From d3d9307c77512c0262aa11dc9c626444a38efeff Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 14:38:13 +0000 Subject: [PATCH 01/32] Isolate some common logic into abstract contract --- .../interfaces/IInterchainModuleEvents.sol | 6 +++ .../contracts/modules/InterchainModule.sol | 52 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 packages/contracts-communication/contracts/interfaces/IInterchainModuleEvents.sol create mode 100644 packages/contracts-communication/contracts/modules/InterchainModule.sol diff --git a/packages/contracts-communication/contracts/interfaces/IInterchainModuleEvents.sol b/packages/contracts-communication/contracts/interfaces/IInterchainModuleEvents.sol new file mode 100644 index 0000000000..cdf89e9420 --- /dev/null +++ b/packages/contracts-communication/contracts/interfaces/IInterchainModuleEvents.sol @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +interface IInterchainModuleEvents { + event VerificationRequested(uint256 indexed destChainId, bytes entry); +} diff --git a/packages/contracts-communication/contracts/modules/InterchainModule.sol b/packages/contracts-communication/contracts/modules/InterchainModule.sol new file mode 100644 index 0000000000..fa726a933f --- /dev/null +++ b/packages/contracts-communication/contracts/modules/InterchainModule.sol @@ -0,0 +1,52 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {IInterchainDB} from "../interfaces/IInterchainDB.sol"; +import {IInterchainModule} from "../interfaces/IInterchainModule.sol"; +import {IInterchainModuleEvents} from "../interfaces/IInterchainModuleEvents.sol"; + +import {InterchainEntry} from "../libs/InterchainEntry.sol"; + +/// @notice Common logic for all Interchain Modules. +abstract contract InterchainModule is IInterchainModule, IInterchainModuleEvents { + address public immutable INTERCHAIN_DB; + + error InterchainModule__NotInterchainDB(); + error InterchainModule__InsufficientFee(uint256 actual, uint256 required); + + constructor(address interchainDB) { + INTERCHAIN_DB = interchainDB; + } + + /// @inheritdoc IInterchainModule + function requestVerification(uint256 destChainId, InterchainEntry memory entry) external payable { + if (msg.sender != INTERCHAIN_DB) { + revert InterchainModule__NotInterchainDB(); + } + uint256 requiredFee = _getModuleFee(destChainId); + if (msg.value < requiredFee) { + revert InterchainModule__InsufficientFee({actual: msg.value, required: requiredFee}); + } + bytes memory encodedEntry = abi.encode(entry); + _requestVerification(destChainId, encodedEntry); + emit VerificationRequested(destChainId, encodedEntry); + } + + /// @inheritdoc IInterchainModule + function getModuleFee(uint256 destChainId) external view returns (uint256) { + return _getModuleFee(destChainId); + } + + /// @dev Should be called once the Module has verified the entry and needs to signal this + /// to the InterchainDB. + function _verifyEntry(bytes memory encodedEntry) internal { + InterchainEntry memory entry = abi.decode(encodedEntry, (InterchainEntry)); + IInterchainDB(INTERCHAIN_DB).verifyEntry(entry); + } + + /// @dev Internal logic to request the verification of an entry on the destination chain. + function _requestVerification(uint256 destChainId, bytes memory encodedEntry) internal virtual; + + /// @dev Internal logic to get the module fee for verifying an entry on the specified destination chain. + function _getModuleFee(uint256 destChainId) internal view virtual returns (uint256); +} From b4cd92a877faab229d045c0f96d2260ea58bfe98 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 14:57:18 +0000 Subject: [PATCH 02/32] Scaffold `ThresholdECDSAModule` --- .../interfaces/IThresholdECDSAModule.sol | 42 ++++++++++++++++ .../modules/ThresholdECDSAModule.sol | 48 +++++++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol create mode 100644 packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol diff --git a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol new file mode 100644 index 0000000000..6b2e2274f2 --- /dev/null +++ b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol @@ -0,0 +1,42 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import {IInterchainModule} from "./IInterchainModule.sol"; + +interface IThresholdECDSAModule is IInterchainModule { + // ═══════════════════════════════════════════════ PERMISSIONED ════════════════════════════════════════════════════ + + /// @notice Adds a new verifier to the module. + /// @dev Could be only called by the owner. Will revert if the verifier is already added. + /// @param verifier The address of the verifier to add + function addVerifier(address verifier) external; + + /// @notice Removes a verifier from the module. + /// @dev Could be only called by the owner. Will revert if the verifier is not added. + /// @param verifier The address of the verifier to remove + function removeVerifier(address verifier) external; + + /// @notice Sets the threshold of the module. + /// @dev Could be only called by the owner. Will revert if the threshold is zero. + /// @param threshold The new threshold value + function setThreshold(uint256 threshold) external; + + /// @notice Sets the address of the gas oracle to be used for estimating the verification fees. + /// @dev Could be only called by the owner. Will revert if the gas oracle is not a contract. + /// @param gasOracle The address of the gas oracle contract + function setGasOracle(address gasOracle) external; + + // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ + + /// @notice Verifies an entry using a set of verifier signatures. + /// If the threshold is met, the entry will be marked as verified in the Interchain DataBase. + /// @param encodedEntry The encoded entry to verify + /// @param signatures An array of signatures used to verify the entry + function verifyEntry(bytes calldata encodedEntry, bytes[] calldata signatures) external; + + // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ + + /// @notice Gets the threshold of the module. + /// This is the minimum number of signatures required for verification. + function getThreshold() external view returns (uint256); +} diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol new file mode 100644 index 0000000000..6a2236289c --- /dev/null +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.20; + +import {InterchainModule} from "./InterchainModule.sol"; + +import {IThresholdECDSAModule} from "../interfaces/IThresholdECDSAModule.sol"; + +import {InterchainEntry} from "../libs/InterchainEntry.sol"; +import {ThresholdECDSA} from "../libs/ThresholdECDSA.sol"; + +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; + +contract ThresholdECDSAModule is InterchainModule, Ownable, IThresholdECDSAModule { + constructor(address interchainDB, address initialOwner) InterchainModule(interchainDB) Ownable(initialOwner) {} + + // ═══════════════════════════════════════════════ PERMISSIONED ════════════════════════════════════════════════════ + + /// @inheritdoc IThresholdECDSAModule + function addVerifier(address verifier) external onlyOwner {} + + /// @inheritdoc IThresholdECDSAModule + function removeVerifier(address verifier) external onlyOwner {} + + /// @inheritdoc IThresholdECDSAModule + function setThreshold(uint256 threshold) external onlyOwner {} + + /// @inheritdoc IThresholdECDSAModule + function setGasOracle(address gasOracle) external onlyOwner {} + + // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ + + /// @inheritdoc IThresholdECDSAModule + function verifyEntry(bytes calldata encodedEntry, bytes[] calldata signatures) external {} + + // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ + + function getThreshold() external view returns (uint256) {} + + // ══════════════════════════════════════════════ INTERNAL LOGIC ═══════════════════════════════════════════════════ + + /// @dev Internal logic to request the verification of an entry on the destination chain. + function _requestVerification(uint256 destChainId, bytes memory encodedEntry) internal override {} + + // ══════════════════════════════════════════════ INTERNAL VIEWS ═══════════════════════════════════════════════════ + + /// @dev Internal logic to get the module fee for verifying an entry on the specified destination chain. + function _getModuleFee(uint256 destChainId) internal view override returns (uint256) {} +} From f7c13cd2000bd3a04533d42895ab259ba5fc4e8e Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 15:56:44 +0000 Subject: [PATCH 03/32] Add more views --- .../contracts/interfaces/IThresholdECDSAModule.sol | 13 +++++++++++-- .../contracts/modules/ThresholdECDSAModule.sol | 12 +++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol index 6b2e2274f2..d4713b221b 100644 --- a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol @@ -23,8 +23,8 @@ interface IThresholdECDSAModule is IInterchainModule { /// @notice Sets the address of the gas oracle to be used for estimating the verification fees. /// @dev Could be only called by the owner. Will revert if the gas oracle is not a contract. - /// @param gasOracle The address of the gas oracle contract - function setGasOracle(address gasOracle) external; + /// @param gasOracle_ The address of the gas oracle contract + function setGasOracle(address gasOracle_) external; // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ @@ -36,7 +36,16 @@ interface IThresholdECDSAModule is IInterchainModule { // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ + /// @notice Returns the address of the gas oracle used for estimating the verification fees. + function gasOracle() external view returns (address); + + /// @notice Returns the list of verifiers for the module. + function getVerifiers() external view returns (address[] memory); + /// @notice Gets the threshold of the module. /// This is the minimum number of signatures required for verification. function getThreshold() external view returns (uint256); + + /// @notice Checks if the given account is a verifier for the module. + function isVerifier(address account) external view returns (bool); } diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol index 6a2236289c..f23b1f4eb2 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -11,6 +11,9 @@ import {ThresholdECDSA} from "../libs/ThresholdECDSA.sol"; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; contract ThresholdECDSAModule is InterchainModule, Ownable, IThresholdECDSAModule { + /// @inheritdoc IThresholdECDSAModule + address public gasOracle; + constructor(address interchainDB, address initialOwner) InterchainModule(interchainDB) Ownable(initialOwner) {} // ═══════════════════════════════════════════════ PERMISSIONED ════════════════════════════════════════════════════ @@ -25,7 +28,7 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, IThresholdECDSAModul function setThreshold(uint256 threshold) external onlyOwner {} /// @inheritdoc IThresholdECDSAModule - function setGasOracle(address gasOracle) external onlyOwner {} + function setGasOracle(address gasOracle_) external onlyOwner {} // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ @@ -34,8 +37,15 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, IThresholdECDSAModul // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ + /// @inheritdoc IThresholdECDSAModule function getThreshold() external view returns (uint256) {} + /// @inheritdoc IThresholdECDSAModule + function getVerifiers() external view returns (address[] memory) {} + + /// @inheritdoc IThresholdECDSAModule + function isVerifier(address account) external view returns (bool) {} + // ══════════════════════════════════════════════ INTERNAL LOGIC ═══════════════════════════════════════════════════ /// @dev Internal logic to request the verification of an entry on the destination chain. From dff57245aa7ddddca5f9abdb908058d687f680f0 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 16:19:46 +0000 Subject: [PATCH 04/32] Implement management tests --- .../interfaces/IThresholdECDSAModule.sol | 2 + .../IThresholdECDSAModuleEvents.sol | 9 + .../test/mocks/GasOracleMock.sol | 4 + .../ThresholdECDSAModule.Management.t.sol | 170 ++++++++++++++++++ 4 files changed, 185 insertions(+) create mode 100644 packages/contracts-communication/contracts/interfaces/IThresholdECDSAModuleEvents.sol create mode 100644 packages/contracts-communication/test/mocks/GasOracleMock.sol create mode 100644 packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol diff --git a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol index d4713b221b..1c178cb0d5 100644 --- a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol @@ -4,6 +4,8 @@ pragma solidity ^0.8.0; import {IInterchainModule} from "./IInterchainModule.sol"; interface IThresholdECDSAModule is IInterchainModule { + error ThresholdECDSAModule__GasOracleNotContract(address gasOracle); + // ═══════════════════════════════════════════════ PERMISSIONED ════════════════════════════════════════════════════ /// @notice Adds a new verifier to the module. diff --git a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModuleEvents.sol b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModuleEvents.sol new file mode 100644 index 0000000000..7de91d452b --- /dev/null +++ b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModuleEvents.sol @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +interface IThresholdECDSAModuleEvents { + event VerifierAdded(address verifier); + event VerifierRemoved(address verifier); + event ThresholdChanged(uint256 threshold); + event GasOracleChanged(address gasOracle); +} diff --git a/packages/contracts-communication/test/mocks/GasOracleMock.sol b/packages/contracts-communication/test/mocks/GasOracleMock.sol new file mode 100644 index 0000000000..9c075aec1a --- /dev/null +++ b/packages/contracts-communication/test/mocks/GasOracleMock.sol @@ -0,0 +1,4 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.20; + +contract GasOracleMock {} diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol new file mode 100644 index 0000000000..04f4799eea --- /dev/null +++ b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol @@ -0,0 +1,170 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.20; + +import {IThresholdECDSAModuleEvents} from "../../contracts/interfaces/IThresholdECDSAModuleEvents.sol"; +import {ThresholdECDSAModule, IThresholdECDSAModule} from "../../contracts/modules/ThresholdECDSAModule.sol"; +import {ThresholdECDSALib} from "../../contracts/libs/ThresholdECDSA.sol"; + +import {GasOracleMock} from "../mocks/GasOracleMock.sol"; + +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; + +import {Test} from "forge-std/Test.sol"; + +contract ThresholdECDSAModuleManagementTest is Test, IThresholdECDSAModuleEvents { + ThresholdECDSAModule public module; + GasOracleMock public gasOracle; + + address public interchainDB = makeAddr("InterchainDB"); + address public owner = makeAddr("Owner"); + + address public constant VERIFIER_1 = address(1); + address public constant VERIFIER_2 = address(2); + address public constant VERIFIER_3 = address(3); + + function setUp() public { + module = new ThresholdECDSAModule(interchainDB, owner); + gasOracle = new GasOracleMock(); + } + + function basicSetup() internal { + vm.startPrank(owner); + module.addVerifier(VERIFIER_1); + module.addVerifier(VERIFIER_2); + module.addVerifier(VERIFIER_3); + vm.stopPrank(); + } + + function test_setup() public { + assertEq(module.owner(), owner); + assertEq(module.INTERCHAIN_DB(), interchainDB); + assertEq(module.getThreshold(), type(uint256).max); + assertEq(module.gasOracle(), address(0)); + } + + function test_basicSetup() public { + basicSetup(); + assertTrue(module.isVerifier(VERIFIER_1)); + assertTrue(module.isVerifier(VERIFIER_2)); + assertTrue(module.isVerifier(VERIFIER_3)); + address[] memory verifiers = module.getVerifiers(); + assertEq(verifiers.length, 3); + assertEq(verifiers[0], VERIFIER_1); + assertEq(verifiers[1], VERIFIER_2); + assertEq(verifiers[2], VERIFIER_3); + } + + function test_addSigner_addsSinger() public { + vm.prank(owner); + module.addVerifier(VERIFIER_1); + assertTrue(module.isVerifier(VERIFIER_1)); + } + + function test_addSigner_emitsEvent() public { + vm.expectEmit(address(module)); + emit VerifierAdded(VERIFIER_1); + vm.prank(owner); + module.addVerifier(VERIFIER_1); + } + + function test_addSigner_revert_alreadyAdded() public { + basicSetup(); + vm.expectRevert(abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__AlreadySigner.selector, VERIFIER_1)); + vm.prank(owner); + module.addVerifier(VERIFIER_1); + } + + function test_addSigner_revert_notOwner(address notOwner) public { + vm.assume(notOwner != owner); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); + vm.prank(notOwner); + module.addVerifier(VERIFIER_1); + } + + function test_removeSigner_removesSigner() public { + basicSetup(); + vm.prank(owner); + module.removeVerifier(VERIFIER_1); + assertFalse(module.isVerifier(VERIFIER_1)); + } + + function test_removeSigner_emitsEvent() public { + basicSetup(); + vm.expectEmit(address(module)); + emit VerifierRemoved(VERIFIER_1); + vm.prank(owner); + module.removeVerifier(VERIFIER_1); + } + + function test_removeSigner_revert_notAdded() public { + vm.expectRevert(abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__NotSigner.selector, VERIFIER_1)); + vm.prank(owner); + module.removeVerifier(VERIFIER_1); + } + + function test_removeSigner_revert_notOwner(address notOwner) public { + vm.assume(notOwner != owner); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); + vm.prank(notOwner); + module.removeVerifier(VERIFIER_1); + } + + function test_setThreshold_setsThreshold() public { + vm.prank(owner); + module.setThreshold(3); + assertEq(module.getThreshold(), 3); + } + + function test_setThreshold_emitsEvent() public { + vm.expectEmit(address(module)); + emit ThresholdChanged(3); + vm.prank(owner); + module.setThreshold(3); + } + + function test_setThreshold_revert_zeroThreshold() public { + vm.expectRevert(abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__ZeroThreshold.selector)); + vm.prank(owner); + module.setThreshold(0); + } + + function test_setThreshold_revert_notOwner(address notOwner) public { + vm.assume(notOwner != owner); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); + vm.prank(notOwner); + module.setThreshold(3); + } + + function test_setGasOracle_setsGasOracle() public { + vm.prank(owner); + module.setGasOracle(address(gasOracle)); + assertEq(module.gasOracle(), address(gasOracle)); + } + + function test_setGasOracle_emitsEvent() public { + vm.expectEmit(address(module)); + emit GasOracleChanged(address(gasOracle)); + vm.prank(owner); + module.setGasOracle(address(gasOracle)); + } + + function test_setGasOracle_revert_notOwner(address notOwner) public { + vm.assume(notOwner != owner); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); + vm.prank(notOwner); + module.setGasOracle(address(gasOracle)); + } + + function test_setGasOracle_revert_notContract() public { + address notContract = makeAddr("NotContract"); + // Sanity check + require(notContract.code.length == 0); + vm.expectRevert( + abi.encodeWithSelector( + IThresholdECDSAModule.ThresholdECDSAModule__GasOracleNotContract.selector, notContract + ) + ); + vm.prank(owner); + module.setGasOracle(notContract); + } +} From 93606620d79e04993d17184f1a0726e0c17ed762 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 16:29:19 +0000 Subject: [PATCH 05/32] Implement management actions --- .../modules/ThresholdECDSAModule.sol | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol index f23b1f4eb2..0b632fc1ec 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -4,31 +4,52 @@ pragma solidity 0.8.20; import {InterchainModule} from "./InterchainModule.sol"; import {IThresholdECDSAModule} from "../interfaces/IThresholdECDSAModule.sol"; +import {IThresholdECDSAModuleEvents} from "../interfaces/IThresholdECDSAModuleEvents.sol"; import {InterchainEntry} from "../libs/InterchainEntry.sol"; import {ThresholdECDSA} from "../libs/ThresholdECDSA.sol"; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; -contract ThresholdECDSAModule is InterchainModule, Ownable, IThresholdECDSAModule { +contract ThresholdECDSAModule is InterchainModule, Ownable, IThresholdECDSAModule, IThresholdECDSAModuleEvents { + /// @dev Struct to hold the verifiers and the threshold for the module. + ThresholdECDSA internal _verifiers; + /// @inheritdoc IThresholdECDSAModule address public gasOracle; - constructor(address interchainDB, address initialOwner) InterchainModule(interchainDB) Ownable(initialOwner) {} + constructor(address interchainDB, address initialOwner) InterchainModule(interchainDB) Ownable(initialOwner) { + // Explicitly disable the module functionality until the threshold is set. + _setThreshold(type(uint256).max); + } // ═══════════════════════════════════════════════ PERMISSIONED ════════════════════════════════════════════════════ /// @inheritdoc IThresholdECDSAModule - function addVerifier(address verifier) external onlyOwner {} + function addVerifier(address verifier) external onlyOwner { + _verifiers.addSigner(verifier); + emit VerifierAdded(verifier); + } /// @inheritdoc IThresholdECDSAModule - function removeVerifier(address verifier) external onlyOwner {} + function removeVerifier(address verifier) external onlyOwner { + _verifiers.removeSigner(verifier); + emit VerifierRemoved(verifier); + } /// @inheritdoc IThresholdECDSAModule - function setThreshold(uint256 threshold) external onlyOwner {} + function setThreshold(uint256 threshold) external onlyOwner { + _setThreshold(threshold); + } /// @inheritdoc IThresholdECDSAModule - function setGasOracle(address gasOracle_) external onlyOwner {} + function setGasOracle(address gasOracle_) external onlyOwner { + if (gasOracle_.code.length == 0) { + revert ThresholdECDSAModule__GasOracleNotContract(gasOracle_); + } + gasOracle = gasOracle_; + emit GasOracleChanged(gasOracle_); + } // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ @@ -38,16 +59,28 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, IThresholdECDSAModul // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ /// @inheritdoc IThresholdECDSAModule - function getThreshold() external view returns (uint256) {} + function getThreshold() external view returns (uint256) { + return _verifiers.getThreshold(); + } /// @inheritdoc IThresholdECDSAModule - function getVerifiers() external view returns (address[] memory) {} + function getVerifiers() external view returns (address[] memory) { + return _verifiers.getSigners(); + } /// @inheritdoc IThresholdECDSAModule - function isVerifier(address account) external view returns (bool) {} + function isVerifier(address account) external view returns (bool) { + return _verifiers.isSigner(account); + } // ══════════════════════════════════════════════ INTERNAL LOGIC ═══════════════════════════════════════════════════ + /// @dev Sets the threshold for the module. Permissions should be checked in the calling function. + function _setThreshold(uint256 threshold) internal { + _verifiers.modifyThreshold(threshold); + emit ThresholdChanged(threshold); + } + /// @dev Internal logic to request the verification of an entry on the destination chain. function _requestVerification(uint256 destChainId, bytes memory encodedEntry) internal override {} From da19b8114f1b7dcc0e3d98ef6961e97e4d0a7ee9 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 16:32:59 +0000 Subject: [PATCH 06/32] Add `InterchainDB` Mock --- .../test/mocks/InterchainDBMock.sol | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 packages/contracts-communication/test/mocks/InterchainDBMock.sol diff --git a/packages/contracts-communication/test/mocks/InterchainDBMock.sol b/packages/contracts-communication/test/mocks/InterchainDBMock.sol new file mode 100644 index 0000000000..4d67e5064f --- /dev/null +++ b/packages/contracts-communication/test/mocks/InterchainDBMock.sol @@ -0,0 +1,45 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.20; + +import {IInterchainDB, InterchainEntry} from "../../contracts/interfaces/IInterchainDB.sol"; + +contract InterchainDBMock is IInterchainDB { + function writeEntry(bytes32 dataHash) external returns (uint256 writerNonce) {} + + function requestVerification( + uint256 destChainId, + address writer, + uint256 writerNonce, + address[] memory srcModules + ) + external + payable + {} + + function writeEntryWithVerification( + uint256 destChainId, + bytes32 dataHash, + address[] memory srcModules + ) + external + payable + returns (uint256 writerNonce) + {} + + function verifyEntry(InterchainEntry memory entry) external {} + + function getInterchainFee(uint256 destChainId, address[] memory srcModules) external view returns (uint256) {} + + function getEntry(address writer, uint256 writerNonce) external view returns (InterchainEntry memory) {} + + function getWriterNonce(address writer) external view returns (uint256) {} + + function readEntry( + address dstModule, + InterchainEntry memory entry + ) + external + view + returns (uint256 moduleVerifiedAt) + {} +} From d1b7c257b3f3f1539aa09fff2d04ae27ce4d3883 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 16:57:00 +0000 Subject: [PATCH 07/32] Reorg events following #2049 --- .../InterchainModuleEvents.sol} | 2 +- .../ThresholdECDSAModuleEvents.sol} | 2 +- .../contracts/modules/InterchainModule.sol | 4 ++-- .../contracts/modules/ThresholdECDSAModule.sol | 4 ++-- .../test/modules/ThresholdECDSAModule.Management.t.sol | 4 ++-- 5 files changed, 8 insertions(+), 8 deletions(-) rename packages/contracts-communication/contracts/{interfaces/IInterchainModuleEvents.sol => events/InterchainModuleEvents.sol} (75%) rename packages/contracts-communication/contracts/{interfaces/IThresholdECDSAModuleEvents.sol => events/ThresholdECDSAModuleEvents.sol} (83%) diff --git a/packages/contracts-communication/contracts/interfaces/IInterchainModuleEvents.sol b/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol similarity index 75% rename from packages/contracts-communication/contracts/interfaces/IInterchainModuleEvents.sol rename to packages/contracts-communication/contracts/events/InterchainModuleEvents.sol index cdf89e9420..2c4f009665 100644 --- a/packages/contracts-communication/contracts/interfaces/IInterchainModuleEvents.sol +++ b/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol @@ -1,6 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -interface IInterchainModuleEvents { +abstract contract InterchainModuleEvents { event VerificationRequested(uint256 indexed destChainId, bytes entry); } diff --git a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModuleEvents.sol b/packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol similarity index 83% rename from packages/contracts-communication/contracts/interfaces/IThresholdECDSAModuleEvents.sol rename to packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol index 7de91d452b..0200fc5142 100644 --- a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModuleEvents.sol +++ b/packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -interface IThresholdECDSAModuleEvents { +abstract contract ThresholdECDSAModuleEvents { event VerifierAdded(address verifier); event VerifierRemoved(address verifier); event ThresholdChanged(uint256 threshold); diff --git a/packages/contracts-communication/contracts/modules/InterchainModule.sol b/packages/contracts-communication/contracts/modules/InterchainModule.sol index fa726a933f..cc1480485f 100644 --- a/packages/contracts-communication/contracts/modules/InterchainModule.sol +++ b/packages/contracts-communication/contracts/modules/InterchainModule.sol @@ -1,14 +1,14 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +import {InterchainModuleEvents} from "../events/InterchainModuleEvents.sol"; import {IInterchainDB} from "../interfaces/IInterchainDB.sol"; import {IInterchainModule} from "../interfaces/IInterchainModule.sol"; -import {IInterchainModuleEvents} from "../interfaces/IInterchainModuleEvents.sol"; import {InterchainEntry} from "../libs/InterchainEntry.sol"; /// @notice Common logic for all Interchain Modules. -abstract contract InterchainModule is IInterchainModule, IInterchainModuleEvents { +abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule { address public immutable INTERCHAIN_DB; error InterchainModule__NotInterchainDB(); diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol index 0b632fc1ec..313625e54e 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -3,15 +3,15 @@ pragma solidity 0.8.20; import {InterchainModule} from "./InterchainModule.sol"; +import {ThresholdECDSAModuleEvents} from "../events/ThresholdECDSAModuleEvents.sol"; import {IThresholdECDSAModule} from "../interfaces/IThresholdECDSAModule.sol"; -import {IThresholdECDSAModuleEvents} from "../interfaces/IThresholdECDSAModuleEvents.sol"; import {InterchainEntry} from "../libs/InterchainEntry.sol"; import {ThresholdECDSA} from "../libs/ThresholdECDSA.sol"; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; -contract ThresholdECDSAModule is InterchainModule, Ownable, IThresholdECDSAModule, IThresholdECDSAModuleEvents { +contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModuleEvents, IThresholdECDSAModule { /// @dev Struct to hold the verifiers and the threshold for the module. ThresholdECDSA internal _verifiers; diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol index 04f4799eea..33a723c38b 100644 --- a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol +++ b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.20; -import {IThresholdECDSAModuleEvents} from "../../contracts/interfaces/IThresholdECDSAModuleEvents.sol"; +import {ThresholdECDSAModuleEvents} from "../../contracts/events/ThresholdECDSAModuleEvents.sol"; import {ThresholdECDSAModule, IThresholdECDSAModule} from "../../contracts/modules/ThresholdECDSAModule.sol"; import {ThresholdECDSALib} from "../../contracts/libs/ThresholdECDSA.sol"; @@ -11,7 +11,7 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import {Test} from "forge-std/Test.sol"; -contract ThresholdECDSAModuleManagementTest is Test, IThresholdECDSAModuleEvents { +contract ThresholdECDSAModuleManagementTest is Test, ThresholdECDSAModuleEvents { ThresholdECDSAModule public module; GasOracleMock public gasOracle; From 8051a7ecbbcf59483288f000957a0a7b1de2533c Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 17:00:20 +0000 Subject: [PATCH 08/32] Also emit ethSingedMsg hash --- .../contracts/events/InterchainModuleEvents.sol | 2 +- .../contracts/modules/InterchainModule.sol | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol b/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol index 2c4f009665..93f0c2085d 100644 --- a/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol +++ b/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol @@ -2,5 +2,5 @@ pragma solidity ^0.8.0; abstract contract InterchainModuleEvents { - event VerificationRequested(uint256 indexed destChainId, bytes entry); + event VerificationRequested(uint256 indexed destChainId, bytes entry, bytes32 ethSignedEntryHash); } diff --git a/packages/contracts-communication/contracts/modules/InterchainModule.sol b/packages/contracts-communication/contracts/modules/InterchainModule.sol index cc1480485f..c2a72238af 100644 --- a/packages/contracts-communication/contracts/modules/InterchainModule.sol +++ b/packages/contracts-communication/contracts/modules/InterchainModule.sol @@ -7,6 +7,8 @@ import {IInterchainModule} from "../interfaces/IInterchainModule.sol"; import {InterchainEntry} from "../libs/InterchainEntry.sol"; +import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; + /// @notice Common logic for all Interchain Modules. abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule { address public immutable INTERCHAIN_DB; @@ -29,7 +31,7 @@ abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule } bytes memory encodedEntry = abi.encode(entry); _requestVerification(destChainId, encodedEntry); - emit VerificationRequested(destChainId, encodedEntry); + emit VerificationRequested(destChainId, encodedEntry, MessageHashUtils.toEthSignedMessageHash(encodedEntry)); } /// @inheritdoc IInterchainModule From 1668581e17bbd7bdd673d83b29ad2a4565d2de30 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 17:40:01 +0000 Subject: [PATCH 09/32] More `InterchainModule` errors --- .../contracts/interfaces/IInterchainModule.sol | 5 +++++ .../contracts/modules/InterchainModule.sol | 9 ++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/contracts-communication/contracts/interfaces/IInterchainModule.sol b/packages/contracts-communication/contracts/interfaces/IInterchainModule.sol index b1f2fd4002..e2710f4641 100644 --- a/packages/contracts-communication/contracts/interfaces/IInterchainModule.sol +++ b/packages/contracts-communication/contracts/interfaces/IInterchainModule.sol @@ -7,6 +7,11 @@ import {InterchainEntry} from "../libs/InterchainEntry.sol"; /// @notice Every Module may opt a different method to confirm the verified entries on destination chain, /// therefore this is not a part of a common interface. interface IInterchainModule { + error InterchainModule__NotInterchainDB(); + error InterchainModule__IncorrectSourceChainId(uint256 chainId); + error InterchainModule__InsufficientFee(uint256 actual, uint256 required); + error InterchainModule__SameChainId(); + /// @notice Request the verification of an entry in the Interchain DataBase by the module. /// Note: a fee is paid to the module for verification, and could be retrieved by using `getModuleFee`. /// Note: this will eventually trigger `InterchainDB.verifyEntry(entry)` function on destination chain, diff --git a/packages/contracts-communication/contracts/modules/InterchainModule.sol b/packages/contracts-communication/contracts/modules/InterchainModule.sol index c2a72238af..3264fe9e7c 100644 --- a/packages/contracts-communication/contracts/modules/InterchainModule.sol +++ b/packages/contracts-communication/contracts/modules/InterchainModule.sol @@ -13,9 +13,6 @@ import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/Messa abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule { address public immutable INTERCHAIN_DB; - error InterchainModule__NotInterchainDB(); - error InterchainModule__InsufficientFee(uint256 actual, uint256 required); - constructor(address interchainDB) { INTERCHAIN_DB = interchainDB; } @@ -25,6 +22,12 @@ abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule if (msg.sender != INTERCHAIN_DB) { revert InterchainModule__NotInterchainDB(); } + if (destChainId == block.chainid) { + revert InterchainModule__SameChainId(); + } + if (entry.srcChainId != block.chainid) { + revert InterchainModule__IncorrectSourceChainId({chainId: entry.srcChainId}); + } uint256 requiredFee = _getModuleFee(destChainId); if (msg.value < requiredFee) { revert InterchainModule__InsufficientFee({actual: msg.value, required: requiredFee}); From 25f7f0306c359ef32805de7c47e0858037b6706f Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 17:52:06 +0000 Subject: [PATCH 10/32] Implement tests for fee collector management --- .../events/ThresholdECDSAModuleEvents.sol | 2 ++ .../interfaces/IThresholdECDSAModule.sol | 8 +++++++ .../ThresholdECDSAModule.Management.t.sol | 21 +++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol b/packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol index 0200fc5142..8139351bd9 100644 --- a/packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol +++ b/packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol @@ -5,5 +5,7 @@ abstract contract ThresholdECDSAModuleEvents { event VerifierAdded(address verifier); event VerifierRemoved(address verifier); event ThresholdChanged(uint256 threshold); + + event FeeCollectorChanged(address feeCollector); event GasOracleChanged(address gasOracle); } diff --git a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol index 1c178cb0d5..de819e200d 100644 --- a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol @@ -23,6 +23,11 @@ interface IThresholdECDSAModule is IInterchainModule { /// @param threshold The new threshold value function setThreshold(uint256 threshold) external; + /// @notice Sets the address of the fee collector, which will have the verification fees forwarded to it. + /// @dev Could be only called by the owner. + /// @param feeCollector_ The address of the fee collector + function setFeeCollector(address feeCollector_) external; + /// @notice Sets the address of the gas oracle to be used for estimating the verification fees. /// @dev Could be only called by the owner. Will revert if the gas oracle is not a contract. /// @param gasOracle_ The address of the gas oracle contract @@ -38,6 +43,9 @@ interface IThresholdECDSAModule is IInterchainModule { // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ + /// @notice Returns the address of the fee collector for the module. + function feeCollector() external view returns (address); + /// @notice Returns the address of the gas oracle used for estimating the verification fees. function gasOracle() external view returns (address); diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol index 33a723c38b..5eaa5e3542 100644 --- a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol +++ b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol @@ -17,6 +17,7 @@ contract ThresholdECDSAModuleManagementTest is Test, ThresholdECDSAModuleEvents address public interchainDB = makeAddr("InterchainDB"); address public owner = makeAddr("Owner"); + address public feeCollector = makeAddr("FeeCollector"); address public constant VERIFIER_1 = address(1); address public constant VERIFIER_2 = address(2); @@ -135,6 +136,26 @@ contract ThresholdECDSAModuleManagementTest is Test, ThresholdECDSAModuleEvents module.setThreshold(3); } + function test_setFeeCollector_setsFeeCollector() public { + vm.prank(owner); + module.setFeeCollector(feeCollector); + assertEq(module.feeCollector(), feeCollector); + } + + function test_setFeeCollector_emitsEvent() public { + vm.expectEmit(address(module)); + emit FeeCollectorChanged(feeCollector); + vm.prank(owner); + module.setFeeCollector(feeCollector); + } + + function test_setFeeCollector_revert_notOwner(address notOwner) public { + vm.assume(notOwner != owner); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); + vm.prank(notOwner); + module.setFeeCollector(feeCollector); + } + function test_setGasOracle_setsGasOracle() public { vm.prank(owner); module.setGasOracle(address(gasOracle)); From d632f51d4758f21c933dda111477e1b1818fc3ad Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 17:59:45 +0000 Subject: [PATCH 11/32] Fix: use the entry hash for signing --- .../contracts/modules/InterchainModule.sol | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/contracts-communication/contracts/modules/InterchainModule.sol b/packages/contracts-communication/contracts/modules/InterchainModule.sol index 3264fe9e7c..a91162664d 100644 --- a/packages/contracts-communication/contracts/modules/InterchainModule.sol +++ b/packages/contracts-communication/contracts/modules/InterchainModule.sol @@ -33,8 +33,9 @@ abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule revert InterchainModule__InsufficientFee({actual: msg.value, required: requiredFee}); } bytes memory encodedEntry = abi.encode(entry); + bytes32 ethSignedEntryHash = MessageHashUtils.toEthSignedMessageHash(keccak256(encodedEntry)); _requestVerification(destChainId, encodedEntry); - emit VerificationRequested(destChainId, encodedEntry, MessageHashUtils.toEthSignedMessageHash(encodedEntry)); + emit VerificationRequested(destChainId, encodedEntry, ethSignedEntryHash); } /// @inheritdoc IInterchainModule From 9af670e8333ca46949ed967aebff44bfe308f6ef Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 18:00:11 +0000 Subject: [PATCH 12/32] Implement `feeCollector` management --- .../contracts/modules/ThresholdECDSAModule.sol | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol index 313625e54e..50d7e4505a 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -15,6 +15,9 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule /// @dev Struct to hold the verifiers and the threshold for the module. ThresholdECDSA internal _verifiers; + /// @inheritdoc IThresholdECDSAModule + address public feeCollector; + /// @inheritdoc IThresholdECDSAModule address public gasOracle; @@ -42,6 +45,11 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule _setThreshold(threshold); } + /// @inheritdoc IThresholdECDSAModule + function setFeeCollector(address feeCollector_) external onlyOwner { + _setFeeCollector(feeCollector_); + } + /// @inheritdoc IThresholdECDSAModule function setGasOracle(address gasOracle_) external onlyOwner { if (gasOracle_.code.length == 0) { @@ -81,6 +89,13 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule emit ThresholdChanged(threshold); } + /// @dev Internal logic to set the address of the fee collector. + /// Permissions should be checked in the calling function. + function _setFeeCollector(address feeCollector_) internal { + feeCollector = feeCollector_; + emit FeeCollectorChanged(feeCollector_); + } + /// @dev Internal logic to request the verification of an entry on the destination chain. function _requestVerification(uint256 destChainId, bytes memory encodedEntry) internal override {} From cd3dccf611fd6bfedf31f44aed6bbb7930305015 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 18:08:25 +0000 Subject: [PATCH 13/32] Start doing source tests --- .../modules/ThresholdECDSAModule.Source.t.sol | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol new file mode 100644 index 0000000000..ce3280a1f4 --- /dev/null +++ b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol @@ -0,0 +1,94 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.20; + +import {InterchainModuleEvents} from "../../contracts/events/InterchainModuleEvents.sol"; +import {ThresholdECDSAModuleEvents} from "../../contracts/events/ThresholdECDSAModuleEvents.sol"; +import {IInterchainModule} from "../../contracts/interfaces/IInterchainModule.sol"; +import { + ThresholdECDSAModule, + InterchainEntry, + IThresholdECDSAModule +} from "../../contracts/modules/ThresholdECDSAModule.sol"; + +import {GasOracleMock} from "../mocks/GasOracleMock.sol"; + +import {Test} from "forge-std/Test.sol"; + +contract ThresholdECDSAModuleSourceTest is Test, InterchainModuleEvents, ThresholdECDSAModuleEvents { + ThresholdECDSAModule public module; + GasOracleMock public gasOracle; + + address public interchainDB = makeAddr("InterchainDB"); + address public feeCollector = makeAddr("FeeCollector"); + address public owner = makeAddr("Owner"); + + uint256 public constant SRC_CHAIN_ID = 1337; + uint256 public constant DST_CHAIN_ID = 7331; + + uint256 public constant FEE = 100; + + InterchainEntry public mockEntry = InterchainEntry({ + srcChainId: SRC_CHAIN_ID, + srcWriter: bytes32(uint256(2)), + writerNonce: 3, + dataHash: bytes32(uint256(4)) + }); + + function setUp() public { + vm.chainId(SRC_CHAIN_ID); + module = new ThresholdECDSAModule(interchainDB, owner); + gasOracle = new GasOracleMock(); + vm.startPrank(owner); + module.setGasOracle(address(gasOracle)); + module.addVerifier(address(1)); + module.setThreshold(1); + vm.stopPrank(); + // TODO: set up GasOracle mocking + } + + function mockRequestVerification(uint256 msgValue, InterchainEntry memory entry) internal { + deal(interchainDB, msgValue); + vm.prank(interchainDB); + module.requestVerification{value: msgValue}(DST_CHAIN_ID, entry); + } + + function test_setup() public { + assertEq(module.owner(), owner); + assertEq(module.INTERCHAIN_DB(), interchainDB); + assertTrue(module.isVerifier(address(1))); + assertEq(module.getThreshold(), 1); + assertEq(module.gasOracle(), address(gasOracle)); + } + + function test_requestVerification_emitsEvent() public { + bytes memory encodedEntry = abi.encode(mockEntry); + bytes32 entryHash = keccak256(encodedEntry); + bytes32 ethSignedHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", entryHash)); + vm.expectEmit(address(module)); + emit VerificationRequested(DST_CHAIN_ID, encodedEntry, ethSignedHash); + mockRequestVerification(FEE, mockEntry); + } + + function test_requestVerification_transfersFeeToCollector() public { + mockRequestVerification(FEE, mockEntry); + assertEq(feeCollector.balance, FEE); + } + + function test_requestVerification_feeAboveRequired() public { + // Should not revert + mockRequestVerification(FEE + 1, mockEntry); + } + + function test_requestVerification_revert_feeBelowRequired() public { + vm.expectRevert( + abi.encodeWithSelector(IInterchainModule.InterchainModule__InsufficientFee.selector, FEE - 1, FEE) + ); + mockRequestVerification(FEE - 1, mockEntry); + } + + function test_getModuleFee() public { + // TODO: fill the calldata + vm.expectCall(address(gasOracle), ""); + assertEq(module.getModuleFee(DST_CHAIN_ID), FEE); + } +} From 05fe117dfee1499f37c0c1dd74040fae2470c22b Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 18:24:18 +0000 Subject: [PATCH 14/32] Add GasOracle interface --- .../contracts/interfaces/IGasOracle.sol | 40 +++++++++++++++++++ .../test/mocks/GasOracleMock.sol | 36 ++++++++++++++++- 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 packages/contracts-communication/contracts/interfaces/IGasOracle.sol diff --git a/packages/contracts-communication/contracts/interfaces/IGasOracle.sol b/packages/contracts-communication/contracts/interfaces/IGasOracle.sol new file mode 100644 index 0000000000..985c23192e --- /dev/null +++ b/packages/contracts-communication/contracts/interfaces/IGasOracle.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +interface IGasOracle { + /// @notice Convert a value from the native token of a remote chain to the local native token. + /// @dev Will revert if no price is available for the remote chain. + /// @param remoteChainId The chain id of the remote chain. + /// @param value The value to convert. + function convertRemoteValueToLocalUnits(uint256 remoteChainId, uint256 value) external view returns (uint256); + + /// @notice Estimate the cost of execution a transaction on a remote chain, + /// and convert it to the local native token. + /// @dev Will revert if no price is available for the remote chain. + /// @param remoteChainId The chain id of the remote chain. + /// @param gasLimit The gas limit of the transaction. + /// @param calldataSize The size of the transaction calldata. + function estimateTxCostInLocalUnits( + uint256 remoteChainId, + uint256 gasLimit, + uint256 calldataSize + ) + external + view + returns (uint256); + + /// @notice Estimate the cost of execution a transaction on a remote chain, + /// and return it as is in the remote chain's native token. + /// @dev Will revert if no price is available for the remote chain. + /// @param remoteChainId The chain id of the remote chain. + /// @param gasLimit The gas limit of the transaction. + /// @param calldataSize The size of the transaction calldata. + function estimateTxCostInRemoteUnits( + uint256 remoteChainId, + uint256 gasLimit, + uint256 calldataSize + ) + external + view + returns (uint256); +} diff --git a/packages/contracts-communication/test/mocks/GasOracleMock.sol b/packages/contracts-communication/test/mocks/GasOracleMock.sol index 9c075aec1a..ef38762239 100644 --- a/packages/contracts-communication/test/mocks/GasOracleMock.sol +++ b/packages/contracts-communication/test/mocks/GasOracleMock.sol @@ -1,4 +1,38 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.20; -contract GasOracleMock {} +import {IGasOracle} from "../../contracts/interfaces/IGasOracle.sol"; + +contract GasOracleMock is IGasOracle { + function convertRemoteValueToLocalUnits( + uint256 remoteChainId, + uint256 value + ) + external + view + override + returns (uint256) + {} + + function estimateTxCostInLocalUnits( + uint256 remoteChainId, + uint256 gasLimit, + uint256 calldataSize + ) + external + view + override + returns (uint256) + {} + + function estimateTxCostInRemoteUnits( + uint256 remoteChainId, + uint256 gasLimit, + uint256 calldataSize + ) + external + view + override + returns (uint256) + {} +} From 3e96e60a1defd08d56988a076edb99b18aeb67e2 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 19:43:41 +0000 Subject: [PATCH 15/32] Update `verifyEntry()` interface --- .../contracts/interfaces/IThresholdECDSAModule.sol | 5 +++-- .../contracts/modules/ThresholdECDSAModule.sol | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol index de819e200d..c00a5a3c6b 100644 --- a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol @@ -37,9 +37,10 @@ interface IThresholdECDSAModule is IInterchainModule { /// @notice Verifies an entry using a set of verifier signatures. /// If the threshold is met, the entry will be marked as verified in the Interchain DataBase. + /// @dev List of recovered signers from the signatures must be sorted in the ascending order. /// @param encodedEntry The encoded entry to verify - /// @param signatures An array of signatures used to verify the entry - function verifyEntry(bytes calldata encodedEntry, bytes[] calldata signatures) external; + /// @param signatures Signatures used to verify the entry, concatenated + function verifyEntry(bytes calldata encodedEntry, bytes calldata signatures) external; // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol index 50d7e4505a..d36fffdb87 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -62,7 +62,7 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ /// @inheritdoc IThresholdECDSAModule - function verifyEntry(bytes calldata encodedEntry, bytes[] calldata signatures) external {} + function verifyEntry(bytes calldata encodedEntry, bytes calldata signatures) external {} // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ From 82ba2df827da1736d705986c5ccf93448c584901 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 20:27:22 +0000 Subject: [PATCH 16/32] Add source chain tests --- .../modules/ThresholdECDSAModule.Source.t.sol | 66 +++++++++++++++---- 1 file changed, 55 insertions(+), 11 deletions(-) diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol index ce3280a1f4..1450876f10 100644 --- a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol +++ b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol @@ -25,6 +25,9 @@ contract ThresholdECDSAModuleSourceTest is Test, InterchainModuleEvents, Thresho uint256 public constant SRC_CHAIN_ID = 1337; uint256 public constant DST_CHAIN_ID = 7331; + // TODO: this should be configurable + uint256 public constant EXPECTED_GAS_LIMIT = 100_000; + uint256 public constant FEE = 100; InterchainEntry public mockEntry = InterchainEntry({ @@ -40,10 +43,17 @@ contract ThresholdECDSAModuleSourceTest is Test, InterchainModuleEvents, Thresho gasOracle = new GasOracleMock(); vm.startPrank(owner); module.setGasOracle(address(gasOracle)); + module.setFeeCollector(feeCollector); module.addVerifier(address(1)); - module.setThreshold(1); + module.addVerifier(address(2)); + module.setThreshold(2); vm.stopPrank(); - // TODO: set up GasOracle mocking + // Mock: gasOracle.estimateTxCostInLocalUnits(DST_CHAIN_ID, *, *) to return FEE + vm.mockCall( + address(gasOracle), + abi.encodeWithSelector(GasOracleMock.estimateTxCostInLocalUnits.selector, DST_CHAIN_ID), + abi.encode(FEE) + ); } function mockRequestVerification(uint256 msgValue, InterchainEntry memory entry) internal { @@ -52,18 +62,25 @@ contract ThresholdECDSAModuleSourceTest is Test, InterchainModuleEvents, Thresho module.requestVerification{value: msgValue}(DST_CHAIN_ID, entry); } + function encodeAndHashEntry(InterchainEntry memory entry) + internal + pure + returns (bytes memory encodedEntry, bytes32 ethSignedHash) + { + encodedEntry = abi.encode(entry); + ethSignedHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", keccak256(encodedEntry))); + } + function test_setup() public { assertEq(module.owner(), owner); assertEq(module.INTERCHAIN_DB(), interchainDB); assertTrue(module.isVerifier(address(1))); - assertEq(module.getThreshold(), 1); + assertEq(module.getThreshold(), 2); assertEq(module.gasOracle(), address(gasOracle)); } function test_requestVerification_emitsEvent() public { - bytes memory encodedEntry = abi.encode(mockEntry); - bytes32 entryHash = keccak256(encodedEntry); - bytes32 ethSignedHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", entryHash)); + (bytes memory encodedEntry, bytes32 ethSignedHash) = encodeAndHashEntry(mockEntry); vm.expectEmit(address(module)); emit VerificationRequested(DST_CHAIN_ID, encodedEntry, ethSignedHash); mockRequestVerification(FEE, mockEntry); @@ -74,11 +91,18 @@ contract ThresholdECDSAModuleSourceTest is Test, InterchainModuleEvents, Thresho assertEq(feeCollector.balance, FEE); } - function test_requestVerification_feeAboveRequired() public { - // Should not revert + function test_requestVerification_feeAboveRequired_emitsEvent() public { + (bytes memory encodedEntry, bytes32 ethSignedHash) = encodeAndHashEntry(mockEntry); + vm.expectEmit(address(module)); + emit VerificationRequested(DST_CHAIN_ID, encodedEntry, ethSignedHash); mockRequestVerification(FEE + 1, mockEntry); } + function test_requestVerification_feeAboveRequired_transfersFeeToCollector() public { + mockRequestVerification(FEE + 1, mockEntry); + assertEq(feeCollector.balance, FEE + 1); + } + function test_requestVerification_revert_feeBelowRequired() public { vm.expectRevert( abi.encodeWithSelector(IInterchainModule.InterchainModule__InsufficientFee.selector, FEE - 1, FEE) @@ -86,9 +110,29 @@ contract ThresholdECDSAModuleSourceTest is Test, InterchainModuleEvents, Thresho mockRequestVerification(FEE - 1, mockEntry); } - function test_getModuleFee() public { - // TODO: fill the calldata - vm.expectCall(address(gasOracle), ""); + function test_getModuleFee_thresholdTwo() public { assertEq(module.getModuleFee(DST_CHAIN_ID), FEE); } + + function test_getModuleFee_callsGasOracle_twoSigners() public { + bytes memory mockedSignatures = new bytes(2 * 65); + bytes memory remoteCalldata = abi.encodeCall(module.verifyEntry, (abi.encode(mockEntry), mockedSignatures)); + bytes memory expectedCalldata = abi.encodeCall( + gasOracle.estimateTxCostInLocalUnits, (DST_CHAIN_ID, EXPECTED_GAS_LIMIT, remoteCalldata.length) + ); + vm.expectCall(address(gasOracle), expectedCalldata); + module.getModuleFee(DST_CHAIN_ID); + } + + function test_getModuleFee_callsGasOracle_threeSigners() public { + vm.prank(owner); + module.setThreshold(3); + bytes memory mockedSignatures = new bytes(3 * 65); + bytes memory remoteCalldata = abi.encodeCall(module.verifyEntry, (abi.encode(mockEntry), mockedSignatures)); + bytes memory expectedCalldata = abi.encodeCall( + gasOracle.estimateTxCostInLocalUnits, (DST_CHAIN_ID, EXPECTED_GAS_LIMIT, remoteCalldata.length) + ); + vm.expectCall(address(gasOracle), expectedCalldata); + module.getModuleFee(DST_CHAIN_ID); + } } From 8f94f6b0a6ec96d3120f85dfc36e097ee2402250 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 20:28:36 +0000 Subject: [PATCH 17/32] Implement source chain funcs --- .../modules/ThresholdECDSAModule.sol | 39 +++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol index d36fffdb87..79d0597d9f 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -4,14 +4,18 @@ pragma solidity 0.8.20; import {InterchainModule} from "./InterchainModule.sol"; import {ThresholdECDSAModuleEvents} from "../events/ThresholdECDSAModuleEvents.sol"; +import {IGasOracle} from "../interfaces/IGasOracle.sol"; import {IThresholdECDSAModule} from "../interfaces/IThresholdECDSAModule.sol"; import {InterchainEntry} from "../libs/InterchainEntry.sol"; import {ThresholdECDSA} from "../libs/ThresholdECDSA.sol"; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; +import {Address} from "@openzeppelin/contracts/utils/Address.sol"; contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModuleEvents, IThresholdECDSAModule { + uint256 public constant VERIFY_GAS_LIMIT = 100_000; + /// @dev Struct to hold the verifiers and the threshold for the module. ThresholdECDSA internal _verifiers; @@ -66,11 +70,6 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ - /// @inheritdoc IThresholdECDSAModule - function getThreshold() external view returns (uint256) { - return _verifiers.getThreshold(); - } - /// @inheritdoc IThresholdECDSAModule function getVerifiers() external view returns (address[] memory) { return _verifiers.getSigners(); @@ -81,6 +80,11 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule return _verifiers.isSigner(account); } + /// @inheritdoc IThresholdECDSAModule + function getThreshold() public view returns (uint256) { + return _verifiers.getThreshold(); + } + // ══════════════════════════════════════════════ INTERNAL LOGIC ═══════════════════════════════════════════════════ /// @dev Sets the threshold for the module. Permissions should be checked in the calling function. @@ -97,10 +101,31 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule } /// @dev Internal logic to request the verification of an entry on the destination chain. - function _requestVerification(uint256 destChainId, bytes memory encodedEntry) internal override {} + function _requestVerification( + uint256, // destChainId + bytes memory // encodedEntry + ) + internal + override + { + // All the hark work has been done in InterchainModule.requestVerification + Address.sendValue(payable(feeCollector), msg.value); + } // ══════════════════════════════════════════════ INTERNAL VIEWS ═══════════════════════════════════════════════════ /// @dev Internal logic to get the module fee for verifying an entry on the specified destination chain. - function _getModuleFee(uint256 destChainId) internal view override returns (uint256) {} + function _getModuleFee(uint256 destChainId) internal view override returns (uint256) { + // On the remote chain the verifyEntry(entry, signatures) function will be called. + // We need to figure out the calldata size for the remote call. + // selector (4 bytes) + entry + signatures + // entry is 32 (length) + 32*4 (fields) = 160 + // signatures: 32 (length) + 65*threshold (padded up to be a multiple of 32 bytes) + // Total formula is: 4 + 32 (entry offset) + 32 (signatures offset) + 160 + 32 + return IGasOracle(gasOracle).estimateTxCostInLocalUnits({ + remoteChainId: destChainId, + gasLimit: VERIFY_GAS_LIMIT, + calldataSize: 260 + (2 * getThreshold() + 1) * 32 + }); + } } From 02d7cbaf0fcd20e96084cfbd73fcbbe953cfd42f Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 20:40:10 +0000 Subject: [PATCH 18/32] Add tests: `bytes` for signatures instead of `bytes[]` --- .../contracts/libs/ThresholdECDSA.sol | 1 + .../harnesses/ThresholdECDSALibHarness.sol | 2 +- .../test/libs/ThresholdECDSALib.t.sol | 87 ++++++++----------- 3 files changed, 40 insertions(+), 50 deletions(-) diff --git a/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol b/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol index 6a4719a62f..0613f7cb2e 100644 --- a/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol +++ b/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol @@ -15,6 +15,7 @@ library ThresholdECDSALib { using EnumerableSet for EnumerableSet.AddressSet; error ThresholdECDSA__AlreadySigner(address account); + error ThresholdECDSA__IncorrectSignaturesLength(uint256 length); error ThresholdECDSA__InvalidSignature(bytes signature); error ThresholdECDSA__NotEnoughSignatures(uint256 threshold); error ThresholdECDSA__NotSigner(address account); diff --git a/packages/contracts-communication/test/harnesses/ThresholdECDSALibHarness.sol b/packages/contracts-communication/test/harnesses/ThresholdECDSALibHarness.sol index fe25c63923..a41eaa5331 100644 --- a/packages/contracts-communication/test/harnesses/ThresholdECDSALibHarness.sol +++ b/packages/contracts-communication/test/harnesses/ThresholdECDSALibHarness.sol @@ -30,7 +30,7 @@ contract ThresholdECDSALibHarness { return ThresholdECDSALib.getThreshold(thresholdECDSA); } - function verifySignedHash(bytes32 hash, bytes[] memory signatures) external view { + function verifySignedHash(bytes32 hash, bytes calldata signatures) external view { ThresholdECDSALib.verifySignedHash(thresholdECDSA, hash, signatures); } } diff --git a/packages/contracts-communication/test/libs/ThresholdECDSALib.t.sol b/packages/contracts-communication/test/libs/ThresholdECDSALib.t.sol index 03bddd053f..d09d1b9989 100644 --- a/packages/contracts-communication/test/libs/ThresholdECDSALib.t.sol +++ b/packages/contracts-communication/test/libs/ThresholdECDSALib.t.sol @@ -40,30 +40,18 @@ contract ThresholdECDSALibTest is Test { return abi.encodePacked(r, s, v); } - function toArray(bytes memory a) internal pure returns (bytes[] memory arr) { - arr = new bytes[](1); - arr[0] = a; - } - - function toArray(bytes memory a, bytes memory b) internal pure returns (bytes[] memory arr) { - arr = new bytes[](2); - arr[0] = a; - arr[1] = b; - } - - function toArray(bytes memory a, bytes memory b, bytes memory c) internal pure returns (bytes[] memory arr) { - arr = new bytes[](3); - arr[0] = a; - arr[1] = b; - arr[2] = c; - } - // ═══════════════════════════════════════════════ TEST HELPERS ════════════════════════════════════════════════════ function expectAlreadySignerError(address account) internal { vm.expectRevert(abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__AlreadySigner.selector, account)); } + function expectIncorrectSignaturesLengthError(uint256 length) internal { + vm.expectRevert( + abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__IncorrectSignaturesLength.selector, length) + ); + } + function expectInvalidSignatureError(bytes memory signature) internal { vm.expectRevert(abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__InvalidSignature.selector, signature)); } @@ -227,100 +215,100 @@ contract ThresholdECDSALibTest is Test { function test_verifySignedHash_providedUnderThreshold_revert_sorted_allSigners() public { libHarness.modifyThreshold(3); expectNotEnoughSignaturesError(3); - libHarness.verifySignedHash(HASH_0, toArray(sig_0_0, sig_2_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_0_0, sig_2_0)); } function test_verifySignedHash_providedUnderThreshold_revert_sorted_hasNonSigners() public { libHarness.modifyThreshold(3); expectNotEnoughSignaturesError(3); - libHarness.verifySignedHash(HASH_0, toArray(sig_0_0, sig_3_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_0_0, sig_3_0)); } function test_verifySignedHash_providedUnderThreshold_revert_unsorted_allSigners() public { libHarness.modifyThreshold(3); expectNotEnoughSignaturesError(3); - libHarness.verifySignedHash(HASH_0, toArray(sig_2_0, sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_2_0, sig_0_0)); } function test_verifySignedHash_providedUnderThreshold_revert_unsorted_hasNonSigners() public { libHarness.modifyThreshold(3); expectNotEnoughSignaturesError(3); - libHarness.verifySignedHash(HASH_0, toArray(sig_3_0, sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_3_0, sig_0_0)); } function test_verifySignedHash_providedUnderThreshold_revert_unsorted_hasDuplicates() public { libHarness.modifyThreshold(3); expectNotEnoughSignaturesError(3); - libHarness.verifySignedHash(HASH_0, toArray(sig_0_0, sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_0_0, sig_0_0)); } function test_verifySignedHash_providedExactlyThreshold_sorted_allSigners() public view { // Should not revert - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, sig_0_0)); } function test_verifySignedHash_providedExactlyThreshold_revert_sorted_hasNonSigners() public { expectNotEnoughSignaturesError(2); - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, sig_3_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, sig_3_0)); } function test_verifySignedHash_providedExactlyThreshold_revert_unsorted_allSigners() public { expectRecoveredSignersNotSortedError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_0_0, sig_1_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_0_0, sig_1_0)); } function test_verifySignedHash_providedExactlyThreshold_revert_unsorted_hasNonSigners() public { expectRecoveredSignersNotSortedError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_3_0, sig_1_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_3_0, sig_1_0)); } function test_verifySignedHash_providedExactlyThreshold_revert_unsorted_hasDuplicates() public { expectRecoveredSignersNotSortedError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_0_0, sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_0_0, sig_0_0)); } function test_verifySignedHash_providedOverThreshold_sorted_allSigners() public view { // Should not revert - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, sig_0_0, sig_2_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, sig_0_0, sig_2_0)); } function test_verifySignedHash_providedOverThreshold_sorted_hasNonSigners_enoughSigners() public view { // Should not revert - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, sig_3_0, sig_2_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, sig_3_0, sig_2_0)); } function test_verifySignedHash_providedOverThreshold_revert_sorted_hasNonSigners_notEnoughSigners() public { libHarness.removeSigner(SIGNER_2); expectNotEnoughSignaturesError(2); - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, sig_3_0, sig_2_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, sig_3_0, sig_2_0)); } function test_verifySignedHash_providedOverThreshold_revert_sorted_hasDuplicates() public { expectRecoveredSignersNotSortedError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, sig_0_0, sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, sig_0_0, sig_0_0)); } function test_verifySignedHash_providedOverThreshold_revert_unsorted_allSigners() public { expectRecoveredSignersNotSortedError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, sig_2_0, sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, sig_2_0, sig_0_0)); } function test_verifySignedHash_providedOverThreshold_revert_unsorted_hasNonSigners_enoughSigners() public { expectRecoveredSignersNotSortedError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_0_0, sig_3_0, sig_1_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_0_0, sig_3_0, sig_1_0)); } function test_verifySignedHash_providedOverThreshold_revert_unsorted_hasNonSigners_notEnoughSigners() public { libHarness.removeSigner(SIGNER_2); expectRecoveredSignersNotSortedError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_2_0, sig_3_0, sig_1_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_2_0, sig_3_0, sig_1_0)); } function test_verifySignedHash_providedOverThreshold_revert_unsorted_hasDuplicates() public { expectRecoveredSignersNotSortedError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, sig_0_0, sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, sig_0_0, sig_0_0)); expectRecoveredSignersNotSortedError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, sig_1_0, sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, sig_1_0, sig_0_0)); } function test_verifySignedHash_revert_zeroThreshold_signerSignature() public { @@ -328,7 +316,7 @@ contract ThresholdECDSALibTest is Test { libHarness = new ThresholdECDSALibHarness(); libHarness.addSigner(SIGNER_0); expectZeroThresholdError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_0_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_0_0)); } function test_verifySignedHash_revert_zeroThreshold_notSignerSignature() public { @@ -336,24 +324,25 @@ contract ThresholdECDSALibTest is Test { libHarness = new ThresholdECDSALibHarness(); libHarness.addSigner(SIGNER_0); expectZeroThresholdError(); - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0)); } - function test_verifySignedHash_revert_incorrectSignatureLength(uint8 length) public { - vm.assume(length != 65); - uint256 min = length < 65 ? length : 65; - bytes memory corruptSig0 = new bytes(length); - for (uint256 i = 0; i < min; i++) { - corruptSig0[i] = sig_0_0[i]; - } - expectInvalidSignatureError(corruptSig0); - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, corruptSig0)); + function test_verifySignedHash_revert_emptySignatures() public { + expectIncorrectSignaturesLengthError(0); + libHarness.verifySignedHash(HASH_0, new bytes(0)); + } + + function test_verifySignedHash_revert_incorrectSignaturesLength(uint256 length) public { + length = bound(length, 0, 1000); + vm.assume(length % 65 != 0); + expectIncorrectSignaturesLengthError(length); + libHarness.verifySignedHash(HASH_0, new bytes(length)); } function test_verifySignedHash_revert_invalidSignature() public { bytes memory corruptSig0 = sig_0_0; corruptSig0[64] = 0xFF; expectInvalidSignatureError(corruptSig0); - libHarness.verifySignedHash(HASH_0, toArray(sig_1_0, corruptSig0)); + libHarness.verifySignedHash(HASH_0, bytes.concat(sig_1_0, corruptSig0)); } } From a5328693ffb7fee060b7d777b7d046d0eb486004 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 20:43:26 +0000 Subject: [PATCH 19/32] Implement library update --- .../contracts/libs/ThresholdECDSA.sol | 20 ++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol b/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol index 0613f7cb2e..91ca5d3979 100644 --- a/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol +++ b/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol @@ -22,6 +22,8 @@ library ThresholdECDSALib { error ThresholdECDSA__RecoveredSignersNotSorted(); error ThresholdECDSA__ZeroThreshold(); + uint256 private constant SIGNATURE_LENGTH = 65; + /// @notice Adds a new signer to the list of signers. /// @dev Will revert if the account is already a signer. function addSigner(ThresholdECDSA storage self, address account) internal { @@ -70,21 +72,28 @@ library ThresholdECDSALib { /// - Any of the payloads is not a valid signature payload. /// - The number of signatures is less than the threshold. /// - The recovered list of signers is not sorted in the ascending order. - function verifySignedHash(ThresholdECDSA storage self, bytes32 hash, bytes[] memory signatures) internal view { + function verifySignedHash(ThresholdECDSA storage self, bytes32 hash, bytes calldata signatures) internal view { + // Figure out the signaturesAmount of signatures provided + uint256 signaturesAmount = signatures.length / SIGNATURE_LENGTH; + if (signaturesAmount == 0 || signaturesAmount * SIGNATURE_LENGTH != signatures.length) { + revert ThresholdECDSA__IncorrectSignaturesLength(signatures.length); + } // First, check that threshold is configured and enough signatures are provided uint256 threshold = self._threshold; if (threshold == 0) { revert ThresholdECDSA__ZeroThreshold(); } - if (signatures.length < threshold) { + if (signaturesAmount < threshold) { revert ThresholdECDSA__NotEnoughSignatures(threshold); } + uint256 offset = 0; uint256 validSignatures = 0; address lastSigner = address(0); - for (uint256 i = 0; i < signatures.length; ++i) { - (address recovered, ECDSA.RecoverError error,) = ECDSA.tryRecover(hash, signatures[i]); + for (uint256 i = 0; i < signaturesAmount; ++i) { + bytes memory signature = signatures[offset:offset + SIGNATURE_LENGTH]; + (address recovered, ECDSA.RecoverError error,) = ECDSA.tryRecover(hash, signature); if (error != ECDSA.RecoverError.NoError) { - revert ThresholdECDSA__InvalidSignature(signatures[i]); + revert ThresholdECDSA__InvalidSignature(signature); } // Check that the recovered addresses list is strictly increasing if (recovered <= lastSigner) { @@ -95,6 +104,7 @@ library ThresholdECDSALib { if (isSigner(self, recovered)) { validSignatures += 1; } + offset += SIGNATURE_LENGTH; } if (validSignatures < threshold) { revert ThresholdECDSA__NotEnoughSignatures(threshold); From 8b0caa2696529c08fe102eabd53971e2499b96ae Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 20:44:45 +0000 Subject: [PATCH 20/32] Simplify math --- .../contracts/modules/ThresholdECDSAModule.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol index 79d0597d9f..331573f1dd 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -125,7 +125,7 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule return IGasOracle(gasOracle).estimateTxCostInLocalUnits({ remoteChainId: destChainId, gasLimit: VERIFY_GAS_LIMIT, - calldataSize: 260 + (2 * getThreshold() + 1) * 32 + calldataSize: 292 + 64 * getThreshold() }); } } From 50b16a4c05b6aeee976c90d24901359ccc4e2dce Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Fri, 16 Feb 2024 20:49:42 +0000 Subject: [PATCH 21/32] Implement `verifyEntry()` --- .../contracts/events/InterchainModuleEvents.sol | 4 ++++ .../contracts/modules/InterchainModule.sol | 1 + .../contracts/modules/ThresholdECDSAModule.sol | 7 ++++++- 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol b/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol index 93f0c2085d..21cb305655 100644 --- a/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol +++ b/packages/contracts-communication/contracts/events/InterchainModuleEvents.sol @@ -1,6 +1,10 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; +import {InterchainEntry} from "../libs/InterchainEntry.sol"; + abstract contract InterchainModuleEvents { event VerificationRequested(uint256 indexed destChainId, bytes entry, bytes32 ethSignedEntryHash); + + event EntryVerified(InterchainEntry entry); } diff --git a/packages/contracts-communication/contracts/modules/InterchainModule.sol b/packages/contracts-communication/contracts/modules/InterchainModule.sol index a91162664d..310cfd548e 100644 --- a/packages/contracts-communication/contracts/modules/InterchainModule.sol +++ b/packages/contracts-communication/contracts/modules/InterchainModule.sol @@ -48,6 +48,7 @@ abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule function _verifyEntry(bytes memory encodedEntry) internal { InterchainEntry memory entry = abi.decode(encodedEntry, (InterchainEntry)); IInterchainDB(INTERCHAIN_DB).verifyEntry(entry); + emit EntryVerified(entry); } /// @dev Internal logic to request the verification of an entry on the destination chain. diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol index 331573f1dd..fe6692deb3 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -12,6 +12,7 @@ import {ThresholdECDSA} from "../libs/ThresholdECDSA.sol"; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; +import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModuleEvents, IThresholdECDSAModule { uint256 public constant VERIFY_GAS_LIMIT = 100_000; @@ -66,7 +67,11 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ /// @inheritdoc IThresholdECDSAModule - function verifyEntry(bytes calldata encodedEntry, bytes calldata signatures) external {} + function verifyEntry(bytes calldata encodedEntry, bytes calldata signatures) external { + bytes32 ethSignedHash = MessageHashUtils.toEthSignedMessageHash(keccak256(encodedEntry)); + _verifiers.verifySignedHash(ethSignedHash, signatures); + _verifyEntry(encodedEntry); + } // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ From de1cc3565587c8d7df75d4ab4917a9820c4999ab Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:11:37 +0000 Subject: [PATCH 22/32] Add tests for destination chain --- .../ThresholdECDSAModule.Destination.t.sol | 325 ++++++++++++++++++ 1 file changed, 325 insertions(+) create mode 100644 packages/contracts-communication/test/modules/ThresholdECDSAModule.Destination.t.sol diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Destination.t.sol b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Destination.t.sol new file mode 100644 index 0000000000..c48400fd80 --- /dev/null +++ b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Destination.t.sol @@ -0,0 +1,325 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.20; + +import {InterchainModuleEvents} from "../../contracts/events/InterchainModuleEvents.sol"; +import {ThresholdECDSAModuleEvents} from "../../contracts/events/ThresholdECDSAModuleEvents.sol"; +import {IInterchainModule} from "../../contracts/interfaces/IInterchainModule.sol"; +import {ThresholdECDSALib} from "../../contracts/libs/ThresholdECDSA.sol"; +import { + ThresholdECDSAModule, + InterchainEntry, + IThresholdECDSAModule +} from "../../contracts/modules/ThresholdECDSAModule.sol"; + +import {GasOracleMock} from "../mocks/GasOracleMock.sol"; +import {InterchainDBMock, IInterchainDB} from "../mocks/InterchainDBMock.sol"; + +import {Test} from "forge-std/Test.sol"; + +contract ThresholdECDSAModuleDestinationTest is Test, InterchainModuleEvents, ThresholdECDSAModuleEvents { + ThresholdECDSAModule public module; + GasOracleMock public gasOracle; + InterchainDBMock public interchainDB; + + address public feeCollector = makeAddr("FeeCollector"); + address public owner = makeAddr("Owner"); + + uint256 public constant SRC_CHAIN_ID = 1337; + uint256 public constant DST_CHAIN_ID = 7331; + + InterchainEntry public mockEntry = InterchainEntry({ + srcChainId: SRC_CHAIN_ID, + srcWriter: bytes32(uint256(2)), + writerNonce: 3, + dataHash: bytes32(uint256(4)) + }); + + uint256 public constant PK_0 = 1000; + uint256 public constant PK_1 = 2000; + uint256 public constant PK_2 = 3000; + uint256 public constant PK_3 = 4000; + uint256 public constant PK_4 = 5000; + + address public constant SIGNER_0 = 0x7F1d642DbfD62aD4A8fA9810eA619707d09825D0; + address public constant SIGNER_1 = 0x5793e629c061e7FD642ab6A1b4d552CeC0e2D606; + address public constant SIGNER_2 = 0xf6c0eB696e44d15E8dceb3B63A6535e469Be6C62; + address public constant SIGNER_3 = 0xAD1117CAB797E37CAB0Eee8Ca7C30bD2452Ef2a3; + address public constant SIGNER_4 = 0x725D327818161E0B4C6cCA5b8b1567d2A40b5B86; + + function setUp() public { + vm.chainId(DST_CHAIN_ID); + interchainDB = new InterchainDBMock(); + module = new ThresholdECDSAModule(address(interchainDB), owner); + gasOracle = new GasOracleMock(); + vm.startPrank(owner); + module.setGasOracle(address(gasOracle)); + module.setFeeCollector(feeCollector); + module.addVerifier(SIGNER_0); + module.addVerifier(SIGNER_1); + module.addVerifier(SIGNER_2); + module.setThreshold(2); + vm.stopPrank(); + } + + function test_pks() public { + assertEq(SIGNER_0, vm.addr(PK_0)); + assertEq(SIGNER_1, vm.addr(PK_1)); + assertEq(SIGNER_2, vm.addr(PK_2)); + assertEq(SIGNER_3, vm.addr(PK_3)); + assertEq(SIGNER_4, vm.addr(PK_4)); + } + + function toArray(uint256 a) internal pure returns (uint256[] memory arr) { + arr = new uint256[](1); + arr[0] = a; + } + + function toArray(uint256 a, uint256 b) internal pure returns (uint256[] memory arr) { + arr = new uint256[](2); + arr[0] = a; + arr[1] = b; + } + + function toArray(uint256 a, uint256 b, uint256 c) internal pure returns (uint256[] memory arr) { + arr = new uint256[](3); + arr[0] = a; + arr[1] = b; + arr[2] = c; + } + + function signEntry( + InterchainEntry memory entry, + uint256[] memory pks + ) + internal + pure + returns (bytes memory signatures) + { + bytes32 digest = keccak256(abi.encode(entry)); + bytes32 ethSignedHash = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", digest)); + signatures = ""; + for (uint256 i = 0; i < pks.length; ++i) { + (uint8 v, bytes32 r, bytes32 s) = vm.sign(pks[i], ethSignedHash); + signatures = bytes.concat(signatures, abi.encodePacked(r, s, v)); + } + } + + function verifyEntry(InterchainEntry memory entry, bytes memory signatures) internal { + module.verifyEntry(abi.encode(entry), signatures); + } + + // ═══════════════════════════════════════════════ TEST HELPERS ════════════════════════════════════════════════════ + + function expectInterchainDBCall(InterchainEntry memory entry) internal { + bytes memory expectedCallData = abi.encodeCall(IInterchainDB.verifyEntry, (entry)); + // expectCall(address callee, bytes calldata data, uint64 count) + vm.expectCall(address(interchainDB), expectedCallData, 1); + } + + function expectEntryVerifiedEvent(InterchainEntry memory entry) internal { + vm.expectEmit(address(module)); + emit EntryVerified(entry); + } + + function expectIncorrectSignaturesLengthRevert(uint256 length) internal { + vm.expectRevert( + abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__IncorrectSignaturesLength.selector, length) + ); + } + + function expectNotEnoughSignaturesRevert(uint256 threshold) internal { + vm.expectRevert( + abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__NotEnoughSignatures.selector, threshold) + ); + } + + function expectRecoveredSignersNotSortedRevert() internal { + vm.expectRevert(ThresholdECDSALib.ThresholdECDSA__RecoveredSignersNotSorted.selector); + } + + function expectSameChainIdRevert() internal { + vm.expectRevert(IInterchainModule.InterchainModule__SameChainId.selector); + } + + // ════════════════════════════════════════════ TESTS: VERIFY ENTRY ════════════════════════════════════════════════ + + // Signers order sorted by their address: + // SIGNER_1, SIGNER_4, SIGNER_0, SIGNER_3, SIGNER_2 + + // Should be verified if the enough valid signatures, which match the signers ascending order + + function test_verifyEntry_zeroSignatures_revertNotEnoughSignatures() public { + expectIncorrectSignaturesLengthRevert(0); + verifyEntry(mockEntry, ""); + } + + function test_verifyEntry_oneSignature_valid_revertNotEnoughSignatures() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_0)); + expectNotEnoughSignaturesRevert(2); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_oneSignature_invalid_revertNotEnoughSignatures() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_3)); + expectNotEnoughSignaturesRevert(2); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_twoSignatures_valid_sorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_1, PK_0)); + expectInterchainDBCall(mockEntry); + expectEntryVerifiedEvent(mockEntry); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_twoSignatures_valid_duplicate_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_1, PK_1)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_twoSignatures_valid_unsorted_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_0, PK_1)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_twoSignatures_invalidOne_sorted_revertNotEnoughSignatures() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_1, PK_3)); + expectNotEnoughSignaturesRevert(2); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_twoSignatures_invalidOne_unsorted_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_3, PK_1)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_twoSignatures_invalidTwo_sorted_revertNotEnoughSignatures() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_4, PK_3)); + expectNotEnoughSignaturesRevert(2); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_twoSignatures_invalidTwo_duplicate_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_4, PK_4)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_valid_sorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_1, PK_0, PK_2)); + expectInterchainDBCall(mockEntry); + expectEntryVerifiedEvent(mockEntry); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_valid_duplicate_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_1, PK_0, PK_0)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_valid_unsorted_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_0, PK_2, PK_1)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_invalidOne_sorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_4, PK_0, PK_2)); + expectInterchainDBCall(mockEntry); + expectEntryVerifiedEvent(mockEntry); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_invalidOne_duplicate_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_4, PK_0, PK_0)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_invalidOne_unsorted_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_0, PK_4, PK_1)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_invalidTwo_sorted_revertNotEnoughSignatures() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_4, PK_3, PK_2)); + expectNotEnoughSignaturesRevert(2); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_invalidTwo_duplicate_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_0, PK_3, PK_3)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_invalidTwo_unsorted_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_3, PK_0, PK_4)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_invalidThree_sorted_revertNotEnoughSignatures() public { + vm.prank(owner); + module.removeVerifier(SIGNER_0); + bytes memory signatures = signEntry(mockEntry, toArray(PK_4, PK_0, PK_3)); + expectNotEnoughSignaturesRevert(2); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_invalidThree_duplicate_revertNotSorted() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_4, PK_3, PK_3)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_threeSignatures_invalidThree_unsorted_revertNotSorted() public { + vm.prank(owner); + module.removeVerifier(SIGNER_0); + bytes memory signatures = signEntry(mockEntry, toArray(PK_3, PK_4, PK_0)); + expectRecoveredSignersNotSortedRevert(); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_revertSameChainId() public { + InterchainEntry memory entry = mockEntry; + entry.srcChainId = DST_CHAIN_ID; + bytes memory signatures = signEntry(entry, toArray(PK_1, PK_0)); + expectSameChainIdRevert(); + verifyEntry(entry, signatures); + } + + function test_verifyEntry_revertZeroThreshold() public { + // Deploy a module without setting up the threshold + module = new ThresholdECDSAModule(address(interchainDB), owner); + vm.startPrank(owner); + module.addVerifier(SIGNER_0); + module.addVerifier(SIGNER_1); + vm.stopPrank(); + bytes memory signatures = signEntry(mockEntry, toArray(PK_1, PK_0)); + vm.expectRevert(ThresholdECDSALib.ThresholdECDSA__ZeroThreshold.selector); + verifyEntry(mockEntry, signatures); + } + + function test_verifyEntry_revertIncorrectSignaturesLengthTooShort() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_1, PK_0)); + bytes memory signaturesShort = new bytes(signatures.length - 1); + for (uint256 i = 0; i < signaturesShort.length; ++i) { + signaturesShort[i] = signatures[i]; + } + expectIncorrectSignaturesLengthRevert(signaturesShort.length); + verifyEntry(mockEntry, signaturesShort); + } + + function test_verifyEntry_revertIncorrectSignaturesLengthTooLong() public { + bytes memory signatures = signEntry(mockEntry, toArray(PK_1, PK_0)); + bytes memory signaturesLong = bytes.concat(signatures, bytes1(0x2A)); + expectIncorrectSignaturesLengthRevert(signaturesLong.length); + verifyEntry(mockEntry, signaturesLong); + } +} From 494ee354f4f82387247d88b89bdf7e96ce6a6873 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:12:49 +0000 Subject: [PATCH 23/32] Don't use infinity as default threshold --- .../contracts/modules/ThresholdECDSAModule.sol | 3 +-- .../test/modules/ThresholdECDSAModule.Management.t.sol | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol index fe6692deb3..cdeebebac2 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol @@ -27,8 +27,7 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule address public gasOracle; constructor(address interchainDB, address initialOwner) InterchainModule(interchainDB) Ownable(initialOwner) { - // Explicitly disable the module functionality until the threshold is set. - _setThreshold(type(uint256).max); + // ThresholdECDSA throws an explicit error if threshold is not set, so default value is not needed } // ═══════════════════════════════════════════════ PERMISSIONED ════════════════════════════════════════════════════ diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol index 5eaa5e3542..d991fe65df 100644 --- a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol +++ b/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol @@ -39,7 +39,7 @@ contract ThresholdECDSAModuleManagementTest is Test, ThresholdECDSAModuleEvents function test_setup() public { assertEq(module.owner(), owner); assertEq(module.INTERCHAIN_DB(), interchainDB); - assertEq(module.getThreshold(), type(uint256).max); + assertEq(module.getThreshold(), 0); assertEq(module.gasOracle(), address(0)); } From 696c4e080bdff52acc8093c1b4ca262b29bdeaf5 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:13:50 +0000 Subject: [PATCH 24/32] Check source chain id in any InterchainModule --- .../contracts/modules/InterchainModule.sol | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/contracts-communication/contracts/modules/InterchainModule.sol b/packages/contracts-communication/contracts/modules/InterchainModule.sol index 310cfd548e..704294e18e 100644 --- a/packages/contracts-communication/contracts/modules/InterchainModule.sol +++ b/packages/contracts-communication/contracts/modules/InterchainModule.sol @@ -47,6 +47,9 @@ abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule /// to the InterchainDB. function _verifyEntry(bytes memory encodedEntry) internal { InterchainEntry memory entry = abi.decode(encodedEntry, (InterchainEntry)); + if (entry.srcChainId == block.chainid) { + revert InterchainModule__SameChainId(); + } IInterchainDB(INTERCHAIN_DB).verifyEntry(entry); emit EntryVerified(entry); } From 900a52ebfde2a3ad2c8a5d062c13b23ae100558e Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:19:16 +0000 Subject: [PATCH 25/32] Deprecate old SynapseModule --- .../contracts/InterchainModule.sol | 12 -- .../contracts/interfaces/ISynapseModule.sol | 34 --- .../contracts/modules/SynapseGasService.sol | 29 --- .../contracts/modules/SynapseModule.sol | 77 ------- .../test/SynapseModuleE2E.t.sol | 151 -------------- .../test/SynapseModuleTest.sol | 197 ------------------ 6 files changed, 500 deletions(-) delete mode 100644 packages/contracts-communication/contracts/InterchainModule.sol delete mode 100644 packages/contracts-communication/contracts/interfaces/ISynapseModule.sol delete mode 100644 packages/contracts-communication/contracts/modules/SynapseGasService.sol delete mode 100644 packages/contracts-communication/contracts/modules/SynapseModule.sol delete mode 100644 packages/contracts-communication/test/SynapseModuleE2E.t.sol delete mode 100644 packages/contracts-communication/test/SynapseModuleTest.sol diff --git a/packages/contracts-communication/contracts/InterchainModule.sol b/packages/contracts-communication/contracts/InterchainModule.sol deleted file mode 100644 index 8f72c4cc6e..0000000000 --- a/packages/contracts-communication/contracts/InterchainModule.sol +++ /dev/null @@ -1,12 +0,0 @@ -pragma solidity 0.8.20; - -contract InterchainModule { - // TODO: All modules need to have an understanding of the gas limit needed to execute on the destination chain. - - function sendModuleMessage(bytes calldata transaction) public { - // This function would send the transaction to the module. - // As `sendModuleMessage` is not implemented, we're leaving this as a placeholder. - } - - function receiveModuleMessage() public {} -} diff --git a/packages/contracts-communication/contracts/interfaces/ISynapseModule.sol b/packages/contracts-communication/contracts/interfaces/ISynapseModule.sol deleted file mode 100644 index 5a21ca5135..0000000000 --- a/packages/contracts-communication/contracts/interfaces/ISynapseModule.sol +++ /dev/null @@ -1,34 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; - -import {InterchainEntry} from "../libs/InterchainEntry.sol"; - -interface ISynapseModule { - /// @notice Sets the address of the InterchainDB contract to be used for verifying entries. - /// @dev This function can only be called by the contract owner. - /// @param _interchainDB The address of the InterchainDB contract. - function setInterchainDB(address _interchainDB) external; - - /// @notice Sets the required threshold for verification. - /// @dev This function updates the threshold value that determines the minimum number of verifications required for an entry to be considered valid. Can only be called by the contract owner. - /// @param _threshold The new threshold value. - function setRequiredThreshold(uint256 _threshold) external; - - /// @notice Updates the list of verifier addresses. - /// @dev This function sets the addresses that are allowed to act as verifiers for entries. Can only be called by the contract owner. - /// @param _verifiers An array of addresses to be set as verifiers. - function setVerifiers(address[] calldata _verifiers) external; - - /// @notice Requests off-chain verification of an interchain entry for a specified destination chain. This function requires a fee. - /// @dev This function can only be called by the InterchainDB contract. It checks if the sent value covers the module fee for the requested destination chain, then proceeds to pay the fee for execution. Emits a VerificationRequested event upon success. - /// @param destChainId The ID of the destination chain where the entry needs to be verified. - /// @param entry The interchain entry to be verified. - function requestVerification(uint256 destChainId, InterchainEntry memory entry) external payable; - - /// @notice Verifies an interchain entry using a set of verifier signatures. - /// @dev This function checks if the provided signatures meet the required threshold for verification. - /// It then calls the InterchainDB contract to verify the entry. Requires that the number of valid signatures meets or exceeds the required threshold. - /// @param entry The interchain entry to be verified. - /// @param signatures An array of signatures used to verify the entry. - function verifyEntry(InterchainEntry memory entry, bytes[] calldata signatures) external; -} diff --git a/packages/contracts-communication/contracts/modules/SynapseGasService.sol b/packages/contracts-communication/contracts/modules/SynapseGasService.sol deleted file mode 100644 index 06815b20d4..0000000000 --- a/packages/contracts-communication/contracts/modules/SynapseGasService.sol +++ /dev/null @@ -1,29 +0,0 @@ -pragma solidity 0.8.20; - -import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; - -abstract contract SynapseGasService is Ownable { - address payable public executor; - address public gasOracle; - - function getModuleFee(uint256 dstChainId) public pure returns (uint256) { - // Get Latest Posted Destination Gas Price from oracle - // Requires: Access to origin USD Gas Price / Destination USD Gas PRice - // Get current price of origin chain assets - // Get current price of destination chain assets - // Calculate the estiamted fee based on preset gas limit - // return - - // TODO: Right now, we don't have all of the info needed to provide a real fee estimation - we will provide 1 wei to enable other functionality to be built. - return 1; - } - - function setExecutor(address _executor) public onlyOwner { - executor = payable(_executor); - } - - function _payFeesForExecution(uint256 feeAmount) internal { - // Transfer fee to executor - executor.transfer(feeAmount); - } -} diff --git a/packages/contracts-communication/contracts/modules/SynapseModule.sol b/packages/contracts-communication/contracts/modules/SynapseModule.sol deleted file mode 100644 index 4e309a00cc..0000000000 --- a/packages/contracts-communication/contracts/modules/SynapseModule.sol +++ /dev/null @@ -1,77 +0,0 @@ -pragma solidity 0.8.20; - -import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; -import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import {SynapseGasService} from "./SynapseGasService.sol"; -import "../IInterchain.sol"; -import "forge-std/console.sol"; -import {IInterchainDB} from "../interfaces/IInterchainDB.sol"; -import {IInterchainModule} from "../interfaces/IInterchainModule.sol"; - -import {ISynapseModule} from "../interfaces/ISynapseModule.sol"; -import {InterchainEntry} from "../libs/InterchainEntry.sol"; - -import {SynapseModuleEvents} from "../events/SynapseModuleEvents.sol"; - -/// @title Synapse Module for Interchain Communication -/// @notice This contract implements the Synapse Module functionality for interchain communication, including setting verifiers, thresholds, and handling verification requests. -/// @dev Inherits from Ownable, SynapseGasService, and SynapseModuleEvents for event emissions. -contract SynapseModule is Ownable, SynapseGasService, SynapseModuleEvents, ISynapseModule { - address[] public verifiers; - uint256 public requiredThreshold; - address public interchainDB; - - /// @notice Initializes the contract setting the deployer as the owner. - constructor() Ownable(msg.sender) {} - - /// @inheritdoc ISynapseModule - function setInterchainDB(address _interchainDB) public onlyOwner { - interchainDB = _interchainDB; - } - - /// @inheritdoc ISynapseModule - function setRequiredThreshold(uint256 _threshold) public onlyOwner { - requiredThreshold = _threshold; - } - - /// @inheritdoc ISynapseModule - function setVerifiers(address[] calldata _verifiers) public onlyOwner { - verifiers = _verifiers; - } - - /// @inheritdoc ISynapseModule - function requestVerification(uint256 destChainId, InterchainEntry memory entry) external payable { - require(msg.sender == interchainDB, "Only InterchainDB can request verification"); - - require(msg.value >= getModuleFee(destChainId), "Insufficient fee to request verification"); - - _payFeesForExecution(msg.value); - bytes32 signedEntryHash = keccak256(abi.encode(entry)); - emit VerificationRequested(destChainId, entry, signedEntryHash); - } - - /// @inheritdoc ISynapseModule - function verifyEntry(InterchainEntry memory entry, bytes[] calldata signatures) external { - bytes32 signableMessageHash = keccak256(abi.encode(entry)); - - require(signatures.length >= requiredThreshold, "Not enough signatures to meet the threshold"); - - uint256 validSignatures; - for (uint256 i = 0; i < verifiers.length; i++) { - // TODO: Use TryRecover for explicit error handling - address signer = ECDSA.recover(signableMessageHash, signatures[i]); - for (uint256 j = 0; j < verifiers.length; j++) { - if (signer == verifiers[j]) { - validSignatures++; - break; - } - } - } - - require(validSignatures >= requiredThreshold, "Not enough valid signatures to meet the threshold"); - - IInterchainDB(interchainDB).verifyEntry(entry); - - emit EntryVerified(entry, signableMessageHash); - } -} diff --git a/packages/contracts-communication/test/SynapseModuleE2E.t.sol b/packages/contracts-communication/test/SynapseModuleE2E.t.sol deleted file mode 100644 index e5afb29599..0000000000 --- a/packages/contracts-communication/test/SynapseModuleE2E.t.sol +++ /dev/null @@ -1,151 +0,0 @@ -// pragma solidity 0.8.20; - -// import 'forge-std/Test.sol'; -// import '../contracts/modules/SynapseModule.sol'; -// import '../contracts/Interchain.sol'; -// import '../contracts/InterchainApp.sol'; - -// import {ECDSA} from '@openzeppelin/contracts/utils/cryptography/ECDSA.sol'; - -// contract SynapseModuleE2ETest is Test { -// SynapseModule moduleL1; -// SynapseModule moduleL2; -// Vm.Wallet verifier1; -// Vm.Wallet verifier2; -// Vm.Wallet verifier3; -// address[] verifierAddresses; -// uint256[] verifierPrivateKeys; - -// Interchain interchainL1; -// Interchain interchainL2; - -// InterchainApp dstInterchainApp; -// InterchainApp originInterchainApp; - -// address[] l1Modules; - -// address executor; - -// function setUp() public { -// moduleL1 = new SynapseModule(); -// moduleL2 = new SynapseModule(); - -// verifier1 = vm.createWallet( -// 0x4a80c7a42367b3f50bc561c298c52233aca8e93dd511a9116281eb529e57ba14 -// ); -// verifier2 = vm.createWallet( -// 0x1dea3ab3c2dddaea367a91af68d2e5097669417df0a195867db0b9f6079ddeeb -// ); -// verifier3 = vm.createWallet( -// 0xed292910d48741c4072b504ce6127157911b4eb56cefeb87a57e63217e0b7ee3 -// ); -// verifierAddresses = new address[](3); -// verifierAddresses[0] = verifier1.addr; -// verifierAddresses[1] = verifier2.addr; -// verifierAddresses[2] = verifier3.addr; -// verifierPrivateKeys = new uint256[](3); -// verifierPrivateKeys[0] = verifier1.privateKey; -// verifierPrivateKeys[1] = verifier2.privateKey; -// verifierPrivateKeys[ -// 2 -// ] = 0xed292910d48741c4072b504ce6127157911b4eb56cefeb87a57e63217e0b7ee3; - -// interchainL1 = new Interchain(); -// interchainL2 = new Interchain(); -// moduleL1.setInterchain(address(interchainL1)); -// moduleL1.setVerifiers(verifierAddresses); -// moduleL2.setInterchain(address(interchainL2)); -// moduleL2.setVerifiers(verifierAddresses); -// moduleL1.setRequiredThreshold(3); -// moduleL2.setRequiredThreshold(3); -// l1Modules = new address[](1); -// l1Modules[0] = address(moduleL1); -// address[] memory l2Modules = new address[](1); -// l2Modules[0] = address(moduleL2); -// originInterchainApp = new InterchainApp( -// address(interchainL1), -// l1Modules, -// l2Modules -// ); -// dstInterchainApp = new InterchainApp( -// address(interchainL2), -// l2Modules, -// l1Modules -// ); -// executor = address(0x123); - -// moduleL1.setExecutor(executor); -// moduleL2.setExecutor(executor); -// } - -// function testSendAndReceiveSynapseModuleMessage() public { -// bytes32 receiver = interchainL2.convertAddressToBytes32( -// address(dstInterchainApp) -// ); -// uint256 dstChainId = 1; -// bytes memory message = abi.encodeWithSelector( -// InterchainApp.appReceive.selector -// ); -// uint64 initialNonce = interchainL1.nonce(); - -// uint256 estimatedFee = interchainL1.estimateInterchainTransactionFee( -// dstChainId, -// l1Modules -// ); -// uint256 executorInitialBalance = executor.balance; -// originInterchainApp.send{value: estimatedFee}( -// receiver, -// dstChainId, -// message -// ); -// uint256 executorAfterBalance = executor.balance; -// assertEq( -// executorAfterBalance - executorInitialBalance, -// 1, -// 'Executor should receive fee amount' -// ); - -// Interchain.InterchainTransaction memory interTransaction = Interchain -// .InterchainTransaction( -// address(this), -// block.chainid, -// receiver, -// dstChainId, -// message, -// initialNonce, -// keccak256(abi.encodePacked('transactionId')), -// new address[](1), -// 1 -// ); -// interTransaction.modules[0] = address(moduleL1); -// bytes memory bytesInterTransaction = abi.encode(interTransaction); -// assertEq( -// interchainL1.nonce(), -// initialNonce + 1, -// 'Nonce should increment by 1' -// ); - -// // Now message was sent -// // Mock out the verifiers responses -// bytes memory proof; -// bytes[] memory signatures = new bytes[](verifierAddresses.length); -// bytes32 messageHashToSign = keccak256(bytesInterTransaction); -// for (uint256 i = 0; i < verifierAddresses.length; i++) { -// (uint8 v, bytes32 r, bytes32 s) = vm.sign( -// verifierPrivateKeys[i], -// messageHashToSign -// ); -// bytes memory signature = abi.encodePacked(r, s, v); -// (address recovered, ECDSA.RecoverError error, ) = ECDSA.tryRecover( -// messageHashToSign, -// signature -// ); -// require(error == ECDSA.RecoverError.NoError, 'Invalid signature'); -// console.log(recovered); - -// signatures[i] = signature; -// } - -// moduleL2.receiveModuleMessage(bytesInterTransaction, signatures); -// } -// } diff --git a/packages/contracts-communication/test/SynapseModuleTest.sol b/packages/contracts-communication/test/SynapseModuleTest.sol deleted file mode 100644 index 8a836fde28..0000000000 --- a/packages/contracts-communication/test/SynapseModuleTest.sol +++ /dev/null @@ -1,197 +0,0 @@ -pragma solidity 0.8.20; - -import {Test} from "forge-std/Test.sol"; - -import {InterchainDB, InterchainEntry, IInterchainDB, InterchainDBEvents} from "../contracts/InterchainDB.sol"; - -import {SynapseModule, SynapseModuleEvents} from "../contracts/modules/SynapseModule.sol"; - -contract SynapseModuleTest is Test, SynapseModuleEvents { - IInterchainDB icDB; - SynapseModule synapseModule; - - uint256 public constant SRC_CHAIN_ID = 1337; - uint256 public constant DST_CHAIN_ID = 7331; - - uint256 public constant INITIAL_WRITER_F_NONCE = 1; - uint256 public constant INITIAL_WRITER_S_NONCE = 2; - - address public contractOwner = makeAddr("Contract Owner"); - address public requestCaller = makeAddr("Request Caller"); - address public writerF = makeAddr("First Writer"); - address public writerS = makeAddr("Second Writer"); - address public notWriter = makeAddr("Not a Writer"); - - Account verifierA = makeAccount("Verifier A"); - Account verifierB = makeAccount("Verifier B"); - Account verifierC = makeAccount("Verifier C"); - address[] verifiers = [verifierA.addr, verifierB.addr, verifierC.addr]; - uint256[] verifiersPrivateKeys = [verifierA.key, verifierB.key, verifierC.key]; - - function setUp() public { - vm.startPrank(contractOwner); - icDB = new InterchainDB(); - synapseModule = new SynapseModule(); - synapseModule.setInterchainDB(address(icDB)); - synapseModule.setVerifiers(verifiers); - synapseModule.setRequiredThreshold(2); - vm.stopPrank(); - - setupWriterNonce(writerF, INITIAL_WRITER_F_NONCE); - setupWriterNonce(writerS, INITIAL_WRITER_S_NONCE); - } - - function setupWriterNonce(address writer, uint256 nonce) internal { - for (uint256 i = 0; i < nonce; i++) { - writeEntry(writer, getMockDataHash(writer, i)); - } - } - - // ═══════════════════════════════════════════════ TEST HELPERS ════════════════════════════════════════════════════ - - function assertEq(InterchainEntry memory entry, InterchainEntry memory expected) internal { - assertEq(entry.srcChainId, expected.srcChainId, "!srcChainId"); - assertEq(entry.srcWriter, expected.srcWriter, "!srcWriter"); - assertEq(entry.writerNonce, expected.writerNonce, "!writerNonce"); - assertEq(entry.dataHash, expected.dataHash, "!dataHash"); - } - - function getMockDataHash(address writer, uint256 nonce) internal pure returns (bytes32) { - return keccak256(abi.encode(writer, nonce)); - } - - function getMockEntry(address writer, uint256 nonce) internal pure returns (InterchainEntry memory entry) { - return InterchainEntry({ - srcChainId: SRC_CHAIN_ID, - srcWriter: addressToBytes32(writer), - writerNonce: nonce, - dataHash: getMockDataHash(writer, nonce) - }); - } - - function addressToBytes32(address addr) internal pure returns (bytes32) { - return bytes32(uint256(uint160(addr))); - } - - function writeEntry(address writer, bytes32 dataHash) internal returns (uint256 writerNonce) { - vm.prank(writer); - writerNonce = icDB.writeEntry(dataHash); - } - - // ═══════════════════════════════════════════════ TESTS: PERMISSIONING ════════════════════════════════════════════════════ - - function test_setInterchainDB_notOwner() public { - vm.expectRevert(); - vm.prank(requestCaller); - synapseModule.setInterchainDB(address(icDB)); - } - - function test_setInterchainDB_owner() public { - vm.prank(contractOwner); - synapseModule.setInterchainDB(address(icDB)); - assertEq(synapseModule.interchainDB(), address(icDB)); - } - - function test_setRequiredThreshold_notOwner() public { - vm.expectRevert(); - vm.prank(requestCaller); - synapseModule.setRequiredThreshold(2); - } - - function test_setRequiredThreshold_owner() public { - vm.prank(contractOwner); - synapseModule.setRequiredThreshold(2); - assertEq(synapseModule.requiredThreshold(), 2); - } - - function test_setVerifiers_notOwner() public { - vm.expectRevert(); - vm.prank(requestCaller); - synapseModule.setVerifiers(verifiers); - } - - function test_setVerifiers_owner() public { - vm.prank(contractOwner); - synapseModule.setVerifiers(verifiers); - for (uint256 i = 0; i < verifiers.length; i++) { - assertEq(synapseModule.verifiers(i), verifiers[i]); - } - } - - // ═══════════════════════════════════════════════ TESTS: REQUEST VERIFICATION ════════════════════════════════════════════════════ - function test_requestVerification_notInterchainDB() public { - vm.expectRevert(); - synapseModule.requestVerification(DST_CHAIN_ID, getMockEntry(writerF, 1)); - } - - function test_requestVerification_insufficientFee() public { - uint256 expectedFee = synapseModule.getModuleFee(DST_CHAIN_ID); - vm.expectRevert(); - vm.prank(address(icDB)); - synapseModule.requestVerification{value: expectedFee - 1}(DST_CHAIN_ID, getMockEntry(writerF, 1)); - } - - function test_requestVerification_feesPaid() public { - address executor = address(synapseModule.executor()); - uint256 prevBalance = executor.balance; - uint256 expectedFee = synapseModule.getModuleFee(DST_CHAIN_ID); - vm.deal(address(icDB), expectedFee); - vm.prank(address(icDB)); - synapseModule.requestVerification{value: expectedFee}(DST_CHAIN_ID, getMockEntry(writerF, 1)); - assertEq(executor.balance, prevBalance + expectedFee); - } - - function test_requestVerification_eventEmit() public { - uint256 expectedFee = synapseModule.getModuleFee(DST_CHAIN_ID); - vm.deal(address(icDB), expectedFee); - vm.prank(address(icDB)); - InterchainEntry memory mockEntry = getMockEntry(writerF, 1); - vm.expectEmit(); - emit VerificationRequested(DST_CHAIN_ID, mockEntry, keccak256(abi.encode(mockEntry))); - synapseModule.requestVerification{value: expectedFee}(DST_CHAIN_ID, getMockEntry(writerF, 1)); - } - - // ═══════════════════════════════════════════════ TESTS: VERIFY ENTRY ════════════════════════════════════════════════════ - - function test_verifyEntry_notEnoughSignatures() public { - InterchainEntry memory entry = getMockEntry(writerF, 1); - vm.expectRevert(); - synapseModule.verifyEntry(entry, new bytes[](0)); - } - - function test_verifyEntry_notEnoughValidSignatures() public { - InterchainEntry memory entry = getMockEntry(writerF, 1); - bytes[] memory signatures = new bytes[](verifiers.length); - uint256 expectedThreshold = synapseModule.requiredThreshold(); - uint256 plannedValidSignatures; - for (uint256 i = 0; i < verifiers.length; i++) { - if (plannedValidSignatures < expectedThreshold - 1) { - (uint8 v, bytes32 r, bytes32 s) = vm.sign(verifiersPrivateKeys[i], keccak256(abi.encode(entry))); - signatures[i] = abi.encodePacked(r, s, v); - plannedValidSignatures++; - } else { - (uint8 v, bytes32 r, bytes32 s) = - vm.sign(verifiersPrivateKeys[i], keccak256(abi.encode("invalid message"))); - signatures[i] = abi.encodePacked(r, s, v); - } - } - vm.expectRevert(); - synapseModule.verifyEntry(entry, signatures); - } - - function test_verifyEntry_validSignatures() public { - InterchainEntry memory entry = getMockEntry(writerF, 1); - bytes[] memory signatures = new bytes[](verifiers.length); - for (uint256 i = 0; i < verifiers.length; i++) { - (uint8 v, bytes32 r, bytes32 s) = vm.sign(verifiersPrivateKeys[i], keccak256(abi.encode(entry))); - signatures[i] = abi.encodePacked(r, s, v); - } - vm.expectEmit(); - emit EntryVerified(entry, keccak256(abi.encode(entry))); - // set block time to 2 to verify entry at block.timestamp = 2 - vm.warp(2); - synapseModule.verifyEntry(entry, signatures); - uint256 moduleVerifiedAt = icDB.readEntry(address(synapseModule), entry); - assertEq(moduleVerifiedAt, 2); - } -} From e724c38c6f282b39a553fd94035b65bbd671a476 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:23:26 +0000 Subject: [PATCH 26/32] Fix ClientV1 test --- .../test/InterchainClientV1Test.t.sol | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/contracts-communication/test/InterchainClientV1Test.t.sol b/packages/contracts-communication/test/InterchainClientV1Test.t.sol index 4fca87b883..f903898627 100644 --- a/packages/contracts-communication/test/InterchainClientV1Test.t.sol +++ b/packages/contracts-communication/test/InterchainClientV1Test.t.sol @@ -6,14 +6,12 @@ import "../contracts/InterchainDB.sol"; import {InterchainAppMock} from "./mocks/InterchainAppMock.sol"; import {InterchainModuleMock} from "./mocks/InterchainModuleMock.sol"; -import "../contracts/modules/SynapseModule.sol"; import {InterchainEntry} from "../contracts/libs/InterchainEntry.sol"; contract InterchainClientV1Test is Test { InterchainClientV1 icClient; InterchainDB icDB; - SynapseModule synapseModule; InterchainAppMock icApp; InterchainModuleMock icModule; @@ -26,21 +24,28 @@ contract InterchainClientV1Test is Test { vm.startPrank(contractOwner); icClient = new InterchainClientV1(); icDB = new InterchainDB(); - synapseModule = new SynapseModule(); - synapseModule.setInterchainDB(address(icDB)); icClient.setInterchainDB(address(icDB)); icModule = new InterchainModuleMock(); icApp = new InterchainAppMock(); icApp.setReceivingModule(address(icModule)); vm.stopPrank(); + // icModule should return 1 for the module fee + mockModuleFee(icModule, 1); + } + + /// @dev Mocks a return value of module.getModuleFee(DST_CHAIN_ID) + function mockModuleFee(InterchainModuleMock module, uint256 feeValue) internal { + bytes memory callData = abi.encodeCall(module.getModuleFee, (DST_CHAIN_ID)); + bytes memory returnData = abi.encode(feeValue); + vm.mockCall(address(module), callData, returnData); } function test_interchainSend() public { bytes32 receiver = icClient.convertAddressToBytes32(makeAddr("Receiver")); bytes memory message = "Hello World"; address[] memory srcModules = new address[](1); - srcModules[0] = address(synapseModule); + srcModules[0] = address(icModule); uint256 totalModuleFees = 1; uint64 nonce = 1; bytes32 transactionID = keccak256( From 41949cde6acbe76fc1fa52e415b2102fd1be9652 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:25:29 +0000 Subject: [PATCH 27/32] Chore: cleanup --- .../test/InterchainClientV1Test.t.sol | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/contracts-communication/test/InterchainClientV1Test.t.sol b/packages/contracts-communication/test/InterchainClientV1Test.t.sol index f903898627..5742065ee7 100644 --- a/packages/contracts-communication/test/InterchainClientV1Test.t.sol +++ b/packages/contracts-communication/test/InterchainClientV1Test.t.sol @@ -1,13 +1,14 @@ +// SPDX-License-Identifier: MIT pragma solidity 0.8.20; -import "forge-std/Test.sol"; import {InterchainClientV1} from "../contracts/InterchainClientV1.sol"; -import "../contracts/InterchainDB.sol"; -import {InterchainAppMock} from "./mocks/InterchainAppMock.sol"; +import {InterchainDB} from "../contracts/InterchainDB.sol"; +import {InterchainEntry} from "../contracts/libs/InterchainEntry.sol"; +import {InterchainAppMock} from "./mocks/InterchainAppMock.sol"; import {InterchainModuleMock} from "./mocks/InterchainModuleMock.sol"; -import {InterchainEntry} from "../contracts/libs/InterchainEntry.sol"; +import {Test} from "forge-std/Test.sol"; contract InterchainClientV1Test is Test { InterchainClientV1 icClient; @@ -53,7 +54,9 @@ contract InterchainClientV1Test is Test { icClient.convertAddressToBytes32(msg.sender), block.chainid, receiver, DST_CHAIN_ID, message, nonce ) ); - icClient.interchainSend{value: 1}(receiver, DST_CHAIN_ID, message, srcModules); + icClient.interchainSend{value: totalModuleFees}(receiver, DST_CHAIN_ID, message, srcModules); + // TODO: should check transactionID + transactionID; } function test_interchainReceive() public { From da01a1e471a5b66f895d66c4271a6852077a0b4e Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:29:07 +0000 Subject: [PATCH 28/32] Chore: renamoooor --- .../contracts/events/SynapseModuleEvents.sol | 10 ++++--- .../events/ThresholdECDSAModuleEvents.sol | 11 ------- ...holdECDSAModule.sol => ISynapseModule.sol} | 4 +-- ...sholdECDSAModule.sol => SynapseModule.sol} | 30 +++++++++---------- ....t.sol => SynapseModule.Destination.t.sol} | 16 ++++------ ...t.t.sol => SynapseModule.Management.t.sol} | 14 ++++----- ...ource.t.sol => SynapseModule.Source.t.sol} | 14 ++++----- 7 files changed, 40 insertions(+), 59 deletions(-) delete mode 100644 packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol rename packages/contracts-communication/contracts/interfaces/{IThresholdECDSAModule.sol => ISynapseModule.sol} (96%) rename packages/contracts-communication/contracts/modules/{ThresholdECDSAModule.sol => SynapseModule.sol} (87%) rename packages/contracts-communication/test/modules/{ThresholdECDSAModule.Destination.t.sol => SynapseModule.Destination.t.sol} (96%) rename packages/contracts-communication/test/modules/{ThresholdECDSAModule.Management.t.sol => SynapseModule.Management.t.sol} (91%) rename packages/contracts-communication/test/modules/{ThresholdECDSAModule.Source.t.sol => SynapseModule.Source.t.sol} (91%) diff --git a/packages/contracts-communication/contracts/events/SynapseModuleEvents.sol b/packages/contracts-communication/contracts/events/SynapseModuleEvents.sol index 081e86a57c..b08d487d88 100644 --- a/packages/contracts-communication/contracts/events/SynapseModuleEvents.sol +++ b/packages/contracts-communication/contracts/events/SynapseModuleEvents.sol @@ -1,9 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.0; -import {InterchainEntry} from "../libs/InterchainEntry.sol"; - abstract contract SynapseModuleEvents { - event VerificationRequested(uint256 indexed destChainId, InterchainEntry entry, bytes32 signableEntryHash); - event EntryVerified(InterchainEntry entry, bytes32 signableEntryHash); + event VerifierAdded(address verifier); + event VerifierRemoved(address verifier); + event ThresholdChanged(uint256 threshold); + + event FeeCollectorChanged(address feeCollector); + event GasOracleChanged(address gasOracle); } diff --git a/packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol b/packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol deleted file mode 100644 index 8139351bd9..0000000000 --- a/packages/contracts-communication/contracts/events/ThresholdECDSAModuleEvents.sol +++ /dev/null @@ -1,11 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity ^0.8.0; - -abstract contract ThresholdECDSAModuleEvents { - event VerifierAdded(address verifier); - event VerifierRemoved(address verifier); - event ThresholdChanged(uint256 threshold); - - event FeeCollectorChanged(address feeCollector); - event GasOracleChanged(address gasOracle); -} diff --git a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol b/packages/contracts-communication/contracts/interfaces/ISynapseModule.sol similarity index 96% rename from packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol rename to packages/contracts-communication/contracts/interfaces/ISynapseModule.sol index c00a5a3c6b..7b3605f1e7 100644 --- a/packages/contracts-communication/contracts/interfaces/IThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/interfaces/ISynapseModule.sol @@ -3,8 +3,8 @@ pragma solidity ^0.8.0; import {IInterchainModule} from "./IInterchainModule.sol"; -interface IThresholdECDSAModule is IInterchainModule { - error ThresholdECDSAModule__GasOracleNotContract(address gasOracle); +interface ISynapseModule is IInterchainModule { + error SynapseModule__GasOracleNotContract(address gasOracle); // ═══════════════════════════════════════════════ PERMISSIONED ════════════════════════════════════════════════════ diff --git a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol b/packages/contracts-communication/contracts/modules/SynapseModule.sol similarity index 87% rename from packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol rename to packages/contracts-communication/contracts/modules/SynapseModule.sol index cdeebebac2..0d57266496 100644 --- a/packages/contracts-communication/contracts/modules/ThresholdECDSAModule.sol +++ b/packages/contracts-communication/contracts/modules/SynapseModule.sol @@ -3,9 +3,9 @@ pragma solidity 0.8.20; import {InterchainModule} from "./InterchainModule.sol"; -import {ThresholdECDSAModuleEvents} from "../events/ThresholdECDSAModuleEvents.sol"; +import {SynapseModuleEvents} from "../events/SynapseModuleEvents.sol"; import {IGasOracle} from "../interfaces/IGasOracle.sol"; -import {IThresholdECDSAModule} from "../interfaces/IThresholdECDSAModule.sol"; +import {ISynapseModule} from "../interfaces/ISynapseModule.sol"; import {InterchainEntry} from "../libs/InterchainEntry.sol"; import {ThresholdECDSA} from "../libs/ThresholdECDSA.sol"; @@ -14,16 +14,16 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; -contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModuleEvents, IThresholdECDSAModule { +contract SynapseModule is InterchainModule, Ownable, SynapseModuleEvents, ISynapseModule { uint256 public constant VERIFY_GAS_LIMIT = 100_000; /// @dev Struct to hold the verifiers and the threshold for the module. ThresholdECDSA internal _verifiers; - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule address public feeCollector; - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule address public gasOracle; constructor(address interchainDB, address initialOwner) InterchainModule(interchainDB) Ownable(initialOwner) { @@ -32,32 +32,32 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule // ═══════════════════════════════════════════════ PERMISSIONED ════════════════════════════════════════════════════ - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule function addVerifier(address verifier) external onlyOwner { _verifiers.addSigner(verifier); emit VerifierAdded(verifier); } - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule function removeVerifier(address verifier) external onlyOwner { _verifiers.removeSigner(verifier); emit VerifierRemoved(verifier); } - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule function setThreshold(uint256 threshold) external onlyOwner { _setThreshold(threshold); } - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule function setFeeCollector(address feeCollector_) external onlyOwner { _setFeeCollector(feeCollector_); } - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule function setGasOracle(address gasOracle_) external onlyOwner { if (gasOracle_.code.length == 0) { - revert ThresholdECDSAModule__GasOracleNotContract(gasOracle_); + revert SynapseModule__GasOracleNotContract(gasOracle_); } gasOracle = gasOracle_; emit GasOracleChanged(gasOracle_); @@ -65,7 +65,7 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule function verifyEntry(bytes calldata encodedEntry, bytes calldata signatures) external { bytes32 ethSignedHash = MessageHashUtils.toEthSignedMessageHash(keccak256(encodedEntry)); _verifiers.verifySignedHash(ethSignedHash, signatures); @@ -74,17 +74,17 @@ contract ThresholdECDSAModule is InterchainModule, Ownable, ThresholdECDSAModule // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule function getVerifiers() external view returns (address[] memory) { return _verifiers.getSigners(); } - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule function isVerifier(address account) external view returns (bool) { return _verifiers.isSigner(account); } - /// @inheritdoc IThresholdECDSAModule + /// @inheritdoc ISynapseModule function getThreshold() public view returns (uint256) { return _verifiers.getThreshold(); } diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Destination.t.sol b/packages/contracts-communication/test/modules/SynapseModule.Destination.t.sol similarity index 96% rename from packages/contracts-communication/test/modules/ThresholdECDSAModule.Destination.t.sol rename to packages/contracts-communication/test/modules/SynapseModule.Destination.t.sol index c48400fd80..f30ccfbfe7 100644 --- a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Destination.t.sol +++ b/packages/contracts-communication/test/modules/SynapseModule.Destination.t.sol @@ -2,22 +2,18 @@ pragma solidity 0.8.20; import {InterchainModuleEvents} from "../../contracts/events/InterchainModuleEvents.sol"; -import {ThresholdECDSAModuleEvents} from "../../contracts/events/ThresholdECDSAModuleEvents.sol"; +import {SynapseModuleEvents} from "../../contracts/events/SynapseModuleEvents.sol"; import {IInterchainModule} from "../../contracts/interfaces/IInterchainModule.sol"; import {ThresholdECDSALib} from "../../contracts/libs/ThresholdECDSA.sol"; -import { - ThresholdECDSAModule, - InterchainEntry, - IThresholdECDSAModule -} from "../../contracts/modules/ThresholdECDSAModule.sol"; +import {SynapseModule, InterchainEntry, ISynapseModule} from "../../contracts/modules/SynapseModule.sol"; import {GasOracleMock} from "../mocks/GasOracleMock.sol"; import {InterchainDBMock, IInterchainDB} from "../mocks/InterchainDBMock.sol"; import {Test} from "forge-std/Test.sol"; -contract ThresholdECDSAModuleDestinationTest is Test, InterchainModuleEvents, ThresholdECDSAModuleEvents { - ThresholdECDSAModule public module; +contract SynapseModuleDestinationTest is Test, InterchainModuleEvents, SynapseModuleEvents { + SynapseModule public module; GasOracleMock public gasOracle; InterchainDBMock public interchainDB; @@ -49,7 +45,7 @@ contract ThresholdECDSAModuleDestinationTest is Test, InterchainModuleEvents, Th function setUp() public { vm.chainId(DST_CHAIN_ID); interchainDB = new InterchainDBMock(); - module = new ThresholdECDSAModule(address(interchainDB), owner); + module = new SynapseModule(address(interchainDB), owner); gasOracle = new GasOracleMock(); vm.startPrank(owner); module.setGasOracle(address(gasOracle)); @@ -296,7 +292,7 @@ contract ThresholdECDSAModuleDestinationTest is Test, InterchainModuleEvents, Th function test_verifyEntry_revertZeroThreshold() public { // Deploy a module without setting up the threshold - module = new ThresholdECDSAModule(address(interchainDB), owner); + module = new SynapseModule(address(interchainDB), owner); vm.startPrank(owner); module.addVerifier(SIGNER_0); module.addVerifier(SIGNER_1); diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol b/packages/contracts-communication/test/modules/SynapseModule.Management.t.sol similarity index 91% rename from packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol rename to packages/contracts-communication/test/modules/SynapseModule.Management.t.sol index d991fe65df..b07db40423 100644 --- a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Management.t.sol +++ b/packages/contracts-communication/test/modules/SynapseModule.Management.t.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.20; -import {ThresholdECDSAModuleEvents} from "../../contracts/events/ThresholdECDSAModuleEvents.sol"; -import {ThresholdECDSAModule, IThresholdECDSAModule} from "../../contracts/modules/ThresholdECDSAModule.sol"; +import {SynapseModuleEvents} from "../../contracts/events/SynapseModuleEvents.sol"; +import {SynapseModule, ISynapseModule} from "../../contracts/modules/SynapseModule.sol"; import {ThresholdECDSALib} from "../../contracts/libs/ThresholdECDSA.sol"; import {GasOracleMock} from "../mocks/GasOracleMock.sol"; @@ -11,8 +11,8 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import {Test} from "forge-std/Test.sol"; -contract ThresholdECDSAModuleManagementTest is Test, ThresholdECDSAModuleEvents { - ThresholdECDSAModule public module; +contract SynapseModuleManagementTest is Test, SynapseModuleEvents { + SynapseModule public module; GasOracleMock public gasOracle; address public interchainDB = makeAddr("InterchainDB"); @@ -24,7 +24,7 @@ contract ThresholdECDSAModuleManagementTest is Test, ThresholdECDSAModuleEvents address public constant VERIFIER_3 = address(3); function setUp() public { - module = new ThresholdECDSAModule(interchainDB, owner); + module = new SynapseModule(interchainDB, owner); gasOracle = new GasOracleMock(); } @@ -181,9 +181,7 @@ contract ThresholdECDSAModuleManagementTest is Test, ThresholdECDSAModuleEvents // Sanity check require(notContract.code.length == 0); vm.expectRevert( - abi.encodeWithSelector( - IThresholdECDSAModule.ThresholdECDSAModule__GasOracleNotContract.selector, notContract - ) + abi.encodeWithSelector(ISynapseModule.SynapseModule__GasOracleNotContract.selector, notContract) ); vm.prank(owner); module.setGasOracle(notContract); diff --git a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol b/packages/contracts-communication/test/modules/SynapseModule.Source.t.sol similarity index 91% rename from packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol rename to packages/contracts-communication/test/modules/SynapseModule.Source.t.sol index 1450876f10..e1b6fbd6ee 100644 --- a/packages/contracts-communication/test/modules/ThresholdECDSAModule.Source.t.sol +++ b/packages/contracts-communication/test/modules/SynapseModule.Source.t.sol @@ -2,20 +2,16 @@ pragma solidity 0.8.20; import {InterchainModuleEvents} from "../../contracts/events/InterchainModuleEvents.sol"; -import {ThresholdECDSAModuleEvents} from "../../contracts/events/ThresholdECDSAModuleEvents.sol"; +import {SynapseModuleEvents} from "../../contracts/events/SynapseModuleEvents.sol"; import {IInterchainModule} from "../../contracts/interfaces/IInterchainModule.sol"; -import { - ThresholdECDSAModule, - InterchainEntry, - IThresholdECDSAModule -} from "../../contracts/modules/ThresholdECDSAModule.sol"; +import {SynapseModule, InterchainEntry, ISynapseModule} from "../../contracts/modules/SynapseModule.sol"; import {GasOracleMock} from "../mocks/GasOracleMock.sol"; import {Test} from "forge-std/Test.sol"; -contract ThresholdECDSAModuleSourceTest is Test, InterchainModuleEvents, ThresholdECDSAModuleEvents { - ThresholdECDSAModule public module; +contract SynapseModuleSourceTest is Test, InterchainModuleEvents, SynapseModuleEvents { + SynapseModule public module; GasOracleMock public gasOracle; address public interchainDB = makeAddr("InterchainDB"); @@ -39,7 +35,7 @@ contract ThresholdECDSAModuleSourceTest is Test, InterchainModuleEvents, Thresho function setUp() public { vm.chainId(SRC_CHAIN_ID); - module = new ThresholdECDSAModule(interchainDB, owner); + module = new SynapseModule(interchainDB, owner); gasOracle = new GasOracleMock(); vm.startPrank(owner); module.setGasOracle(address(gasOracle)); From 268d32ea2ddc740a1e098cbc43a20db2ddd46e4a Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:29:56 +0000 Subject: [PATCH 29/32] Revert "Fix ClientV1 test" This reverts commit e724c38c6f282b39a553fd94035b65bbd671a476. --- .../test/InterchainClientV1Test.t.sol | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/contracts-communication/test/InterchainClientV1Test.t.sol b/packages/contracts-communication/test/InterchainClientV1Test.t.sol index 5742065ee7..75935821f0 100644 --- a/packages/contracts-communication/test/InterchainClientV1Test.t.sol +++ b/packages/contracts-communication/test/InterchainClientV1Test.t.sol @@ -7,12 +7,14 @@ import {InterchainEntry} from "../contracts/libs/InterchainEntry.sol"; import {InterchainAppMock} from "./mocks/InterchainAppMock.sol"; import {InterchainModuleMock} from "./mocks/InterchainModuleMock.sol"; +import "../contracts/modules/SynapseModule.sol"; import {Test} from "forge-std/Test.sol"; contract InterchainClientV1Test is Test { InterchainClientV1 icClient; InterchainDB icDB; + SynapseModule synapseModule; InterchainAppMock icApp; InterchainModuleMock icModule; @@ -25,28 +27,21 @@ contract InterchainClientV1Test is Test { vm.startPrank(contractOwner); icClient = new InterchainClientV1(); icDB = new InterchainDB(); + synapseModule = new SynapseModule(); + synapseModule.setInterchainDB(address(icDB)); icClient.setInterchainDB(address(icDB)); icModule = new InterchainModuleMock(); icApp = new InterchainAppMock(); icApp.setReceivingModule(address(icModule)); vm.stopPrank(); - // icModule should return 1 for the module fee - mockModuleFee(icModule, 1); - } - - /// @dev Mocks a return value of module.getModuleFee(DST_CHAIN_ID) - function mockModuleFee(InterchainModuleMock module, uint256 feeValue) internal { - bytes memory callData = abi.encodeCall(module.getModuleFee, (DST_CHAIN_ID)); - bytes memory returnData = abi.encode(feeValue); - vm.mockCall(address(module), callData, returnData); } function test_interchainSend() public { bytes32 receiver = icClient.convertAddressToBytes32(makeAddr("Receiver")); bytes memory message = "Hello World"; address[] memory srcModules = new address[](1); - srcModules[0] = address(icModule); + srcModules[0] = address(synapseModule); uint256 totalModuleFees = 1; uint64 nonce = 1; bytes32 transactionID = keccak256( From 553ff99e690c3cc6206e9f96b4d97b26dc5583d1 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:31:02 +0000 Subject: [PATCH 30/32] Revert "Chore: cleanup" This reverts commit 41949cde6acbe76fc1fa52e415b2102fd1be9652. --- .../test/InterchainClientV1Test.t.sol | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/contracts-communication/test/InterchainClientV1Test.t.sol b/packages/contracts-communication/test/InterchainClientV1Test.t.sol index 75935821f0..4fca87b883 100644 --- a/packages/contracts-communication/test/InterchainClientV1Test.t.sol +++ b/packages/contracts-communication/test/InterchainClientV1Test.t.sol @@ -1,15 +1,14 @@ -// SPDX-License-Identifier: MIT pragma solidity 0.8.20; +import "forge-std/Test.sol"; import {InterchainClientV1} from "../contracts/InterchainClientV1.sol"; -import {InterchainDB} from "../contracts/InterchainDB.sol"; -import {InterchainEntry} from "../contracts/libs/InterchainEntry.sol"; - +import "../contracts/InterchainDB.sol"; import {InterchainAppMock} from "./mocks/InterchainAppMock.sol"; + import {InterchainModuleMock} from "./mocks/InterchainModuleMock.sol"; import "../contracts/modules/SynapseModule.sol"; -import {Test} from "forge-std/Test.sol"; +import {InterchainEntry} from "../contracts/libs/InterchainEntry.sol"; contract InterchainClientV1Test is Test { InterchainClientV1 icClient; @@ -49,9 +48,7 @@ contract InterchainClientV1Test is Test { icClient.convertAddressToBytes32(msg.sender), block.chainid, receiver, DST_CHAIN_ID, message, nonce ) ); - icClient.interchainSend{value: totalModuleFees}(receiver, DST_CHAIN_ID, message, srcModules); - // TODO: should check transactionID - transactionID; + icClient.interchainSend{value: 1}(receiver, DST_CHAIN_ID, message, srcModules); } function test_interchainReceive() public { From 0192d5607c7135fbf29bf93ad7b8ead43d7e888a Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:33:13 +0000 Subject: [PATCH 31/32] Fix ClientV1 test --- .../test/InterchainClientV1Test.t.sol | 40 ++++++++++++------- 1 file changed, 25 insertions(+), 15 deletions(-) diff --git a/packages/contracts-communication/test/InterchainClientV1Test.t.sol b/packages/contracts-communication/test/InterchainClientV1Test.t.sol index 3cf4d851b6..b6fdca9a82 100644 --- a/packages/contracts-communication/test/InterchainClientV1Test.t.sol +++ b/packages/contracts-communication/test/InterchainClientV1Test.t.sol @@ -6,25 +6,22 @@ import "../contracts/InterchainDB.sol"; import {InterchainAppMock} from "./mocks/InterchainAppMock.sol"; import {InterchainModuleMock} from "./mocks/InterchainModuleMock.sol"; -import "../contracts/modules/SynapseModule.sol"; - import {InterchainEntry} from "../contracts/libs/InterchainEntry.sol"; import {TypeCasts} from "../contracts/libs/TypeCasts.sol"; import {OptionsLib} from "../contracts/libs/Options.sol"; -import { InterchainClientV1Harness } from "./harnesses/InterchainClientV1Harness.sol"; +import {InterchainClientV1Harness} from "./harnesses/InterchainClientV1Harness.sol"; contract InterchainClientV1Test is Test { InterchainClientV1Harness icClient; InterchainDB icDB; - SynapseModule synapseModule; InterchainAppMock icApp; InterchainModuleMock icModule; // Use default options of V1, 200k gas limit, 0 gas airdrop - bytes options = OptionsLib.encodeOptions(OptionsLib.Options(1, 200000, 0)); + bytes options = OptionsLib.encodeOptions(OptionsLib.Options(1, 200_000, 0)); uint256 public constant SRC_CHAIN_ID = 1337; uint256 public constant DST_CHAIN_ID = 7331; @@ -35,26 +32,41 @@ contract InterchainClientV1Test is Test { vm.startPrank(contractOwner); icClient = new InterchainClientV1Harness(); icDB = new InterchainDB(); - synapseModule = new SynapseModule(); - synapseModule.setInterchainDB(address(icDB)); icClient.setInterchainDB(address(icDB)); icModule = new InterchainModuleMock(); icApp = new InterchainAppMock(); icApp.setReceivingModule(address(icModule)); vm.stopPrank(); + mockModuleFee(icModule, 1); + } + + /// @dev Mocks a return value of module.getModuleFee(DST_CHAIN_ID) + function mockModuleFee(InterchainModuleMock module, uint256 feeValue) internal { + bytes memory callData = abi.encodeCall(module.getModuleFee, (DST_CHAIN_ID)); + bytes memory returnData = abi.encode(feeValue); + vm.mockCall(address(module), callData, returnData); } + // ══════════════════════════════════════════════ INTERNAL TESTS ══════════════════════════════════════════════════ - function test_generateTxId(bytes32 srcSender, + function test_generateTxId( + bytes32 srcSender, uint256 srcChainId, bytes32 dstReceiver, uint256 dstChainId, bytes memory message, uint64 nonce, - bytes memory fuzzOptions) public { - bytes32 txId = icClient.generateTransactionIdHarness(srcSender, srcChainId, dstReceiver, dstChainId, message, nonce, fuzzOptions); - assertEq(txId, keccak256(abi.encode(srcSender, srcChainId, dstReceiver, dstChainId, message, nonce, fuzzOptions))); + bytes memory fuzzOptions + ) + public + { + bytes32 txId = icClient.generateTransactionIdHarness( + srcSender, srcChainId, dstReceiver, dstChainId, message, nonce, fuzzOptions + ); + assertEq( + txId, keccak256(abi.encode(srcSender, srcChainId, dstReceiver, dstChainId, message, nonce, fuzzOptions)) + ); } function test_getFinalizedResponsesCount() public { @@ -91,13 +103,11 @@ contract InterchainClientV1Test is Test { bytes32 receiver = TypeCasts.addressToBytes32(makeAddr("Receiver")); bytes memory message = "Hello World"; address[] memory srcModules = new address[](1); - srcModules[0] = address(synapseModule); + srcModules[0] = address(icModule); uint256 totalModuleFees = 1; uint64 nonce = 1; bytes32 transactionID = keccak256( - abi.encode( - TypeCasts.addressToBytes32(msg.sender), block.chainid, receiver, DST_CHAIN_ID, message, nonce - ) + abi.encode(TypeCasts.addressToBytes32(msg.sender), block.chainid, receiver, DST_CHAIN_ID, message, nonce) ); icClient.interchainSend{value: 1}(receiver, DST_CHAIN_ID, message, options, srcModules); } From 980d1ea4b90566388f7d71cfaea1d28325ec5968 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 19 Feb 2024 12:34:41 +0000 Subject: [PATCH 32/32] Chore: cleanup --- .../test/InterchainClientV1Test.t.sol | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/contracts-communication/test/InterchainClientV1Test.t.sol b/packages/contracts-communication/test/InterchainClientV1Test.t.sol index b6fdca9a82..f9751bb624 100644 --- a/packages/contracts-communication/test/InterchainClientV1Test.t.sol +++ b/packages/contracts-communication/test/InterchainClientV1Test.t.sol @@ -1,19 +1,20 @@ +// SPDX-License-Identifier: MIT pragma solidity 0.8.20; -import "forge-std/Test.sol"; import {InterchainClientV1} from "../contracts/InterchainClientV1.sol"; -import "../contracts/InterchainDB.sol"; -import {InterchainAppMock} from "./mocks/InterchainAppMock.sol"; +import {InterchainDB} from "../contracts/InterchainDB.sol"; -import {InterchainModuleMock} from "./mocks/InterchainModuleMock.sol"; import {InterchainEntry} from "../contracts/libs/InterchainEntry.sol"; - -import {TypeCasts} from "../contracts/libs/TypeCasts.sol"; - import {OptionsLib} from "../contracts/libs/Options.sol"; +import {TypeCasts} from "../contracts/libs/TypeCasts.sol"; import {InterchainClientV1Harness} from "./harnesses/InterchainClientV1Harness.sol"; +import {InterchainAppMock} from "./mocks/InterchainAppMock.sol"; +import {InterchainModuleMock} from "./mocks/InterchainModuleMock.sol"; + +import {Test} from "forge-std/Test.sol"; + contract InterchainClientV1Test is Test { InterchainClientV1Harness icClient; InterchainDB icDB; @@ -109,7 +110,9 @@ contract InterchainClientV1Test is Test { bytes32 transactionID = keccak256( abi.encode(TypeCasts.addressToBytes32(msg.sender), block.chainid, receiver, DST_CHAIN_ID, message, nonce) ); - icClient.interchainSend{value: 1}(receiver, DST_CHAIN_ID, message, options, srcModules); + icClient.interchainSend{value: totalModuleFees}(receiver, DST_CHAIN_ID, message, options, srcModules); + // TODO: should check the transaction ID? + transactionID; } function test_interchainReceive() public {