From e0c904275bf5092f1f662c9d0cf93db6ba6df41d Mon Sep 17 00:00:00 2001 From: CodeSandwich Date: Sat, 23 Jul 2022 23:50:43 +0200 Subject: [PATCH 01/15] Optimize toString --- CHANGELOG.md | 1 + contracts/mocks/StringsMock.sol | 8 +-- contracts/utils/Strings.sol | 65 ++++++++++++++++------- test/utils/Strings.test.js | 94 ++++++++++++++++++++++++++------- 4 files changed, 128 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1200341c93..c31d435e6d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ * `ReentrancyGuard`: Reduce code size impact of the modifier by using internal functions. ([#3515](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3515)) * `SafeCast`: optimize downcasting of signed integers. ([#3565](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3565)) * `ERC20FlashMint`: add an internal `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) + * `Strings`: optimize `toString`. ([#????](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/????)) ### Compatibility Note diff --git a/contracts/mocks/StringsMock.sol b/contracts/mocks/StringsMock.sol index b8622680bab..8cefa05c4cb 100644 --- a/contracts/mocks/StringsMock.sol +++ b/contracts/mocks/StringsMock.sol @@ -5,19 +5,19 @@ pragma solidity ^0.8.0; import "../utils/Strings.sol"; contract StringsMock { - function fromUint256(uint256 value) public pure returns (string memory) { + function toStringDecimal(uint256 value) public pure returns (string memory) { return Strings.toString(value); } - function fromUint256Hex(uint256 value) public pure returns (string memory) { + function toHexString(uint256 value) public pure returns (string memory) { return Strings.toHexString(value); } - function fromUint256HexFixed(uint256 value, uint256 length) public pure returns (string memory) { + function toHexString(uint256 value, uint256 length) public pure returns (string memory) { return Strings.toHexString(value, length); } - function fromAddressHexFixed(address addr) public pure returns (string memory) { + function toHexStringAddress(address addr) public pure returns (string memory) { return Strings.toHexString(addr); } } diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 39e0cf124f8..c1fd15a1476 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -13,26 +13,55 @@ library Strings { /** * @dev Converts a `uint256` to its ASCII `string` decimal representation. */ - function toString(uint256 value) internal pure returns (string memory) { - // Inspired by OraclizeAPI's implementation - MIT licence - // https://github.com/oraclize/ethereum-api/blob/b42146b063c7d6ee1358846c198246239e9360e8/oraclizeAPI_0.4.25.sol + function toString(uint256 value) internal pure returns (string memory result) { + unchecked { + if (value < 10) { + return string(abi.encodePacked(uint8(value + 48))); + } + uint256 length = 0; + uint256 valueCopy = value; + if (valueCopy >= 1000000000000000000000000000000000000000000000000000000000000000) { + valueCopy /= 10000000000000000000000000000000000000000000000000000000000000000; + length += 64; + } + if (valueCopy >= 10000000000000000000000000000000) { + valueCopy /= 100000000000000000000000000000000; + length += 32; + } + if (valueCopy >= 1000000000000000) { + valueCopy /= 10000000000000000; + length += 16; + } + if (valueCopy >= 10000000) { + valueCopy /= 100000000; + length += 8; + } + if (valueCopy >= 1000) { + valueCopy /= 10000; + length += 4; + } + if (valueCopy >= 10) { + valueCopy /= 100; + length += 2; + } + if (valueCopy >= 1) { + length += 1; + } + result = new string(length); + /// @solidity memory-safe-assembly + assembly { + let ptr := add(result, add(length, 32)) + for { - if (value == 0) { - return "0"; - } - uint256 temp = value; - uint256 digits; - while (temp != 0) { - digits++; - temp /= 10; - } - bytes memory buffer = new bytes(digits); - while (value != 0) { - digits -= 1; - buffer[digits] = bytes1(uint8(48 + uint256(value % 10))); - value /= 10; + } gt(value, 0) { + + } { + ptr := sub(ptr, 1) + mstore8(ptr, add(48, mod(value, 10))) + value := div(value, 10) + } + } } - return string(buffer); } /** diff --git a/test/utils/Strings.test.js b/test/utils/Strings.test.js index 8dda829ea96..41cf6e0025e 100644 --- a/test/utils/Strings.test.js +++ b/test/utils/Strings.test.js @@ -5,67 +5,125 @@ const { expect } = require('chai'); const StringsMock = artifacts.require('StringsMock'); contract('Strings', function (accounts) { - beforeEach(async function () { - this.strings = await StringsMock.new(); + let strings; + + before(async function () { + strings = await StringsMock.new(); }); - describe('from uint256 - decimal format', function () { + describe('toString', function () { + async function testToString (input) { + expect(await strings.toStringDecimal(input)).to.equal(input); + } + it('converts 0', async function () { - expect(await this.strings.fromUint256(0)).to.equal('0'); + await testToString('0'); }); - it('converts a positive number', async function () { - expect(await this.strings.fromUint256(4132)).to.equal('4132'); + it('converts 7', async function () { + await testToString('7'); + }); + + it('converts 42', async function () { + await testToString('42'); + }); + + it('converts 123', async function () { + await testToString('123'); + }); + + it('converts 4132', async function () { + await testToString('4132'); + }); + + it('converts 12345', async function () { + await testToString('12345'); + }); + + it('converts 1234567', async function () { + await testToString('1234567'); + }); + + it('converts 1234567890', async function () { + await testToString('1234567890'); + }); + + it('converts 123456789012345', async function () { + await testToString('123456789012345'); + }); + + it('converts 12345678901234567890', async function () { + await testToString('12345678901234567890'); + }); + + it('converts 123456789012345678901234567890', async function () { + await testToString('123456789012345678901234567890'); + }); + + it('converts 1234567890123456789012345678901234567890', async function () { + await testToString('1234567890123456789012345678901234567890'); + }); + + it('converts 12345678901234567890123456789012345678901234567890', async function () { + await testToString('12345678901234567890123456789012345678901234567890'); + }); + + it('converts 123456789012345678901234567890123456789012345678901234567890', async function () { + await testToString('123456789012345678901234567890123456789012345678901234567890'); + }); + + it('converts 1234567890123456789012345678901234567890123456789012345678901234567890', async function () { + await testToString('1234567890123456789012345678901234567890123456789012345678901234567890'); }); it('converts MAX_UINT256', async function () { - expect(await this.strings.fromUint256(constants.MAX_UINT256)).to.equal(constants.MAX_UINT256.toString()); + await testToString(constants.MAX_UINT256.toString()); }); }); - describe('from uint256 - hex format', function () { + describe('toHexString', function () { it('converts 0', async function () { - expect(await this.strings.fromUint256Hex(0)).to.equal('0x00'); + expect(await strings.toHexString(0)).to.equal('0x00'); }); it('converts a positive number', async function () { - expect(await this.strings.fromUint256Hex(0x4132)).to.equal('0x4132'); + expect(await strings.toHexString(0x4132)).to.equal('0x4132'); }); it('converts MAX_UINT256', async function () { - expect(await this.strings.fromUint256Hex(constants.MAX_UINT256)) + expect(await strings.toHexString(constants.MAX_UINT256)) .to.equal(web3.utils.toHex(constants.MAX_UINT256)); }); }); - describe('from uint256 - fixed hex format', function () { + describe('toHexString fixed', function () { it('converts a positive number (long)', async function () { - expect(await this.strings.fromUint256HexFixed(0x4132, 32)) + expect(await strings.toHexString(0x4132, 32)) .to.equal('0x0000000000000000000000000000000000000000000000000000000000004132'); }); it('converts a positive number (short)', async function () { await expectRevert( - this.strings.fromUint256HexFixed(0x4132, 1), + strings.toHexString(0x4132, 1), 'Strings: hex length insufficient', ); }); it('converts MAX_UINT256', async function () { - expect(await this.strings.fromUint256HexFixed(constants.MAX_UINT256, 32)) + expect(await strings.toHexString(constants.MAX_UINT256, 32)) .to.equal(web3.utils.toHex(constants.MAX_UINT256)); }); }); - describe('from address - fixed hex format', function () { + describe('toHexString address', function () { it('converts a random address', async function () { const addr = '0xa9036907dccae6a1e0033479b12e837e5cf5a02f'; - expect(await this.strings.fromAddressHexFixed(addr)).to.equal(addr); + expect(await strings.toHexStringAddress(addr)).to.equal(addr); }); it('converts an address with leading zeros', async function () { const addr = '0x0000e0ca771e21bd00057f54a68c30d400000000'; - expect(await this.strings.fromAddressHexFixed(addr)).to.equal(addr); + expect(await strings.toHexStringAddress(addr)).to.equal(addr); }); }); }); From 2292bf41eb72a75916a9e6d455944f838a9c7228 Mon Sep 17 00:00:00 2001 From: CodeSandwich Date: Sun, 24 Jul 2022 01:16:36 +0200 Subject: [PATCH 02/15] Set PR number --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c31d435e6d6..1c86cfa6749 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ * `ReentrancyGuard`: Reduce code size impact of the modifier by using internal functions. ([#3515](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3515)) * `SafeCast`: optimize downcasting of signed integers. ([#3565](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3565)) * `ERC20FlashMint`: add an internal `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551)) - * `Strings`: optimize `toString`. ([#????](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/????)) + * `Strings`: optimize `toString`. ([#3573](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3573)) ### Compatibility Note From 10a7a90d104c4eba5362febc31d443f1efd8b530 Mon Sep 17 00:00:00 2001 From: CodeSandwich Date: Mon, 25 Jul 2022 14:57:42 +0200 Subject: [PATCH 03/15] Express long numbers as powers --- contracts/utils/Strings.sol | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index c1fd15a1476..79f44bf4ec6 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -20,31 +20,31 @@ library Strings { } uint256 length = 0; uint256 valueCopy = value; - if (valueCopy >= 1000000000000000000000000000000000000000000000000000000000000000) { - valueCopy /= 10000000000000000000000000000000000000000000000000000000000000000; + if(valueCopy >= 10 ** 63) { + valueCopy /= 10 ** 64; length += 64; } - if (valueCopy >= 10000000000000000000000000000000) { - valueCopy /= 100000000000000000000000000000000; + if(valueCopy >= 10 ** 31) { + valueCopy /= 10 ** 32; length += 32; } - if (valueCopy >= 1000000000000000) { - valueCopy /= 10000000000000000; + if(valueCopy >= 10 ** 15) { + valueCopy /= 10 ** 16; length += 16; } - if (valueCopy >= 10000000) { - valueCopy /= 100000000; + if(valueCopy >= 10 ** 7) { + valueCopy /= 10 ** 8; length += 8; } - if (valueCopy >= 1000) { - valueCopy /= 10000; + if(valueCopy >= 10 ** 3) { + valueCopy /= 10 ** 4; length += 4; } - if (valueCopy >= 10) { - valueCopy /= 100; + if(valueCopy >= 10 ** 1) { + valueCopy /= 10 ** 2; length += 2; } - if (valueCopy >= 1) { + if(valueCopy >= 1) { length += 1; } result = new string(length); From 029c82e922cf5d0aea2d623e9b607804b1f79f29 Mon Sep 17 00:00:00 2001 From: CodeSandwich Date: Mon, 25 Jul 2022 15:00:00 +0200 Subject: [PATCH 04/15] lint --- contracts/utils/Strings.sol | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 79f44bf4ec6..edf656a1951 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -20,31 +20,31 @@ library Strings { } uint256 length = 0; uint256 valueCopy = value; - if(valueCopy >= 10 ** 63) { - valueCopy /= 10 ** 64; + if (valueCopy >= 10**63) { + valueCopy /= 10**64; length += 64; } - if(valueCopy >= 10 ** 31) { - valueCopy /= 10 ** 32; + if (valueCopy >= 10**31) { + valueCopy /= 10**32; length += 32; } - if(valueCopy >= 10 ** 15) { - valueCopy /= 10 ** 16; + if (valueCopy >= 10**15) { + valueCopy /= 10**16; length += 16; } - if(valueCopy >= 10 ** 7) { - valueCopy /= 10 ** 8; + if (valueCopy >= 10**7) { + valueCopy /= 10**8; length += 8; } - if(valueCopy >= 10 ** 3) { - valueCopy /= 10 ** 4; + if (valueCopy >= 10**3) { + valueCopy /= 10**4; length += 4; } - if(valueCopy >= 10 ** 1) { - valueCopy /= 10 ** 2; + if (valueCopy >= 10**1) { + valueCopy /= 10**2; length += 2; } - if(valueCopy >= 1) { + if (valueCopy >= 1) { length += 1; } result = new string(length); From 365e07b31a8b2ff9d5fe4e10818934517f431725 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Jul 2022 15:14:45 +0200 Subject: [PATCH 05/15] refactor toString() --- contracts/utils/Strings.sol | 40 ++++++++-------- test/utils/Strings.test.js | 93 +++++++++++-------------------------- 2 files changed, 47 insertions(+), 86 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index c1fd15a1476..df6b4f58f86 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -15,45 +15,47 @@ library Strings { */ function toString(uint256 value) internal pure returns (string memory result) { unchecked { - if (value < 10) { - return string(abi.encodePacked(uint8(value + 48))); - } - uint256 length = 0; + uint256 length = 1; + + // compute log10(value), and add it to length uint256 valueCopy = value; - if (valueCopy >= 1000000000000000000000000000000000000000000000000000000000000000) { - valueCopy /= 10000000000000000000000000000000000000000000000000000000000000000; + if (valueCopy >= 10**64) { + valueCopy /= 10**64; length += 64; } - if (valueCopy >= 10000000000000000000000000000000) { - valueCopy /= 100000000000000000000000000000000; + if (valueCopy >= 10**32) { + valueCopy /= 10**32; length += 32; } - if (valueCopy >= 1000000000000000) { - valueCopy /= 10000000000000000; + if (valueCopy >= 10**16) { + valueCopy /= 10**16; length += 16; } - if (valueCopy >= 10000000) { - valueCopy /= 100000000; + if (valueCopy >= 10**8) { + valueCopy /= 10**8; length += 8; } - if (valueCopy >= 1000) { - valueCopy /= 10000; + if (valueCopy >= 10**4) { + valueCopy /= 10**4; length += 4; } - if (valueCopy >= 10) { - valueCopy /= 100; + if (valueCopy >= 10**2) { + valueCopy /= 10**2; length += 2; } - if (valueCopy >= 1) { + if (valueCopy >= 10**1) { length += 1; } + // now, length is log10(value) + 1 result = new string(length); /// @solidity memory-safe-assembly assembly { - let ptr := add(result, add(length, 32)) + let pos := add(result, 32) + let ptr := add(pos, length) + for { - } gt(value, 0) { + } gt(ptr, pos) { } { ptr := sub(ptr, 1) diff --git a/test/utils/Strings.test.js b/test/utils/Strings.test.js index 41cf6e0025e..f4c5a7781c5 100644 --- a/test/utils/Strings.test.js +++ b/test/utils/Strings.test.js @@ -1,4 +1,4 @@ -const { constants, expectRevert } = require('@openzeppelin/test-helpers'); +const { BN, constants, expectRevert } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); @@ -12,73 +12,32 @@ contract('Strings', function (accounts) { }); describe('toString', function () { - async function testToString (input) { - expect(await strings.toStringDecimal(input)).to.equal(input); + for (const [ key, value ] of Object.entries([ + '0', + '7', + '10', + '99', + '100', + '101', + '123', + '4132', + '12345', + '1234567', + '1234567890', + '123456789012345', + '12345678901234567890', + '123456789012345678901234567890', + '1234567890123456789012345678901234567890', + '12345678901234567890123456789012345678901234567890', + '123456789012345678901234567890123456789012345678901234567890', + '1234567890123456789012345678901234567890123456789012345678901234567890', + ].reduce((acc, value) => Object.assign(acc, { [value]: new BN(value) }), { + MAX_UINT256: constants.MAX_UINT256.toString(), + }))) { + it(`converts ${key}`, async function () { + expect(await strings.toStringDecimal(value)).to.equal(value.toString(10)); + }); } - - it('converts 0', async function () { - await testToString('0'); - }); - - it('converts 7', async function () { - await testToString('7'); - }); - - it('converts 42', async function () { - await testToString('42'); - }); - - it('converts 123', async function () { - await testToString('123'); - }); - - it('converts 4132', async function () { - await testToString('4132'); - }); - - it('converts 12345', async function () { - await testToString('12345'); - }); - - it('converts 1234567', async function () { - await testToString('1234567'); - }); - - it('converts 1234567890', async function () { - await testToString('1234567890'); - }); - - it('converts 123456789012345', async function () { - await testToString('123456789012345'); - }); - - it('converts 12345678901234567890', async function () { - await testToString('12345678901234567890'); - }); - - it('converts 123456789012345678901234567890', async function () { - await testToString('123456789012345678901234567890'); - }); - - it('converts 1234567890123456789012345678901234567890', async function () { - await testToString('1234567890123456789012345678901234567890'); - }); - - it('converts 12345678901234567890123456789012345678901234567890', async function () { - await testToString('12345678901234567890123456789012345678901234567890'); - }); - - it('converts 123456789012345678901234567890123456789012345678901234567890', async function () { - await testToString('123456789012345678901234567890123456789012345678901234567890'); - }); - - it('converts 1234567890123456789012345678901234567890123456789012345678901234567890', async function () { - await testToString('1234567890123456789012345678901234567890123456789012345678901234567890'); - }); - - it('converts MAX_UINT256', async function () { - await testToString(constants.MAX_UINT256.toString()); - }); }); describe('toHexString', function () { From 57af5c4b2489741c18fefb8d2d83371a3d13105d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Jul 2022 15:21:48 +0200 Subject: [PATCH 06/15] use the library name/selector in mocks & test --- contracts/mocks/StringsMock.sol | 4 ++-- test/utils/Strings.test.js | 18 +++++++++--------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/contracts/mocks/StringsMock.sol b/contracts/mocks/StringsMock.sol index 8cefa05c4cb..90a6c94b34b 100644 --- a/contracts/mocks/StringsMock.sol +++ b/contracts/mocks/StringsMock.sol @@ -5,7 +5,7 @@ pragma solidity ^0.8.0; import "../utils/Strings.sol"; contract StringsMock { - function toStringDecimal(uint256 value) public pure returns (string memory) { + function toString(uint256 value) public pure returns (string memory) { return Strings.toString(value); } @@ -17,7 +17,7 @@ contract StringsMock { return Strings.toHexString(value, length); } - function toHexStringAddress(address addr) public pure returns (string memory) { + function toHexString(address addr) public pure returns (string memory) { return Strings.toHexString(addr); } } diff --git a/test/utils/Strings.test.js b/test/utils/Strings.test.js index f4c5a7781c5..1b6ceda698b 100644 --- a/test/utils/Strings.test.js +++ b/test/utils/Strings.test.js @@ -35,41 +35,41 @@ contract('Strings', function (accounts) { MAX_UINT256: constants.MAX_UINT256.toString(), }))) { it(`converts ${key}`, async function () { - expect(await strings.toStringDecimal(value)).to.equal(value.toString(10)); + expect(await strings.methods['toString(uint256)'](value)).to.equal(value.toString(10)); }); } }); describe('toHexString', function () { it('converts 0', async function () { - expect(await strings.toHexString(0)).to.equal('0x00'); + expect(await strings.methods['toHexString(uint256)'](0)).to.equal('0x00'); }); it('converts a positive number', async function () { - expect(await strings.toHexString(0x4132)).to.equal('0x4132'); + expect(await strings.methods['toHexString(uint256)'](0x4132)).to.equal('0x4132'); }); it('converts MAX_UINT256', async function () { - expect(await strings.toHexString(constants.MAX_UINT256)) + expect(await strings.methods['toHexString(uint256)'](constants.MAX_UINT256)) .to.equal(web3.utils.toHex(constants.MAX_UINT256)); }); }); describe('toHexString fixed', function () { it('converts a positive number (long)', async function () { - expect(await strings.toHexString(0x4132, 32)) + expect(await strings.methods['toHexString(uint256,uint256)'](0x4132, 32)) .to.equal('0x0000000000000000000000000000000000000000000000000000000000004132'); }); it('converts a positive number (short)', async function () { await expectRevert( - strings.toHexString(0x4132, 1), + strings.methods['toHexString(uint256,uint256)'](0x4132, 1), 'Strings: hex length insufficient', ); }); it('converts MAX_UINT256', async function () { - expect(await strings.toHexString(constants.MAX_UINT256, 32)) + expect(await strings.methods['toHexString(uint256,uint256)'](constants.MAX_UINT256, 32)) .to.equal(web3.utils.toHex(constants.MAX_UINT256)); }); }); @@ -77,12 +77,12 @@ contract('Strings', function (accounts) { describe('toHexString address', function () { it('converts a random address', async function () { const addr = '0xa9036907dccae6a1e0033479b12e837e5cf5a02f'; - expect(await strings.toHexStringAddress(addr)).to.equal(addr); + expect(await strings.methods['toHexString(address)'](addr)).to.equal(addr); }); it('converts an address with leading zeros', async function () { const addr = '0x0000e0ca771e21bd00057f54a68c30d400000000'; - expect(await strings.toHexStringAddress(addr)).to.equal(addr); + expect(await strings.methods['toHexString(address)'](addr)).to.equal(addr); }); }); }); From e8e61fcf1748fdb3b7444ca5e94c826e6333efba Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Jul 2022 15:43:28 +0200 Subject: [PATCH 07/15] improve length estimation --- contracts/utils/Strings.sol | 38 ++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index df6b4f58f86..8f59ecda3fa 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -47,6 +47,7 @@ library Strings { length += 1; } // now, length is log10(value) + 1 + result = new string(length); /// @solidity memory-safe-assembly assembly { @@ -70,16 +71,35 @@ library Strings { * @dev Converts a `uint256` to its ASCII `string` hexadecimal representation. */ function toHexString(uint256 value) internal pure returns (string memory) { - if (value == 0) { - return "0x00"; - } - uint256 temp = value; - uint256 length = 0; - while (temp != 0) { - length++; - temp >>= 8; + unchecked { + uint256 length = 1; + + // compute log256(value), and add it to length + uint256 valueCopy = value; + if (valueCopy >> 128 > 0) { + valueCopy >>= 128; + length += 16; + } + if (valueCopy >> 64 > 0) { + valueCopy >>= 64; + length += 8; + } + if (valueCopy >> 32 > 0) { + valueCopy >>= 32; + length += 4; + } + if (valueCopy >> 16 > 0) { + valueCopy >>= 16; + length += 2; + } + if (valueCopy >> 8 > 0) { + valueCopy >>= 8; + length += 1; + } + // now, length is log256(value) + 1 + + return toHexString(value, length); } - return toHexString(value, length); } /** From 7e81cc4453847252102055154b15994a4f635b8f Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Jul 2022 15:46:17 +0200 Subject: [PATCH 08/15] minimize PR change --- test/utils/Strings.test.js | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/test/utils/Strings.test.js b/test/utils/Strings.test.js index 1b6ceda698b..7abc859ff3a 100644 --- a/test/utils/Strings.test.js +++ b/test/utils/Strings.test.js @@ -5,10 +5,8 @@ const { expect } = require('chai'); const StringsMock = artifacts.require('StringsMock'); contract('Strings', function (accounts) { - let strings; - before(async function () { - strings = await StringsMock.new(); + this.strings = await StringsMock.new(); }); describe('toString', function () { @@ -35,41 +33,41 @@ contract('Strings', function (accounts) { MAX_UINT256: constants.MAX_UINT256.toString(), }))) { it(`converts ${key}`, async function () { - expect(await strings.methods['toString(uint256)'](value)).to.equal(value.toString(10)); + expect(await this.strings.methods['toString(uint256)'](value)).to.equal(value.toString(10)); }); } }); describe('toHexString', function () { it('converts 0', async function () { - expect(await strings.methods['toHexString(uint256)'](0)).to.equal('0x00'); + expect(await this.strings.methods['toHexString(uint256)'](0)).to.equal('0x00'); }); it('converts a positive number', async function () { - expect(await strings.methods['toHexString(uint256)'](0x4132)).to.equal('0x4132'); + expect(await this.strings.methods['toHexString(uint256)'](0x4132)).to.equal('0x4132'); }); it('converts MAX_UINT256', async function () { - expect(await strings.methods['toHexString(uint256)'](constants.MAX_UINT256)) + expect(await this.strings.methods['toHexString(uint256)'](constants.MAX_UINT256)) .to.equal(web3.utils.toHex(constants.MAX_UINT256)); }); }); describe('toHexString fixed', function () { it('converts a positive number (long)', async function () { - expect(await strings.methods['toHexString(uint256,uint256)'](0x4132, 32)) + expect(await this.strings.methods['toHexString(uint256,uint256)'](0x4132, 32)) .to.equal('0x0000000000000000000000000000000000000000000000000000000000004132'); }); it('converts a positive number (short)', async function () { await expectRevert( - strings.methods['toHexString(uint256,uint256)'](0x4132, 1), + this.strings.methods['toHexString(uint256,uint256)'](0x4132, 1), 'Strings: hex length insufficient', ); }); it('converts MAX_UINT256', async function () { - expect(await strings.methods['toHexString(uint256,uint256)'](constants.MAX_UINT256, 32)) + expect(await this.strings.methods['toHexString(uint256,uint256)'](constants.MAX_UINT256, 32)) .to.equal(web3.utils.toHex(constants.MAX_UINT256)); }); }); @@ -77,12 +75,12 @@ contract('Strings', function (accounts) { describe('toHexString address', function () { it('converts a random address', async function () { const addr = '0xa9036907dccae6a1e0033479b12e837e5cf5a02f'; - expect(await strings.methods['toHexString(address)'](addr)).to.equal(addr); + expect(await this.strings.methods['toHexString(address)'](addr)).to.equal(addr); }); it('converts an address with leading zeros', async function () { const addr = '0x0000e0ca771e21bd00057f54a68c30d400000000'; - expect(await strings.methods['toHexString(address)'](addr)).to.equal(addr); + expect(await this.strings.methods['toHexString(address)'](addr)).to.equal(addr); }); }); }); From eb7f013ffdfde54b3d87594496c3e9d34f05754c Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Jul 2022 16:01:20 +0200 Subject: [PATCH 09/15] minimize PR change --- contracts/utils/Strings.sol | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 8f59ecda3fa..bdc0c993a4a 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -13,7 +13,7 @@ library Strings { /** * @dev Converts a `uint256` to its ASCII `string` decimal representation. */ - function toString(uint256 value) internal pure returns (string memory result) { + function toString(uint256 value) internal pure returns (string memory) { unchecked { uint256 length = 1; @@ -48,10 +48,10 @@ library Strings { } // now, length is log10(value) + 1 - result = new string(length); + string memory buffer = new string(length); /// @solidity memory-safe-assembly assembly { - let pos := add(result, 32) + let pos := add(buffer, 32) let ptr := add(pos, length) for { @@ -64,6 +64,7 @@ library Strings { value := div(value, 10) } } + return buffer; } } From 2eeeb5c1bd9af6b7455d5c1073b15653c93cd243 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Jul 2022 16:39:26 +0200 Subject: [PATCH 10/15] gas optimization in toString() and sqrt() --- contracts/utils/Strings.sol | 10 +++++----- contracts/utils/math/Math.sol | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index bdc0c993a4a..fb1e763fe16 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -77,23 +77,23 @@ library Strings { // compute log256(value), and add it to length uint256 valueCopy = value; - if (valueCopy >> 128 > 0) { + if (valueCopy > 1 << 128 ) { valueCopy >>= 128; length += 16; } - if (valueCopy >> 64 > 0) { + if (valueCopy > 1 << 64 ) { valueCopy >>= 64; length += 8; } - if (valueCopy >> 32 > 0) { + if (valueCopy > 1 << 32 ) { valueCopy >>= 32; length += 4; } - if (valueCopy >> 16 > 0) { + if (valueCopy > 1 << 16 ) { valueCopy >>= 16; length += 2; } - if (valueCopy >> 8 > 0) { + if (valueCopy > 1 << 8 ) { valueCopy >>= 8; length += 1; } diff --git a/contracts/utils/math/Math.sol b/contracts/utils/math/Math.sol index 379407c42e4..0fe773e5c6e 100644 --- a/contracts/utils/math/Math.sol +++ b/contracts/utils/math/Math.sol @@ -169,31 +169,31 @@ library Math { // good first approximation of `sqrt(a)` with at least 1 correct bit. uint256 result = 1; uint256 x = a; - if (x >> 128 > 0) { + if (x > 1 << 128) { x >>= 128; result <<= 64; } - if (x >> 64 > 0) { + if (x > 1 << 64) { x >>= 64; result <<= 32; } - if (x >> 32 > 0) { + if (x > 1 << 32) { x >>= 32; result <<= 16; } - if (x >> 16 > 0) { + if (x > 1 << 16) { x >>= 16; result <<= 8; } - if (x >> 8 > 0) { + if (x > 1 << 8) { x >>= 8; result <<= 4; } - if (x >> 4 > 0) { + if (x > 1 << 4) { x >>= 4; result <<= 2; } - if (x >> 2 > 0) { + if (x > 1 << 2) { result <<= 1; } From 8f84142ffac1c3fc64106b12122577c16bb31d2d Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 25 Jul 2022 16:47:25 +0200 Subject: [PATCH 11/15] =?UTF-8?q?fix=20>=20=E2=86=92=20>=3D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- contracts/utils/Strings.sol | 10 +++++----- contracts/utils/math/Math.sol | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index fb1e763fe16..7cb47dfe3c4 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -77,23 +77,23 @@ library Strings { // compute log256(value), and add it to length uint256 valueCopy = value; - if (valueCopy > 1 << 128 ) { + if (valueCopy >= 1 << 128) { valueCopy >>= 128; length += 16; } - if (valueCopy > 1 << 64 ) { + if (valueCopy >= 1 << 64) { valueCopy >>= 64; length += 8; } - if (valueCopy > 1 << 32 ) { + if (valueCopy >= 1 << 32) { valueCopy >>= 32; length += 4; } - if (valueCopy > 1 << 16 ) { + if (valueCopy >= 1 << 16) { valueCopy >>= 16; length += 2; } - if (valueCopy > 1 << 8 ) { + if (valueCopy >= 1 << 8) { valueCopy >>= 8; length += 1; } diff --git a/contracts/utils/math/Math.sol b/contracts/utils/math/Math.sol index 0fe773e5c6e..539643b0b77 100644 --- a/contracts/utils/math/Math.sol +++ b/contracts/utils/math/Math.sol @@ -169,31 +169,31 @@ library Math { // good first approximation of `sqrt(a)` with at least 1 correct bit. uint256 result = 1; uint256 x = a; - if (x > 1 << 128) { + if (x >= 1 << 128) { x >>= 128; result <<= 64; } - if (x > 1 << 64) { + if (x >= 1 << 64) { x >>= 64; result <<= 32; } - if (x > 1 << 32) { + if (x >= 1 << 32) { x >>= 32; result <<= 16; } - if (x > 1 << 16) { + if (x >= 1 << 16) { x >>= 16; result <<= 8; } - if (x > 1 << 8) { + if (x >= 1 << 8) { x >>= 8; result <<= 4; } - if (x > 1 << 4) { + if (x >= 1 << 4) { x >>= 4; result <<= 2; } - if (x > 1 << 2) { + if (x >= 1 << 2) { result <<= 1; } From 4eace993d5cda2bd3c57afea28a08b1b1af889e8 Mon Sep 17 00:00:00 2001 From: CodeSandwich Date: Tue, 26 Jul 2022 23:10:27 +0200 Subject: [PATCH 12/15] Optimize writing loop --- contracts/utils/Strings.sol | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 7cb47dfe3c4..aca7862e532 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -7,7 +7,7 @@ pragma solidity ^0.8.0; * @dev String operations. */ library Strings { - bytes16 private constant _HEX_SYMBOLS = "0123456789abcdef"; + bytes16 private constant _SYMBOLS = "0123456789abcdef"; uint8 private constant _ADDRESS_LENGTH = 20; /** @@ -51,17 +51,18 @@ library Strings { string memory buffer = new string(length); /// @solidity memory-safe-assembly assembly { - let pos := add(buffer, 32) - let ptr := add(pos, length) - + let ptr := add(buffer, add(32, length)) for { - } gt(ptr, pos) { + } 1 { } { ptr := sub(ptr, 1) - mstore8(ptr, add(48, mod(value, 10))) + mstore8(ptr, byte(mod(value, 10), _SYMBOLS)) value := div(value, 10) + if iszero(value) { + break + } } } return buffer; @@ -111,7 +112,7 @@ library Strings { buffer[0] = "0"; buffer[1] = "x"; for (uint256 i = 2 * length + 1; i > 1; --i) { - buffer[i] = _HEX_SYMBOLS[value & 0xf]; + buffer[i] = _SYMBOLS[value & 0xf]; value >>= 4; } require(value == 0, "Strings: hex length insufficient"); From b9a4ae1c70bf188a5e702dba6fdef79f1f17a806 Mon Sep 17 00:00:00 2001 From: CodeSandwich Date: Thu, 25 Aug 2022 17:09:00 +0200 Subject: [PATCH 13/15] Less assembly --- contracts/utils/Strings.sol | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index aca7862e532..c10e2f7a0ff 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -49,21 +49,19 @@ library Strings { // now, length is log10(value) + 1 string memory buffer = new string(length); + uint ptr; /// @solidity memory-safe-assembly assembly { - let ptr := add(buffer, add(32, length)) - for { - - } 1 { - - } { - ptr := sub(ptr, 1) + ptr := add(buffer, add(32, length)) + } + while(true) { + ptr--; + /// @solidity memory-safe-assembly + assembly { mstore8(ptr, byte(mod(value, 10), _SYMBOLS)) - value := div(value, 10) - if iszero(value) { - break - } } + value /= 10; + if (value == 0) break; } return buffer; } From 8e955af1243e02a298f0b7f9889060b04f27d0e9 Mon Sep 17 00:00:00 2001 From: CodeSandwich Date: Fri, 26 Aug 2022 10:26:00 +0200 Subject: [PATCH 14/15] prettier --- contracts/utils/Strings.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index c10e2f7a0ff..b7127f2264f 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -49,12 +49,12 @@ library Strings { // now, length is log10(value) + 1 string memory buffer = new string(length); - uint ptr; + uint256 ptr; /// @solidity memory-safe-assembly assembly { ptr := add(buffer, add(32, length)) } - while(true) { + while (true) { ptr--; /// @solidity memory-safe-assembly assembly { From d3d66ac3cb36d9b607619f64d17346e323c102f1 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 31 Aug 2022 19:57:25 -0300 Subject: [PATCH 15/15] revert unrelated changes --- contracts/utils/math/Math.sol | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/contracts/utils/math/Math.sol b/contracts/utils/math/Math.sol index 539643b0b77..379407c42e4 100644 --- a/contracts/utils/math/Math.sol +++ b/contracts/utils/math/Math.sol @@ -169,31 +169,31 @@ library Math { // good first approximation of `sqrt(a)` with at least 1 correct bit. uint256 result = 1; uint256 x = a; - if (x >= 1 << 128) { + if (x >> 128 > 0) { x >>= 128; result <<= 64; } - if (x >= 1 << 64) { + if (x >> 64 > 0) { x >>= 64; result <<= 32; } - if (x >= 1 << 32) { + if (x >> 32 > 0) { x >>= 32; result <<= 16; } - if (x >= 1 << 16) { + if (x >> 16 > 0) { x >>= 16; result <<= 8; } - if (x >= 1 << 8) { + if (x >> 8 > 0) { x >>= 8; result <<= 4; } - if (x >= 1 << 4) { + if (x >> 4 > 0) { x >>= 4; result <<= 2; } - if (x >= 1 << 2) { + if (x >> 2 > 0) { result <<= 1; }