From 0067ec774e9608852baa9c6d32801933ca412528 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 19 Feb 2024 20:59:20 +0100 Subject: [PATCH 01/13] add circular buffer --- .changeset/cold-cheetahs-check.md | 5 ++ contracts/utils/structs/CircularBuffer.sol | 57 +++++++++++++++++ test/utils/structs/CircularBuffer.test.js | 71 ++++++++++++++++++++++ 3 files changed, 133 insertions(+) create mode 100644 .changeset/cold-cheetahs-check.md create mode 100644 contracts/utils/structs/CircularBuffer.sol create mode 100644 test/utils/structs/CircularBuffer.test.js diff --git a/.changeset/cold-cheetahs-check.md b/.changeset/cold-cheetahs-check.md new file mode 100644 index 00000000000..5e5abedc7c5 --- /dev/null +++ b/.changeset/cold-cheetahs-check.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`CircularBuffer`: add a datastructure that stored the last N values pushed to it. diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol new file mode 100644 index 00000000000..6c8b185ff87 --- /dev/null +++ b/contracts/utils/structs/CircularBuffer.sol @@ -0,0 +1,57 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import {Math} from "../math/Math.sol"; +import {Arrays} from "../Arrays.sol"; +import {Panic} from "../Panic.sol"; + +library CircularBuffer { + struct Bytes32CircularBuffer { + uint256 index; + bytes32[] data; + } + + function setup(Bytes32CircularBuffer storage self, uint256 length) internal { + clear(self); + Arrays.unsafeSetLength(self.data, length); + } + + function clear(Bytes32CircularBuffer storage self) internal { + self.index = 0; + } + + function push(Bytes32CircularBuffer storage self, bytes32 value) internal { + uint256 index = self.index++; + uint256 length = self.data.length; + Arrays.unsafeAccess(self.data, index % length).value = value; + } + + function count(Bytes32CircularBuffer storage self) internal view returns (uint256) { + return Math.min(self.index, self.data.length); + } + + function size(Bytes32CircularBuffer storage self) internal view returns (uint256) { + return self.data.length; + } + + function last(Bytes32CircularBuffer storage self, uint256 i) internal view returns (bytes32) { + uint256 index = self.index; + if (index <= i) { + Panic.panic(Panic.ARRAY_OUT_OF_BOUNDS); + } + return Arrays.unsafeAccess(self.data, (index - i - 1) % self.data.length).value; + } + + function includes(Bytes32CircularBuffer storage self, bytes32 value) internal view returns (bool) { + uint256 index = self.index; + uint256 length = self.data.length; + for (uint256 i = 1; i <= length; ++i) { + if (i > index) { + return false; + } else if (Arrays.unsafeAccess(self.data, (index - i) % length).value == value) { + return true; + } + } + return false; + } +} diff --git a/test/utils/structs/CircularBuffer.test.js b/test/utils/structs/CircularBuffer.test.js new file mode 100644 index 00000000000..b2582446db5 --- /dev/null +++ b/test/utils/structs/CircularBuffer.test.js @@ -0,0 +1,71 @@ +const { ethers } = require('hardhat'); +const { expect } = require('chai'); +const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { PANIC_CODES } = require('@nomicfoundation/hardhat-chai-matchers/panic'); + +const { generators } = require('../../helpers/random'); + +const LENGTH = 4; + +async function fixture() { + const mock = await ethers.deployContract('$CircularBuffer'); + await mock.$setup(0, LENGTH); + return { mock }; +} + +describe('CircularBuffer', function () { + beforeEach(async function () { + Object.assign(this, await loadFixture(fixture)); + }); + + it('starts empty', async function () { + expect(await this.mock.$count(0)).to.equal(0n); + expect(await this.mock.$size(0)).to.equal(LENGTH); + await expect(this.mock.$last(0, 0)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS); + }); + + it('push', async function () { + const values = Array.from({ length: LENGTH + 3 }, generators.bytes32); + + for (const [i, value] of values.map((v, i) => [i, v])) { + // push value + await this.mock.$push(0, value); + + // view of the values + const pushed = values.slice(0, i + 1); + const stored = pushed.slice(-LENGTH); + const dropped = pushed.slice(0, -LENGTH); + + // check count + expect(await this.mock.$size(0)).to.equal(LENGTH); + expect(await this.mock.$count(0)).to.equal(stored.length); + + // check last + for (const i in stored) { + expect(await this.mock.$last(0, i)).to.equal(stored.at(-i - 1)); + } + + // check included and non-included values + for (const v of stored) { + expect(await this.mock.$includes(0, v)).to.be.true; + } + for (const v of dropped) { + expect(await this.mock.$includes(0, v)).to.be.false; + } + } + }); + + it('clear', async function () { + await this.mock.$push(0, generators.bytes32()); + + expect(await this.mock.$count(0)).to.equal(1n); + expect(await this.mock.$size(0)).to.equal(LENGTH); + await this.mock.$last(0, 0); // not revert + + await this.mock.$clear(0); + + expect(await this.mock.$count(0)).to.equal(0n); + expect(await this.mock.$size(0)).to.equal(LENGTH); + await expect(this.mock.$last(0, 0)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS); + }); +}); From 585db62124e570ff874c11effcc0a5a13c8da0f9 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 20 Feb 2024 16:38:40 +0100 Subject: [PATCH 02/13] inline documentation & testing --- contracts/mocks/ArraysMock.sol | 24 ++++++ contracts/utils/Arrays.sol | 33 ++++++++ contracts/utils/structs/CircularBuffer.sol | 92 +++++++++++++++++----- test/utils/Arrays.test.js | 48 +++++++---- test/utils/structs/CircularBuffer.test.js | 13 ++- 5 files changed, 173 insertions(+), 37 deletions(-) diff --git a/contracts/mocks/ArraysMock.sol b/contracts/mocks/ArraysMock.sol index 0e39485fce0..63f4c8e69f3 100644 --- a/contracts/mocks/ArraysMock.sol +++ b/contracts/mocks/ArraysMock.sol @@ -48,6 +48,14 @@ contract Uint256ArraysMock { function _reverse(uint256 a, uint256 b) private pure returns (bool) { return a > b; } + + function unsafeSetLength(uint256 newLength) external { + _array.unsafeSetLength(newLength); + } + + function length() external view returns (uint256) { + return _array.length; + } } contract AddressArraysMock { @@ -74,6 +82,14 @@ contract AddressArraysMock { function _reverse(address a, address b) private pure returns (bool) { return uint160(a) > uint160(b); } + + function unsafeSetLength(uint256 newLength) external { + _array.unsafeSetLength(newLength); + } + + function length() external view returns (uint256) { + return _array.length; + } } contract Bytes32ArraysMock { @@ -100,4 +116,12 @@ contract Bytes32ArraysMock { function _reverse(bytes32 a, bytes32 b) private pure returns (bool) { return uint256(a) > uint256(b); } + + function unsafeSetLength(uint256 newLength) external { + _array.unsafeSetLength(newLength); + } + + function length() external view returns (uint256) { + return _array.length; + } } diff --git a/contracts/utils/Arrays.sol b/contracts/utils/Arrays.sol index 6b2d75f50e1..02252059898 100644 --- a/contracts/utils/Arrays.sol +++ b/contracts/utils/Arrays.sol @@ -440,4 +440,37 @@ library Arrays { res := mload(add(add(arr, 0x20), mul(pos, 0x20))) } } + + /** + * @dev Helper to set the length of an dynamic array. Directly writing to `.length` is forbidden. + * + * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. + */ + function unsafeSetLength(address[] storage array, uint256 len) internal { + assembly { + sstore(array.slot, len) + } + } + + /** + * @dev Helper to set the length of an dynamic array. Directly writing to `.length` is forbidden. + * + * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. + */ + function unsafeSetLength(bytes32[] storage array, uint256 len) internal { + assembly { + sstore(array.slot, len) + } + } + + /** + * @dev Helper to set the length of an dynamic array. Directly writing to `.length` is forbidden. + * + * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. + */ + function unsafeSetLength(uint256[] storage array, uint256 len) internal { + assembly { + sstore(array.slot, len) + } + } } diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol index 6c8b185ff87..524b33581c3 100644 --- a/contracts/utils/structs/CircularBuffer.sol +++ b/contracts/utils/structs/CircularBuffer.sol @@ -5,50 +5,106 @@ import {Math} from "../math/Math.sol"; import {Arrays} from "../Arrays.sol"; import {Panic} from "../Panic.sol"; +/** + * @dev A buffer of items of fixed size. When a new item is pushed, it takes the place of the oldest one in the buffer + * so that at all times, only the last N elements are kept. Items cannot be removed. The entier buffer can be reset. + * Last N elements can be accessed using their index from the end. + * + * Complexity: + * - insertion (`push`): O(1) + * - lookup (`last`): O(1) + * - inclusion (`includes`): O(N) (worst case) + * - reset (`clear`): O(1) + * + * * The struct is called `Bytes32CircularBuffer`. Other types can be cast to and from `bytes32`. This data structure + * can only be used in storage, and not in memory. + * ```solidity + * CircularBuffer.Bytes32CircularBuffer buffer; + * ``` + */ library CircularBuffer { + /** + * @dev Counts the number of items that have been pushed to the buffer. The residu modulo _data.length indicates + * where the next value should be stored. + * + * Struct members have an underscore prefix indicating that they are "private" and should not be read or written to + * directly. Use the functions provided below instead. Modifying the struct manually may violate assumptions and + * lead to unexpected behavior. + * + * The last item is at data[(index - 1) % data.length] and the last item is at data[index % data.length]. This + * range can wrap around. + */ struct Bytes32CircularBuffer { - uint256 index; - bytes32[] data; + uint256 _count; + bytes32[] _data; } + /** + * @dev Initialize a new CircularBuffer of given length. + * + * If the CircularBuffer was already setup and used, calling that function again will reset it to a blank state. + */ function setup(Bytes32CircularBuffer storage self, uint256 length) internal { clear(self); - Arrays.unsafeSetLength(self.data, length); + Arrays.unsafeSetLength(self._data, length); } + /** + * @dev Clear all data in the buffer, keeping the existing length. + */ function clear(Bytes32CircularBuffer storage self) internal { - self.index = 0; + self._count = 0; } + /** + * @dev Push a new value to the buffer. If the buffer is already full, the new value replaces the oldest value in + * the buffer. + */ function push(Bytes32CircularBuffer storage self, bytes32 value) internal { - uint256 index = self.index++; - uint256 length = self.data.length; - Arrays.unsafeAccess(self.data, index % length).value = value; + uint256 index = self._count++; + uint256 length = self._data.length; + Arrays.unsafeAccess(self._data, index % length).value = value; } + /** + * @dev Number of values currently in the buffer. This values is 0 for empty buffer, and cannot exceed the size of + * the buffer. + */ function count(Bytes32CircularBuffer storage self) internal view returns (uint256) { - return Math.min(self.index, self.data.length); + return Math.min(self._count, self._data.length); } + /** + * @dev Length of the buffer. This is the maximum number of elements kepts in the buffer. + */ function size(Bytes32CircularBuffer storage self) internal view returns (uint256) { - return self.data.length; + return self._data.length; } + /** + * @dev Getter for the i-th value in the buffer, from the end. + * + * Reverts with {Panic-ARRAY_OUT_OF_BOUNDS} if trying to access an element that was not pushed, or that was + * dropped to make room for newer elements. + */ function last(Bytes32CircularBuffer storage self, uint256 i) internal view returns (bytes32) { - uint256 index = self.index; - if (index <= i) { + uint256 index = self._count; + uint256 length = self._data.length; + if (index <= i || length <= i) { Panic.panic(Panic.ARRAY_OUT_OF_BOUNDS); } - return Arrays.unsafeAccess(self.data, (index - i - 1) % self.data.length).value; + return Arrays.unsafeAccess(self._data, (index - i - 1) % self._data.length).value; } + /** + * @dev Check if a given value is in the buffer. + */ function includes(Bytes32CircularBuffer storage self, bytes32 value) internal view returns (bool) { - uint256 index = self.index; - uint256 length = self.data.length; - for (uint256 i = 1; i <= length; ++i) { - if (i > index) { - return false; - } else if (Arrays.unsafeAccess(self.data, (index - i) % length).value == value) { + uint256 index = self._count; + uint256 total = count(self); + uint256 length = self._data.length; + for (uint256 i = 1; i <= total; ++i) { + if (Arrays.unsafeAccess(self._data, (index - i) % length).value == value) { return true; } } diff --git a/test/utils/Arrays.test.js b/test/utils/Arrays.test.js index 8edf75fa71f..30685199333 100644 --- a/test/utils/Arrays.test.js +++ b/test/utils/Arrays.test.js @@ -17,6 +17,7 @@ const upperBound = (array, value) => { }; const bigintSign = x => (x > 0n ? 1 : x < 0n ? -1 : 0); +const comparator = (a, b) => bigintSign(ethers.toBigInt(a) - ethers.toBigInt(b)); const hasDuplicates = array => array.some((v, i) => array.indexOf(v) != i); describe('Arrays', function () { @@ -116,23 +117,22 @@ describe('Arrays', function () { } }); - for (const [type, { artifact, elements, comp }] of Object.entries({ + for (const [type, { artifact, format }] of Object.entries({ address: { artifact: 'AddressArraysMock', - elements: Array.from({ length: 10 }, generators.address), - comp: (a, b) => bigintSign(ethers.toBigInt(a) - ethers.toBigInt(b)), + format: x => ethers.getAddress(ethers.toBeHex(x, 20)), }, bytes32: { artifact: 'Bytes32ArraysMock', - elements: Array.from({ length: 10 }, generators.bytes32), - comp: (a, b) => bigintSign(ethers.toBigInt(a) - ethers.toBigInt(b)), + format: x => ethers.toBeHex(x, 32), }, uint256: { artifact: 'Uint256ArraysMock', - elements: Array.from({ length: 10 }, generators.uint256), - comp: (a, b) => bigintSign(a - b), + format: x => ethers.toBigInt(x), }, })) { + const elements = Array.from({ length: 10 }, generators[type]); + describe(type, function () { const fixture = async () => { return { instance: await ethers.deployContract(artifact, [elements]) }; @@ -146,14 +146,14 @@ describe('Arrays', function () { for (const length of [0, 1, 2, 8, 32, 128]) { describe(`${type}[] of length ${length}`, function () { beforeEach(async function () { - this.elements = Array.from({ length }, generators[type]); + this.array = Array.from({ length }, generators[type]); }); afterEach(async function () { - const expected = Array.from(this.elements).sort(comp); + const expected = Array.from(this.array).sort(comparator); const reversed = Array.from(expected).reverse(); - expect(await this.instance.sort(this.elements)).to.deep.equal(expected); - expect(await this.instance.sortReverse(this.elements)).to.deep.equal(reversed); + expect(await this.instance.sort(this.array)).to.deep.equal(expected); + expect(await this.instance.sortReverse(this.array)).to.deep.equal(reversed); }); it('sort array', async function () { @@ -163,23 +163,23 @@ describe('Arrays', function () { if (length > 1) { it('sort array for identical elements', async function () { // duplicate the first value to all elements - this.elements.fill(this.elements.at(0)); + this.array.fill(this.array.at(0)); }); it('sort already sorted array', async function () { // pre-sort the elements - this.elements.sort(comp); + this.array.sort(comparator); }); it('sort reversed array', async function () { // pre-sort in reverse order - this.elements.sort(comp).reverse(); + this.array.sort(comparator).reverse(); }); it('sort almost sorted array', async function () { // pre-sort + rotate (move the last element to the front) for an almost sorted effect - this.elements.sort(comp); - this.elements.unshift(this.elements.pop()); + this.array.sort(comparator); + this.array.unshift(this.array.pop()); }); } }); @@ -197,6 +197,14 @@ describe('Arrays', function () { it('unsafeAccess outside bounds', async function () { await expect(this.instance.unsafeAccess(elements.length)).to.not.be.rejected; }); + + it('unsafeSetLength changes the length or the array', async function () { + const newLength = generators.uint256(); + + expect(await this.instance.length()).to.equal(elements.length); + await expect(this.instance.unsafeSetLength(newLength)).to.not.be.rejected; + expect(await this.instance.length()).to.equal(newLength); + }); }); describe('memory', function () { @@ -211,6 +219,14 @@ describe('Arrays', function () { it('unsafeMemoryAccess outside bounds', async function () { await expect(this.mock[fragment](elements, elements.length)).to.not.be.rejected; }); + + it('unsafeMemoryAccess loop around', async function () { + for (let i = 251n; i < 256n; ++i) { + expect(await this.mock[fragment](elements, 2n ** i - 1n)).to.equal(format(elements.length)); + expect(await this.mock[fragment](elements, 2n ** i + 0n)).to.equal(elements[0]); + expect(await this.mock[fragment](elements, 2n ** i + 1n)).to.equal(elements[1]); + } + }); }); }); }); diff --git a/test/utils/structs/CircularBuffer.test.js b/test/utils/structs/CircularBuffer.test.js index b2582446db5..b442077b253 100644 --- a/test/utils/structs/CircularBuffer.test.js +++ b/test/utils/structs/CircularBuffer.test.js @@ -21,6 +21,7 @@ describe('CircularBuffer', function () { it('starts empty', async function () { expect(await this.mock.$count(0)).to.equal(0n); expect(await this.mock.$size(0)).to.equal(LENGTH); + expect(await this.mock.$includes(0, ethers.ZeroHash)).to.be.false; await expect(this.mock.$last(0, 0)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS); }); @@ -41,9 +42,10 @@ describe('CircularBuffer', function () { expect(await this.mock.$count(0)).to.equal(stored.length); // check last - for (const i in stored) { - expect(await this.mock.$last(0, i)).to.equal(stored.at(-i - 1)); + for (const j in stored) { + expect(await this.mock.$last(0, j)).to.equal(stored.at(-j - 1)); } + await expect(this.mock.$last(0, stored.length + 1)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS); // check included and non-included values for (const v of stored) { @@ -52,20 +54,25 @@ describe('CircularBuffer', function () { for (const v of dropped) { expect(await this.mock.$includes(0, v)).to.be.false; } + expect(await this.mock.$includes(0, ethers.ZeroHash)).to.be.false; } + }); it('clear', async function () { - await this.mock.$push(0, generators.bytes32()); + const value = generators.bytes32(); + await this.mock.$push(0, value); expect(await this.mock.$count(0)).to.equal(1n); expect(await this.mock.$size(0)).to.equal(LENGTH); + expect(await this.mock.$includes(0, value)).to.be.true; await this.mock.$last(0, 0); // not revert await this.mock.$clear(0); expect(await this.mock.$count(0)).to.equal(0n); expect(await this.mock.$size(0)).to.equal(LENGTH); + expect(await this.mock.$includes(0, value)).to.be.false; await expect(this.mock.$last(0, 0)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS); }); }); From 42db594c6454efb74b972b1e501cbd585bf9174a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 20 Feb 2024 16:46:27 +0100 Subject: [PATCH 03/13] update README.adoc --- contracts/utils/README.adoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contracts/utils/README.adoc b/contracts/utils/README.adoc index 68e228321f7..0ffaf51c1e0 100644 --- a/contracts/utils/README.adoc +++ b/contracts/utils/README.adoc @@ -19,6 +19,7 @@ Miscellaneous contracts and libraries containing utility functions you can use t * {EnumerableMap}: A type like Solidity's https://solidity.readthedocs.io/en/latest/types.html#mapping-types[`mapping`], but with key-value _enumeration_: this will let you know how many entries a mapping has, and iterate over them (which is not possible with `mapping`). * {EnumerableSet}: Like {EnumerableMap}, but for https://en.wikipedia.org/wiki/Set_(abstract_data_type)[sets]. Can be used to store privileged accounts, issued IDs, etc. * {DoubleEndedQueue}: An implementation of a https://en.wikipedia.org/wiki/Double-ended_queue[double ended queue] whose values can be removed added or remove from both sides. Useful for FIFO and LIFO structures. + * {CircularBuffer}: A data structure to store the last N values pushed to it. * {Checkpoints}: A data structure to store values mapped to an strictly increasing key. Can be used for storing and accessing values over time. * {Create2}: Wrapper around the https://blog.openzeppelin.com/getting-the-most-out-of-create2/[`CREATE2` EVM opcode] for safe use without having to deal with low-level assembly. * {Address}: Collection of functions for overloading Solidity's https://docs.soliditylang.org/en/latest/types.html#address[`address`] type. @@ -86,6 +87,8 @@ Ethereum contracts have no native concept of an interface, so applications must {{DoubleEndedQueue}} +{{CircularBuffer}} + {{Checkpoints}} == Libraries From b56796fc7409eed573dd9bd4ecdd04cddfa93f5b Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 20 Feb 2024 16:51:39 +0100 Subject: [PATCH 04/13] fix lint --- contracts/utils/structs/CircularBuffer.sol | 9 +++++---- test/utils/structs/CircularBuffer.test.js | 5 +++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol index 524b33581c3..979e8249cda 100644 --- a/contracts/utils/structs/CircularBuffer.sol +++ b/contracts/utils/structs/CircularBuffer.sol @@ -90,7 +90,8 @@ library CircularBuffer { function last(Bytes32CircularBuffer storage self, uint256 i) internal view returns (bytes32) { uint256 index = self._count; uint256 length = self._data.length; - if (index <= i || length <= i) { + uint256 total = Math.min(index, length); // count(self) + if (i >= total) { Panic.panic(Panic.ARRAY_OUT_OF_BOUNDS); } return Arrays.unsafeAccess(self._data, (index - i - 1) % self._data.length).value; @@ -101,10 +102,10 @@ library CircularBuffer { */ function includes(Bytes32CircularBuffer storage self, bytes32 value) internal view returns (bool) { uint256 index = self._count; - uint256 total = count(self); uint256 length = self._data.length; - for (uint256 i = 1; i <= total; ++i) { - if (Arrays.unsafeAccess(self._data, (index - i) % length).value == value) { + uint256 total = Math.min(index, length); // count(self) + for (uint256 i = 0; i < total; ++i) { + if (Arrays.unsafeAccess(self._data, (index - i - 1) % length).value == value) { return true; } } diff --git a/test/utils/structs/CircularBuffer.test.js b/test/utils/structs/CircularBuffer.test.js index b442077b253..b3c5f2382fa 100644 --- a/test/utils/structs/CircularBuffer.test.js +++ b/test/utils/structs/CircularBuffer.test.js @@ -45,7 +45,9 @@ describe('CircularBuffer', function () { for (const j in stored) { expect(await this.mock.$last(0, j)).to.equal(stored.at(-j - 1)); } - await expect(this.mock.$last(0, stored.length + 1)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS); + await expect(this.mock.$last(0, stored.length + 1)).to.be.revertedWithPanic( + PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS, + ); // check included and non-included values for (const v of stored) { @@ -56,7 +58,6 @@ describe('CircularBuffer', function () { } expect(await this.mock.$includes(0, ethers.ZeroHash)).to.be.false; } - }); it('clear', async function () { From 5cd33bc83e98b19f72df0b7e650f0190c92a8f31 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 20 Feb 2024 16:59:28 +0100 Subject: [PATCH 05/13] Update contracts/utils/structs/CircularBuffer.sol --- contracts/utils/structs/CircularBuffer.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol index 979e8249cda..8341193a8c3 100644 --- a/contracts/utils/structs/CircularBuffer.sol +++ b/contracts/utils/structs/CircularBuffer.sol @@ -94,7 +94,7 @@ library CircularBuffer { if (i >= total) { Panic.panic(Panic.ARRAY_OUT_OF_BOUNDS); } - return Arrays.unsafeAccess(self._data, (index - i - 1) % self._data.length).value; + return Arrays.unsafeAccess(self._data, (index - i - 1) % length).value; } /** From a2b343c2299c77083c936b183ab928e1c166b2b5 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 20 Feb 2024 17:29:05 +0100 Subject: [PATCH 06/13] update Stateless.sol --- contracts/mocks/Stateless.sol | 2 ++ 1 file changed, 2 insertions(+) diff --git a/contracts/mocks/Stateless.sol b/contracts/mocks/Stateless.sol index 56f5b4c6610..48983b4e9c3 100644 --- a/contracts/mocks/Stateless.sol +++ b/contracts/mocks/Stateless.sol @@ -10,6 +10,7 @@ import {AuthorityUtils} from "../access/manager/AuthorityUtils.sol"; import {Base64} from "../utils/Base64.sol"; import {BitMaps} from "../utils/structs/BitMaps.sol"; import {Checkpoints} from "../utils/structs/Checkpoints.sol"; +import {CircularBuffer} from "../utils/structs/CircularBuffer.sol"; import {Clones} from "../proxy/Clones.sol"; import {Create2} from "../utils/Create2.sol"; import {DoubleEndedQueue} from "../utils/structs/DoubleEndedQueue.sol"; @@ -24,6 +25,7 @@ import {ERC721Holder} from "../token/ERC721/utils/ERC721Holder.sol"; import {Math} from "../utils/math/Math.sol"; import {MerkleProof} from "../utils/cryptography/MerkleProof.sol"; import {MessageHashUtils} from "../utils/cryptography/MessageHashUtils.sol"; +import {Panic} from "../utils/Panic.sol"; import {SafeCast} from "../utils/math/SafeCast.sol"; import {SafeERC20} from "../token/ERC20/utils/SafeERC20.sol"; import {ShortStrings} from "../utils/ShortStrings.sol"; From f160bc7d7e36354c32687bb288c9d5ecb47b0f83 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Mon, 25 Mar 2024 14:14:06 +0000 Subject: [PATCH 07/13] Apply trivial PR suggestions --- .changeset/cold-cheetahs-check.md | 2 +- contracts/utils/Arrays.sol | 3 +++ contracts/utils/structs/CircularBuffer.sol | 10 +++++----- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.changeset/cold-cheetahs-check.md b/.changeset/cold-cheetahs-check.md index 5e5abedc7c5..0697dcdf7b4 100644 --- a/.changeset/cold-cheetahs-check.md +++ b/.changeset/cold-cheetahs-check.md @@ -2,4 +2,4 @@ 'openzeppelin-solidity': minor --- -`CircularBuffer`: add a datastructure that stored the last N values pushed to it. +`CircularBuffer`: Add a data structure that stores the last `N` values pushed to it. diff --git a/contracts/utils/Arrays.sol b/contracts/utils/Arrays.sol index 02252059898..2a35a73eec8 100644 --- a/contracts/utils/Arrays.sol +++ b/contracts/utils/Arrays.sol @@ -447,6 +447,7 @@ library Arrays { * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. */ function unsafeSetLength(address[] storage array, uint256 len) internal { + /// @solidity memory-safe-assembly assembly { sstore(array.slot, len) } @@ -458,6 +459,7 @@ library Arrays { * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. */ function unsafeSetLength(bytes32[] storage array, uint256 len) internal { + /// @solidity memory-safe-assembly assembly { sstore(array.slot, len) } @@ -469,6 +471,7 @@ library Arrays { * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. */ function unsafeSetLength(uint256[] storage array, uint256 len) internal { + /// @solidity memory-safe-assembly assembly { sstore(array.slot, len) } diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol index 8341193a8c3..86068599b83 100644 --- a/contracts/utils/structs/CircularBuffer.sol +++ b/contracts/utils/structs/CircularBuffer.sol @@ -11,10 +11,10 @@ import {Panic} from "../Panic.sol"; * Last N elements can be accessed using their index from the end. * * Complexity: - * - insertion (`push`): O(1) - * - lookup (`last`): O(1) - * - inclusion (`includes`): O(N) (worst case) - * - reset (`clear`): O(1) + * - insertion ({push}): O(1) + * - lookup ({last}): O(1) + * - inclusion ({includes}): O(N) (worst case) + * - reset ({clear}): O(1) * * * The struct is called `Bytes32CircularBuffer`. Other types can be cast to and from `bytes32`. This data structure * can only be used in storage, and not in memory. @@ -67,7 +67,7 @@ library CircularBuffer { } /** - * @dev Number of values currently in the buffer. This values is 0 for empty buffer, and cannot exceed the size of + * @dev Number of values currently in the buffer. This value is 0 for an empty buffer, and cannot exceed the size of * the buffer. */ function count(Bytes32CircularBuffer storage self) internal view returns (uint256) { From 63ff766674acfed9f943bb1edea2ff083e890930 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Mar 2024 23:12:42 +0100 Subject: [PATCH 08/13] Update contracts/utils/structs/CircularBuffer.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto García --- contracts/utils/structs/CircularBuffer.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol index 86068599b83..df64266235a 100644 --- a/contracts/utils/structs/CircularBuffer.sol +++ b/contracts/utils/structs/CircularBuffer.sol @@ -77,7 +77,7 @@ library CircularBuffer { /** * @dev Length of the buffer. This is the maximum number of elements kepts in the buffer. */ - function size(Bytes32CircularBuffer storage self) internal view returns (uint256) { + function length(Bytes32CircularBuffer storage self) internal view returns (uint256) { return self._data.length; } From 72a37944a07984022d42db973c4d00cc8ff59341 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Mar 2024 23:22:01 +0100 Subject: [PATCH 09/13] rename internal vars to avoid name conflicts --- contracts/utils/structs/CircularBuffer.sol | 20 ++++++++++---------- test/utils/structs/CircularBuffer.test.js | 8 ++++---- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol index df64266235a..b84750dd264 100644 --- a/contracts/utils/structs/CircularBuffer.sol +++ b/contracts/utils/structs/CircularBuffer.sol @@ -44,9 +44,9 @@ library CircularBuffer { * * If the CircularBuffer was already setup and used, calling that function again will reset it to a blank state. */ - function setup(Bytes32CircularBuffer storage self, uint256 length) internal { + function setup(Bytes32CircularBuffer storage self, uint256 size) internal { clear(self); - Arrays.unsafeSetLength(self._data, length); + Arrays.unsafeSetLength(self._data, size); } /** @@ -62,8 +62,8 @@ library CircularBuffer { */ function push(Bytes32CircularBuffer storage self, bytes32 value) internal { uint256 index = self._count++; - uint256 length = self._data.length; - Arrays.unsafeAccess(self._data, index % length).value = value; + uint256 module = self._data.length; + Arrays.unsafeAccess(self._data, index % module).value = value; } /** @@ -89,12 +89,12 @@ library CircularBuffer { */ function last(Bytes32CircularBuffer storage self, uint256 i) internal view returns (bytes32) { uint256 index = self._count; - uint256 length = self._data.length; - uint256 total = Math.min(index, length); // count(self) + uint256 module = self._data.length; + uint256 total = Math.min(index, module); // count(self) if (i >= total) { Panic.panic(Panic.ARRAY_OUT_OF_BOUNDS); } - return Arrays.unsafeAccess(self._data, (index - i - 1) % length).value; + return Arrays.unsafeAccess(self._data, (index - i - 1) % module).value; } /** @@ -102,10 +102,10 @@ library CircularBuffer { */ function includes(Bytes32CircularBuffer storage self, bytes32 value) internal view returns (bool) { uint256 index = self._count; - uint256 length = self._data.length; - uint256 total = Math.min(index, length); // count(self) + uint256 module = self._data.length; + uint256 total = Math.min(index, module); // count(self) for (uint256 i = 0; i < total; ++i) { - if (Arrays.unsafeAccess(self._data, (index - i - 1) % length).value == value) { + if (Arrays.unsafeAccess(self._data, (index - i - 1) % module).value == value) { return true; } } diff --git a/test/utils/structs/CircularBuffer.test.js b/test/utils/structs/CircularBuffer.test.js index b3c5f2382fa..c3d61aaa823 100644 --- a/test/utils/structs/CircularBuffer.test.js +++ b/test/utils/structs/CircularBuffer.test.js @@ -20,7 +20,7 @@ describe('CircularBuffer', function () { it('starts empty', async function () { expect(await this.mock.$count(0)).to.equal(0n); - expect(await this.mock.$size(0)).to.equal(LENGTH); + expect(await this.mock.$length(0)).to.equal(LENGTH); expect(await this.mock.$includes(0, ethers.ZeroHash)).to.be.false; await expect(this.mock.$last(0, 0)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS); }); @@ -38,7 +38,7 @@ describe('CircularBuffer', function () { const dropped = pushed.slice(0, -LENGTH); // check count - expect(await this.mock.$size(0)).to.equal(LENGTH); + expect(await this.mock.$length(0)).to.equal(LENGTH); expect(await this.mock.$count(0)).to.equal(stored.length); // check last @@ -65,14 +65,14 @@ describe('CircularBuffer', function () { await this.mock.$push(0, value); expect(await this.mock.$count(0)).to.equal(1n); - expect(await this.mock.$size(0)).to.equal(LENGTH); + expect(await this.mock.$length(0)).to.equal(LENGTH); expect(await this.mock.$includes(0, value)).to.be.true; await this.mock.$last(0, 0); // not revert await this.mock.$clear(0); expect(await this.mock.$count(0)).to.equal(0n); - expect(await this.mock.$size(0)).to.equal(LENGTH); + expect(await this.mock.$length(0)).to.equal(LENGTH); expect(await this.mock.$includes(0, value)).to.be.false; await expect(this.mock.$last(0, 0)).to.be.revertedWithPanic(PANIC_CODES.ARRAY_ACCESS_OUT_OF_BOUNDS); }); From 53d6aace2901386eb620bb08c7c51154042abcf2 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Mar 2024 23:25:52 +0100 Subject: [PATCH 10/13] Update contracts/utils/structs/CircularBuffer.sol MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Ernesto García --- contracts/utils/structs/CircularBuffer.sol | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol index b84750dd264..cca21ddcd6c 100644 --- a/contracts/utils/structs/CircularBuffer.sol +++ b/contracts/utils/structs/CircularBuffer.sol @@ -6,9 +6,13 @@ import {Arrays} from "../Arrays.sol"; import {Panic} from "../Panic.sol"; /** - * @dev A buffer of items of fixed size. When a new item is pushed, it takes the place of the oldest one in the buffer - * so that at all times, only the last N elements are kept. Items cannot be removed. The entier buffer can be reset. - * Last N elements can be accessed using their index from the end. + * @dev A fixed-length buffer for keeping `bytes32` items in storage. + * + * This data structure allows for pushing elements to it, and when its size exceeds the specified fixed length, + * new items take the place of the oldest element in the buffer, keeping at most `N` elements in the + * structure. + * + * Elements can't be removed but the data structure can be cleared. See {clear}. * * Complexity: * - insertion ({push}): O(1) From e5458fdb1e2940d7fc3c68b718e7aebbe79d6ad8 Mon Sep 17 00:00:00 2001 From: ernestognw Date: Mon, 22 Apr 2024 16:21:45 -0600 Subject: [PATCH 11/13] Nit --- contracts/utils/structs/CircularBuffer.sol | 29 ++++++++++++---------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol index cca21ddcd6c..4c4ed1bb951 100644 --- a/contracts/utils/structs/CircularBuffer.sol +++ b/contracts/utils/structs/CircularBuffer.sol @@ -6,9 +6,9 @@ import {Arrays} from "../Arrays.sol"; import {Panic} from "../Panic.sol"; /** - * @dev A fixed-length buffer for keeping `bytes32` items in storage. + * @dev A fixed-size buffer for keeping `bytes32` items in storage. * - * This data structure allows for pushing elements to it, and when its size exceeds the specified fixed length, + * This data structure allows for pushing elements to it, and when its length exceeds the specified fixed size, * new items take the place of the oldest element in the buffer, keeping at most `N` elements in the * structure. * @@ -28,7 +28,7 @@ import {Panic} from "../Panic.sol"; */ library CircularBuffer { /** - * @dev Counts the number of items that have been pushed to the buffer. The residu modulo _data.length indicates + * @dev Counts the number of items that have been pushed to the buffer. The residuo modulo _data.length indicates * where the next value should be stored. * * Struct members have an underscore prefix indicating that they are "private" and should not be read or written to @@ -44,9 +44,12 @@ library CircularBuffer { } /** - * @dev Initialize a new CircularBuffer of given length. + * @dev Initialize a new CircularBuffer of given size. * * If the CircularBuffer was already setup and used, calling that function again will reset it to a blank state. + * + * NOTE: The size of the buffer will affect the execution of {includes} function, as it has a complexity of O(N). + * Consider a large buffer size may render the function unusable. */ function setup(Bytes32CircularBuffer storage self, uint256 size) internal { clear(self); @@ -54,7 +57,7 @@ library CircularBuffer { } /** - * @dev Clear all data in the buffer, keeping the existing length. + * @dev Clear all data in the buffer without resetting memory, keeping the existing size. */ function clear(Bytes32CircularBuffer storage self) internal { self._count = 0; @@ -66,8 +69,8 @@ library CircularBuffer { */ function push(Bytes32CircularBuffer storage self, bytes32 value) internal { uint256 index = self._count++; - uint256 module = self._data.length; - Arrays.unsafeAccess(self._data, index % module).value = value; + uint256 modulus = self._data.length; + Arrays.unsafeAccess(self._data, index % modulus).value = value; } /** @@ -93,12 +96,12 @@ library CircularBuffer { */ function last(Bytes32CircularBuffer storage self, uint256 i) internal view returns (bytes32) { uint256 index = self._count; - uint256 module = self._data.length; - uint256 total = Math.min(index, module); // count(self) + uint256 modulus = self._data.length; + uint256 total = Math.min(index, modulus); // count(self) if (i >= total) { Panic.panic(Panic.ARRAY_OUT_OF_BOUNDS); } - return Arrays.unsafeAccess(self._data, (index - i - 1) % module).value; + return Arrays.unsafeAccess(self._data, (index - i - 1) % modulus).value; } /** @@ -106,10 +109,10 @@ library CircularBuffer { */ function includes(Bytes32CircularBuffer storage self, bytes32 value) internal view returns (bool) { uint256 index = self._count; - uint256 module = self._data.length; - uint256 total = Math.min(index, module); // count(self) + uint256 modulus = self._data.length; + uint256 total = Math.min(index, modulus); // count(self) for (uint256 i = 0; i < total; ++i) { - if (Arrays.unsafeAccess(self._data, (index - i - 1) % module).value == value) { + if (Arrays.unsafeAccess(self._data, (index - i - 1) % modulus).value == value) { return true; } } From f6d92b42219875987458863cd467f95434cef30d Mon Sep 17 00:00:00 2001 From: ernestognw Date: Mon, 22 Apr 2024 16:23:11 -0600 Subject: [PATCH 12/13] Improve example usage --- contracts/utils/structs/CircularBuffer.sol | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/contracts/utils/structs/CircularBuffer.sol b/contracts/utils/structs/CircularBuffer.sol index 4c4ed1bb951..be9af9f8cbd 100644 --- a/contracts/utils/structs/CircularBuffer.sol +++ b/contracts/utils/structs/CircularBuffer.sol @@ -22,8 +22,17 @@ import {Panic} from "../Panic.sol"; * * * The struct is called `Bytes32CircularBuffer`. Other types can be cast to and from `bytes32`. This data structure * can only be used in storage, and not in memory. + * + * Example usage: + * * ```solidity - * CircularBuffer.Bytes32CircularBuffer buffer; + * contract Example { + * // Add the library methods + * using CircularBuffer for CircularBuffer.Bytes32CircularBuffer; + * + * // Declare a buffer storage variable + * CircularBuffer.Bytes32CircularBuffer private myBuffer; + * } * ``` */ library CircularBuffer { From 64351ed0deabdcba552b2d6122226521344e6c6b Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 23 Apr 2024 14:33:57 +0200 Subject: [PATCH 13/13] fix generation --- scripts/generate/templates/Arrays.js | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/generate/templates/Arrays.js b/scripts/generate/templates/Arrays.js index bc279466b7f..6166d6f6de2 100644 --- a/scripts/generate/templates/Arrays.js +++ b/scripts/generate/templates/Arrays.js @@ -356,6 +356,7 @@ const unsafeSetLength = type => ` * WARNING: this does not clear elements if length is reduced, of initialize elements if length is increased. */ function unsafeSetLength(${type}[] storage array, uint256 len) internal { + /// @solidity memory-safe-assembly assembly { sstore(array.slot, len) }