-
Notifications
You must be signed in to change notification settings - Fork 12.4k
Refactor DoubleEndedQueue #4150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
fd880a5
1199e60
1294d4b
734bf8e
0b3da8c
2201f65
909af00
d5b67d3
3b3e144
8c64c4c
aea94da
825d8fe
53ac10d
3a2ec00
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| 'openzeppelin-solidity': major | ||
| --- | ||
|
|
||
| `DoubleEndedQueue`: refactor internal structure to use `uint128` instead of `int128`. This has no effect on the library interface. |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -2,8 +2,6 @@ | |||||||||
| // OpenZeppelin Contracts (last updated v4.9.0) (utils/structs/DoubleEndedQueue.sol) | ||||||||||
| pragma solidity ^0.8.19; | ||||||||||
|
|
||||||||||
| import {SafeCast} from "../math/SafeCast.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 | ||||||||||
|
|
@@ -22,6 +20,11 @@ library DoubleEndedQueue { | |||||||||
| */ | ||||||||||
| 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. | ||||||||||
| */ | ||||||||||
|
|
@@ -40,18 +43,19 @@ library DoubleEndedQueue { | |||||||||
| * data[end - 1]. | ||||||||||
| */ | ||||||||||
| struct Bytes32Deque { | ||||||||||
| int128 _begin; | ||||||||||
| int128 _end; | ||||||||||
| mapping(int128 => bytes32) _data; | ||||||||||
| uint128 _begin; | ||||||||||
| uint128 _end; | ||||||||||
| mapping(uint128 => bytes32) _data; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @dev Inserts an item at the end of the queue. | ||||||||||
| */ | ||||||||||
| function pushBack(Bytes32Deque storage deque, bytes32 value) internal { | ||||||||||
| int128 backIndex = deque._end; | ||||||||||
| deque._data[backIndex] = value; | ||||||||||
| unchecked { | ||||||||||
| uint128 backIndex = deque._end; | ||||||||||
| if (backIndex + 1 == deque._begin) revert QueueFull(); | ||||||||||
| deque._data[backIndex] = value; | ||||||||||
| deque._end = backIndex + 1; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
@@ -62,26 +66,26 @@ library DoubleEndedQueue { | |||||||||
| * Reverts with `QueueEmpty` if the queue is empty. | ||||||||||
| */ | ||||||||||
| function popBack(Bytes32Deque storage deque) internal returns (bytes32 value) { | ||||||||||
| if (empty(deque)) revert QueueEmpty(); | ||||||||||
| int128 backIndex; | ||||||||||
| unchecked { | ||||||||||
| backIndex = deque._end - 1; | ||||||||||
| uint128 backIndex = deque._end; | ||||||||||
| if (backIndex == deque._begin) revert QueueEmpty(); | ||||||||||
| --backIndex; | ||||||||||
| value = deque._data[backIndex]; | ||||||||||
| delete deque._data[backIndex]; | ||||||||||
| deque._end = backIndex; | ||||||||||
| } | ||||||||||
| value = deque._data[backIndex]; | ||||||||||
| delete deque._data[backIndex]; | ||||||||||
| deque._end = backIndex; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @dev Inserts an item at the beginning of the queue. | ||||||||||
| */ | ||||||||||
| function pushFront(Bytes32Deque storage deque, bytes32 value) internal { | ||||||||||
| int128 frontIndex; | ||||||||||
| unchecked { | ||||||||||
| frontIndex = deque._begin - 1; | ||||||||||
| uint128 frontIndex = deque._begin - 1; | ||||||||||
| if (frontIndex == deque._end) revert QueueFull(); | ||||||||||
| deque._data[frontIndex] = value; | ||||||||||
| deque._begin = frontIndex; | ||||||||||
| } | ||||||||||
| deque._data[frontIndex] = value; | ||||||||||
| deque._begin = frontIndex; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
|
|
@@ -90,11 +94,11 @@ library DoubleEndedQueue { | |||||||||
| * Reverts with `QueueEmpty` if the queue is empty. | ||||||||||
| */ | ||||||||||
| function popFront(Bytes32Deque storage deque) internal returns (bytes32 value) { | ||||||||||
| if (empty(deque)) revert QueueEmpty(); | ||||||||||
| int128 frontIndex = deque._begin; | ||||||||||
| value = deque._data[frontIndex]; | ||||||||||
| delete deque._data[frontIndex]; | ||||||||||
| unchecked { | ||||||||||
| uint128 frontIndex = deque._begin; | ||||||||||
| if (frontIndex == deque._end) revert QueueEmpty(); | ||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We've been consistent with this formatting. We can discuss it further because I prefer this. Same for others. Not blocking.
Suggested change
|
||||||||||
| value = deque._data[frontIndex]; | ||||||||||
| delete deque._data[frontIndex]; | ||||||||||
| deque._begin = frontIndex + 1; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
@@ -106,8 +110,7 @@ library DoubleEndedQueue { | |||||||||
| */ | ||||||||||
| function front(Bytes32Deque storage deque) internal view returns (bytes32 value) { | ||||||||||
| if (empty(deque)) revert QueueEmpty(); | ||||||||||
| int128 frontIndex = deque._begin; | ||||||||||
| return deque._data[frontIndex]; | ||||||||||
| return deque._data[deque._begin]; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
|
|
@@ -117,11 +120,9 @@ library DoubleEndedQueue { | |||||||||
| */ | ||||||||||
| function back(Bytes32Deque storage deque) internal view returns (bytes32 value) { | ||||||||||
| if (empty(deque)) revert QueueEmpty(); | ||||||||||
| int128 backIndex; | ||||||||||
| unchecked { | ||||||||||
| backIndex = deque._end - 1; | ||||||||||
| return deque._data[deque._end - 1]; | ||||||||||
| } | ||||||||||
| return deque._data[backIndex]; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
|
|
@@ -131,10 +132,11 @@ library DoubleEndedQueue { | |||||||||
| * Reverts with `QueueOutOfBounds` if the index is out of bounds. | ||||||||||
| */ | ||||||||||
| function at(Bytes32Deque storage deque, uint256 index) internal view returns (bytes32 value) { | ||||||||||
| // int256(deque._begin) is a safe upcast | ||||||||||
| int128 idx = SafeCast.toInt128(int256(deque._begin) + SafeCast.toInt256(index)); | ||||||||||
| if (idx >= deque._end) revert QueueOutOfBounds(); | ||||||||||
| return deque._data[idx]; | ||||||||||
| if (index >= length(deque)) revert QueueOutOfBounds(); | ||||||||||
| // 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)]; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
|
|
@@ -152,17 +154,15 @@ library DoubleEndedQueue { | |||||||||
| * @dev Returns the number of items in the queue. | ||||||||||
| */ | ||||||||||
| function length(Bytes32Deque storage deque) internal view returns (uint256) { | ||||||||||
| // The interface preserves the invariant that begin <= end so we assume this will not overflow. | ||||||||||
| // We also assume there are at most int256.max items in the queue. | ||||||||||
| unchecked { | ||||||||||
| return uint256(int256(deque._end) - int256(deque._begin)); | ||||||||||
| return uint256(deque._end - deque._begin); | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * @dev Returns true if the queue is empty. | ||||||||||
| */ | ||||||||||
| function empty(Bytes32Deque storage deque) internal view returns (bool) { | ||||||||||
| return deque._end <= deque._begin; | ||||||||||
| return deque._end == deque._begin; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| certora-cli==3.6.4 | ||
| certora-cli==4.3.1 | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we merge this will all the existing specs stop working?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They already don't work ... because 3.6.4 is no longer supported
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We'll have to do a "bump FV to new format" task during the quality control sprint. I can take care of it. |
||
Uh oh!
There was an error while loading. Please reload this page.