Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve gas efficiency of EnumerableMap #2518

Merged
merged 23 commits into from
Feb 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
dad7bc2
Addin support for eth-gas-reporter
Amxx Feb 1, 2021
1f48c6f
export ethgasreport to file
Amxx Feb 3, 2021
fe4c470
Fix hardhat config to support optimizations
Amxx Feb 3, 2021
6813eb3
add eth-gas-report to github workflows
Amxx Feb 3, 2021
26e84ef
save gas usage report produced through actions
Amxx Feb 3, 2021
7c74c73
save eth-gas-report from file
Amxx Feb 3, 2021
d166ca8
Integrate eth-gas-report in the test workflow + log to stdout
Amxx Feb 4, 2021
d0b0b95
Merge remote-tracking branch 'origin/master' into tooling/eth-gas-rep…
Amxx Feb 4, 2021
7630b6a
use an envvar (ENABLE_GAS_REPORT) to enable/disable gas-report
Amxx Feb 4, 2021
520f46d
Merge branch 'tooling/eth-gas-reporter' into refactor/new-enumerablemap
Amxx Feb 8, 2021
0841c1c
Add a EnumerableSet based version of EnumerableMap
Amxx Feb 8, 2021
487c92f
rename
Amxx Feb 8, 2021
04033fe
use assembly to optimize EnumerableSet
Amxx Feb 9, 2021
6cb0966
moving enumerablemap2 changes into enumerablemap
Amxx Feb 9, 2021
7402f58
Merge branch 'master' into refactor/new-enumerablemap
Amxx Feb 9, 2021
27a0a89
assembly isn't more efficient for EnumerableSet.add
Amxx Feb 9, 2021
97d8ce8
re-enable assembly version of EnumerableSet._add
Amxx Feb 9, 2021
a3e228c
Merge branch 'master' into refactor/new-enumerablemap
Amxx Feb 12, 2021
eb8b45d
Merge branch 'master' into refactor/new-enumerablemap
Amxx Feb 17, 2021
bc69528
Update contracts/utils/EnumerableSet.sol
Amxx Feb 17, 2021
2392d70
revert back to non-assembly EnumerableSet
Amxx Feb 18, 2021
3958bd3
add changelog entry
Amxx Feb 18, 2021
acf188f
reword changelog entry
frangio Feb 18, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* `Context`: making `_msgData` return `bytes calldata` instead of `bytes memory` ([#2492](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2492))
* `ERC20`: Removed the `_setDecimals` function and the storage slot associated to decimals. ([#2502](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2502))
* `Strings`: addition of a `toHexString` function. ([#2504](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2504))
* `EnumerableMap`: change implementation to optimize for `key → value` lookups instead of enumeration. ([#2518](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2518))

## 3.4.0 (2021-02-02)

Expand Down
96 changes: 27 additions & 69 deletions contracts/utils/EnumerableMap.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

pragma solidity ^0.8.0;

import "./EnumerableSet.sol";

/**
* @dev Library for managing an enumerable variant of Solidity's
* https://solidity.readthedocs.io/en/latest/types.html#mapping-types[`mapping`]
Expand All @@ -27,6 +29,8 @@ pragma solidity ^0.8.0;
* supported.
*/
library EnumerableMap {
using EnumerableSet for EnumerableSet.Bytes32Set;

// To implement this library for multiple types with as little code
// repetition as possible, we write it in terms of a generic Map type with
// bytes32 keys and values.
Expand All @@ -36,18 +40,11 @@ library EnumerableMap {
// This means that we can only create new EnumerableMaps for types that fit
// in bytes32.

struct MapEntry {
bytes32 _key;
bytes32 _value;
}

struct Map {
// Storage of map keys and values
MapEntry[] _entries;
// Storage of keys
EnumerableSet.Bytes32Set _keys;

// Position of the entry defined by a key in the `entries` array, plus 1
// because index 0 means a key is not in the map.
mapping (bytes32 => uint256) _indexes;
mapping (bytes32 => bytes32) _values;
}

/**
Expand All @@ -58,19 +55,8 @@ library EnumerableMap {
* already present.
*/
function _set(Map storage map, bytes32 key, bytes32 value) private returns (bool) {
// We read and store the key's index to prevent multiple reads from the same storage slot
uint256 keyIndex = map._indexes[key];

if (keyIndex == 0) { // Equivalent to !contains(map, key)
map._entries.push(MapEntry({ _key: key, _value: value }));
// The entry is stored at length-1, but we add 1 to all indexes
// and use 0 as a sentinel value
map._indexes[key] = map._entries.length;
return true;
} else {
map._entries[keyIndex - 1]._value = value;
return false;
}
map._values[key] = value;
return map._keys.add(key);
}

/**
Expand All @@ -79,51 +65,22 @@ library EnumerableMap {
* Returns true if the key was removed from the map, that is if it was present.
*/
function _remove(Map storage map, bytes32 key) private returns (bool) {
// We read and store the key's index to prevent multiple reads from the same storage slot
uint256 keyIndex = map._indexes[key];

if (keyIndex != 0) { // Equivalent to contains(map, key)
// To delete a key-value pair from the _entries array in O(1), we swap the entry to delete with the last one
// in the array, and then remove the last entry (sometimes called as 'swap and pop').
// This modifies the order of the array, as noted in {at}.

uint256 toDeleteIndex = keyIndex - 1;
uint256 lastIndex = map._entries.length - 1;

// When the entry to delete is the last one, the swap operation is unnecessary. However, since this occurs
// so rarely, we still do the swap anyway to avoid the gas cost of adding an 'if' statement.

MapEntry storage lastEntry = map._entries[lastIndex];

// Move the last entry to the index where the entry to delete is
map._entries[toDeleteIndex] = lastEntry;
// Update the index for the moved entry
map._indexes[lastEntry._key] = toDeleteIndex + 1; // All indexes are 1-based

// Delete the slot where the moved entry was stored
map._entries.pop();

// Delete the index for the deleted slot
delete map._indexes[key];

return true;
} else {
return false;
}
delete map._values[key];
return map._keys.remove(key);
}

/**
* @dev Returns true if the key is in the map. O(1).
*/
function _contains(Map storage map, bytes32 key) private view returns (bool) {
return map._indexes[key] != 0;
return map._keys.contains(key);
}

/**
* @dev Returns the number of key-value pairs in the map. O(1).
*/
function _length(Map storage map) private view returns (uint256) {
return map._entries.length;
return map._keys.length();
}

/**
Expand All @@ -137,20 +94,21 @@ library EnumerableMap {
* - `index` must be strictly less than {length}.
*/
function _at(Map storage map, uint256 index) private view returns (bytes32, bytes32) {
require(map._entries.length > index, "EnumerableMap: index out of bounds");

MapEntry storage entry = map._entries[index];
return (entry._key, entry._value);
bytes32 key = map._keys.at(index);
return (key, map._values[key]);
}

/**
* @dev Tries to returns the value associated with `key`. O(1).
* Does not revert if `key` is not in the map.
*/
function _tryGet(Map storage map, bytes32 key) private view returns (bool, bytes32) {
uint256 keyIndex = map._indexes[key];
if (keyIndex == 0) return (false, 0); // Equivalent to contains(map, key)
return (true, map._entries[keyIndex - 1]._value); // All indexes are 1-based
bytes32 value = map._values[key];
if (value == bytes32(0)) {
return (_contains(map, key), bytes32(0));
} else {
return (true, value);
}
}

/**
Expand All @@ -161,9 +119,9 @@ library EnumerableMap {
* - `key` must be in the map.
*/
function _get(Map storage map, bytes32 key) private view returns (bytes32) {
uint256 keyIndex = map._indexes[key];
require(keyIndex != 0, "EnumerableMap: nonexistent key"); // Equivalent to contains(map, key)
return map._entries[keyIndex - 1]._value; // All indexes are 1-based
bytes32 value = map._values[key];
require(value != 0 || _contains(map, key), "EnumerableMap: nonexistent key");
return value;
}

/**
Expand All @@ -173,9 +131,9 @@ library EnumerableMap {
* message unnecessarily. For custom revert reasons use {_tryGet}.
*/
function _get(Map storage map, bytes32 key, string memory errorMessage) private view returns (bytes32) {
frangio marked this conversation as resolved.
Show resolved Hide resolved
uint256 keyIndex = map._indexes[key];
require(keyIndex != 0, errorMessage); // Equivalent to contains(map, key)
return map._entries[keyIndex - 1]._value; // All indexes are 1-based
bytes32 value = map._values[key];
require(value != 0 || _contains(map, key), errorMessage);
return value;
}

// UintToAddressMap
Expand Down
4 changes: 2 additions & 2 deletions test/token/ERC721/ERC721.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ contract('ERC721', function (accounts) {

it('reverts if index is greater than supply', async function () {
await expectRevert(
this.token.tokenByIndex(2), 'EnumerableMap: index out of bounds',
this.token.tokenByIndex(2), 'EnumerableSet: index out of bounds',
);
});

Expand Down Expand Up @@ -908,7 +908,7 @@ contract('ERC721', function (accounts) {
await this.token.burn(secondTokenId, { from: owner });
expect(await this.token.totalSupply()).to.be.bignumber.equal('0');
await expectRevert(
this.token.tokenByIndex(0), 'EnumerableMap: index out of bounds',
this.token.tokenByIndex(0), 'EnumerableSet: index out of bounds',
);
});

Expand Down