From d43ff2d932294b8e20fdd2a4d07aa7705cb77f22 Mon Sep 17 00:00:00 2001 From: RenanSouza2 Date: Tue, 18 Apr 2023 08:56:15 -0300 Subject: [PATCH 1/9] first version --- test/governance/Governor.test.js | 2 +- test/governance/TimelockController.test.js | 2 +- test/token/ERC1155/ERC1155.behavior.js | 2 +- test/token/ERC1155/ERC1155.test.js | 4 ++-- test/token/ERC1155/extensions/ERC1155Burnable.test.js | 4 ++-- test/token/ERC1155/extensions/ERC1155Pausable.test.js | 2 +- test/token/ERC1155/extensions/ERC1155Supply.test.js | 8 ++++---- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index a66020ed959..fe136504441 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -594,7 +594,7 @@ contract('Governor', function (accounts) { }); }); - describe('ERC1155', function () { + describe.only('ERC1155', function () { const uri = 'https://token-cdn-domain/{id}.json'; const tokenIds = { 1: new BN(1000), diff --git a/test/governance/TimelockController.test.js b/test/governance/TimelockController.test.js index 607f8bc8358..40a43dd99eb 100644 --- a/test/governance/TimelockController.test.js +++ b/test/governance/TimelockController.test.js @@ -1038,7 +1038,7 @@ contract('TimelockController', function (accounts) { }); }); - describe('ERC1155', function () { + describe.only('ERC1155', function () { const uri = 'https://token-cdn-domain/{id}.json'; const tokenIds = { 1: new BN(1000), diff --git a/test/token/ERC1155/ERC1155.behavior.js b/test/token/ERC1155/ERC1155.behavior.js index 96d448a9e2c..8f49c6ae8e5 100644 --- a/test/token/ERC1155/ERC1155.behavior.js +++ b/test/token/ERC1155/ERC1155.behavior.js @@ -18,7 +18,7 @@ function shouldBehaveLikeERC1155([minter, firstTokenHolder, secondTokenHolder, m const RECEIVER_SINGLE_MAGIC_VALUE = '0xf23a6e61'; const RECEIVER_BATCH_MAGIC_VALUE = '0xbc197c81'; - describe('like an ERC1155', function () { + describe.only('like an ERC1155', function () { describe('balanceOf', function () { it('reverts when queried about the zero address', async function () { await expectRevert( diff --git a/test/token/ERC1155/ERC1155.test.js b/test/token/ERC1155/ERC1155.test.js index 48197eeb562..24390670528 100644 --- a/test/token/ERC1155/ERC1155.test.js +++ b/test/token/ERC1155/ERC1155.test.js @@ -17,7 +17,7 @@ contract('ERC1155', function (accounts) { shouldBehaveLikeERC1155(otherAccounts); - describe('internal functions', function () { + describe.only('internal functions', function () { const tokenId = new BN(1990); const mintAmount = new BN(9001); const burnAmount = new BN(3000); @@ -205,7 +205,7 @@ contract('ERC1155', function (accounts) { }); }); - describe('ERC1155MetadataURI', function () { + describe.only('ERC1155MetadataURI', function () { const firstTokenID = new BN('42'); const secondTokenID = new BN('1337'); diff --git a/test/token/ERC1155/extensions/ERC1155Burnable.test.js b/test/token/ERC1155/extensions/ERC1155Burnable.test.js index f80d9935ac6..01be37ca320 100644 --- a/test/token/ERC1155/extensions/ERC1155Burnable.test.js +++ b/test/token/ERC1155/extensions/ERC1155Burnable.test.js @@ -19,7 +19,7 @@ contract('ERC1155Burnable', function (accounts) { await this.token.$_mint(holder, tokenIds[1], amounts[1], '0x'); }); - describe('burn', function () { + describe.only('burn', function () { it('holder can burn their tokens', async function () { await this.token.burn(holder, tokenIds[0], amounts[0].subn(1), { from: holder }); @@ -41,7 +41,7 @@ contract('ERC1155Burnable', function (accounts) { }); }); - describe('burnBatch', function () { + describe.only('burnBatch', function () { it('holder can burn their tokens', async function () { await this.token.burnBatch(holder, tokenIds, [amounts[0].subn(1), amounts[1].subn(2)], { from: holder }); diff --git a/test/token/ERC1155/extensions/ERC1155Pausable.test.js b/test/token/ERC1155/extensions/ERC1155Pausable.test.js index f4d5cedec57..372baf45caa 100644 --- a/test/token/ERC1155/extensions/ERC1155Pausable.test.js +++ b/test/token/ERC1155/extensions/ERC1155Pausable.test.js @@ -13,7 +13,7 @@ contract('ERC1155Pausable', function (accounts) { this.token = await ERC1155Pausable.new(uri); }); - context('when token is paused', function () { + context.only('when token is paused', function () { const firstTokenId = new BN('37'); const firstTokenAmount = new BN('42'); diff --git a/test/token/ERC1155/extensions/ERC1155Supply.test.js b/test/token/ERC1155/extensions/ERC1155Supply.test.js index 22a75c84f9a..2d6ba96cb05 100644 --- a/test/token/ERC1155/extensions/ERC1155Supply.test.js +++ b/test/token/ERC1155/extensions/ERC1155Supply.test.js @@ -21,7 +21,7 @@ contract('ERC1155Supply', function (accounts) { this.token = await ERC1155Supply.new(uri); }); - context('before mint', function () { + context.only('before mint', function () { it('exist', async function () { expect(await this.token.exists(firstTokenId)).to.be.equal(false); }); @@ -32,7 +32,7 @@ contract('ERC1155Supply', function (accounts) { }); }); - context('after mint', function () { + context.only('after mint', function () { context('single', function () { beforeEach(async function () { await this.token.$_mint(holder, firstTokenId, firstTokenAmount, '0x'); @@ -75,7 +75,7 @@ contract('ERC1155Supply', function (accounts) { }); }); - context('after burn', function () { + context.only('after burn', function () { context('single', function () { beforeEach(async function () { await this.token.$_mint(holder, firstTokenId, firstTokenAmount, '0x'); @@ -116,7 +116,7 @@ contract('ERC1155Supply', function (accounts) { }); }); - context('other', function () { + context.only('other', function () { it('supply unaffected by no-op', async function () { this.token.safeTransferFrom(ZERO_ADDRESS, ZERO_ADDRESS, firstTokenId, firstTokenAmount, '0x', { from: ZERO_ADDRESS, From 1b3190bbd3f43fb7acdb39af012d15cc65475ac3 Mon Sep 17 00:00:00 2001 From: RenanSouza2 Date: Tue, 18 Apr 2023 09:05:18 -0300 Subject: [PATCH 2/9] assembly version --- contracts/token/ERC1155/ERC1155.sol | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index c720267340c..3fc08170a56 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -207,8 +207,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { function _safeTransferFrom(address from, address to, uint256 id, uint256 amount, bytes memory data) internal { require(to != address(0), "ERC1155: transfer to the zero address"); require(from != address(0), "ERC1155: transfer from the zero address"); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); + uint256[] memory ids; + uint256[] memory amounts; + (ids, amounts) = _asSingletonArrays(id, amount); _update(from, to, ids, amounts, data); } @@ -383,4 +384,23 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { return array; } + + function _asSingletonArrays(uint256 element1, uint256 element2) private pure returns (uint256[] memory, uint256[] memory) { + uint256[] memory array1; + uint256[] memory array2; + + assembly { + array1 := mload(0x40) + mstore(array1, 1) + mstore(add(array1, 0x20), element1) + + array2 := add(array1, 0x40) + mstore(array2, 1) + mstore(add(array2, 0x20), element2) + + mstore(0x40, add(array2, 0x40)) + } + + return (array1, array2); + } } From a3598e4af0b866ad1631dcfa3f8b4a7dc12cc8dd Mon Sep 17 00:00:00 2001 From: RenanSouza2 Date: Wed, 26 Apr 2023 11:55:10 -0300 Subject: [PATCH 3/9] Benchmark commit --- contracts/token/ERC1155/ERC1155.sol | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 3fc08170a56..49bb49db51d 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -271,8 +271,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { */ function _mint(address to, uint256 id, uint256 amount, bytes memory data) internal { require(to != address(0), "ERC1155: mint to the zero address"); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); + uint256[] memory ids; + uint256[] memory amounts; + (ids, amounts) = _asSingletonArrays(id, amount); _update(address(0), to, ids, amounts, data); } @@ -304,8 +305,9 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { */ function _burn(address from, uint256 id, uint256 amount) internal { require(from != address(0), "ERC1155: burn from the zero address"); - uint256[] memory ids = _asSingletonArray(id); - uint256[] memory amounts = _asSingletonArray(amount); + uint256[] memory ids; + uint256[] memory amounts; + (ids, amounts) = _asSingletonArrays(id, amount); _update(from, address(0), ids, amounts, ""); } @@ -378,13 +380,6 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { } } - function _asSingletonArray(uint256 element) private pure returns (uint256[] memory) { - uint256[] memory array = new uint256[](1); - array[0] = element; - - return array; - } - function _asSingletonArrays(uint256 element1, uint256 element2) private pure returns (uint256[] memory, uint256[] memory) { uint256[] memory array1; uint256[] memory array2; From 87c586809b877d1991d00c7b344abce76417fdfe Mon Sep 17 00:00:00 2001 From: RenanSouza2 Date: Wed, 26 Apr 2023 12:03:27 -0300 Subject: [PATCH 4/9] Enable tests --- test/governance/Governor.test.js | 2 +- test/governance/TimelockController.test.js | 2 +- test/token/ERC1155/ERC1155.behavior.js | 2 +- test/token/ERC1155/ERC1155.test.js | 4 ++-- test/token/ERC1155/extensions/ERC1155Burnable.test.js | 4 ++-- test/token/ERC1155/extensions/ERC1155Pausable.test.js | 2 +- test/token/ERC1155/extensions/ERC1155Supply.test.js | 8 ++++---- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js index fe136504441..a66020ed959 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -594,7 +594,7 @@ contract('Governor', function (accounts) { }); }); - describe.only('ERC1155', function () { + describe('ERC1155', function () { const uri = 'https://token-cdn-domain/{id}.json'; const tokenIds = { 1: new BN(1000), diff --git a/test/governance/TimelockController.test.js b/test/governance/TimelockController.test.js index 40a43dd99eb..607f8bc8358 100644 --- a/test/governance/TimelockController.test.js +++ b/test/governance/TimelockController.test.js @@ -1038,7 +1038,7 @@ contract('TimelockController', function (accounts) { }); }); - describe.only('ERC1155', function () { + describe('ERC1155', function () { const uri = 'https://token-cdn-domain/{id}.json'; const tokenIds = { 1: new BN(1000), diff --git a/test/token/ERC1155/ERC1155.behavior.js b/test/token/ERC1155/ERC1155.behavior.js index 8f49c6ae8e5..96d448a9e2c 100644 --- a/test/token/ERC1155/ERC1155.behavior.js +++ b/test/token/ERC1155/ERC1155.behavior.js @@ -18,7 +18,7 @@ function shouldBehaveLikeERC1155([minter, firstTokenHolder, secondTokenHolder, m const RECEIVER_SINGLE_MAGIC_VALUE = '0xf23a6e61'; const RECEIVER_BATCH_MAGIC_VALUE = '0xbc197c81'; - describe.only('like an ERC1155', function () { + describe('like an ERC1155', function () { describe('balanceOf', function () { it('reverts when queried about the zero address', async function () { await expectRevert( diff --git a/test/token/ERC1155/ERC1155.test.js b/test/token/ERC1155/ERC1155.test.js index 24390670528..48197eeb562 100644 --- a/test/token/ERC1155/ERC1155.test.js +++ b/test/token/ERC1155/ERC1155.test.js @@ -17,7 +17,7 @@ contract('ERC1155', function (accounts) { shouldBehaveLikeERC1155(otherAccounts); - describe.only('internal functions', function () { + describe('internal functions', function () { const tokenId = new BN(1990); const mintAmount = new BN(9001); const burnAmount = new BN(3000); @@ -205,7 +205,7 @@ contract('ERC1155', function (accounts) { }); }); - describe.only('ERC1155MetadataURI', function () { + describe('ERC1155MetadataURI', function () { const firstTokenID = new BN('42'); const secondTokenID = new BN('1337'); diff --git a/test/token/ERC1155/extensions/ERC1155Burnable.test.js b/test/token/ERC1155/extensions/ERC1155Burnable.test.js index 01be37ca320..f80d9935ac6 100644 --- a/test/token/ERC1155/extensions/ERC1155Burnable.test.js +++ b/test/token/ERC1155/extensions/ERC1155Burnable.test.js @@ -19,7 +19,7 @@ contract('ERC1155Burnable', function (accounts) { await this.token.$_mint(holder, tokenIds[1], amounts[1], '0x'); }); - describe.only('burn', function () { + describe('burn', function () { it('holder can burn their tokens', async function () { await this.token.burn(holder, tokenIds[0], amounts[0].subn(1), { from: holder }); @@ -41,7 +41,7 @@ contract('ERC1155Burnable', function (accounts) { }); }); - describe.only('burnBatch', function () { + describe('burnBatch', function () { it('holder can burn their tokens', async function () { await this.token.burnBatch(holder, tokenIds, [amounts[0].subn(1), amounts[1].subn(2)], { from: holder }); diff --git a/test/token/ERC1155/extensions/ERC1155Pausable.test.js b/test/token/ERC1155/extensions/ERC1155Pausable.test.js index 372baf45caa..f4d5cedec57 100644 --- a/test/token/ERC1155/extensions/ERC1155Pausable.test.js +++ b/test/token/ERC1155/extensions/ERC1155Pausable.test.js @@ -13,7 +13,7 @@ contract('ERC1155Pausable', function (accounts) { this.token = await ERC1155Pausable.new(uri); }); - context.only('when token is paused', function () { + context('when token is paused', function () { const firstTokenId = new BN('37'); const firstTokenAmount = new BN('42'); diff --git a/test/token/ERC1155/extensions/ERC1155Supply.test.js b/test/token/ERC1155/extensions/ERC1155Supply.test.js index 2d6ba96cb05..22a75c84f9a 100644 --- a/test/token/ERC1155/extensions/ERC1155Supply.test.js +++ b/test/token/ERC1155/extensions/ERC1155Supply.test.js @@ -21,7 +21,7 @@ contract('ERC1155Supply', function (accounts) { this.token = await ERC1155Supply.new(uri); }); - context.only('before mint', function () { + context('before mint', function () { it('exist', async function () { expect(await this.token.exists(firstTokenId)).to.be.equal(false); }); @@ -32,7 +32,7 @@ contract('ERC1155Supply', function (accounts) { }); }); - context.only('after mint', function () { + context('after mint', function () { context('single', function () { beforeEach(async function () { await this.token.$_mint(holder, firstTokenId, firstTokenAmount, '0x'); @@ -75,7 +75,7 @@ contract('ERC1155Supply', function (accounts) { }); }); - context.only('after burn', function () { + context('after burn', function () { context('single', function () { beforeEach(async function () { await this.token.$_mint(holder, firstTokenId, firstTokenAmount, '0x'); @@ -116,7 +116,7 @@ contract('ERC1155Supply', function (accounts) { }); }); - context.only('other', function () { + context('other', function () { it('supply unaffected by no-op', async function () { this.token.safeTransferFrom(ZERO_ADDRESS, ZERO_ADDRESS, firstTokenId, firstTokenAmount, '0x', { from: ZERO_ADDRESS, From bfa132321598a0c283eddfa68c9bfb50bd393f6a Mon Sep 17 00:00:00 2001 From: RenanSouza2 Date: Thu, 1 Jun 2023 21:03:17 -0300 Subject: [PATCH 5/9] add memory safety comment --- contracts/token/ERC1155/ERC1155.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index c5e471201b8..dced6f46467 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -383,6 +383,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { uint256[] memory array1; uint256[] memory array2; + /// @solidity memory-safe-assembly assembly { array1 := mload(0x40) mstore(array1, 1) From 98ac862db7b33306400ef6d04b6a307feb82de47 Mon Sep 17 00:00:00 2001 From: RenanSouza2 Date: Thu, 1 Jun 2023 21:16:16 -0300 Subject: [PATCH 6/9] fix linter issues --- contracts/token/ERC1155/ERC1155.sol | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index dced6f46467..d4ea3013e7c 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -379,10 +379,13 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { } } - function _asSingletonArrays(uint256 element1, uint256 element2) private pure returns (uint256[] memory, uint256[] memory) { + function _asSingletonArrays( + uint256 element1, + uint256 element2 + ) private pure returns (uint256[] memory, uint256[] memory) { uint256[] memory array1; uint256[] memory array2; - + /// @solidity memory-safe-assembly assembly { array1 := mload(0x40) From ba81ae9982ee779b7d298a094dc24a099562f7c2 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Fri, 2 Jun 2023 01:23:19 -0300 Subject: [PATCH 7/9] add changeset --- .changeset/serious-books-lie.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/serious-books-lie.md diff --git a/.changeset/serious-books-lie.md b/.changeset/serious-books-lie.md new file mode 100644 index 00000000000..6f0a0a73284 --- /dev/null +++ b/.changeset/serious-books-lie.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`ERC1155`: Optimize array allocation. From 78491914077b80f47f618bfa200fe18f132892fa Mon Sep 17 00:00:00 2001 From: RenanSouza2 Date: Fri, 2 Jun 2023 10:29:33 -0300 Subject: [PATCH 8/9] change variable declaration --- contracts/token/ERC1155/ERC1155.sol | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index d4ea3013e7c..4b22e319ce8 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -206,9 +206,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { function _safeTransferFrom(address from, address to, uint256 id, uint256 amount, bytes memory data) internal { require(to != address(0), "ERC1155: transfer to the zero address"); require(from != address(0), "ERC1155: transfer from the zero address"); - uint256[] memory ids; - uint256[] memory amounts; - (ids, amounts) = _asSingletonArrays(id, amount); + (uint256[] memory ids, uint256[] memory amounts) = _asSingletonArrays(id, amount); _update(from, to, ids, amounts, data); } @@ -270,9 +268,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { */ function _mint(address to, uint256 id, uint256 amount, bytes memory data) internal { require(to != address(0), "ERC1155: mint to the zero address"); - uint256[] memory ids; - uint256[] memory amounts; - (ids, amounts) = _asSingletonArrays(id, amount); + (uint256[] memory ids, uint256[] memory amounts) = _asSingletonArrays(id, amount); _update(address(0), to, ids, amounts, data); } @@ -304,9 +300,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { */ function _burn(address from, uint256 id, uint256 amount) internal { require(from != address(0), "ERC1155: burn from the zero address"); - uint256[] memory ids; - uint256[] memory amounts; - (ids, amounts) = _asSingletonArrays(id, amount); + (uint256[] memory ids, uint256[] memory amounts) = _asSingletonArrays(id, amount); _update(from, address(0), ids, amounts, ""); } From 6f9b86a3b35fcad63d307911e0ea4ea9b0f1d6a7 Mon Sep 17 00:00:00 2001 From: RenanSouza2 Date: Fri, 2 Jun 2023 10:36:41 -0300 Subject: [PATCH 9/9] use declared return variables instead of return --- contracts/token/ERC1155/ERC1155.sol | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/contracts/token/ERC1155/ERC1155.sol b/contracts/token/ERC1155/ERC1155.sol index 4b22e319ce8..6761edd248b 100644 --- a/contracts/token/ERC1155/ERC1155.sol +++ b/contracts/token/ERC1155/ERC1155.sol @@ -376,10 +376,7 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { function _asSingletonArrays( uint256 element1, uint256 element2 - ) private pure returns (uint256[] memory, uint256[] memory) { - uint256[] memory array1; - uint256[] memory array2; - + ) private pure returns (uint256[] memory array1, uint256[] memory array2) { /// @solidity memory-safe-assembly assembly { array1 := mload(0x40) @@ -392,7 +389,5 @@ contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI { mstore(0x40, add(array2, 0x40)) } - - return (array1, array2); } }