NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Replace custom errors with native panic codes in DoubleEndedQueue (#4872)

Browse Source 
Co-authored-by: ernestognw <ernestognw@gmail.com>
This commit is contained in:
Hadrien Croubois
2024-02-06 21:02:01 +01:00
committed by GitHub
parent e73913c3c1
commit 036c3cbef2
9 changed files with 71 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
.changeset/gentle-bulldogs-turn.md Normal file
View File

@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---
`DoubleEndedQueue`: Custom errors replaced with native panic codes.

2
.changeset/smart-bugs-switch.md
View File

@ -2,4 +2,4 @@
'openzeppelin-solidity': minor
---
`Math`: MathOverflowedMulDiv error was replaced with native panic codes.
`Math`: Custom errors replaced with native panic codes.

2
contracts/utils/Panic.sol
View File

@ -17,7 +17,7 @@ pragma solidity ^0.8.20;
* }
* ```
*
* Follows the list from libsolutil: https://github.com/ethereum/solidity/blob/v0.8.24/libsolutil/ErrorCodes.h
* Follows the list from https://github.com/ethereum/solidity/blob/v0.8.24/libsolutil/ErrorCodes.h[libsolutil].
*/
// slither-disable-next-line unused-state
library Panic {

3
contracts/utils/README.adoc
View File

@ -29,6 +29,7 @@ Miscellaneous contracts and libraries containing utility functions you can use t
* {StorageSlot}: Methods for accessing specific storage slots formatted as common primitive types.
* {Multicall}: Abstract contract with an utility to allow batching together multiple calls in a single transaction. Useful for allowing EOAs to perform multiple operations at once.
* {Context}: An utility for abstracting the sender and calldata in the current execution context.
* {Panic}: A library to revert with https://docs.soliditylang.org/en/v0.8.20/control-structures.html#panic-via-assert-and-error-via-require[Solidity panic codes].
[NOTE]
====
@ -106,3 +107,5 @@ Ethereum contracts have no native concept of an interface, so applications must
{{Multicall}}
{{Context}}
{{Panic}}

45
contracts/utils/structs/DoubleEndedQueue.sol
View File

@ -2,6 +2,8 @@
// OpenZeppelin Contracts (last updated v5.0.0) (utils/structs/DoubleEndedQueue.sol)
pragma solidity ^0.8.20;
import {Panic} from "../Panic.sol";
/**
* @dev A sequence of items with the ability to efficiently push and pop items (i.e. insert and remove) on both ends of
* the sequence (called front and back). Among other access patterns, it can be used to implement efficient LIFO and
@ -15,21 +17,6 @@ pragma solidity ^0.8.20;
* ```
*/
library DoubleEndedQueue {
/**
* @dev An operation (e.g. {front}) couldn't be completed due to the queue being empty.
*/
error QueueEmpty();
/**
* @dev A push operation couldn't be completed due to the queue being full.
*/
error QueueFull();
/**
* @dev An operation (e.g. {at}) couldn't be completed due to an index being out of bounds.
*/
error QueueOutOfBounds();
/**
* @dev Indices are 128 bits so begin and end are packed in a single storage slot for efficient access.
*
@ -48,12 +35,12 @@ library DoubleEndedQueue {
/**
* @dev Inserts an item at the end of the queue.
*
* Reverts with {QueueFull} if the queue is full.
* Reverts with {Panic-RESOURCE_ERROR} if the queue is full.
*/
function pushBack(Bytes32Deque storage deque, bytes32 value) internal {
unchecked {
uint128 backIndex = deque._end;
if (backIndex + 1 == deque._begin) revert QueueFull();
if (backIndex + 1 == deque._begin) Panic.panic(Panic.RESOURCE_ERROR);
deque._data[backIndex] = value;
deque._end = backIndex + 1;
}
@ -62,12 +49,12 @@ library DoubleEndedQueue {
/**
* @dev Removes the item at the end of the queue and returns it.
*
* Reverts with {QueueEmpty} if the queue is empty.
* Reverts with {Panic-EMPTY_ARRAY_POP} if the queue is empty.
*/
function popBack(Bytes32Deque storage deque) internal returns (bytes32 value) {
unchecked {
uint128 backIndex = deque._end;
if (backIndex == deque._begin) revert QueueEmpty();
if (backIndex == deque._begin) Panic.panic(Panic.EMPTY_ARRAY_POP);
--backIndex;
value = deque._data[backIndex];
delete deque._data[backIndex];
@ -78,12 +65,12 @@ library DoubleEndedQueue {
/**
* @dev Inserts an item at the beginning of the queue.
*
* Reverts with {QueueFull} if the queue is full.
* Reverts with {Panic-RESOURCE_ERROR} if the queue is full.
*/
function pushFront(Bytes32Deque storage deque, bytes32 value) internal {
unchecked {
uint128 frontIndex = deque._begin - 1;
if (frontIndex == deque._end) revert QueueFull();
if (frontIndex == deque._end) Panic.panic(Panic.RESOURCE_ERROR);
deque._data[frontIndex] = value;
deque._begin = frontIndex;
}
@ -92,12 +79,12 @@ library DoubleEndedQueue {
/**
* @dev Removes the item at the beginning of the queue and returns it.
*
* Reverts with `QueueEmpty` if the queue is empty.
* Reverts with {Panic-EMPTY_ARRAY_POP} if the queue is empty.
*/
function popFront(Bytes32Deque storage deque) internal returns (bytes32 value) {
unchecked {
uint128 frontIndex = deque._begin;
if (frontIndex == deque._end) revert QueueEmpty();
if (frontIndex == deque._end) Panic.panic(Panic.EMPTY_ARRAY_POP);
value = deque._data[frontIndex];
delete deque._data[frontIndex];
deque._begin = frontIndex + 1;
@ -107,20 +94,20 @@ library DoubleEndedQueue {
/**
* @dev Returns the item at the beginning of the queue.
*
* Reverts with `QueueEmpty` if the queue is empty.
* Reverts with {Panic-ARRAY_OUT_OF_BOUNDS} if the queue is empty.
*/
function front(Bytes32Deque storage deque) internal view returns (bytes32 value) {
if (empty(deque)) revert QueueEmpty();
if (empty(deque)) Panic.panic(Panic.ARRAY_OUT_OF_BOUNDS);
return deque._data[deque._begin];
}
/**
* @dev Returns the item at the end of the queue.
*
* Reverts with `QueueEmpty` if the queue is empty.
* Reverts with {Panic-ARRAY_OUT_OF_BOUNDS} if the queue is empty.
*/
function back(Bytes32Deque storage deque) internal view returns (bytes32 value) {
if (empty(deque)) revert QueueEmpty();
if (empty(deque)) Panic.panic(Panic.ARRAY_OUT_OF_BOUNDS);
unchecked {
return deque._data[deque._end - 1];
}
@ -130,10 +117,10 @@ library DoubleEndedQueue {
* @dev Return the item at a position in the queue given by `index`, with the first item at 0 and last item at
* `length(deque) - 1`.
*
* Reverts with `QueueOutOfBounds` if the index is out of bounds.
* Reverts with {Panic-ARRAY_OUT_OF_BOUNDS} if the index is out of bounds.
*/
function at(Bytes32Deque storage deque, uint256 index) internal view returns (bytes32 value) {
if (index >= length(deque)) revert QueueOutOfBounds();
if (index >= length(deque)) Panic.panic(Panic.ARRAY_OUT_OF_BOUNDS);
// By construction, length is a uint128, so the check above ensures that index can be safely downcast to uint128
unchecked {
return deque._data[deque._begin + uint128(index)];

26
docs/templates/contract.hbs vendored
View File

@ -74,6 +74,23 @@ import "@openzeppelin/{{__item_context.file.absolutePath}}";
--
{{/if}}
{{#if has-internal-variables}}
[.contract-index]
.Internal Variables
--
{{#each inheritance}}
{{#unless @first}}
[.contract-subindex-inherited]
.{{name}}
{{/unless}}
{{#each internal-variables}}
* {xref-{{anchor~}} }[`++{{typeDescriptions.typeString}} {{#if constant}}constant{{/if}} {{name}}++`]
{{/each}}
{{/each}}
--
{{/if}}
{{#each modifiers}}
[.contract-item]
[[{{anchor}}]]
@ -109,3 +126,12 @@ import "@openzeppelin/{{__item_context.file.absolutePath}}";
{{{natspec.dev}}}
{{/each}}
{{#each internal-variables}}
[.contract-item]
[[{{anchor}}]]
==== `{{typeDescriptions.typeString}} [.contract-item-name]#++{{name}}++#` [.item-kind]#internal{{#if constant}} constant{{/if}}#
{{{natspec.dev}}}
{{/each}}

8
docs/templates/properties.js vendored
View File

@ -39,6 +39,14 @@ module.exports['has-errors'] = function ({ item }) {
return item.inheritance.some(c => c.errors.length > 0);
};
module.exports['internal-variables'] = function ({ item }) {
return item.variables.filter(({ visibility }) => visibility === 'internal');
};
module.exports['has-internal-variables'] = function ({ item }) {
return module.exports['internal-variables']({ item }).length > 0;
};
module.exports.functions = function ({ item }) {
return [
...[...findAll('FunctionDefinition', item)].filter(f => f.visibility !== 'private'),

6
test/governance/extensions/GovernorTimelockControl.test.js
View File

@ -2,6 +2,7 @@ const { ethers } = require('hardhat');
const { expect } = require('chai');
const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
const { anyValue } = require('@nomicfoundation/hardhat-chai-matchers/withArgs');
const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic');
const { GovernorHelper, timelockSalt } = require('../../helpers/governance');
const { OperationState, ProposalState, VoteType } = require('../../helpers/enums');
@ -377,9 +378,8 @@ describe('GovernorTimelockControl', function () {
await time.increaseBy.timestamp(delay);
// Error bubbled up from Governor
await expect(this.timelock.connect(this.owner).execute(...call)).to.be.revertedWithCustomError(
this.mock,
'QueueEmpty',
await expect(this.timelock.connect(this.owner).execute(...call)).to.be.revertedWithPanic(
PANIC_CODES.POP_ON_EMPTY_ARRAY,
);
});
});

13
test/utils/structs/DoubleEndedQueue.test.js
View File

@ -1,6 +1,7 @@
const { ethers } = require('hardhat');
const { expect } = require('chai');
const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic');
async function fixture() {
const mock = await ethers.deployContract('$DoubleEndedQueue');
@ -36,10 +37,10 @@ describe('DoubleEndedQueue', function () {
});
it('reverts on accesses', async function () {
await expect(this.mock.$popBack(0)).to.be.revertedWithCustomError(this.mock, 'QueueEmpty');
await expect(this.mock.$popFront(0)).to.be.revertedWithCustomError(this.mock, 'QueueEmpty');
await expect(this.mock.$back(0)).to.be.revertedWithCustomError(this.mock, 'QueueEmpty');
await expect(this.mock.$front(0)).to.be.revertedWithCustomError(this.mock, 'QueueEmpty');
await expect(this.mock.$popBack(0)).to.be.revertedWithPanic(PANIC_CODES.POP_ON_EMPTY_ARRAY);
await expect(this.mock.$popFront(0)).to.be.revertedWithPanic(PANIC_CODES.POP_ON_EMPTY_ARRAY);
await expect(this.mock.$back(0)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS);
await expect(this.mock.$front(0)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS);
});
});
@ -60,7 +61,9 @@ describe('DoubleEndedQueue', function () {
});
it('out of bounds access', async function () {
await expect(this.mock.$at(0, this.content.length)).to.be.revertedWithCustomError(this.mock, 'QueueOutOfBounds');
await expect(this.mock.$at(0, this.content.length)).to.be.revertedWithPanic(
PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS,
);
});
describe('push', function () {