diff --git a/.changeset/gentle-bulldogs-turn.md b/.changeset/gentle-bulldogs-turn.md new file mode 100644 index 00000000000..12bc87a2dfb --- /dev/null +++ b/.changeset/gentle-bulldogs-turn.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`DoubleEndedQueue`: Custom errors replaced with native panic codes. diff --git a/.changeset/smart-bugs-switch.md b/.changeset/smart-bugs-switch.md index 6bdea5f2dd4..8a001ae58a1 100644 --- a/.changeset/smart-bugs-switch.md +++ b/.changeset/smart-bugs-switch.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`Math`: MathOverflowedMulDiv error was replaced with native panic codes. +`Math`: Custom errors replaced with native panic codes. diff --git a/contracts/utils/Panic.sol b/contracts/utils/Panic.sol index 4561c7a1365..a24dc624549 100644 --- a/contracts/utils/Panic.sol +++ b/contracts/utils/Panic.sol @@ -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 { diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index b3975335f3d..68e228321f7 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -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}} diff --git a/contracts/utils/structs/DoubleEndedQueue.sol b/contracts/utils/structs/DoubleEndedQueue.sol index 218a2fbd9e4..48f0d68c1eb 100644 --- a/contracts/utils/structs/DoubleEndedQueue.sol +++ b/contracts/utils/structs/DoubleEndedQueue.sol @@ -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)]; diff --git a/docs/templates/contract.hbs b/docs/templates/contract.hbs index e4c8e92060c..aaca0a3cc56 100644 --- a/docs/templates/contract.hbs +++ b/docs/templates/contract.hbs @@ -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}} diff --git a/docs/templates/properties.js b/docs/templates/properties.js index 7e041056776..52eebac547e 100644 --- a/docs/templates/properties.js +++ b/docs/templates/properties.js @@ -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'), diff --git a/test/governance/extensions/GovernorTimelockControl.test.js b/test/governance/extensions/GovernorTimelockControl.test.js index e492d854c6b..c1156a50993 100644 --- a/test/governance/extensions/GovernorTimelockControl.test.js +++ b/test/governance/extensions/GovernorTimelockControl.test.js @@ -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, ); }); }); diff --git a/test/utils/structs/DoubleEndedQueue.test.js b/test/utils/structs/DoubleEndedQueue.test.js index 1f6e782b5bd..3547072ee3b 100644 --- a/test/utils/structs/DoubleEndedQueue.test.js +++ b/test/utils/structs/DoubleEndedQueue.test.js @@ -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 () {