From 25b41164d5641dff3a96af31e2777f5545ca7439 Mon Sep 17 00:00:00 2001 From: Franco NG Date: Thu, 14 Dec 2023 17:02:59 +0100 Subject: [PATCH] Updated NatSpecs, error msg, and testcases --- script/Utils.sol | 7 ++--- src/L2/L2Claim.sol | 57 ++++++++++++++++++++++++++----------- test/L2/L2Claim.t.sol | 66 +++++++++++++++++++++++++++++++------------ 3 files changed, 91 insertions(+), 39 deletions(-) diff --git a/script/Utils.sol b/script/Utils.sol index 04fe1b44..c1b7daf8 100644 --- a/script/Utils.sol +++ b/script/Utils.sol @@ -38,7 +38,7 @@ contract Utils is Script { /// @notice This struct is used to read MerkleTree from JSON file. struct MerkleTree { - MerkleTreeLeaf[] leaf; + MerkleTreeLeaf[] leaves; bytes32 merkleRoot; } @@ -79,12 +79,11 @@ contract Utils is Script { finalJson.write(string.concat("deployment/l2addresses.json")); } - /// @notice This function reads MerkleTree from JSON file. - /// @return L2ClaimConfig struct containing merkle root. + /// @return MerkleTree struct containing merkle root. function readMerkleTreeFile() external view returns (MerkleTree memory) { string memory root = vm.projectRoot(); - string memory addressPath = string.concat(root, "/script/merkleTree.json"); + string memory addressPath = string.concat(root, "/script/data/devnet/merkleTree.json"); string memory addressJson = vm.readFile(addressPath); bytes memory addressRaw = vm.parseJson(addressJson); return abi.decode(addressRaw, (MerkleTree)); diff --git a/src/L2/L2Claim.sol b/src/L2/L2Claim.sol index d4e47c18..8b73a5dc 100644 --- a/src/L2/L2Claim.sol +++ b/src/L2/L2Claim.sol @@ -18,7 +18,7 @@ struct ED25519Signature { } /// @title L2Claim -/// @notice L2Claim lets user claim their LSK token from LSK Chain using Merkle Tree method. +/// @notice L2Claim lets user claim their LSK token from Lisk Chain using Merkle Tree method. contract L2Claim { /// @notice LSK originally has 8 d.p., L2 LSK has 18. uint256 public constant LSK_MULTIPLIER = 10 ** 10; @@ -26,7 +26,7 @@ contract L2Claim { /// @notice address of L2 LSK Token. IERC20 public immutable l2LiskToken; - /// @notice Merkle Tree for the claim. + /// @notice Merkle Root for the claim. bytes32 public immutable merkleRoot; // @notice Records claimed addresses (lskAddress => boolean). @@ -35,29 +35,46 @@ contract L2Claim { /// @notice Emitted when an address has claimed the LSK. event LSKClaimed(bytes20 lskAddress, address recipient, uint256 amount); - /// @notice _l2LiskToken L2 LSK Token Address - /// @notice _merkleRoot Merkle Tree Root + /// @notice Constructs the L2Claim contract. + /// @param _l2LiskToken L2 LSK Token Address + /// @param _merkleRoot Merkle Tree Root constructor(address _l2LiskToken, bytes32 _merkleRoot) { l2LiskToken = IERC20(_l2LiskToken); merkleRoot = _merkleRoot; } /// @notice Verifies ED25519 Signature, throws error when verification fails. - /// @param _pubKey Public Key of the address in LSK Chain. - /// @param _r r-value of the ED25519 signature. - /// @param _s s-value of the ED25519 signature. - /// @param _message Message to be verified. - function verifySignature(bytes32 _pubKey, bytes32 _r, bytes32 _s, bytes32 _message) internal pure { - require(Ed25519.check(_pubKey, _r, _s, _message, bytes9(0)), "Invalid Signature"); + /// @param _pubKey Public Key of the address in Lisk Chain. + /// @param _r r-value of the ED25519 signature. + /// @param _s s-value of the ED25519 signature. + /// @param _message Message to be verified. + /// @param _errorMessage Message the contract should throw, when the check returns false. + function verifySignature( + bytes32 _pubKey, + bytes32 _r, + bytes32 _s, + bytes32 _message, + string memory _errorMessage + ) + internal + pure + { + require(Ed25519.check(_pubKey, _r, _s, _message, bytes9(0)), _errorMessage); } /// @notice Hash a message twice using Keccak-256. /// @param _message Message to be hashed. + /// @return double keccak256 hashed bytes32 function doubleKeccak256(bytes memory _message) internal pure returns (bytes32) { return keccak256(bytes.concat(keccak256(_message))); } /// @notice Internal function called by both regular and multisig claims. + /// @param _lskAddress LSK Address in bytes format. + /// @param _amount Amount of LSK (In Beddows [1 LSK = 10**8 Beddow]). + /// @param _proof Array of hashes that proves existence of the leaf. + /// @param _leaf Double-hashed leaf by combining address, amount and signatures. + /// @param _recipient Destination address at L2 Chain. function claim( bytes20 _lskAddress, uint64 _amount, @@ -78,8 +95,8 @@ contract L2Claim { /// @notice Claim LSK from a regular account. /// @param _proof Array of hashes that proves existence of the leaf. - /// @param _pubKey Public Key of LSK Address. - /// @param _amount Amount of LSK (In Beddows). + /// @param _pubKey Public Key of the address in Lisk Chain. + /// @param _amount Amount of LSK (In Beddows [1 LSK = 10**8 Beddow]). /// @param _recipient Destination address at L2 Chain. /// @param _sig ED25519 signature pair. function claimRegularAccount( @@ -94,7 +111,7 @@ contract L2Claim { bytes20 lskAddress = bytes20(sha256(abi.encode(_pubKey))); bytes32 leaf = doubleKeccak256(abi.encode(lskAddress, _amount, uint32(0), new bytes32[](0), new bytes32[](0))); - verifySignature(_pubKey, _sig.r, _sig.s, keccak256(abi.encode(leaf, _recipient))); + verifySignature(_pubKey, _sig.r, _sig.s, keccak256(abi.encode(leaf, _recipient)), "Invalid Signature"); claim(lskAddress, _amount, _proof, leaf, _recipient); } @@ -102,7 +119,7 @@ contract L2Claim { /// @notice Claim LSK from a multisig account. /// @param _proof Array of hashes that proves existence of the leaf. /// @param _lskAddress LSK Address in bytes format. - /// @param _amount Amount of LSK (In Beddows). + /// @param _amount Amount of LSK (In Beddows [1 LSK = 10**8 Beddow]). /// @param _keys Structs of Mandatory Keys and Optional Keys. /// @param _recipient Destination address at L2 Chain. /// @param _sigs Array of ED25519 signature pair. @@ -116,7 +133,10 @@ contract L2Claim { ) external { - require(_sigs.length == _keys.optionalKeys.length + _keys.mandatoryKeys.length, "Invalid Signature Length"); + require( + _sigs.length == _keys.optionalKeys.length + _keys.mandatoryKeys.length, + "Signatures array has invalid length" + ); // If numberOfSignatures passes MerkleProof in later stage, that means this value is correct. uint32 numberOfSignatures = uint32(_keys.mandatoryKeys.length); @@ -135,7 +155,9 @@ contract L2Claim { bytes32 message = keccak256(abi.encode(leaf, _recipient)); for (uint256 i = 0; i < _keys.mandatoryKeys.length; i++) { - verifySignature(_keys.mandatoryKeys[i], _sigs[i].r, _sigs[i].s, message); + verifySignature( + _keys.mandatoryKeys[i], _sigs[i].r, _sigs[i].s, message, "Invalid signature for mandatoryKey" + ); } for (uint256 i = 0; i < _keys.optionalKeys.length; i++) { @@ -146,7 +168,8 @@ contract L2Claim { _keys.optionalKeys[i], _sigs[i + _keys.mandatoryKeys.length].r, _sigs[i + _keys.mandatoryKeys.length].s, - message + message, + "Invalid signature for optionalKey" ); } diff --git a/test/L2/L2Claim.t.sol b/test/L2/L2Claim.t.sol index 6ea4a9ad..c016e984 100644 --- a/test/L2/L2Claim.t.sol +++ b/test/L2/L2Claim.t.sol @@ -26,6 +26,7 @@ contract L2ClaimTest is Test { ERC20 public lsk; L2Claim public l2Claim; + Utils public utils; string public merkleTreeJson; string public signatureJson; @@ -66,7 +67,7 @@ contract L2ClaimTest is Test { function test_claimRegularAccount_RevertWhenInvalidProof() public { uint256 accountIndex = 0; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); leaf.proof[0] = bytes32AddOne(leaf.proof[0]); @@ -83,7 +84,7 @@ contract L2ClaimTest is Test { function test_claimRegularAccount_RevertWhenValidProofInvalidSig() public { uint256 accountIndex = 0; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); vm.expectRevert(); @@ -107,7 +108,7 @@ contract L2ClaimTest is Test { function claimRegularAccount(uint256 _accountIndex) internal { uint256 originalBalance = lsk.balanceOf(address(this)); - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[_accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[_accountIndex]; Signature memory signature = getSignature(_accountIndex); l2Claim.claimRegularAccount( @@ -132,7 +133,7 @@ contract L2ClaimTest is Test { uint256 claimIndex = 0; claimRegularAccount(claimIndex); - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[claimIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[claimIndex]; Signature memory signature = getSignature(claimIndex); vm.expectRevert("Already Claimed"); @@ -148,7 +149,7 @@ contract L2ClaimTest is Test { // Multisig settings refers to: lisk-merkle-tree-builder/data/example/create-balances.ts function test_claimMultisigAccount_RevertWhenIncorrectProof() public { uint256 accountIndex = 50; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); ED25519Signature[] memory ed25519Signatures = new ED25519Signature[](leaf.numberOfSignatures); @@ -170,9 +171,9 @@ contract L2ClaimTest is Test { ); } - function test_claimMultisigAccount_RevertWhenValidProofInvalidSig() public { + function test_claimMultisigAccount_RevertWhenValidProofInvalidMandatorySig() public { uint256 accountIndex = 50; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); ED25519Signature[] memory ed25519Signatures = new ED25519Signature[](leaf.numberOfSignatures); @@ -183,7 +184,34 @@ contract L2ClaimTest is Test { ed25519Signatures[0].r = bytes32AddOne(ed25519Signatures[0].r); - vm.expectRevert("Invalid Signature"); + vm.expectRevert("Invalid signature for mandatoryKey"); + l2Claim.claimMultisigAccount( + leaf.proof, + bytes20(leaf.b32Address << 96), + leaf.balanceBeddows, + MultisigKeys(leaf.mandatoryKeys, leaf.optionalKeys), + address(this), + ed25519Signatures + ); + } + + function test_claimMultisigAccount_RevertWhenValidProofInvalidOptionalSig() public { + uint256 accountIndex = 51; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; + Signature memory signature = getSignature(accountIndex); + + ED25519Signature[] memory ed25519Signatures = + new ED25519Signature[](leaf.mandatoryKeys.length + leaf.optionalKeys.length); + + for (uint256 i; i < leaf.numberOfSignatures; i++) { + ed25519Signatures[i] = ED25519Signature(signature.sigs[i].r, signature.sigs[i].s); + } + + // Shifting byte of the last sig (i.e. one of the optionalKey sig) + ed25519Signatures[leaf.numberOfSignatures - 1].r = + bytes32AddOne(ed25519Signatures[leaf.numberOfSignatures - 1].r); + + vm.expectRevert("Invalid signature for optionalKey"); l2Claim.claimMultisigAccount( leaf.proof, bytes20(leaf.b32Address << 96), @@ -196,7 +224,7 @@ contract L2ClaimTest is Test { function test_claimMultisigAccount_RevertWhenValidProofInsufficientSig() public { uint256 accountIndex = 50; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); ED25519Signature[] memory ed25519Signatures = new ED25519Signature[](leaf.numberOfSignatures); @@ -205,7 +233,7 @@ contract L2ClaimTest is Test { ed25519Signatures[i] = ED25519Signature(signature.sigs[i].r, signature.sigs[i].s); } - vm.expectRevert("Invalid Signature"); + vm.expectRevert("Invalid signature for mandatoryKey"); l2Claim.claimMultisigAccount( leaf.proof, @@ -219,7 +247,7 @@ contract L2ClaimTest is Test { function test_claimMultisigAccount_RevertWhenSigLengthLongerThanManKeysAndOpKeys() public { uint256 accountIndex = 50; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); ED25519Signature[] memory ed25519Signatures = new ED25519Signature[](leaf.numberOfSignatures + 1); @@ -240,10 +268,12 @@ contract L2ClaimTest is Test { } // numberOfSignatures are calculated by number of non-empty signatures, hence providing more signature than needed - // would result in sig error + // would result in sig error at mandatoryKey stage function test_claimMultisigAccount_RevertWhenSigOversupplied() public { + // 1m + 2o, numberOfSignatures = 2 uint256 accountIndex = 51; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); ED25519Signature[] memory ed25519Signatures = @@ -253,7 +283,7 @@ contract L2ClaimTest is Test { ed25519Signatures[i] = ED25519Signature(signature.sigs[i].r, signature.sigs[i].s); } - vm.expectRevert("Invalid Signature"); + vm.expectRevert("Invalid signature for mandatoryKey"); l2Claim.claimMultisigAccount( leaf.proof, bytes20(leaf.b32Address << 96), @@ -266,7 +296,7 @@ contract L2ClaimTest is Test { function test_claimMultisigAccount_SuccessClaim_3M() public { uint256 accountIndex = 50; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); ED25519Signature[] memory ed25519Signatures = new ED25519Signature[](leaf.numberOfSignatures); @@ -288,7 +318,7 @@ contract L2ClaimTest is Test { function test_claimMultisigAccount_SuccessClaim_1M_2O() public { uint256 accountIndex = 51; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); ED25519Signature[] memory ed25519Signatures = @@ -314,7 +344,7 @@ contract L2ClaimTest is Test { function test_claimMultisigAccount_SuccessClaim_3M_3O() public { uint256 accountIndex = 52; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); ED25519Signature[] memory ed25519Signatures = @@ -340,7 +370,7 @@ contract L2ClaimTest is Test { function test_claimMultisigAccount_SuccessClaim_64M() public { uint256 accountIndex = 53; - Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaf[accountIndex]; + Utils.MerkleTreeLeaf memory leaf = getMerkleTree().leaves[accountIndex]; Signature memory signature = getSignature(accountIndex); ED25519Signature[] memory ed25519Signatures =