Skip to content

Commit

Permalink
addressing the memory leak issue in EnumerableMap
Browse files Browse the repository at this point in the history
  • Loading branch information
Amxx committed Jan 15, 2021
1 parent 23b65b2 commit 2b69162
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 2 deletions.
8 changes: 8 additions & 0 deletions contracts/mocks/EnumerableMapMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,15 @@ contract EnumerableMapMock {
}


function tryGet(uint256 key) public view returns (bool, address) {
return _map.tryGet(key);
}

function get(uint256 key) public view returns (address) {
return _map.get(key);
}

function getWithMessage(uint256 key, string calldata errorMessage) public view returns (address) {
return _map.get(key, errorMessage);
}
}
23 changes: 22 additions & 1 deletion contracts/utils/EnumerableMap.sol
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,16 @@ library EnumerableMap {
return (entry._key, entry._value);
}

/**
* @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
}

/**
* @dev Returns the value associated with `key`. O(1).
*
Expand All @@ -151,7 +161,9 @@ library EnumerableMap {
* - `key` must be in the map.
*/
function _get(Map storage map, bytes32 key) private view returns (bytes32) {
return _get(map, key, "EnumerableMap: nonexistent key");
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
}

/**
Expand Down Expand Up @@ -217,6 +229,15 @@ library EnumerableMap {
return (uint256(key), address(uint160(uint256(value))));
}

/**
* @dev Tries to returns the value associated with `key`. O(1).
* Does not revert if `key` is not in the map.
*/
function tryGet(UintToAddressMap storage map, uint256 key) internal view returns (bool, address) {
(bool success, bytes32 value) = _tryGet(map._inner, bytes32(key));
return (success, address(uint160(uint256(value))));
}

/**
* @dev Returns the value associated with `key`. O(1).
*
Expand Down
42 changes: 41 additions & 1 deletion test/utils/EnumerableMap.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { BN, expectEvent } = require('@openzeppelin/test-helpers');
const { BN, constants, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');
const { expect } = require('chai');

const zip = require('lodash.zip');
Expand Down Expand Up @@ -139,4 +139,44 @@ contract('EnumerableMap', function (accounts) {
expect(await this.map.contains(keyB)).to.equal(false);
});
});

describe('read', function () {
beforeEach(async function () {
await this.map.set(keyA, accountA);
});

describe('get', function () {
it('existing value', async function () {
expect(await this.map.get(keyA)).to.be.equal(accountA);
});
it('missing value', async function () {
await expectRevert(this.map.get(keyB), "EnumerableMap: nonexistent key");
});
});

describe('get with message', function () {
it('existing value', async function () {
expect(await this.map.getWithMessage(keyA, "custom error string")).to.be.equal(accountA);
});
it('missing value', async function () {
await expectRevert(this.map.getWithMessage(keyB, "custom error string"), "custom error string");
});
});

describe('tryGet', function () {
it('existing value', async function () {
expect(await this.map.tryGet(keyA)).to.be.deep.equal({
'0': true,
'1': accountA
});
});
it('missing value', async function () {
expect(await this.map.tryGet(keyB)).to.be.deep.equal({
'0': false,
'1': constants.ZERO_ADDRESS
});
});
});

});
});

0 comments on commit 2b69162

Please sign in to comment.