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

Co-authored-by: ernestognw <[email protected]>

Author: Amxx

.changeset/gentle-bulldogs-turn.md

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

.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.

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 {

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}}

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)];

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}}

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'),

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,
             );
           });
         });

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 () {