From 4ba5d5b73e10939327757720aad32dc3b2c28548 Mon Sep 17 00:00:00 2001 From: Philippe Dumonet Date: Wed, 20 Dec 2023 20:46:29 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9A=A1=EF=B8=8F=20Optimize=20`PurityChecker`?= =?UTF-8?q?=20(#22)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: apply fmt * ⚡ Optimize `PurityChecker` * nit: clean up code * nit: add comment explaining non-push case --- .gas-snapshot | 34 +++++ script/Deploy.s.sol | 1 - src/tokens/KingERC721.sol | 10 +- src/utils/PurityChecker.sol | 62 ++++---- src/utils/metadata/KingArt.sol | 13 +- src/utils/metadata/ParArt.sol | 251 ++++++++++++++++----------------- 6 files changed, 194 insertions(+), 177 deletions(-) create mode 100644 .gas-snapshot diff --git a/.gas-snapshot b/.gas-snapshot new file mode 100644 index 0000000..90dff4c --- /dev/null +++ b/.gas-snapshot @@ -0,0 +1,34 @@ +CurtaGolfTest:test_addCourse() (gas: 74494) +CurtaGolfTest:test_addCourse_NotOwner_Unauthorized(address) (runs: 256, μ: 13559, ~: 13559) +CurtaGolfTest:test_addCourse_ZeroAddress_Reverts() (gas: 11517) +CurtaGolfTest:test_commit(bytes32) (runs: 256, μ: 38350, ~: 38350) +CurtaGolfTest:test_commit_KeyAlreadyCommitted_Reverts(bytes32) (runs: 256, μ: 34803, ~: 34803) +CurtaGolfTest:test_par_Equality() (gas: 8022) +CurtaGolfTest:test_setAllowedOpcodes() (gas: 23461) +CurtaGolfTest:test_setAllowedOpcodes_CourseDoesNotExist_Reverts() (gas: 13931) +CurtaGolfTest:test_setAllowedOpcodes_NotOwner_Unauthorized(address) (runs: 256, μ: 11488, ~: 11488) +CurtaGolfTest:test_setPurityChecker() (gas: 148218) +CurtaGolfTest:test_setPurityChecker_NotOwner_Unauthorized(address) (runs: 256, μ: 13956, ~: 13956) +CurtaGolfTest:test_setPurityChecker_ZeroAddress_Reverts() (gas: 12260) +CurtaGolfTest:test_submit() (gas: 751363) +CurtaGolfTest:test_submitDirectly() (gas: 664460) +CurtaGolfTest:test_submit_CommitTooNew_Reverts(uint256) (runs: 256, μ: 43348, ~: 43392) +CurtaGolfTest:test_submit_IncorrectSolution_Reverts() (gas: 131877) +CurtaGolfTest:test_submit_KeyNotCommitted_Reverts(uint256) (runs: 256, μ: 15489, ~: 15489) +CurtaGolfTest:test_submit_NonexistentCourse_Reverts() (gas: 42342) +CurtaGolfTest:test_submit_PollutedSolution_Reverts() (gas: 65405) +CurtaGolfTest:test_tokenURI_MintedToken_Succeeds() (gas: 45725077) +CurtaGolfTest:test_tokenURI_UnmintedToken_Reverts() (gas: 13480) +MockCourseTest:test_name_ReturnsNonEmptyString() (gas: 6086) +MockCourseTest:test_run_CorrectSolution_DoesNotRevert(uint256) (runs: 256, μ: 10734, ~: 10734) +MockCourseTest:test_run_IncorrectSolution_Reverts(uint256) (runs: 256, μ: 13775, ~: 13775) +ParTest:test_curtaGolf_Equality() (gas: 8080) +ParTest:test_tokenURI_MintedToken_Succeeds() (gas: 1993252) +ParTest:test_tokenURI_UnmintedToken_Reverts() (gas: 13145) +ParTest:test_upmint(address,uint32,uint32) (runs: 256, μ: 67620, ~: 67620) +ParTest:test_upmint_ExistingTokenBeatsRecord_UpdatesStorage() (gas: 65541) +ParTest:test_upmint_ExistingTokenDoesntBeatRecord_DoesntUpdateStorage() (gas: 65490) +ParTest:test_upmint_NotCurtaGolf_Unauthorized(address) (runs: 256, μ: 9996, ~: 9996) +PurityCheckerTest:test_bitmask() (gas: 2460) +PurityCheckerTest:test_check() (gas: 25695) +PurityCheckerTest:test_customBitmap() (gas: 11733) \ No newline at end of file diff --git a/script/Deploy.s.sol b/script/Deploy.s.sol index 35f5e89..71347d6 100644 --- a/script/Deploy.s.sol +++ b/script/Deploy.s.sol @@ -9,7 +9,6 @@ import { Par } from "src/Par.sol"; import { PurityChecker } from "src/utils/PurityChecker.sol"; contract Deploy is Script { - // ------------------------------------------------------------------------- // Deployment addresses // ------------------------------------------------------------------------- diff --git a/src/tokens/KingERC721.sol b/src/tokens/KingERC721.sol index 0e3d515..ceeb26f 100644 --- a/src/tokens/KingERC721.sol +++ b/src/tokens/KingERC721.sol @@ -112,10 +112,7 @@ abstract contract KingERC721 { } // Set new owner. - _tokenData[_id] = TokenData({ - owner: _to, - metadata: uint96(uint160(_to) >> 64) - }); + _tokenData[_id] = TokenData({ owner: _to, metadata: uint96(uint160(_to) >> 64) }); // Clear previous approval data for the token. delete getApproved[_id]; @@ -136,10 +133,7 @@ abstract contract KingERC721 { } // Set new owner - _tokenData[_id] = TokenData({ - owner: _to, - metadata: uint96(uint160(_to) >> 64) - }); + _tokenData[_id] = TokenData({ owner: _to, metadata: uint96(uint160(_to) >> 64) }); // Emit event. emit Transfer(address(0), _to, _id); diff --git a/src/utils/PurityChecker.sol b/src/utils/PurityChecker.sol index 8edca49..5a8310d 100644 --- a/src/utils/PurityChecker.sol +++ b/src/utils/PurityChecker.sol @@ -17,43 +17,37 @@ contract PurityChecker is IPurityChecker { returns (bool isPure) { assembly ("memory-safe") { - function perform(code, bitmap) -> ret { - // `offset` is the memory position where the bytecode starts; we - // add `0x20` to skip the portion that stores the length of the - // bytecode. - let offset := add(code, 0x20) - // `end` is the memory position where the bytecode ends. - let end := add(offset, mload(code)) + // Code is pure by default unless disallowed byte is found. + isPure := 1 - // Iterate through the opcodes in the bytecode until we reach - // the end. For each byte that is an opcode (i.e. not data - // pushed onto the stack via the `PUSH` opcodes), we break the - // loop early if it is not allowed (i.e. the corresponding LSb - // bit in `_allowedOpcodes` is `0`). Since `ret` is set to `0`, - // or `false`, by default, this correctly results in `check` - // returning `false` if the contract is not pure. - for { let i := offset } lt(offset, end) { offset := add(offset, 1) } { - let opcode := byte(0, mload(offset)) - // If the opcode is not allowed, the contract is not pure. - if iszero(and(shr(opcode, bitmap), 1)) { leave } - // `0xffffffff000000000000000000000000` is a bitmap where - // LSb bits `[0x5f + 1, 0x7f]` are 1, i.e. the `PUSH1`, ..., - // `PUSH32`, opcodes. We want to skip the number of bytes - // pushed onto the stack because they are arbitrary data, - // and we should not apply the purity check to them. - if and(shr(opcode, 0xffffffff000000000000000000000000), 1) { - // `opcode - 0x5f` is the number of bytes pushed onto - // the stack. - offset := add(offset, sub(opcode, 0x5f)) - } - } + // `offset` is the memory position where the bytecode starts; we + // add `0x20` to skip the portion that stores the length of the + // bytecode. + let offset := add(_code, 0x20) + // `end` is the memory position where the bytecode ends. + let end := add(offset, mload(_code)) - // If we reached the end of the bytecode, the contract is pure, - // so return `true`. - ret := 1 - } + for { } lt(offset, end) { } { + let opcode := byte(0, mload(offset)) + + // Set `isPure` to 0 if any indexed bit in the loop was ever 0. + isPure := and(isPure, shr(opcode, _allowedOpcodes)) - isPure := perform(_code, _allowedOpcodes) + // Always increments the offset by at least 1. + // If an opcode is a PUSH1-PUSH32 opcode (byte ∈ [0x60, 0x80)) the resulting value + // after subtracting 0x60 will be a valid index ∈ [0, 32). This is then indexed into + // a byte array representing the push bytes of the opcodes (1-32). Bytes outside of + // that range will result in an index larger than 31, which for `BYTE(n, x)` always + // returns 0. + offset := + add( + add(offset, 1), + byte( + sub(opcode, 0x60), + 0x0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20 + ) + ) + } } } } diff --git a/src/utils/metadata/KingArt.sol b/src/utils/metadata/KingArt.sol index 43a0bb8..33dc7d2 100644 --- a/src/utils/metadata/KingArt.sol +++ b/src/utils/metadata/KingArt.sol @@ -159,8 +159,7 @@ library KingArt { '="18" r="4"/>Curta | Golf'; + 's="b"> | Golf'; /// @notice Starting string for the island's SVG. /// @dev The island's SVG's width and height are computed to perfectly @@ -328,19 +327,17 @@ library KingArt { '
King
', _formatNumber(_solves), '
Solves
<' - 'div class="b">', + "-2-2H6a2 2 0 0 0-2 2v18m10-9h2a2 2 0 0 1 2 2v2a2 2 0 0 0 2 2h0a2 2" + ' 0 0 0 2-2V9.83a2 2 0 0 0-.59-1.42L18 5"/>
<' 'div class="b">', _formatNumber(_gasUsed), - '
Gas used
' + '
Gas used
" ); } diff --git a/src/utils/metadata/ParArt.sol b/src/utils/metadata/ParArt.sol index ae98ea2..a754d98 100644 --- a/src/utils/metadata/ParArt.sol +++ b/src/utils/metadata/ParArt.sol @@ -18,81 +18,81 @@ library ParArt { string constant SVG_START = 'Golf
<' - '/svg>
'; + '2 0"/><' "/svg>
"; // ------------------------------------------------------------------------- // `render` and `_renderIsland`