From 60c3fa6faef4738f11ef9a887e760b5b9f658092 Mon Sep 17 00:00:00 2001 From: dmfxyz <100147743+dmfxyz@users.noreply.github.com> Date: Sat, 23 Mar 2024 13:44:59 -0400 Subject: [PATCH] update testing/validation pipelines and forge fmt all files (#13) * forge fmt * update workflow to fuzz 10,000 times * update slither workflow version * use --fuzz-runs everywhere instead of environment variable --- .github/workflows/run_tests.yml | 4 +- .github/workflows/slither.yml | 5 +- script/Merkle.s.sol | 23 +++++-- script/common/ScriptHelper.sol | 34 +++++------ script/test/Merkle.s.t.sol | 12 +--- src/Merkle.sol | 35 +++++------ src/Xorkle.sol | 14 ++--- src/common/MurkyBase.sol | 105 +++++++++++++++++--------------- src/test/Merkle.t.sol | 9 +-- src/test/MurkyBase.t.sol | 10 ++- src/test/StandardInput.t.sol | 21 +++---- src/test/Xorkle.t.sol | 9 +-- 12 files changed, 144 insertions(+), 137 deletions(-) diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index a6cdce9..49a2a7e 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -24,13 +24,13 @@ jobs: version: nightly - name: Run Fuzzed Unit Tests - run: forge test --no-match-path src/test/StandardInput.t.sol + run: forge test --no-match-path src/test/StandardInput.t.sol --fuzz-runs 10000 - name: Run Differential Tests run: | npm --prefix differential_testing/scripts/ install npm --prefix differential_testing/scripts/ run compile - FOUNDRY_FUZZ_RUNS=512 forge test --ffi --contracts differential_testing/test/DifferentialTests.t.sol + forge test --ffi --contracts differential_testing/test/DifferentialTests.t.sol --fuzz-runs 512 - name: Run Standard Gas Snapshotting run: forge snapshot --gas-report --ffi --match-path src/test/StandardInput.t.sol \ No newline at end of file diff --git a/.github/workflows/slither.yml b/.github/workflows/slither.yml index 5c23a9f..dcbefce 100644 --- a/.github/workflows/slither.yml +++ b/.github/workflows/slither.yml @@ -9,8 +9,7 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v3 - - uses: crytic/slither-action@v0.2.0 + - uses: crytic/slither-action@v0.3.1 with: - target: 'src/' - slither-args: '--exclude-informational --checklist' + slither-args: '--exclude-informational --checklist --show-ignored-findings' fail-on: 'low' diff --git a/script/Merkle.s.sol b/script/Merkle.s.sol index 5cf00c6..09ff9c5 100644 --- a/script/Merkle.s.sol +++ b/script/Merkle.s.sol @@ -27,7 +27,6 @@ contract MerkleScript is Script, ScriptHelper { string[] private types = elements.readStringArray(".types"); uint256 private count = elements.readUint(".count"); - bytes32[] private leafs = new bytes32[](count); string[] private inputs = new string[](count); @@ -41,13 +40,25 @@ contract MerkleScript is Script, ScriptHelper { } /// @dev Generate the JSON entries for the output file - function generateJsonEntries(string memory _inputs, string memory _proof, string memory _root, string memory _leaf) internal pure returns (string memory) { + function generateJsonEntries(string memory _inputs, string memory _proof, string memory _root, string memory _leaf) + internal + pure + returns (string memory) + { string memory result = string.concat( "{", - "\"inputs\":", _inputs, ",", - "\"proof\":", _proof, ",", - "\"root\":\"", _root, "\",", - "\"leaf\":\"", _leaf, "\"", + "\"inputs\":", + _inputs, + ",", + "\"proof\":", + _proof, + ",", + "\"root\":\"", + _root, + "\",", + "\"leaf\":\"", + _leaf, + "\"", "}" ); diff --git a/script/common/ScriptHelper.sol b/script/common/ScriptHelper.sol index e8dd4ae..6a05cba 100644 --- a/script/common/ScriptHelper.sol +++ b/script/common/ScriptHelper.sol @@ -5,7 +5,6 @@ import "forge-std/Script.sol"; /// @notice Helper functions for scripts contract ScriptHelper is Script { - /// @dev Compares two strings and returns true iff they are equal. function compareStrings(string memory a, string memory b) internal pure returns (bool) { return (keccak256(abi.encodePacked((a))) == keccak256(abi.encodePacked((b)))); @@ -15,7 +14,7 @@ contract ScriptHelper is Script { function ltrim64(bytes memory _bytes) internal pure returns (bytes memory) { return slice(_bytes, 64, _bytes.length - 64); } - + /// @dev Returns a slice of `_bytes` starting at index `_start` and of length `_length`. /// referenece: https://github.com/GNSPS/solidity-bytes-utils/blob/6458fb2780a3092bc756e737f246be1de6d3d362/contracts/BytesLib.sol#L228 function slice(bytes memory _bytes, uint256 _start, uint256 _length) internal pure returns (bytes memory) { @@ -33,14 +32,10 @@ contract ScriptHelper is Script { let mc := add(add(tempBytes, lengthmod), mul(0x20, iszero(lengthmod))) let end := add(mc, _length) - for { - let cc := add(add(add(_bytes, lengthmod), mul(0x20, iszero(lengthmod))), _start) - } lt(mc, end) { + for { let cc := add(add(add(_bytes, lengthmod), mul(0x20, iszero(lengthmod))), _start) } lt(mc, end) { mc := add(mc, 0x20) cc := add(cc, 0x20) - } { - mstore(mc, mload(cc)) - } + } { mstore(mc, mload(cc)) } mstore(tempBytes, _length) mstore(0x40, and(add(mc, 31), not(31))) @@ -59,11 +54,12 @@ contract ScriptHelper is Script { function stringArrayToString(string[] memory array) internal pure returns (string memory) { string memory result = "["; - for (uint i = 0; i < array.length; i++) { - if (i != array.length - 1) + for (uint256 i = 0; i < array.length; i++) { + if (i != array.length - 1) { result = string.concat(result, "\"", array[i], "\","); - else + } else { result = string.concat(result, "\"", array[i], "\""); + } } return string.concat(result, "]"); @@ -73,11 +69,12 @@ contract ScriptHelper is Script { function stringArrayToArrayString(string[] memory array) internal pure returns (string memory) { string memory result = "["; - for (uint i = 0; i < array.length; i++) { - if (i != array.length - 1) + for (uint256 i = 0; i < array.length; i++) { + if (i != array.length - 1) { result = string.concat(result, array[i], ","); - else + } else { result = string.concat(result, array[i]); + } } return string.concat(result, "]"); @@ -87,13 +84,14 @@ contract ScriptHelper is Script { function bytes32ArrayToString(bytes32[] memory array) internal pure returns (string memory) { string memory result = "["; - for (uint i = 0; i < array.length; i++) { - if (i != array.length - 1) + for (uint256 i = 0; i < array.length; i++) { + if (i != array.length - 1) { result = string.concat(result, "\"", vm.toString(array[i]), "\","); - else + } else { result = string.concat(result, "\"", vm.toString(array[i]), "\""); + } } return string.concat(result, "]"); } -} \ No newline at end of file +} diff --git a/script/test/Merkle.s.t.sol b/script/test/Merkle.s.t.sol index 3f51466..db1b0bf 100644 --- a/script/test/Merkle.s.t.sol +++ b/script/test/Merkle.s.t.sol @@ -8,7 +8,6 @@ import "openzeppelin-contracts/contracts/utils/cryptography/MerkleProof.sol"; import "../common/ScriptHelper.sol"; contract MerkleScriptOutputTest is Test, ScriptHelper { - // TODO: hardcoding the output from /script/target/output.json for simplicity now bytes32[] proof = [ @@ -16,10 +15,8 @@ contract MerkleScriptOutputTest is Test, ScriptHelper { bytes32(0x7040f273bcf51ac5aeb606be19c784802b4a81469d609428e2487c5fa23e855e), bytes32(0x00d9335e1f2b15d085d331fc240a20196e4ae5037b7d317f05d597c855a329b4) ]; - bytes32 root = - 0xb61922e9343b32c54f90f4cca2561cc277e090b279ba4f9e75d1a9994b895123; - bytes32 leaf = - 0x035e33df50de019c2fdafb75e088976405fe8806b0341fa28db67c78e5e7f0e7; + bytes32 root = 0xb61922e9343b32c54f90f4cca2561cc277e090b279ba4f9e75d1a9994b895123; + bytes32 leaf = 0x035e33df50de019c2fdafb75e088976405fe8806b0341fa28db67c78e5e7f0e7; address addr = 0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266; uint256 value = 10000; @@ -39,10 +36,7 @@ contract MerkleScriptOutputTest is Test, ScriptHelper { _bytes[1] = b_bytes; _bytes[2] = c_bytes; - assertEq( - abi.encode(a, b, c), - ltrim64(abi.encode(_bytes)) - ); + assertEq(abi.encode(a, b, c), ltrim64(abi.encode(_bytes))); } function testComputeLeaf() public { diff --git a/src/Merkle.sol b/src/Merkle.sol index 540abef..20e7549 100644 --- a/src/Merkle.sol +++ b/src/Merkle.sol @@ -7,24 +7,25 @@ import "./common/MurkyBase.sol"; /// @author dmfxyz /// @dev Note Generic Merkle Tree contract Merkle is MurkyBase { - - /******************** - * HASHING FUNCTION * - ********************/ + /** + * + * HASHING FUNCTION * + * + */ /// ascending sort and concat prior to hashing function hashLeafPairs(bytes32 left, bytes32 right) public pure override returns (bytes32 _hash) { - assembly { - switch lt(left, right) - case 0 { - mstore(0x0, right) - mstore(0x20, left) - } - default { - mstore(0x0, left) - mstore(0x20, right) - } - _hash := keccak256(0x0, 0x40) - } + assembly { + switch lt(left, right) + case 0 { + mstore(0x0, right) + mstore(0x20, left) + } + default { + mstore(0x0, left) + mstore(0x20, right) + } + _hash := keccak256(0x0, 0x40) + } } -} \ No newline at end of file +} diff --git a/src/Xorkle.sol b/src/Xorkle.sol index 7314a30..efb4ea6 100644 --- a/src/Xorkle.sol +++ b/src/Xorkle.sol @@ -7,16 +7,16 @@ import "./common/MurkyBase.sol"; /// @author dmfxyz /// @dev Note Xor Based "Merkle" Tree contract Xorkle is MurkyBase { - - /******************** - * HASHING FUNCTION * - ********************/ - + /** + * + * HASHING FUNCTION * + * + */ function hashLeafPairs(bytes32 left, bytes32 right) public pure override returns (bytes32 _hash) { // saves a few gas lol assembly { - mstore(0x0, xor(left,right)) - _hash := keccak256(0x0, 0x20) + mstore(0x0, xor(left, right)) + _hash := keccak256(0x0, 0x20) } } } diff --git a/src/common/MurkyBase.sol b/src/common/MurkyBase.sol index 06d6f37..4bf0b96 100644 --- a/src/common/MurkyBase.sol +++ b/src/common/MurkyBase.sol @@ -2,40 +2,45 @@ pragma solidity ^0.8.4; abstract contract MurkyBase { - /*************** - * CONSTRUCTOR * - ***************/ + /** + * + * CONSTRUCTOR * + * + */ constructor() {} - /******************** - * VIRTUAL HASHING FUNCTIONS * - ********************/ + /** + * + * VIRTUAL HASHING FUNCTIONS * + * + */ function hashLeafPairs(bytes32 left, bytes32 right) public pure virtual returns (bytes32 _hash); - - /********************** - * PROOF VERIFICATION * - **********************/ - + /** + * + * PROOF VERIFICATION * + * + */ function verifyProof(bytes32 root, bytes32[] memory proof, bytes32 valueToProve) external pure returns (bool) { // proof length must be less than max array size bytes32 rollingHash = valueToProve; uint256 length = proof.length; unchecked { - for(uint i = 0; i < length; ++i){ + for (uint256 i = 0; i < length; ++i) { rollingHash = hashLeafPairs(rollingHash, proof[i]); } } return root == rollingHash; } - /******************** - * PROOF GENERATION * - ********************/ - + /** + * + * PROOF GENERATION * + * + */ function getRoot(bytes32[] memory data) public pure returns (bytes32) { require(data.length > 1, "won't generate root for single leaf"); - while(data.length > 1) { + while (data.length > 1) { data = hashLevel(data); } return data[0]; @@ -43,23 +48,21 @@ abstract contract MurkyBase { function getProof(bytes32[] memory data, uint256 node) public pure returns (bytes32[] memory) { require(data.length > 1, "won't generate proof for single leaf"); - // The size of the proof is equal to the ceiling of log2(numLeaves) + // The size of the proof is equal to the ceiling of log2(numLeaves) bytes32[] memory result = new bytes32[](log2ceilBitMagic(data.length)); uint256 pos = 0; // Two overflow risks: node, pos // node: max array size is 2**256-1. Largest index in the array will be 1 less than that. Also, - // for dynamic arrays, size is limited to 2**64-1 + // for dynamic arrays, size is limited to 2**64-1 // pos: pos is bounded by log2(data.length), which should be less than type(uint256).max - while(data.length > 1) { + while (data.length > 1) { unchecked { - if(node & 0x1 == 1) { + if (node & 0x1 == 1) { result[pos] = data[node - 1]; - } - else if (node + 1 == data.length) { - result[pos] = bytes32(0); - } - else { + } else if (node + 1 == data.length) { + result[pos] = bytes32(0); + } else { result[pos] = data[node + 1]; } ++pos; @@ -76,36 +79,38 @@ abstract contract MurkyBase { // Function is private, and all internal callers check that data.length >=2. // Underflow is not possible as lowest possible value for data/result index is 1 - // overflow should be safe as length is / 2 always. + // overflow should be safe as length is / 2 always. unchecked { uint256 length = data.length; - if (length & 0x1 == 1){ + if (length & 0x1 == 1) { result = new bytes32[](length / 2 + 1); result[result.length - 1] = hashLeafPairs(data[length - 1], bytes32(0)); } else { result = new bytes32[](length / 2); - } - // pos is upper bounded by data.length / 2, so safe even if array is at max size + } + // pos is upper bounded by data.length / 2, so safe even if array is at max size uint256 pos = 0; - for (uint256 i = 0; i < length-1; i+=2){ - result[pos] = hashLeafPairs(data[i], data[i+1]); + for (uint256 i = 0; i < length - 1; i += 2) { + result[pos] = hashLeafPairs(data[i], data[i + 1]); ++pos; } } return result; } - /****************** - * MATH "LIBRARY" * - ******************/ - + /** + * + * MATH "LIBRARY" * + * + */ + /// @dev Note that x is assumed > 0 function log2ceil(uint256 x) public pure returns (uint256) { uint256 ceil = 0; - uint pOf2; + uint256 pOf2; // If x is a power of 2, then this function will return a ceiling // that is 1 greater than the actual ceiling. So we need to check if - // x is a power of 2, and subtract one from ceil if so. + // x is a power of 2, and subtract one from ceil if so. assembly { // we check by seeing if x == (~x + 1) & x. This applies a mask // to find the lowest set bit of x and then checks it for equality @@ -126,11 +131,11 @@ abstract contract MurkyBase { // we do some assembly magic to treat the bool as an integer later on pOf2 := eq(and(add(not(x), 1), x), x) } - + // if x == type(uint256).max, than ceil is capped at 256 // if x == 0, then pO2 == 0, so ceil won't underflow unchecked { - while( x > 0) { + while (x > 0) { x >>= 1; ceil++; } @@ -141,41 +146,41 @@ abstract contract MurkyBase { /// Original bitmagic adapted from https://github.com/paulrberg/prb-math/blob/main/contracts/PRBMath.sol /// @dev Note that x assumed > 1 - function log2ceilBitMagic(uint256 x) public pure returns (uint256){ + function log2ceilBitMagic(uint256 x) public pure returns (uint256) { if (x <= 1) { return 0; } uint256 msb = 0; uint256 _x = x; - if (x >= 2**128) { + if (x >= 2 ** 128) { x >>= 128; msb += 128; } - if (x >= 2**64) { + if (x >= 2 ** 64) { x >>= 64; msb += 64; } - if (x >= 2**32) { + if (x >= 2 ** 32) { x >>= 32; msb += 32; } - if (x >= 2**16) { + if (x >= 2 ** 16) { x >>= 16; msb += 16; } - if (x >= 2**8) { + if (x >= 2 ** 8) { x >>= 8; msb += 8; } - if (x >= 2**4) { + if (x >= 2 ** 4) { x >>= 4; msb += 4; } - if (x >= 2**2) { + if (x >= 2 ** 2) { x >>= 2; msb += 2; } - if (x >= 2**1) { + if (x >= 2 ** 1) { msb += 1; } @@ -186,4 +191,4 @@ abstract contract MurkyBase { return msb + 1; } } -} \ No newline at end of file +} diff --git a/src/test/Merkle.t.sol b/src/test/Merkle.t.sol index e0227c8..fae0fe7 100644 --- a/src/test/Merkle.t.sol +++ b/src/test/Merkle.t.sol @@ -7,6 +7,7 @@ import "openzeppelin-contracts/contracts/utils/cryptography/MerkleProof.sol"; contract ContractTest is Test { Merkle m; + function setUp() public { m = new Merkle(); } @@ -22,7 +23,7 @@ contract ContractTest is Test { bytes32 hNaive = keccak256(packed); assertEq(hAssem, hNaive); } - + function testGenerateProof(bytes32[] memory data, uint256 node) public { vm.assume(data.length > 1); vm.assume(node < data.length); @@ -31,7 +32,7 @@ contract ContractTest is Test { bytes32 valueToProve = data[node]; bytes32 rollingHash = valueToProve; - for(uint i = 0; i < proof.length; ++i){ + for (uint256 i = 0; i < proof.length; ++i) { rollingHash = m.hashLeafPairs(rollingHash, proof[i]); } assertEq(rollingHash, root); @@ -66,7 +67,7 @@ contract ContractTest is Test { function testWontGetRootSingleLeaf() public { bytes32[] memory data = new bytes32[](1); - data[0] = bytes32(0x0); + data[0] = bytes32(0x0); vm.expectRevert("won't generate root for single leaf"); m.getRoot(data); } @@ -79,7 +80,7 @@ contract ContractTest is Test { } function valueNotInArray(bytes32[] memory data, bytes32 value) public pure returns (bool) { - for (uint i = 0; i < data.length; ++i) { + for (uint256 i = 0; i < data.length; ++i) { if (data[i] == value) return false; } return true; diff --git a/src/test/MurkyBase.t.sol b/src/test/MurkyBase.t.sol index 2576c6a..db38c5d 100644 --- a/src/test/MurkyBase.t.sol +++ b/src/test/MurkyBase.t.sol @@ -4,28 +4,26 @@ import "forge-std/Test.sol"; import "../common/MurkyBase.sol"; contract MurkyBaseTest is Test, MurkyBase { - // Hacky way to test the base functions until transitioned to library function hashLeafPairs(bytes32, bytes32) public pure virtual override returns (bytes32) { return bytes32(0x0); } - function testLogCeil(uint256 x) public{ + function testLogCeil(uint256 x) public view { vm.assume(x > 0); this.log2ceil(x); } - function testLogCeilBitMagic(uint256 x) public { + function testLogCeilBitMagic(uint256 x) public view { vm.assume(x > 0); this.log2ceilBitMagic(x); } - function testLogCeil_KnownPowerOf2() public { assertEq(3, this.log2ceilBitMagic(8)); } + function testLogCeil_Known() public { assertEq(8, this.log2ceilBitMagic((129))); } - -} \ No newline at end of file +} diff --git a/src/test/StandardInput.t.sol b/src/test/StandardInput.t.sol index 221c90b..8e77cdf 100644 --- a/src/test/StandardInput.t.sol +++ b/src/test/StandardInput.t.sol @@ -14,23 +14,22 @@ contract StandardizedInputTest is Test { string[] memory inputs = new string[](2); inputs[0] = "cat"; inputs[1] = "src/test/standard_data/StandardInput.txt"; - bytes memory result = vm.ffi(inputs); + bytes memory result = vm.ffi(inputs); data = abi.decode(result, (bytes32[100])); x = new Xorkle(); m = new Merkle(); } - + function testXorkleGenerateProofStandard() public view { - bytes32[] memory _data = _getData(); - for (uint i = 0; i < leaves.length; ++i) { + bytes32[] memory _data = _getData(); + for (uint256 i = 0; i < leaves.length; ++i) { x.getProof(_data, leaves[i]); } - } function testMerkleGenerateProofStandard() public view { bytes32[] memory _data = _getData(); - for(uint i = 0; i < leaves.length; ++i) { + for (uint256 i = 0; i < leaves.length; ++i) { m.getProof(_data, leaves[i]); } } @@ -38,7 +37,7 @@ contract StandardizedInputTest is Test { function testXorkleVerifyProofStandard() public { bytes32[] memory _data = _getData(); bytes32 root = x.getRoot(_data); - for (uint i = 0; i < leaves.length; ++i) { + for (uint256 i = 0; i < leaves.length; ++i) { bytes32[] memory proof = x.getProof(_data, leaves[i]); assertTrue(x.verifyProof(root, proof, _data[leaves[i]])); } @@ -47,7 +46,7 @@ contract StandardizedInputTest is Test { function testMerkleVerifyProofStandard() public { bytes32[] memory _data = _getData(); bytes32 root = m.getRoot(_data); - for (uint i = 0; i < leaves.length; ++i) { + for (uint256 i = 0; i < leaves.length; ++i) { bytes32[] memory proof = m.getProof(_data, leaves[i]); assertTrue(m.verifyProof(root, proof, _data[leaves[i]])); } @@ -55,10 +54,10 @@ contract StandardizedInputTest is Test { function _getData() public view returns (bytes32[] memory) { bytes32[] memory _data = new bytes32[](data.length); - uint length = data.length; - for (uint i = 0; i < length; ++i) { + uint256 length = data.length; + for (uint256 i = 0; i < length; ++i) { _data[i] = data[i]; } return _data; } -} \ No newline at end of file +} diff --git a/src/test/Xorkle.t.sol b/src/test/Xorkle.t.sol index f67daf3..099d98c 100644 --- a/src/test/Xorkle.t.sol +++ b/src/test/Xorkle.t.sol @@ -7,6 +7,7 @@ import "forge-std/Test.sol"; contract ContractTest is DSTest { Xorkle m; Vm vm = Vm(HEVM_ADDRESS); + function setUp() public { m = new Xorkle(); } @@ -16,7 +17,7 @@ contract ContractTest is DSTest { bytes32 hNaive = keccak256(abi.encode(left ^ right)); assertEq(hAssem, hNaive); } - + function testGenerateProof(bytes32[] memory data, uint256 node) public { vm.assume(data.length > 1); vm.assume(node < data.length); @@ -25,7 +26,7 @@ contract ContractTest is DSTest { bytes32 valueToProve = data[node]; bytes32 rollingHash = valueToProve; - for(uint i = 0; i < proof.length; ++i){ + for (uint256 i = 0; i < proof.length; ++i) { rollingHash = m.hashLeafPairs(rollingHash, proof[i]); } assertEq(rollingHash, root); @@ -51,7 +52,7 @@ contract ContractTest is DSTest { function testWontGetRootSingleLeaf() public { bytes32[] memory data = new bytes32[](1); - data[0] = bytes32(0x0); + data[0] = bytes32(0x0); vm.expectRevert("won't generate root for single leaf"); m.getRoot(data); } @@ -64,7 +65,7 @@ contract ContractTest is DSTest { } function valueNotInArray(bytes32[] memory data, bytes32 value) public pure returns (bool) { - for (uint i = 0; i < data.length; ++i) { + for (uint256 i = 0; i < data.length; ++i) { if (data[i] == value) return false; } return true;