From fa203a3e001077771162cc22b900b894c8a67815 Mon Sep 17 00:00:00 2001 From: Haider-Ali-DS Date: Mon, 23 Dec 2024 21:25:40 +0500 Subject: [PATCH 1/2] emit events only when something changed add test for shard events --- src/Gateway.sol | 28 +++++++++------- src/interfaces/IExecutor.sol | 5 +++ src/storage/Shards.sol | 39 +++++++++++++++++----- test/Gateway.t.sol | 64 +++++++++++++++++++++++++++++++++++- 4 files changed, 115 insertions(+), 21 deletions(-) diff --git a/src/Gateway.sol b/src/Gateway.sol index 5890327..bfc2070 100644 --- a/src/Gateway.sol +++ b/src/Gateway.sol @@ -429,10 +429,12 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { */ function setShard(TssKey calldata publicKey) external { require(msg.sender == ERC1967.getAdmin(), "unauthorized"); - ShardStore.getMainStorage().register(publicKey); - TssKey[] memory keys = new TssKey[](1); - keys[0] = publicKey; - emit ShardsRegistered(keys); + bool isSuccess = ShardStore.getMainStorage().register(publicKey); + if (isSuccess) { + TssKey[] memory keys = new TssKey[](1); + keys[0] = publicKey; + emit ShardsRegistered(keys); + } } /** @@ -456,19 +458,23 @@ contract Gateway is IGateway, IExecutor, IUpgradable, GatewayEIP712 { */ function revokeShard(TssKey calldata publicKey) external { require(msg.sender == ERC1967.getAdmin(), "unauthorized"); - ShardStore.getMainStorage().revoke(publicKey); - TssKey[] memory keys = new TssKey[](1); - keys[0] = publicKey; - emit ShardsUnregistered(keys); + bool isSuccess = ShardStore.getMainStorage().revoke(publicKey); + if (isSuccess) { + TssKey[] memory keys = new TssKey[](1); + keys[0] = publicKey; + emit ShardsUnregistered(keys); + } } /** * @dev Revoke Shards in batch. */ - function revokeShard(TssKey[] calldata publicKeys) external { + function revokeShards(TssKey[] calldata publicKeys) external { require(msg.sender == ERC1967.getAdmin(), "unauthorized"); - ShardStore.getMainStorage().revokeKeys(publicKeys); - emit ShardsUnregistered(publicKeys); + TssKey[] memory revokedKeys = ShardStore.getMainStorage().revokeKeys(publicKeys); + if (revokedKeys.length > 0) { + emit ShardsUnregistered(revokedKeys); + } } /*////////////////////////////////////////////////////////////// diff --git a/src/interfaces/IExecutor.sol b/src/interfaces/IExecutor.sol index 1710208..35a7451 100644 --- a/src/interfaces/IExecutor.sol +++ b/src/interfaces/IExecutor.sol @@ -52,6 +52,11 @@ interface IExecutor { */ function revokeShard(TssKey calldata publicKey) external; + /** + * @dev Revoke a single shard TSS Key. + */ + function revokeShards(TssKey[] calldata publicKey) external; + /** * @dev List all shards currently registered in the gateway. */ diff --git a/src/storage/Shards.sol b/src/storage/Shards.sol index 9ac67d8..c4d1556 100644 --- a/src/storage/Shards.sol +++ b/src/storage/Shards.sol @@ -305,22 +305,27 @@ library ShardStore { * Requirements: * - The `keys` must be registered. */ - function revoke(MainStorage storage store, TssKey calldata key) internal { + function revoke(MainStorage storage store, TssKey calldata key) internal returns (bool) { // Read shard from storage ShardID id = ShardID.wrap(bytes32(key.xCoord)); - ShardInfo memory stored = get(store, id); + (bool exists, ShardInfo memory stored) = tryGet(store, id); - // Check y-parity - require(stored.yParity == (key.yParity & 1), "y parity mismatch, cannot revoke key"); - _revoke(store, id); + if (exists) { + // Check y-parity + require(stored.yParity == (key.yParity & 1), "y parity mismatch, cannot revoke key"); + return _revoke(store, id); + } + + return exists; } /** * @dev Revoke Shards keys. */ - function _revoke(MainStorage storage store, ShardID id) private { + function _revoke(MainStorage storage store, ShardID id) private returns (bool) { // Remove from the set - store.shards.remove(ShardID.unwrap(id)); + StoragePtr ptr = store.shards.remove(ShardID.unwrap(id)); + return ptr.isNull(); } /** @@ -328,11 +333,27 @@ library ShardStore { * Requirements: * - The `publicKeys` must be registered. */ - function revokeKeys(MainStorage storage store, TssKey[] calldata publicKeys) internal { + function revokeKeys(MainStorage storage store, TssKey[] calldata publicKeys) + internal + returns (TssKey[] memory revokedKeys) + { // Revoke tss keys + uint256 keysLength = publicKeys.length; + revokedKeys = new TssKey[](keysLength); + uint256 revokedCount = 0; + for (uint256 i = 0; i < publicKeys.length; i++) { - revoke(store, publicKeys[i]); + if (revoke(store, publicKeys[i])) { + revokedKeys[revokedCount++] = publicKeys[i]; + } + } + + if (revokedKeys.length != keysLength) { + assembly { + mstore(revokedKeys, revokedCount) + } } + return revokedKeys; } function _t(MainStorage storage store) internal view returns (TssKey[] memory) {} diff --git a/test/Gateway.t.sol b/test/Gateway.t.sol index 7960b5b..965291f 100644 --- a/test/Gateway.t.sol +++ b/test/Gateway.t.sol @@ -3,7 +3,8 @@ pragma solidity >=0.8.0; -import {Test, console} from "forge-std/Test.sol"; +import {Test, console, Vm} from "forge-std/Test.sol"; +import "forge-std/console.sol"; import {VmSafe} from "forge-std/Vm.sol"; import {TestUtils, SigningKey, SigningUtils} from "./TestUtils.sol"; import {Gateway, GatewayEIP712} from "../src/Gateway.sol"; @@ -243,6 +244,67 @@ contract GatewayBase is Test { } } + function test_shardEvents() external { + TssKey[] memory keys = new TssKey[](10); + + // create random shard keys + SigningKey memory signer; + for (uint256 i = 0; i < keys.length; i++) { + signer = TestUtils.signerFromEntropy(bytes32(i)); + keys[i] = TssKey({yParity: signer.yParity() == 28 ? 3 : 2, xCoord: signer.xCoord()}); + } + + _sortTssKeys(keys); + + // set shards + vm.prank(ADMIN, ADMIN); + // vm.expectEmit(false, false, false, true); + // emit IExecutor.ShardsRegistered(keys); + gateway.setShards(keys); + + // set a shard which is already registered and verify that is does not emit a event. + vm.prank(ADMIN, ADMIN); + vm.recordLogs(); + gateway.setShard(keys[0]); + Vm.Log[] memory entries = vm.getRecordedLogs(); + assertEq(entries.length, 0); + + // Revoke a registered shard thats not registered. + uint256 unregisteredSignerKey = 11; + signer = TestUtils.signerFromEntropy(bytes32(unregisteredSignerKey)); + TssKey memory nonRegisteredKey = TssKey({yParity: signer.yParity() == 28 ? 3 : 2, xCoord: signer.xCoord()}); + vm.prank(ADMIN, ADMIN); + vm.recordLogs(); + gateway.revokeShard(nonRegisteredKey); + Vm.Log[] memory entries1 = vm.getRecordedLogs(); + assertEq(entries1.length, 0); + + // Revoke a registered shard + vm.prank(ADMIN, ADMIN); + TssKey[] memory unregisteredShardKey = new TssKey[](1); + unregisteredShardKey[0] = keys[0]; + // vm.expectEmit(false, false, false, true); + // emit IExecutor.ShardsUnRegistered(unregisteredShardKey); + gateway.revokeShard(keys[0]); + + // Register a revokedShard + vm.prank(ADMIN, ADMIN); + // vm.expectEmit(false, false, false, true); + // emit IExecutor.ShardsUnRegistered(unregisteredShardKey); + gateway.setShard(unregisteredShardKey[0]); + + // Revoke half of the keys + vm.prank(ADMIN, ADMIN); + uint256 halfKeysLength = keys.length / 2; + TssKey[] memory halfKeys = new TssKey[](halfKeysLength); + for (uint256 i = 0; i < halfKeysLength; i++){ + halfKeys[i] = keys[i]; + } + // vm.expectEmit(false, false, false, true); + // emit IExecutor.ShardsUnRegistered(halfKeys); + gateway.revokeShards(keys); + } + function test_Receiver() external { bytes memory testEncodedCall = abi.encodeCall( IGmpReceiver.onGmpReceived, From a776c6931fb03cfc2b5aad9cefaa17a36a83335b Mon Sep 17 00:00:00 2001 From: Haider-Ali-DS Date: Tue, 24 Dec 2024 01:19:04 +0500 Subject: [PATCH 2/2] fix tests --- src/storage/Shards.sol | 13 ++++++------- src/utils/GasUtils.sol | 2 +- test/Example.t.sol | 2 +- test/Gateway.t.sol | 43 ++++++++++++++++++++++++++---------------- test/TestUtils.sol | 2 +- 5 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/storage/Shards.sol b/src/storage/Shards.sol index c4d1556..26aed8e 100644 --- a/src/storage/Shards.sol +++ b/src/storage/Shards.sol @@ -196,7 +196,7 @@ library ShardStore { */ function register(MainStorage storage store, TssKey calldata newKey) internal returns (bool) { // Check y-parity - require(newKey.yParity == (newKey.yParity & 3), "y parity bit must be 2 or 3, cannot register shard"); + require((newKey.yParity == 2 || newKey.yParity == 3), "y parity bit must be 2 or 3, cannot register shard"); // Read shard from storage ShardID id = ShardID.wrap(bytes32(newKey.xCoord)); @@ -204,7 +204,7 @@ library ShardStore { // Check if the shard is already registered if (!created) { - require(stored.nonce == 1 || newKey.yParity == stored.yParity, "tsskey.yParity mismatch"); + require(stored.nonce == 1 || newKey.yParity == (stored.yParity | 2), "tsskey.yParity mismatch"); return false; } @@ -212,7 +212,7 @@ library ShardStore { ShardInfo memory shard = stored; require( - shard.createdAtBlock == 0 || shard.yParity == newKey.yParity, + shard.createdAtBlock == 0 || (shard.yParity | 2) == newKey.yParity, "the provided y-parity doesn't match the existing y-parity, cannot register shard" ); @@ -269,7 +269,7 @@ library ShardStore { if (register(store, key)) { // Shard registered - created[createdCount++] = TssKey({yParity: key.yParity + 2, xCoord: key.xCoord}); + created[createdCount++] = TssKey({yParity: key.yParity, xCoord: key.xCoord}); } else { // Shard already registered, remove it from the revoke list. uint256 len = revoked.length; @@ -315,8 +315,7 @@ library ShardStore { require(stored.yParity == (key.yParity & 1), "y parity mismatch, cannot revoke key"); return _revoke(store, id); } - - return exists; + return false; } /** @@ -325,7 +324,7 @@ library ShardStore { function _revoke(MainStorage storage store, ShardID id) private returns (bool) { // Remove from the set StoragePtr ptr = store.shards.remove(ShardID.unwrap(id)); - return ptr.isNull(); + return !ptr.isNull(); } /** diff --git a/src/utils/GasUtils.sol b/src/utils/GasUtils.sol index 80f7959..ee55181 100644 --- a/src/utils/GasUtils.sol +++ b/src/utils/GasUtils.sol @@ -25,7 +25,7 @@ library GasUtils { /** * @dev Base cost of the `IGateway.submitMessage` method. */ - uint256 internal constant SUBMIT_BASE_COST = 23525 - 22; + uint256 internal constant SUBMIT_BASE_COST = 23525; /** * @dev Extra gas cost of the first `IGateway.submitMessage` method. diff --git a/test/Example.t.sol b/test/Example.t.sol index a7de3db..111dac7 100644 --- a/test/Example.t.sol +++ b/test/Example.t.sol @@ -50,7 +50,7 @@ contract ExampleTest is Test { returns (Network[] memory networks) { TssKey[] memory keys = new TssKey[](1); - keys[0] = TssKey({yParity: signer.pubkey.yParity() == 28 ? 1 : 0, xCoord: signer.pubkey.px}); + keys[0] = TssKey({yParity: signer.pubkey.yParity() == 28 ? 3 : 2, xCoord: signer.pubkey.px}); networks = new Network[](networkIds.length); for (uint256 i = 0; i < networks.length; i++) { diff --git a/test/Gateway.t.sol b/test/Gateway.t.sol index 965291f..af049c8 100644 --- a/test/Gateway.t.sol +++ b/test/Gateway.t.sol @@ -4,7 +4,6 @@ pragma solidity >=0.8.0; import {Test, console, Vm} from "forge-std/Test.sol"; -import "forge-std/console.sol"; import {VmSafe} from "forge-std/Vm.sol"; import {TestUtils, SigningKey, SigningUtils} from "./TestUtils.sol"; import {Gateway, GatewayEIP712} from "../src/Gateway.sol"; @@ -253,13 +252,12 @@ contract GatewayBase is Test { signer = TestUtils.signerFromEntropy(bytes32(i)); keys[i] = TssKey({yParity: signer.yParity() == 28 ? 3 : 2, xCoord: signer.xCoord()}); } - _sortTssKeys(keys); // set shards vm.prank(ADMIN, ADMIN); - // vm.expectEmit(false, false, false, true); - // emit IExecutor.ShardsRegistered(keys); + vm.expectEmit(false, false, false, true); + emit IExecutor.ShardsRegistered(keys); gateway.setShards(keys); // set a shard which is already registered and verify that is does not emit a event. @@ -283,26 +281,39 @@ contract GatewayBase is Test { vm.prank(ADMIN, ADMIN); TssKey[] memory unregisteredShardKey = new TssKey[](1); unregisteredShardKey[0] = keys[0]; - // vm.expectEmit(false, false, false, true); - // emit IExecutor.ShardsUnRegistered(unregisteredShardKey); + vm.expectEmit(false, false, false, true); + emit IExecutor.ShardsUnregistered(unregisteredShardKey); gateway.revokeShard(keys[0]); - // Register a revokedShard + // Register a revoked shard vm.prank(ADMIN, ADMIN); - // vm.expectEmit(false, false, false, true); - // emit IExecutor.ShardsUnRegistered(unregisteredShardKey); + vm.expectEmit(false, false, false, true); + emit IExecutor.ShardsRegistered(unregisteredShardKey); gateway.setShard(unregisteredShardKey[0]); - // Revoke half of the keys + // Revoke half of the keys and verify event length vm.prank(ADMIN, ADMIN); uint256 halfKeysLength = keys.length / 2; - TssKey[] memory halfKeys = new TssKey[](halfKeysLength); - for (uint256 i = 0; i < halfKeysLength; i++){ - halfKeys[i] = keys[i]; + uint256 secondHalfLength = keys.length - halfKeysLength; + TssKey[] memory firstHalf = new TssKey[](halfKeysLength); + TssKey[] memory secondHalf = new TssKey[](secondHalfLength); + for (uint256 i = 0; i < keys.length; i++){ + if (i < halfKeysLength) { + firstHalf[i] = keys[i]; + } else { + secondHalf[i - halfKeysLength] = keys[i]; + } } - // vm.expectEmit(false, false, false, true); - // emit IExecutor.ShardsUnRegistered(halfKeys); - gateway.revokeShards(keys); + vm.expectEmit(false, false, false, true); + emit IExecutor.ShardsUnregistered(firstHalf); + gateway.revokeShards(firstHalf); + + // register first half keys and check if the other half is unregistered + vm.prank(ADMIN, ADMIN); + vm.expectEmit(false, false, false, true); + emit IExecutor.ShardsRegistered(firstHalf); + emit IExecutor.ShardsUnregistered(secondHalf); + gateway.setShards(firstHalf); } function test_Receiver() external { diff --git a/test/TestUtils.sol b/test/TestUtils.sol index b89a0e2..2fd7399 100644 --- a/test/TestUtils.sol +++ b/test/TestUtils.sol @@ -441,7 +441,7 @@ library TestUtils { require(FACTORY == TestUtils.deployFactory(), "UniversalFactory not deployed"); SigningKey memory signer = TestUtils.createSigner(admin.privateKey); TssKey[] memory keys = new TssKey[](1); - keys[0] = TssKey({yParity: SigningUtils.yParity(signer) == 28 ? 1 : 0, xCoord: SigningUtils.xCoord(signer)}); // Shard key + keys[0] = TssKey({yParity: SigningUtils.yParity(signer) == 28 ? 3 : 2, xCoord: SigningUtils.xCoord(signer)}); // Shard key Network[] memory networks = new Network[](2); address proxyAddr = computeGatewayProxyAddress(admin.addr, salt); networks[0].id = srcRoute; // sepolia network id