From 09dfd7ea32183d68929ff9a025bfd4579dd10c77 Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Tue, 3 Jan 2023 18:40:15 +0400 Subject: [PATCH 01/20] Improve gas testing for AddressArray.get() method --- test/contracts/AddressArray.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/contracts/AddressArray.test.ts b/test/contracts/AddressArray.test.ts index cb559b35..86f9f72e 100644 --- a/test/contracts/AddressArray.test.ts +++ b/test/contracts/AddressArray.test.ts @@ -21,12 +21,14 @@ describe('AddressArray', function () { describe('length', function () { it('should calculate length 0', async function () { const { addressArrayMock } = await loadFixture(deployAddressArrayMock); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.length()); expect(await addressArrayMock.length()).to.be.equal('0'); }); it('should calculate length 1', async function () { const { addressArrayMock } = await loadFixture(deployAddressArrayMock); await addressArrayMock.push(signer1.address); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.length()); expect(await addressArrayMock.length()).to.be.equal('1'); }); }); @@ -57,12 +59,14 @@ describe('AddressArray', function () { describe('get', function () { it('should get empty array', async function () { const { addressArrayMock } = await loadFixture(deployAddressArrayMock); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); expect(await addressArrayMock.get()).to.be.deep.equal([]); }); it('should get array with 1 element', async function () { const { addressArrayMock } = await loadFixture(deployAddressArrayMock); await addressArrayMock.push(signer1.address); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); expect(await addressArrayMock.get()).to.be.deep.equal([signer1.address]); }); @@ -70,6 +74,7 @@ describe('AddressArray', function () { const { addressArrayMock } = await loadFixture(deployAddressArrayMock); await addressArrayMock.push(signer1.address); await addressArrayMock.push(signer2.address); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); expect(await addressArrayMock.get()).to.be.deep.equal([signer1.address, signer2.address]); }); @@ -78,6 +83,7 @@ describe('AddressArray', function () { await addressArrayMock.push(signer1.address); await addressArrayMock.push(signer2.address); await addressArrayMock.push(signer3.address); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); expect(await addressArrayMock.get()).to.be.deep.equal([signer1.address, signer2.address, signer3.address]); }); }); @@ -118,6 +124,7 @@ describe('AddressArray', function () { const { addressArrayMock } = await loadFixture(deployAddressArrayMock); await addressArrayMock.push(signer1.address); await addressArrayMock.pop(); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); expect(await addressArrayMock.get()).to.be.deep.equal([]); }); @@ -126,6 +133,7 @@ describe('AddressArray', function () { await addressArrayMock.push(signer1.address); await addressArrayMock.push(signer2.address); await addressArrayMock.pop(); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); expect(await addressArrayMock.get()).to.be.deep.equal([signer1.address]); }); @@ -135,6 +143,7 @@ describe('AddressArray', function () { await addressArrayMock.push(signer2.address); await addressArrayMock.push(signer3.address); await addressArrayMock.pop(); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); expect(await addressArrayMock.get()).to.be.deep.equal([signer1.address, signer2.address]); }); @@ -159,6 +168,7 @@ describe('AddressArray', function () { const { addressArrayMock } = await loadFixture(deployAddressArrayMock); await addressArrayMock.push(signer1.address); await addressArrayMock.set(0, signer2.address); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); expect(await addressArrayMock.get()).to.be.deep.equal([signer2.address]); }); @@ -167,6 +177,7 @@ describe('AddressArray', function () { await addressArrayMock.push(signer1.address); await addressArrayMock.push(signer2.address); await addressArrayMock.set(0, signer3.address); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); expect(await addressArrayMock.get()).to.be.deep.equal([signer3.address, signer2.address]); }); @@ -175,6 +186,8 @@ describe('AddressArray', function () { await addressArrayMock.push(signer1.address); await addressArrayMock.push(signer2.address); await addressArrayMock.set(1, signer3.address); + await signer1.sendTransaction(await addressArrayMock.populateTransaction.get()); + expect(await addressArrayMock.get()).to.be.deep.equal([signer1.address, signer3.address]); }); }); From d8d06a39de16588f91dd9cdb6af8991691b37078 Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Tue, 3 Jan 2023 19:48:20 +0400 Subject: [PATCH 02/20] Reimplement AddressArray in assembly --- contracts/libraries/AddressArray.sol | 188 +++++++++++++++++++-------- 1 file changed, 134 insertions(+), 54 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index baf0ed59..c98616c6 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -9,92 +9,172 @@ library AddressArray { error PopFromEmptyArray(); error OutputArrayTooSmall(); + uint256 internal constant ZERO_ADDRESS = 0x8000000000000000000000000000000000000000000000000000000000000000; // Next tx gas optimization + uint256 internal constant LENGTH_MASK = 0x0000000000000000ffffffff0000000000000000000000000000000000000000; + uint256 internal constant ADDRESS_MASK = 0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff; + uint256 internal constant ONE_LENGTH = 0x0000000000000000000000010000000000000000000000000000000000000000; + /// @dev Data struct containing raw mapping. struct Data { mapping(uint256 => uint256) _raw; } /// @dev Length of array. - function length(Data storage self) internal view returns (uint256) { - return self._raw[0] >> 160; + function length(Data storage self) internal view returns (uint256 res) { + mapping(uint256 => uint256) storage raw = self._raw; + /// @solidity memory-safe-assembly + assembly { // solhint-disable-line no-inline-assembly + mstore(0x20, raw.offset) + mstore(0x00, 0) + res := shr(160, and(sload(keccak256(0, 0x40)), LENGTH_MASK)) + } } /// @dev Returns data item from `self` storage at `i`. - function at(Data storage self, uint256 i) internal view returns (address) { - return address(uint160(self._raw[i])); + function at(Data storage self, uint256 i) internal view returns (address res) { + mapping(uint256 => uint256) storage raw = self._raw; + /// @solidity memory-safe-assembly + assembly { // solhint-disable-line no-inline-assembly + mstore(0x20, raw.offset) + mstore(0x00, i) + res := and(sload(keccak256(0, 0x40)), ADDRESS_MASK) + } } /// @dev Returns list of addresses from storage `self`. - function get(Data storage self) internal view returns (address[] memory arr) { - uint256 lengthAndFirst = self._raw[0]; - arr = new address[](lengthAndFirst >> 160); - _get(self, arr, lengthAndFirst); + function get(Data storage self) internal view returns (address[] memory output) { + mapping(uint256 => uint256) storage raw = self._raw; + /// @solidity memory-safe-assembly + assembly { // solhint-disable-line no-inline-assembly + mstore(0x20, raw.offset) + mstore(0x00, 0) + let lengthAndFirst := sload(keccak256(0, 0x40)) + let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let fst := and(lengthAndFirst, ADDRESS_MASK) + + // Allocate array + output := mload(0x40) + mstore(0x40, add(output, mul(0x20, add(1, len)))) + mstore(output, len) + + // Copy first element and the rest in loop + let ptr := add(output, 0x20) + mstore(ptr, fst) + for { let i := 1 } lt(i, len) { i:= add(i, 1) } { + mstore(0x00, i) + let item := sload(keccak256(0, 0x40)) + mstore(add(ptr, mul(0x20, i)), item) + } + } } /// @dev Puts list of addresses from `self` storage into `output` array. - function get(Data storage self, address[] memory output) internal view returns (address[] memory) { - return _get(self, output, self._raw[0]); - } + function get(Data storage self, address[] memory input) internal view returns (address[] memory output) { + output = input; + mapping(uint256 => uint256) storage raw = self._raw; + bool exception; + /// @solidity memory-safe-assembly + assembly { // solhint-disable-line no-inline-assembly + mstore(0x20, raw.offset) + mstore(0x00, 0) + let lengthAndFirst := sload(keccak256(0, 0x40)) + let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let fst := and(lengthAndFirst, ADDRESS_MASK) + + if gt(len, mload(input)) { + exception := true + } - function _get( - Data storage self, - address[] memory output, - uint256 lengthAndFirst - ) private view returns (address[] memory) { - uint256 len = lengthAndFirst >> 160; - if (len > output.length) revert OutputArrayTooSmall(); - if (len > 0) { - output[0] = address(uint160(lengthAndFirst)); - unchecked { - for (uint256 i = 1; i < len; i++) { - output[i] = address(uint160(self._raw[i])); - } + // Copy first element and the rest in loop + let ptr := add(output, 0x20) + mstore(ptr, fst) + for { let i := 1 } lt(i, len) { i:= add(i, 1) } { + mstore(0x00, i) + let item := and(sload(keccak256(0, 0x40)), ADDRESS_MASK) + mstore(add(ptr, mul(0x20, i)), item) } } - return output; + if (exception) revert OutputArrayTooSmall(); } /// @dev Array push back `account` operation on storage `self`. - function push(Data storage self, address account) internal returns (uint256) { - unchecked { - uint256 lengthAndFirst = self._raw[0]; - uint256 len = lengthAndFirst >> 160; - if (len == 0) { - self._raw[0] = (1 << 160) + uint160(account); - } else { - self._raw[0] = lengthAndFirst + (1 << 160); - self._raw[len] = uint160(account); + function push(Data storage self, address account) internal returns (uint256 res) { + mapping(uint256 => uint256) storage raw = self._raw; + /// @solidity memory-safe-assembly + assembly { // solhint-disable-line no-inline-assembly + mstore(0x20, raw.offset) + mstore(0x00, 0) + let sptr := keccak256(0, 0x40) + let lengthAndFirst := sload(sptr) + let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let fst := and(lengthAndFirst, ADDRESS_MASK) + + switch len + case 0 { + sstore(sptr, or(or(account, ONE_LENGTH), ZERO_ADDRESS)) + } + default { + sstore(sptr, add(lengthAndFirst, ONE_LENGTH)) + mstore(0x00, len) + sstore(keccak256(0, 0x40), or(account, ZERO_ADDRESS)) } - return len + 1; + res := add(len, 1) } } /// @dev Array pop back operation for storage `self`. function pop(Data storage self) internal { - unchecked { - uint256 lengthAndFirst = self._raw[0]; - uint256 len = lengthAndFirst >> 160; - if (len == 0) revert PopFromEmptyArray(); - self._raw[len - 1] = 0; - if (len > 1) { - self._raw[0] = lengthAndFirst - (1 << 160); + mapping(uint256 => uint256) storage raw = self._raw; + bool exception; + /// @solidity memory-safe-assembly + assembly { // solhint-disable-line no-inline-assembly + mstore(0x20, raw.offset) + mstore(0x00, 0) + let sptr := keccak256(0, 0x40) + let lengthAndFirst := sload(sptr) + let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let fst := and(lengthAndFirst, ADDRESS_MASK) + + switch len + case 0 { + exception := true + } + case 1 { + sstore(sptr, ZERO_ADDRESS) + } + default { + sstore(sptr, sub(lengthAndFirst, ONE_LENGTH)) } } + if (exception) revert PopFromEmptyArray(); } /// @dev Set element for storage `self` at `index` to `account`. - function set( - Data storage self, - uint256 index, - address account - ) internal { - uint256 len = length(self); - if (index >= len) revert IndexOutOfBounds(); - - if (index == 0) { - self._raw[0] = (len << 160) | uint160(account); - } else { - self._raw[index] = uint160(account); + function set(Data storage self, uint256 index, address account) internal { + mapping(uint256 => uint256) storage raw = self._raw; + bool exception; + /// @solidity memory-safe-assembly + assembly { // solhint-disable-line no-inline-assembly + mstore(0x20, raw.offset) + mstore(0x00, 0) + let sptr := keccak256(0, 0x40) + let lengthAndFirst := sload(sptr) + let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let fst := and(lengthAndFirst, ADDRESS_MASK) + + if iszero(lt(index, len)) { + exception := true + } + + switch index + case 0 { + sstore(sptr, or(xor(lengthAndFirst, fst), account)) + } + default { + mstore(0x00, index) + sstore(keccak256(0, 0x40), or(account, ZERO_ADDRESS)) + } } + if (exception) revert IndexOutOfBounds(); } } From 9dd72da399bc93b9d3a00ead55ab4eae73dcc830 Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Tue, 3 Jan 2023 19:58:30 +0400 Subject: [PATCH 03/20] Reimplement AddressArray on static array --- contracts/libraries/AddressArray.sol | 70 ++++++++++------------------ 1 file changed, 25 insertions(+), 45 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index c98616c6..686007c3 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -16,39 +16,34 @@ library AddressArray { /// @dev Data struct containing raw mapping. struct Data { - mapping(uint256 => uint256) _raw; + uint256[1 << 32] _raw; } /// @dev Length of array. function length(Data storage self) internal view returns (uint256 res) { - mapping(uint256 => uint256) storage raw = self._raw; + uint256[1 << 32] storage raw = self._raw; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - mstore(0x20, raw.offset) - mstore(0x00, 0) - res := shr(160, and(sload(keccak256(0, 0x40)), LENGTH_MASK)) + res := shr(160, and(sload(raw.offset), LENGTH_MASK)) } } /// @dev Returns data item from `self` storage at `i`. function at(Data storage self, uint256 i) internal view returns (address res) { - mapping(uint256 => uint256) storage raw = self._raw; + if (i >= 1 << 32) revert IndexOutOfBounds(); + uint256[1 << 32] storage raw = self._raw; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - mstore(0x20, raw.offset) - mstore(0x00, i) - res := and(sload(keccak256(0, 0x40)), ADDRESS_MASK) + res := and(sload(add(raw.offset, i)), ADDRESS_MASK) } } /// @dev Returns list of addresses from storage `self`. function get(Data storage self) internal view returns (address[] memory output) { - mapping(uint256 => uint256) storage raw = self._raw; + uint256[1 << 32] storage raw = self._raw; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - mstore(0x20, raw.offset) - mstore(0x00, 0) - let lengthAndFirst := sload(keccak256(0, 0x40)) + let lengthAndFirst := sload(raw.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) @@ -61,8 +56,7 @@ library AddressArray { let ptr := add(output, 0x20) mstore(ptr, fst) for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - mstore(0x00, i) - let item := sload(keccak256(0, 0x40)) + let item := sload(add(raw.offset, i)) mstore(add(ptr, mul(0x20, i)), item) } } @@ -71,13 +65,11 @@ library AddressArray { /// @dev Puts list of addresses from `self` storage into `output` array. function get(Data storage self, address[] memory input) internal view returns (address[] memory output) { output = input; - mapping(uint256 => uint256) storage raw = self._raw; + uint256[1 << 32] storage raw = self._raw; bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - mstore(0x20, raw.offset) - mstore(0x00, 0) - let lengthAndFirst := sload(keccak256(0, 0x40)) + let lengthAndFirst := sload(raw.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) @@ -89,8 +81,7 @@ library AddressArray { let ptr := add(output, 0x20) mstore(ptr, fst) for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - mstore(0x00, i) - let item := and(sload(keccak256(0, 0x40)), ADDRESS_MASK) + let item := and(sload(add(raw.offset, i)), ADDRESS_MASK) mstore(add(ptr, mul(0x20, i)), item) } } @@ -99,24 +90,20 @@ library AddressArray { /// @dev Array push back `account` operation on storage `self`. function push(Data storage self, address account) internal returns (uint256 res) { - mapping(uint256 => uint256) storage raw = self._raw; + uint256[1 << 32] storage raw = self._raw; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - mstore(0x20, raw.offset) - mstore(0x00, 0) - let sptr := keccak256(0, 0x40) - let lengthAndFirst := sload(sptr) + let lengthAndFirst := sload(raw.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) switch len case 0 { - sstore(sptr, or(or(account, ONE_LENGTH), ZERO_ADDRESS)) + sstore(raw.offset, or(or(account, ONE_LENGTH), ZERO_ADDRESS)) } default { - sstore(sptr, add(lengthAndFirst, ONE_LENGTH)) - mstore(0x00, len) - sstore(keccak256(0, 0x40), or(account, ZERO_ADDRESS)) + sstore(raw.offset, add(lengthAndFirst, ONE_LENGTH)) + sstore(add(raw.offset, len), or(account, ZERO_ADDRESS)) } res := add(len, 1) } @@ -124,14 +111,11 @@ library AddressArray { /// @dev Array pop back operation for storage `self`. function pop(Data storage self) internal { - mapping(uint256 => uint256) storage raw = self._raw; + uint256[1 << 32] storage raw = self._raw; bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - mstore(0x20, raw.offset) - mstore(0x00, 0) - let sptr := keccak256(0, 0x40) - let lengthAndFirst := sload(sptr) + let lengthAndFirst := sload(raw.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) @@ -140,10 +124,10 @@ library AddressArray { exception := true } case 1 { - sstore(sptr, ZERO_ADDRESS) + sstore(raw.offset, ZERO_ADDRESS) } default { - sstore(sptr, sub(lengthAndFirst, ONE_LENGTH)) + sstore(raw.offset, sub(lengthAndFirst, ONE_LENGTH)) } } if (exception) revert PopFromEmptyArray(); @@ -151,14 +135,11 @@ library AddressArray { /// @dev Set element for storage `self` at `index` to `account`. function set(Data storage self, uint256 index, address account) internal { - mapping(uint256 => uint256) storage raw = self._raw; + uint256[1 << 32] storage raw = self._raw; bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - mstore(0x20, raw.offset) - mstore(0x00, 0) - let sptr := keccak256(0, 0x40) - let lengthAndFirst := sload(sptr) + let lengthAndFirst := sload(raw.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) @@ -168,11 +149,10 @@ library AddressArray { switch index case 0 { - sstore(sptr, or(xor(lengthAndFirst, fst), account)) + sstore(raw.offset, or(xor(lengthAndFirst, fst), account)) } default { - mstore(0x00, index) - sstore(keccak256(0, 0x40), or(account, ZERO_ADDRESS)) + sstore(add(raw.offset, index), or(account, ZERO_ADDRESS)) } } if (exception) revert IndexOutOfBounds(); From eecf410ddbe7ce6d732860c652e146f74e56b75b Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Tue, 3 Jan 2023 20:39:37 +0400 Subject: [PATCH 04/20] Introduce AddressArray.erase() --- contracts/libraries/AddressArray.sol | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 686007c3..0536329b 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -157,4 +157,13 @@ library AddressArray { } if (exception) revert IndexOutOfBounds(); } + + /// @dev Erase length of the array + function erase(Data storage self) internal { + uint256[1 << 32] storage raw = self._raw; + /// @solidity memory-safe-assembly + assembly { // solhint-disable-line no-inline-assembly + sstore(raw.offset, ADDRESS_MASK) + } + } } From 204a2fc43b2fa8e4c6ab38d2741d91dcd5734f22 Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Tue, 3 Jan 2023 20:41:22 +0400 Subject: [PATCH 05/20] Refactor storage assess --- contracts/libraries/AddressArray.sol | 42 +++++++++++----------------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 0536329b..95901483 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -21,29 +21,26 @@ library AddressArray { /// @dev Length of array. function length(Data storage self) internal view returns (uint256 res) { - uint256[1 << 32] storage raw = self._raw; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - res := shr(160, and(sload(raw.offset), LENGTH_MASK)) + res := shr(160, and(sload(self.offset), LENGTH_MASK)) } } /// @dev Returns data item from `self` storage at `i`. function at(Data storage self, uint256 i) internal view returns (address res) { if (i >= 1 << 32) revert IndexOutOfBounds(); - uint256[1 << 32] storage raw = self._raw; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - res := and(sload(add(raw.offset, i)), ADDRESS_MASK) + res := and(sload(add(self.offset, i)), ADDRESS_MASK) } } /// @dev Returns list of addresses from storage `self`. function get(Data storage self) internal view returns (address[] memory output) { - uint256[1 << 32] storage raw = self._raw; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(raw.offset) + let lengthAndFirst := sload(self.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) @@ -56,7 +53,7 @@ library AddressArray { let ptr := add(output, 0x20) mstore(ptr, fst) for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - let item := sload(add(raw.offset, i)) + let item := sload(add(self.offset, i)) mstore(add(ptr, mul(0x20, i)), item) } } @@ -65,11 +62,10 @@ library AddressArray { /// @dev Puts list of addresses from `self` storage into `output` array. function get(Data storage self, address[] memory input) internal view returns (address[] memory output) { output = input; - uint256[1 << 32] storage raw = self._raw; bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(raw.offset) + let lengthAndFirst := sload(self.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) @@ -81,7 +77,7 @@ library AddressArray { let ptr := add(output, 0x20) mstore(ptr, fst) for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - let item := and(sload(add(raw.offset, i)), ADDRESS_MASK) + let item := and(sload(add(self.offset, i)), ADDRESS_MASK) mstore(add(ptr, mul(0x20, i)), item) } } @@ -90,20 +86,19 @@ library AddressArray { /// @dev Array push back `account` operation on storage `self`. function push(Data storage self, address account) internal returns (uint256 res) { - uint256[1 << 32] storage raw = self._raw; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(raw.offset) + let lengthAndFirst := sload(self.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) switch len case 0 { - sstore(raw.offset, or(or(account, ONE_LENGTH), ZERO_ADDRESS)) + sstore(self.offset, or(or(account, ONE_LENGTH), ZERO_ADDRESS)) } default { - sstore(raw.offset, add(lengthAndFirst, ONE_LENGTH)) - sstore(add(raw.offset, len), or(account, ZERO_ADDRESS)) + sstore(self.offset, add(lengthAndFirst, ONE_LENGTH)) + sstore(add(self.offset, len), or(account, ZERO_ADDRESS)) } res := add(len, 1) } @@ -111,11 +106,10 @@ library AddressArray { /// @dev Array pop back operation for storage `self`. function pop(Data storage self) internal { - uint256[1 << 32] storage raw = self._raw; bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(raw.offset) + let lengthAndFirst := sload(self.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) @@ -124,10 +118,10 @@ library AddressArray { exception := true } case 1 { - sstore(raw.offset, ZERO_ADDRESS) + sstore(self.offset, ZERO_ADDRESS) } default { - sstore(raw.offset, sub(lengthAndFirst, ONE_LENGTH)) + sstore(self.offset, sub(lengthAndFirst, ONE_LENGTH)) } } if (exception) revert PopFromEmptyArray(); @@ -135,11 +129,10 @@ library AddressArray { /// @dev Set element for storage `self` at `index` to `account`. function set(Data storage self, uint256 index, address account) internal { - uint256[1 << 32] storage raw = self._raw; bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(raw.offset) + let lengthAndFirst := sload(self.offset) let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) @@ -149,10 +142,10 @@ library AddressArray { switch index case 0 { - sstore(raw.offset, or(xor(lengthAndFirst, fst), account)) + sstore(self.offset, or(xor(lengthAndFirst, fst), account)) } default { - sstore(add(raw.offset, index), or(account, ZERO_ADDRESS)) + sstore(add(self.offset, index), or(account, ZERO_ADDRESS)) } } if (exception) revert IndexOutOfBounds(); @@ -160,10 +153,9 @@ library AddressArray { /// @dev Erase length of the array function erase(Data storage self) internal { - uint256[1 << 32] storage raw = self._raw; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - sstore(raw.offset, ADDRESS_MASK) + sstore(self.offset, ADDRESS_MASK) } } } From d028998262b6b077e6e330c6ec7e251be2874b65 Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Tue, 3 Jan 2023 20:51:15 +0400 Subject: [PATCH 06/20] Introduce AddressSet.erase() --- contracts/libraries/AddressSet.sol | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/contracts/libraries/AddressSet.sol b/contracts/libraries/AddressSet.sol index c002fb63..9abaf65c 100644 --- a/contracts/libraries/AddressSet.sol +++ b/contracts/libraries/AddressSet.sol @@ -63,4 +63,18 @@ library AddressSet { delete s.lookup[item]; return true; } + + /// @dev Erases set from storage `s` and returns all removed items + function erase(Data storage s) internal returns(address[] memory items) { + items = s.items.get(); + uint256 len = items.length; + if (len > 0) { + s.items.erase(); + unchecked { + for (uint256 i = 0; i < len; i++) { + delete s.lookup[items[i]]; + } + } + } + } } From 8ef4fe4e763771e5a59c162442433ed7f58e81cd Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Tue, 3 Jan 2023 21:04:29 +0400 Subject: [PATCH 07/20] Optimize AddressSet.remove() --- contracts/libraries/AddressArray.sol | 4 +++- contracts/libraries/AddressSet.sol | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 95901483..389dd32d 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -105,7 +105,7 @@ library AddressArray { } /// @dev Array pop back operation for storage `self`. - function pop(Data storage self) internal { + function pop(Data storage self) internal returns(address res) { bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly @@ -118,9 +118,11 @@ library AddressArray { exception := true } case 1 { + res := and(sload(self.offset), ADDRESS_MASK) sstore(self.offset, ZERO_ADDRESS) } default { + res := and(sload(add(self.offset, sub(len, 1))), ADDRESS_MASK) sstore(self.offset, sub(lengthAndFirst, ONE_LENGTH)) } } diff --git a/contracts/libraries/AddressSet.sol b/contracts/libraries/AddressSet.sol index 9abaf65c..100cab98 100644 --- a/contracts/libraries/AddressSet.sol +++ b/contracts/libraries/AddressSet.sol @@ -49,18 +49,18 @@ library AddressSet { /// @dev Removes `item` from storage `s` and returns true if successful. function remove(Data storage s, address item) internal returns (bool) { uint256 index = s.lookup[item]; + delete s.lookup[item]; if (index == 0) { return false; } + + address lastItem = s.items.pop(); if (index < s.items.length()) { unchecked { - address lastItem = s.items.at(s.items.length() - 1); s.items.set(index - 1, lastItem); s.lookup[lastItem] = index; } } - s.items.pop(); - delete s.lookup[item]; return true; } From 17eb49b3470343fd85e9ecca58fd607c79683fd4 Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Tue, 3 Jan 2023 21:11:53 +0400 Subject: [PATCH 08/20] Refactor AddressArray --- contracts/libraries/AddressArray.sol | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 389dd32d..0dac759d 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -13,6 +13,7 @@ library AddressArray { uint256 internal constant LENGTH_MASK = 0x0000000000000000ffffffff0000000000000000000000000000000000000000; uint256 internal constant ADDRESS_MASK = 0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff; uint256 internal constant ONE_LENGTH = 0x0000000000000000000000010000000000000000000000000000000000000000; + uint256 internal constant LENGTH_OFFSET = 160; /// @dev Data struct containing raw mapping. struct Data { @@ -23,7 +24,7 @@ library AddressArray { function length(Data storage self) internal view returns (uint256 res) { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - res := shr(160, and(sload(self.offset), LENGTH_MASK)) + res := shr(LENGTH_OFFSET, and(sload(self.offset), LENGTH_MASK)) } } @@ -41,7 +42,7 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) // Allocate array @@ -66,7 +67,7 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) if gt(len, mload(input)) { @@ -89,7 +90,7 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) switch len @@ -110,7 +111,7 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) switch len @@ -135,7 +136,7 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(160, and(lengthAndFirst, LENGTH_MASK)) + let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) let fst := and(lengthAndFirst, ADDRESS_MASK) if iszero(lt(index, len)) { From b730b96195e3ada9ff94ed99974bcdd7a716391e Mon Sep 17 00:00:00 2001 From: Anton Bukov Date: Wed, 4 Jan 2023 11:14:22 +0100 Subject: [PATCH 09/20] Optimize AddressSet for no gas refund --- contracts/libraries/AddressSet.sol | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/contracts/libraries/AddressSet.sol b/contracts/libraries/AddressSet.sol index 100cab98..7651072e 100644 --- a/contracts/libraries/AddressSet.sol +++ b/contracts/libraries/AddressSet.sol @@ -14,9 +14,9 @@ import "./AddressArray.sol"; library AddressSet { using AddressArray for AddressArray.Data; - /** @dev Data struct from AddressArray.Data items - * and lookup mapping address => index in data array. - */ + uint256 internal constant NULL_INDEX = type(uint256).max; + + /// @dev Data struct from AddressArray.Data items and lookup mapping address => index in data array. struct Data { AddressArray.Data items; mapping(address => uint256) lookup; @@ -34,12 +34,14 @@ library AddressSet { /// @dev Returns true if storage `s` has `item`. function contains(Data storage s, address item) internal view returns (bool) { - return s.lookup[item] != 0; + uint256 index = s.lookup[item]; + return index != 0 && index != NULL_INDEX; } /// @dev Adds `item` into storage `s` and returns true if successful. function add(Data storage s, address item) internal returns (bool) { - if (s.lookup[item] > 0) { + uint256 index = s.lookup[item]; + if (index != 0 && index != NULL_INDEX) { return false; } s.lookup[item] = s.items.push(item); @@ -49,8 +51,8 @@ library AddressSet { /// @dev Removes `item` from storage `s` and returns true if successful. function remove(Data storage s, address item) internal returns (bool) { uint256 index = s.lookup[item]; - delete s.lookup[item]; - if (index == 0) { + s.lookup[item] = NULL_INDEX; + if (index == 0 || index == NULL_INDEX) { return false; } @@ -72,7 +74,7 @@ library AddressSet { s.items.erase(); unchecked { for (uint256 i = 0; i < len; i++) { - delete s.lookup[items[i]]; + s.lookup[items[i]] = NULL_INDEX; } } } From 8b3d6af2f0ffac1ce189654323fb33aaedcca0c2 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 12 Jan 2023 14:22:21 +0400 Subject: [PATCH 10/20] linter --- contracts/libraries/AddressArray.sol | 52 +++++++++++++--------------- contracts/libraries/AddressSet.sol | 12 +++---- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 0dac759d..41c21d88 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -9,11 +9,11 @@ library AddressArray { error PopFromEmptyArray(); error OutputArrayTooSmall(); - uint256 internal constant ZERO_ADDRESS = 0x8000000000000000000000000000000000000000000000000000000000000000; // Next tx gas optimization - uint256 internal constant LENGTH_MASK = 0x0000000000000000ffffffff0000000000000000000000000000000000000000; - uint256 internal constant ADDRESS_MASK = 0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff; - uint256 internal constant ONE_LENGTH = 0x0000000000000000000000010000000000000000000000000000000000000000; - uint256 internal constant LENGTH_OFFSET = 160; + uint256 internal constant _ZERO_ADDRESS = 0x8000000000000000000000000000000000000000000000000000000000000000; // Next tx gas optimization + uint256 internal constant _LENGTH_MASK = 0x0000000000000000ffffffff0000000000000000000000000000000000000000; + uint256 internal constant _ADDRESS_MASK = 0x000000000000000000000000ffffffffffffffffffffffffffffffffffffffff; + uint256 internal constant _ONE_LENGTH = 0x0000000000000000000000010000000000000000000000000000000000000000; + uint256 internal constant _LENGTH_OFFSET = 160; /// @dev Data struct containing raw mapping. struct Data { @@ -24,7 +24,7 @@ library AddressArray { function length(Data storage self) internal view returns (uint256 res) { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - res := shr(LENGTH_OFFSET, and(sload(self.offset), LENGTH_MASK)) + res := shr(_LENGTH_OFFSET, and(sload(self.offset), _LENGTH_MASK)) } } @@ -33,7 +33,7 @@ library AddressArray { if (i >= 1 << 32) revert IndexOutOfBounds(); /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - res := and(sload(add(self.offset, i)), ADDRESS_MASK) + res := and(sload(add(self.offset, i)), _ADDRESS_MASK) } } @@ -42,8 +42,8 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) - let fst := and(lengthAndFirst, ADDRESS_MASK) + let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) + let fst := and(lengthAndFirst, _ADDRESS_MASK) // Allocate array output := mload(0x40) @@ -67,8 +67,8 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) - let fst := and(lengthAndFirst, ADDRESS_MASK) + let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) + let fst := and(lengthAndFirst, _ADDRESS_MASK) if gt(len, mload(input)) { exception := true @@ -78,7 +78,7 @@ library AddressArray { let ptr := add(output, 0x20) mstore(ptr, fst) for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - let item := and(sload(add(self.offset, i)), ADDRESS_MASK) + let item := and(sload(add(self.offset, i)), _ADDRESS_MASK) mstore(add(ptr, mul(0x20, i)), item) } } @@ -90,16 +90,15 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) - let fst := and(lengthAndFirst, ADDRESS_MASK) + let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) switch len case 0 { - sstore(self.offset, or(or(account, ONE_LENGTH), ZERO_ADDRESS)) + sstore(self.offset, or(or(account, _ONE_LENGTH), _ZERO_ADDRESS)) } default { - sstore(self.offset, add(lengthAndFirst, ONE_LENGTH)) - sstore(add(self.offset, len), or(account, ZERO_ADDRESS)) + sstore(self.offset, add(lengthAndFirst, _ONE_LENGTH)) + sstore(add(self.offset, len), or(account, _ZERO_ADDRESS)) } res := add(len, 1) } @@ -111,20 +110,19 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) - let fst := and(lengthAndFirst, ADDRESS_MASK) + let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) switch len case 0 { exception := true } case 1 { - res := and(sload(self.offset), ADDRESS_MASK) - sstore(self.offset, ZERO_ADDRESS) + res := and(sload(self.offset), _ADDRESS_MASK) + sstore(self.offset, _ZERO_ADDRESS) } default { - res := and(sload(add(self.offset, sub(len, 1))), ADDRESS_MASK) - sstore(self.offset, sub(lengthAndFirst, ONE_LENGTH)) + res := and(sload(add(self.offset, sub(len, 1))), _ADDRESS_MASK) + sstore(self.offset, sub(lengthAndFirst, _ONE_LENGTH)) } } if (exception) revert PopFromEmptyArray(); @@ -136,8 +134,8 @@ library AddressArray { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.offset) - let len := shr(LENGTH_OFFSET, and(lengthAndFirst, LENGTH_MASK)) - let fst := and(lengthAndFirst, ADDRESS_MASK) + let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) + let fst := and(lengthAndFirst, _ADDRESS_MASK) if iszero(lt(index, len)) { exception := true @@ -148,7 +146,7 @@ library AddressArray { sstore(self.offset, or(xor(lengthAndFirst, fst), account)) } default { - sstore(add(self.offset, index), or(account, ZERO_ADDRESS)) + sstore(add(self.offset, index), or(account, _ZERO_ADDRESS)) } } if (exception) revert IndexOutOfBounds(); @@ -158,7 +156,7 @@ library AddressArray { function erase(Data storage self) internal { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - sstore(self.offset, ADDRESS_MASK) + sstore(self.offset, _ADDRESS_MASK) } } } diff --git a/contracts/libraries/AddressSet.sol b/contracts/libraries/AddressSet.sol index 7651072e..a4923aed 100644 --- a/contracts/libraries/AddressSet.sol +++ b/contracts/libraries/AddressSet.sol @@ -14,7 +14,7 @@ import "./AddressArray.sol"; library AddressSet { using AddressArray for AddressArray.Data; - uint256 internal constant NULL_INDEX = type(uint256).max; + uint256 internal constant _NULL_INDEX = type(uint256).max; /// @dev Data struct from AddressArray.Data items and lookup mapping address => index in data array. struct Data { @@ -35,13 +35,13 @@ library AddressSet { /// @dev Returns true if storage `s` has `item`. function contains(Data storage s, address item) internal view returns (bool) { uint256 index = s.lookup[item]; - return index != 0 && index != NULL_INDEX; + return index != 0 && index != _NULL_INDEX; } /// @dev Adds `item` into storage `s` and returns true if successful. function add(Data storage s, address item) internal returns (bool) { uint256 index = s.lookup[item]; - if (index != 0 && index != NULL_INDEX) { + if (index != 0 && index != _NULL_INDEX) { return false; } s.lookup[item] = s.items.push(item); @@ -51,8 +51,8 @@ library AddressSet { /// @dev Removes `item` from storage `s` and returns true if successful. function remove(Data storage s, address item) internal returns (bool) { uint256 index = s.lookup[item]; - s.lookup[item] = NULL_INDEX; - if (index == 0 || index == NULL_INDEX) { + s.lookup[item] = _NULL_INDEX; + if (index == 0 || index == _NULL_INDEX) { return false; } @@ -74,7 +74,7 @@ library AddressSet { s.items.erase(); unchecked { for (uint256 i = 0; i < len; i++) { - s.lookup[items[i]] = NULL_INDEX; + s.lookup[items[i]] = _NULL_INDEX; } } } From 4bbdcb936bb93da3ecc5b03f8f3c0c88b02602cd Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Wed, 20 Mar 2024 14:51:10 +0400 Subject: [PATCH 11/20] remove some assembly with no gas difference --- contracts/libraries/AddressArray.sol | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 96b8443b..5209388e 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -20,20 +20,14 @@ library AddressArray { } /// @dev Length of array. - function length(Data storage self) internal view returns (uint256 res) { - /// @solidity memory-safe-assembly - assembly { // solhint-disable-line no-inline-assembly - res := shr(_LENGTH_OFFSET, and(sload(self.offset), _LENGTH_MASK)) - } + function length(Data storage self) internal view returns (uint256) { + return (self._raw[0] & _LENGTH_MASK) >> _LENGTH_OFFSET; } /// @dev Returns data item from `self` storage at `i`. - function at(Data storage self, uint256 i) internal view returns (address res) { + function at(Data storage self, uint256 i) internal view returns (address) { if (i >= 1 << 32) revert IndexOutOfBounds(); - /// @solidity memory-safe-assembly - assembly { // solhint-disable-line no-inline-assembly - res := and(sload(add(self.offset, i)), _ADDRESS_MASK) - } + return address(uint160(self._raw[i] & _ADDRESS_MASK)); } /// @dev Returns list of addresses from storage `self`. From 20eecb46b46090ca569ef99b98e977408e3ce777 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Wed, 20 Mar 2024 17:24:28 +0400 Subject: [PATCH 12/20] fix assembly --- contracts/libraries/AddressArray.sol | 34 ++++++++++++++-------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 5209388e..3b213be9 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -34,7 +34,7 @@ library AddressArray { function get(Data storage self) internal view returns (address[] memory output) { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(self.offset) + let lengthAndFirst := sload(self.slot) let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) let fst := and(lengthAndFirst, _ADDRESS_MASK) @@ -47,7 +47,7 @@ library AddressArray { let ptr := add(output, 0x20) mstore(ptr, fst) for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - let item := sload(add(self.offset, i)) + let item := sload(add(self.slot, i)) mstore(add(ptr, mul(0x20, i)), item) } } @@ -59,7 +59,7 @@ library AddressArray { bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(self.offset) + let lengthAndFirst := sload(self.slot) let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) let fst := and(lengthAndFirst, _ADDRESS_MASK) @@ -71,7 +71,7 @@ library AddressArray { let ptr := add(output, 0x20) mstore(ptr, fst) for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - let item := and(sload(add(self.offset, i)), _ADDRESS_MASK) + let item := and(sload(add(self.slot, i)), _ADDRESS_MASK) mstore(add(ptr, mul(0x20, i)), item) } } @@ -82,16 +82,16 @@ library AddressArray { function push(Data storage self, address account) internal returns (uint256 res) { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(self.offset) + let lengthAndFirst := sload(self.slot) let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) switch len case 0 { - sstore(self.offset, or(or(account, _ONE_LENGTH), _ZERO_ADDRESS)) + sstore(self.slot, or(or(account, _ONE_LENGTH), _ZERO_ADDRESS)) } default { - sstore(self.offset, add(lengthAndFirst, _ONE_LENGTH)) - sstore(add(self.offset, len), or(account, _ZERO_ADDRESS)) + sstore(self.slot, add(lengthAndFirst, _ONE_LENGTH)) + sstore(add(self.slot, len), or(account, _ZERO_ADDRESS)) } res := add(len, 1) } @@ -102,7 +102,7 @@ library AddressArray { bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(self.offset) + let lengthAndFirst := sload(self.slot) let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) switch len @@ -110,12 +110,12 @@ library AddressArray { exception := true } case 1 { - res := and(sload(self.offset), _ADDRESS_MASK) - sstore(self.offset, _ZERO_ADDRESS) + res := and(sload(self.slot), _ADDRESS_MASK) + sstore(self.slot, _ZERO_ADDRESS) } default { - res := and(sload(add(self.offset, sub(len, 1))), _ADDRESS_MASK) - sstore(self.offset, sub(lengthAndFirst, _ONE_LENGTH)) + res := and(sload(add(self.slot, sub(len, 1))), _ADDRESS_MASK) + sstore(self.slot, sub(lengthAndFirst, _ONE_LENGTH)) } } if (exception) revert PopFromEmptyArray(); @@ -126,7 +126,7 @@ library AddressArray { bool exception; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - let lengthAndFirst := sload(self.offset) + let lengthAndFirst := sload(self.slot) let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) let fst := and(lengthAndFirst, _ADDRESS_MASK) @@ -136,10 +136,10 @@ library AddressArray { switch index case 0 { - sstore(self.offset, or(xor(lengthAndFirst, fst), account)) + sstore(self.slot, or(xor(lengthAndFirst, fst), account)) } default { - sstore(add(self.offset, index), or(account, _ZERO_ADDRESS)) + sstore(add(self.slot, index), or(account, _ZERO_ADDRESS)) } } if (exception) revert IndexOutOfBounds(); @@ -149,7 +149,7 @@ library AddressArray { function erase(Data storage self) internal { /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly - sstore(self.offset, _ADDRESS_MASK) + sstore(self.slot, _ADDRESS_MASK) } } } From 58ac66850dd2bb9239bdae649fba6c1581ed1296 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 21 Mar 2024 13:36:55 +0400 Subject: [PATCH 13/20] fix memory safety --- contracts/libraries/AddressArray.sol | 35 ++++++++++++++++------------ 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 3b213be9..fb7f037b 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -43,12 +43,14 @@ library AddressArray { mstore(0x40, add(output, mul(0x20, add(1, len)))) mstore(output, len) - // Copy first element and the rest in loop - let ptr := add(output, 0x20) - mstore(ptr, fst) - for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - let item := sload(add(self.slot, i)) - mstore(add(ptr, mul(0x20, i)), item) + if len { + // Copy first element and then the rest in a loop + let ptr := add(output, 0x20) + mstore(ptr, fst) + for { let i := 1 } lt(i, len) { i:= add(i, 1) } { + let item := sload(add(self.slot, i)) + mstore(add(ptr, mul(0x20, i)), item) + } } } } @@ -63,16 +65,19 @@ library AddressArray { let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) let fst := and(lengthAndFirst, _ADDRESS_MASK) - if gt(len, mload(input)) { + switch gt(len, mload(input)) + case 1 { exception := true - } - - // Copy first element and the rest in loop - let ptr := add(output, 0x20) - mstore(ptr, fst) - for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - let item := and(sload(add(self.slot, i)), _ADDRESS_MASK) - mstore(add(ptr, mul(0x20, i)), item) + } default { + if len { + // Copy first element and then the rest in a loop + let ptr := add(output, 0x20) + mstore(ptr, fst) + for { let i := 1 } lt(i, len) { i:= add(i, 1) } { + let item := and(sload(add(self.slot, i)), _ADDRESS_MASK) + mstore(add(ptr, mul(0x20, i)), item) + } + } } } if (exception) revert OutputArrayTooSmall(); From d0506f4de34993e9e3b329a5a79adebf7ffaa007 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 21 Mar 2024 13:40:14 +0400 Subject: [PATCH 14/20] sanitize addresses in get() --- contracts/libraries/AddressArray.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index fb7f037b..72496555 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -48,7 +48,7 @@ library AddressArray { let ptr := add(output, 0x20) mstore(ptr, fst) for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - let item := sload(add(self.slot, i)) + let item := and(sload(add(self.slot, i)), _ADDRESS_MASK) mstore(add(ptr, mul(0x20, i)), item) } } From f037233d73e002b4a496519996417c6c8d1b574a Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 21 Mar 2024 14:04:05 +0400 Subject: [PATCH 15/20] optimizations --- contracts/libraries/AddressArray.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 72496555..8bed6b8f 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -92,7 +92,7 @@ library AddressArray { switch len case 0 { - sstore(self.slot, or(or(account, _ONE_LENGTH), _ZERO_ADDRESS)) + sstore(self.slot, or(account, _ONE_LENGTH)) } default { sstore(self.slot, add(lengthAndFirst, _ONE_LENGTH)) @@ -115,7 +115,7 @@ library AddressArray { exception := true } case 1 { - res := and(sload(self.slot), _ADDRESS_MASK) + res := and(lengthAndFirst, _ADDRESS_MASK) sstore(self.slot, _ZERO_ADDRESS) } default { From b62d227bc1d16ed696c75af130001ff0c8285f15 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 21 Mar 2024 15:49:10 +0400 Subject: [PATCH 16/20] optimize reverting --- contracts/libraries/AddressArray.sol | 38 ++++++++++------------ contracts/tests/mocks/AddressArrayMock.sol | 2 ++ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index 8bed6b8f..a5f739da 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -58,29 +58,27 @@ library AddressArray { /// @dev Puts list of addresses from `self` storage into `output` array. function get(Data storage self, address[] memory input) internal view returns (address[] memory output) { output = input; - bool exception; + bytes4 err = OutputArrayTooSmall.selector; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.slot) let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) let fst := and(lengthAndFirst, _ADDRESS_MASK) - switch gt(len, mload(input)) - case 1 { - exception := true - } default { - if len { - // Copy first element and then the rest in a loop - let ptr := add(output, 0x20) - mstore(ptr, fst) - for { let i := 1 } lt(i, len) { i:= add(i, 1) } { - let item := and(sload(add(self.slot, i)), _ADDRESS_MASK) - mstore(add(ptr, mul(0x20, i)), item) - } + if gt(len, mload(input)) { + mstore(0, err) + revert(0, 4) + } + if len { + // Copy first element and then the rest in a loop + let ptr := add(output, 0x20) + mstore(ptr, fst) + for { let i := 1 } lt(i, len) { i:= add(i, 1) } { + let item := and(sload(add(self.slot, i)), _ADDRESS_MASK) + mstore(add(ptr, mul(0x20, i)), item) } } } - if (exception) revert OutputArrayTooSmall(); } /// @dev Array push back `account` operation on storage `self`. @@ -104,7 +102,7 @@ library AddressArray { /// @dev Array pop back operation for storage `self`. function pop(Data storage self) internal returns(address res) { - bool exception; + bytes4 err = PopFromEmptyArray.selector; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.slot) @@ -112,7 +110,8 @@ library AddressArray { switch len case 0 { - exception := true + mstore(0, err) + revert(0, 4) } case 1 { res := and(lengthAndFirst, _ADDRESS_MASK) @@ -123,12 +122,11 @@ library AddressArray { sstore(self.slot, sub(lengthAndFirst, _ONE_LENGTH)) } } - if (exception) revert PopFromEmptyArray(); } /// @dev Set element for storage `self` at `index` to `account`. function set(Data storage self, uint256 index, address account) internal { - bool exception; + bytes4 err = IndexOutOfBounds.selector; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly let lengthAndFirst := sload(self.slot) @@ -136,7 +134,8 @@ library AddressArray { let fst := and(lengthAndFirst, _ADDRESS_MASK) if iszero(lt(index, len)) { - exception := true + mstore(0, err) + revert(0, 4) } switch index @@ -147,7 +146,6 @@ library AddressArray { sstore(add(self.slot, index), or(account, _ZERO_ADDRESS)) } } - if (exception) revert IndexOutOfBounds(); } /// @dev Erase length of the array diff --git a/contracts/tests/mocks/AddressArrayMock.sol b/contracts/tests/mocks/AddressArrayMock.sol index cb968e70..30a0c65e 100644 --- a/contracts/tests/mocks/AddressArrayMock.sol +++ b/contracts/tests/mocks/AddressArrayMock.sol @@ -9,6 +9,8 @@ contract AddressArrayMock { AddressArray.Data private _self; + error PopFromEmptyArray(); + function length() external view returns (uint256) { return AddressArray.length(_self); } From acba39c2277326a1d266924f9fe74f2a81a2e547 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 21 Mar 2024 16:25:18 +0400 Subject: [PATCH 17/20] split pop into pop and popGet --- contracts/libraries/AddressArray.sol | 24 +++++++++++++++++++++++- contracts/libraries/AddressSet.sol | 2 +- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/contracts/libraries/AddressArray.sol b/contracts/libraries/AddressArray.sol index a5f739da..9b39c871 100644 --- a/contracts/libraries/AddressArray.sol +++ b/contracts/libraries/AddressArray.sol @@ -101,7 +101,29 @@ library AddressArray { } /// @dev Array pop back operation for storage `self`. - function pop(Data storage self) internal returns(address res) { + function pop(Data storage self) internal { + bytes4 err = PopFromEmptyArray.selector; + /// @solidity memory-safe-assembly + assembly { // solhint-disable-line no-inline-assembly + let lengthAndFirst := sload(self.slot) + let len := shr(_LENGTH_OFFSET, and(lengthAndFirst, _LENGTH_MASK)) + + switch len + case 0 { + mstore(0, err) + revert(0, 4) + } + case 1 { + sstore(self.slot, _ZERO_ADDRESS) + } + default { + sstore(self.slot, sub(lengthAndFirst, _ONE_LENGTH)) + } + } + } + + /// @dev Array pop back operation for storage `self` that returns popped element. + function popGet(Data storage self) internal returns(address res) { bytes4 err = PopFromEmptyArray.selector; /// @solidity memory-safe-assembly assembly { // solhint-disable-line no-inline-assembly diff --git a/contracts/libraries/AddressSet.sol b/contracts/libraries/AddressSet.sol index 41199272..cc3528dd 100644 --- a/contracts/libraries/AddressSet.sol +++ b/contracts/libraries/AddressSet.sol @@ -55,7 +55,7 @@ library AddressSet { return false; } - address lastItem = s.items.pop(); + address lastItem = s.items.popGet(); if (index < s.items.length()) { unchecked { s.items.set(index - 1, lastItem); From 75de8a9b8f4ae21995ac2db355f3da253dd46a30 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 21 Mar 2024 18:36:19 +0400 Subject: [PATCH 18/20] add getters to set and fix test --- contracts/libraries/AddressSet.sol | 12 +++++++++++- contracts/tests/mocks/AddressSetMock.sol | 4 ++++ test/contracts/AddressSet.test.ts | 3 +-- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/contracts/libraries/AddressSet.sol b/contracts/libraries/AddressSet.sol index cc3528dd..28c26bc6 100644 --- a/contracts/libraries/AddressSet.sol +++ b/contracts/libraries/AddressSet.sol @@ -37,6 +37,16 @@ library AddressSet { return index != 0 && index != _NULL_INDEX; } + /// @dev Returns list of addresses from storage `s`. + function get(Data storage s) internal view returns (address[] memory) { + return s.items.get(); + } + + /// @dev Puts list of addresses from `s` storage into `output` array. + function get(Data storage s, address[] memory input) internal view returns (address[] memory) { + return s.items.get(input); + } + /// @dev Adds `item` into storage `s` and returns true if successful. function add(Data storage s, address item) internal returns (bool) { uint256 index = s.lookup[item]; @@ -56,7 +66,7 @@ library AddressSet { } address lastItem = s.items.popGet(); - if (index < s.items.length()) { + if (index < s.items.length() + 1) { unchecked { s.items.set(index - 1, lastItem); s.lookup[lastItem] = index; diff --git a/contracts/tests/mocks/AddressSetMock.sol b/contracts/tests/mocks/AddressSetMock.sol index 2844a645..579c39a4 100644 --- a/contracts/tests/mocks/AddressSetMock.sol +++ b/contracts/tests/mocks/AddressSetMock.sol @@ -21,6 +21,10 @@ contract AddressSetMock { return AddressSet.contains(_self, item); } + function get() external view returns (address[] memory) { + return AddressSet.get(_self); + } + function add(address item) external returns (bool) { return AddressSet.add(_self, item); } diff --git a/test/contracts/AddressSet.test.ts b/test/contracts/AddressSet.test.ts index 7376872b..527c5ec6 100644 --- a/test/contracts/AddressSet.test.ts +++ b/test/contracts/AddressSet.test.ts @@ -142,8 +142,7 @@ describe('AddressSet', function () { const isRemoved = await addressSetMock.remove.staticCall(signer1); await addressSetMock.remove(signer1); expect(isRemoved).to.be.true; - expect(await addressSetMock.contains(signer1)).to.be.false; - expect(await addressSetMock.contains(signer2)).to.be.true; + expect(await addressSetMock.get()).to.be.deep.equal([signer2.address]); }); }); }); From 325129dd28135e50b829deacf4359a53c91d834b Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Thu, 21 Mar 2024 18:38:47 +0400 Subject: [PATCH 19/20] optimize a bit --- contracts/libraries/AddressSet.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/libraries/AddressSet.sol b/contracts/libraries/AddressSet.sol index 28c26bc6..9fa707ce 100644 --- a/contracts/libraries/AddressSet.sol +++ b/contracts/libraries/AddressSet.sol @@ -66,7 +66,7 @@ library AddressSet { } address lastItem = s.items.popGet(); - if (index < s.items.length() + 1) { + if (lastItem != item) { unchecked { s.items.set(index - 1, lastItem); s.lookup[lastItem] = index; From cdee296bda7e7f02d8deb80d1647e70351a89c4e Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Fri, 22 Mar 2024 13:04:54 +0400 Subject: [PATCH 20/20] add tests for multi push + multi pop --- test/contracts/AddressArray.test.ts | 14 ++++++++++++++ test/contracts/AddressSet.test.ts | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/test/contracts/AddressArray.test.ts b/test/contracts/AddressArray.test.ts index afa77a91..14633332 100644 --- a/test/contracts/AddressArray.test.ts +++ b/test/contracts/AddressArray.test.ts @@ -191,4 +191,18 @@ describe('AddressArray', function () { expect(await addressArrayMock.get()).to.be.deep.equal([signer1.address, signer3.address]); }); }); + + describe('multiple add/remove', function () { + it('should add and remove multiple times', async function () { + const { addressArrayMock } = await loadFixture(deployAddressArrayMock); + await addressArrayMock.push(signer1); + await addressArrayMock.push(signer2); + await addressArrayMock.pop(); + await addressArrayMock.pop(); + await addressArrayMock.push(signer1); + await addressArrayMock.push(signer2); + await addressArrayMock.pop(); + await addressArrayMock.pop(); + }); + }); }); diff --git a/test/contracts/AddressSet.test.ts b/test/contracts/AddressSet.test.ts index 527c5ec6..f74bba29 100644 --- a/test/contracts/AddressSet.test.ts +++ b/test/contracts/AddressSet.test.ts @@ -145,4 +145,18 @@ describe('AddressSet', function () { expect(await addressSetMock.get()).to.be.deep.equal([signer2.address]); }); }); + + describe('multiple add/remove', function () { + it('should add and remove multiple times', async function () { + const { addressSetMock } = await loadFixture(deployAddressSetMock); + await addressSetMock.add(signer1); + await addressSetMock.add(signer2); + await addressSetMock.remove(signer2); + await addressSetMock.remove(signer1); + await addressSetMock.add(signer1); + await addressSetMock.add(signer2); + await addressSetMock.remove(signer2); + await addressSetMock.remove(signer1); + }); + }); });