diff --git a/packages/contracts-communication/contracts/events/SynapseModuleEvents.sol b/packages/contracts-communication/contracts/events/SynapseModuleEvents.sol index b08d487d88..5defbe3fba 100644 --- a/packages/contracts-communication/contracts/events/SynapseModuleEvents.sol +++ b/packages/contracts-communication/contracts/events/SynapseModuleEvents.sol @@ -8,4 +8,8 @@ abstract contract SynapseModuleEvents { event FeeCollectorChanged(address feeCollector); event GasOracleChanged(address gasOracle); + event VerifyGasLimitChanged(uint256 chainId, uint256 gasLimit); + + event ClaimFeeFractionChanged(uint256 claimFeeFraction); + event FeesClaimed(address feeCollector, uint256 collectedFees, address claimer, uint256 claimerFee); } diff --git a/packages/contracts-communication/contracts/interfaces/ISynapseModule.sol b/packages/contracts-communication/contracts/interfaces/ISynapseModule.sol index 7b3605f1e7..02ba033590 100644 --- a/packages/contracts-communication/contracts/interfaces/ISynapseModule.sol +++ b/packages/contracts-communication/contracts/interfaces/ISynapseModule.sol @@ -4,7 +4,10 @@ pragma solidity ^0.8.0; import {IInterchainModule} from "./IInterchainModule.sol"; interface ISynapseModule is IInterchainModule { + error SynapseModule__ClaimFeeFractionExceedsMax(uint256 claimFeeFraction); + error SynapseModule__FeeCollectorNotSet(); error SynapseModule__GasOracleNotContract(address gasOracle); + error SynapseModule__NoFeesToClaim(); // ═══════════════════════════════════════════════ PERMISSIONED ════════════════════════════════════════════════════ @@ -13,11 +16,21 @@ interface ISynapseModule is IInterchainModule { /// @param verifier The address of the verifier to add function addVerifier(address verifier) external; + /// @notice Adds a list of new verifiers to the module. + /// @dev Could be only called by the owner. Will revert if any of the verifiers is already added. + /// @param verifiers The list of addresses of the verifiers to add + function addVerifiers(address[] calldata verifiers) 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 Removes a list of verifiers from the module. + /// @dev Could be only called by the owner. Will revert if any of the verifiers is not added. + /// @param verifiers The list of addresses of the verifiers to remove + function removeVerifiers(address[] calldata verifiers) 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 @@ -28,13 +41,32 @@ interface ISynapseModule is IInterchainModule { /// @param feeCollector_ The address of the fee collector function setFeeCollector(address feeCollector_) external; + /// @notice Sets the fraction of the accumulated fees to be paid to caller of `claimFees`. + /// This encourages rational actors to call the function as soon as claim fee is higher than the gas cost. + /// @dev Could be only called by the owner. Could not exceed 1%. + /// @param claimFeeFraction The fraction of the fees to be paid to the claimer (100% = 1e18) + function setClaimFeeFraction(uint256 claimFeeFraction) 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; + /// @notice Sets the estimated gas limit for verifying an entry on the given chain. + /// @dev Could be only called by the owner. + /// @param chainId The chain ID for which to set the gas limit + /// @param gasLimit The new gas limit + function setVerifyGasLimit(uint256 chainId, uint256 gasLimit) external; + // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ + /// @notice Transfers the accumulated fees to the fee collector. + /// Message caller receives a percentage of the fees, this ensures that the module is self-sustainable. + /// The claim fee amount could be retrieved using `getClaimFeeAmount`. The rest of the fees + /// will be transferred to the fee collector. + /// @dev Will revert if the fee collector is not set. + function claimFees() external; + /// @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. @@ -47,6 +79,13 @@ interface ISynapseModule is IInterchainModule { /// @notice Returns the address of the fee collector for the module. function feeCollector() external view returns (address); + /// @notice Returns the current claim fee to be paid to the caller of `claimFees`. + function getClaimFeeAmount() external view returns (uint256); + + /// @notice Returns the fraction of the fees to be paid to the caller of `claimFees`. + /// The returned value is in the range [0, 1e18], where 1e18 corresponds to 100% of the fees. + function getClaimFeeFraction() external view returns (uint256); + /// @notice Returns the address of the gas oracle used for estimating the verification fees. function gasOracle() external view returns (address); @@ -59,4 +98,8 @@ interface ISynapseModule is IInterchainModule { /// @notice Checks if the given account is a verifier for the module. function isVerifier(address account) external view returns (bool); + + /// @notice Returns the estimated gas limit for verifying an entry on the given chain. + /// Note: this defaults to DEFAULT_VERIFY_GAS_LIMIT if not set. + function getVerifyGasLimit(uint256 chainId) external view returns (uint256); } diff --git a/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol b/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol index 91ca5d3979..bdf14be94c 100644 --- a/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol +++ b/packages/contracts-communication/contracts/libs/ThresholdECDSA.sol @@ -11,22 +11,25 @@ struct ThresholdECDSA { using ThresholdECDSALib for ThresholdECDSA global; +// solhint-disable code-complexity library ThresholdECDSALib { using EnumerableSet for EnumerableSet.AddressSet; + uint256 private constant SIGNATURE_LENGTH = 65; + 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); error ThresholdECDSA__RecoveredSignersNotSorted(); + error ThresholdECDSA__ZeroAddress(); 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 { + if (account == address(0)) revert ThresholdECDSA__ZeroAddress(); bool added = self._signers.add(account); if (!added) { revert ThresholdECDSA__AlreadySigner(account); diff --git a/packages/contracts-communication/contracts/modules/InterchainModule.sol b/packages/contracts-communication/contracts/modules/InterchainModule.sol index 6a0a989c63..bf93677d9e 100644 --- a/packages/contracts-communication/contracts/modules/InterchainModule.sol +++ b/packages/contracts-communication/contracts/modules/InterchainModule.sol @@ -38,10 +38,6 @@ abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule emit VerificationRequested(destChainId, encodedEntry, ethSignedEntryHash); } - function decodeEntry(bytes memory entry) external pure returns (InterchainEntry memory) { - return abi.decode(entry, (InterchainEntry)); - } - /// @inheritdoc IInterchainModule function getModuleFee(uint256 destChainId) external view returns (uint256) { return _getModuleFee(destChainId); @@ -61,7 +57,8 @@ abstract contract InterchainModule is InterchainModuleEvents, IInterchainModule } /// @dev Internal logic to request the verification of an entry on the destination chain. - function _requestVerification(uint256 destChainId, bytes memory encodedEntry) internal virtual; + // solhint-disable-next-line no-empty-blocks + 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); diff --git a/packages/contracts-communication/contracts/modules/SynapseModule.sol b/packages/contracts-communication/contracts/modules/SynapseModule.sol index 0d57266496..3ce3c73c2c 100644 --- a/packages/contracts-communication/contracts/modules/SynapseModule.sol +++ b/packages/contracts-communication/contracts/modules/SynapseModule.sol @@ -7,7 +7,6 @@ import {SynapseModuleEvents} from "../events/SynapseModuleEvents.sol"; import {IGasOracle} from "../interfaces/IGasOracle.sol"; import {ISynapseModule} from "../interfaces/ISynapseModule.sol"; -import {InterchainEntry} from "../libs/InterchainEntry.sol"; import {ThresholdECDSA} from "../libs/ThresholdECDSA.sol"; import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; @@ -15,14 +14,21 @@ import {Address} from "@openzeppelin/contracts/utils/Address.sol"; import {MessageHashUtils} from "@openzeppelin/contracts/utils/cryptography/MessageHashUtils.sol"; contract SynapseModule is InterchainModule, Ownable, SynapseModuleEvents, ISynapseModule { - uint256 public constant VERIFY_GAS_LIMIT = 100_000; + // TODO: make sure this is a good enough default value + uint256 public constant DEFAULT_VERIFY_GAS_LIMIT = 100_000; + + uint256 internal constant MAX_CLAIM_FEE_FRACTION = 0.01e18; // 1% + uint256 internal constant FEE_PRECISION = 1e18; /// @dev Struct to hold the verifiers and the threshold for the module. ThresholdECDSA internal _verifiers; + /// @dev Claim fee fraction, 100% = 1e18 + uint256 internal _claimFeeFraction; + /// @dev Gas limit for the verifyEntry function on the remote chain. + mapping(uint256 chainId => uint256 gasLimit) internal _verifyGasLimit; /// @inheritdoc ISynapseModule address public feeCollector; - /// @inheritdoc ISynapseModule address public gasOracle; @@ -34,24 +40,49 @@ contract SynapseModule is InterchainModule, Ownable, SynapseModuleEvents, ISynap /// @inheritdoc ISynapseModule function addVerifier(address verifier) external onlyOwner { - _verifiers.addSigner(verifier); - emit VerifierAdded(verifier); + _addVerifier(verifier); + } + + /// @inheritdoc ISynapseModule + function addVerifiers(address[] calldata verifiers) external onlyOwner { + uint256 length = verifiers.length; + for (uint256 i = 0; i < length; ++i) { + _addVerifier(verifiers[i]); + } } /// @inheritdoc ISynapseModule function removeVerifier(address verifier) external onlyOwner { - _verifiers.removeSigner(verifier); - emit VerifierRemoved(verifier); + _removeVerifier(verifier); + } + + /// @inheritdoc ISynapseModule + function removeVerifiers(address[] calldata verifiers) external onlyOwner { + uint256 length = verifiers.length; + for (uint256 i = 0; i < length; ++i) { + _removeVerifier(verifiers[i]); + } } /// @inheritdoc ISynapseModule function setThreshold(uint256 threshold) external onlyOwner { - _setThreshold(threshold); + _verifiers.modifyThreshold(threshold); + emit ThresholdChanged(threshold); } /// @inheritdoc ISynapseModule function setFeeCollector(address feeCollector_) external onlyOwner { - _setFeeCollector(feeCollector_); + feeCollector = feeCollector_; + emit FeeCollectorChanged(feeCollector_); + } + + /// @inheritdoc ISynapseModule + function setClaimFeeFraction(uint256 claimFeeFraction) external onlyOwner { + if (claimFeeFraction > MAX_CLAIM_FEE_FRACTION) { + revert SynapseModule__ClaimFeeFractionExceedsMax(claimFeeFraction); + } + _claimFeeFraction = claimFeeFraction; + emit ClaimFeeFractionChanged(claimFeeFraction); } /// @inheritdoc ISynapseModule @@ -63,8 +94,29 @@ contract SynapseModule is InterchainModule, Ownable, SynapseModuleEvents, ISynap emit GasOracleChanged(gasOracle_); } + /// @inheritdoc ISynapseModule + function setVerifyGasLimit(uint256 chainId, uint256 gasLimit) external onlyOwner { + _verifyGasLimit[chainId] = gasLimit; + emit VerifyGasLimitChanged(chainId, gasLimit); + } + // ══════════════════════════════════════════════ PERMISSIONLESS ═══════════════════════════════════════════════════ + /// @inheritdoc ISynapseModule + function claimFees() external { + if (feeCollector == address(0)) { + revert SynapseModule__FeeCollectorNotSet(); + } + if (address(this).balance == 0) { + revert SynapseModule__NoFeesToClaim(); + } + uint256 claimFee = getClaimFeeAmount(); + uint256 collectedFee = address(this).balance - claimFee; + Address.sendValue(payable(feeCollector), collectedFee); + Address.sendValue(payable(msg.sender), claimFee); + emit FeesClaimed(feeCollector, collectedFee, msg.sender, claimFee); + } + /// @inheritdoc ISynapseModule function verifyEntry(bytes calldata encodedEntry, bytes calldata signatures) external { bytes32 ethSignedHash = MessageHashUtils.toEthSignedMessageHash(keccak256(encodedEntry)); @@ -74,6 +126,11 @@ contract SynapseModule is InterchainModule, Ownable, SynapseModuleEvents, ISynap // ═══════════════════════════════════════════════════ VIEWS ═══════════════════════════════════════════════════════ + /// @inheritdoc ISynapseModule + function getClaimFeeFraction() external view returns (uint256) { + return _claimFeeFraction; + } + /// @inheritdoc ISynapseModule function getVerifiers() external view returns (address[] memory) { return _verifiers.getSigners(); @@ -84,36 +141,36 @@ contract SynapseModule is InterchainModule, Ownable, SynapseModuleEvents, ISynap return _verifiers.isSigner(account); } + /// @inheritdoc ISynapseModule + function getClaimFeeAmount() public view returns (uint256) { + return address(this).balance * _claimFeeFraction / FEE_PRECISION; + } + /// @inheritdoc ISynapseModule 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. - function _setThreshold(uint256 threshold) internal { - _verifiers.modifyThreshold(threshold); - emit ThresholdChanged(threshold); + /// @inheritdoc ISynapseModule + function getVerifyGasLimit(uint256 chainId) public view override returns (uint256 gasLimit) { + gasLimit = _verifyGasLimit[chainId]; + if (gasLimit == 0) { + gasLimit = DEFAULT_VERIFY_GAS_LIMIT; + } } - /// @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_); + // ══════════════════════════════════════════════ INTERNAL LOGIC ═══════════════════════════════════════════════════ + + /// @dev Adds a verifier to the module. Permissions should be checked in the calling function. + function _addVerifier(address verifier) internal { + _verifiers.addSigner(verifier); + emit VerifierAdded(verifier); } - /// @dev Internal logic to request the verification of an entry on the destination chain. - 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); + /// @dev Removes a verifier from the module. Permissions should be checked in the calling function. + function _removeVerifier(address verifier) internal { + _verifiers.removeSigner(verifier); + emit VerifierRemoved(verifier); } // ══════════════════════════════════════════════ INTERNAL VIEWS ═══════════════════════════════════════════════════ @@ -128,7 +185,7 @@ contract SynapseModule is InterchainModule, Ownable, SynapseModuleEvents, ISynap // Total formula is: 4 + 32 (entry offset) + 32 (signatures offset) + 160 + 32 return IGasOracle(gasOracle).estimateTxCostInLocalUnits({ remoteChainId: destChainId, - gasLimit: VERIFY_GAS_LIMIT, + gasLimit: getVerifyGasLimit(destChainId), calldataSize: 292 + 64 * getThreshold() }); } diff --git a/packages/contracts-communication/test/libs/ThresholdECDSALib.t.sol b/packages/contracts-communication/test/libs/ThresholdECDSALib.t.sol index d09d1b9989..c0ac173c5b 100644 --- a/packages/contracts-communication/test/libs/ThresholdECDSALib.t.sol +++ b/packages/contracts-communication/test/libs/ThresholdECDSALib.t.sol @@ -5,6 +5,9 @@ import {ThresholdECDSALib, ThresholdECDSALibHarness} from "../harnesses/Threshol import {Test} from "forge-std/Test.sol"; +// solhint-disable func-name-mixedcase +// solhint-disable ordering +// solhint-disable var-name-mixedcase contract ThresholdECDSALibTest is Test { ThresholdECDSALibHarness public libHarness; @@ -70,6 +73,10 @@ contract ThresholdECDSALibTest is Test { vm.expectRevert(abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__RecoveredSignersNotSorted.selector)); } + function expectZeroAddressError() internal { + vm.expectRevert(ThresholdECDSALib.ThresholdECDSA__ZeroAddress.selector); + } + function expectZeroThresholdError() internal { vm.expectRevert(abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__ZeroThreshold.selector)); } @@ -194,6 +201,11 @@ contract ThresholdECDSALibTest is Test { libHarness.addSigner(SIGNER_2); } + function test_addSigner_revert_zeroAddress() public { + expectZeroAddressError(); + libHarness.addSigner(address(0)); + } + function test_removeSigner_revert_notSigner() public { expectNotSignerError(SIGNER_3); libHarness.removeSigner(SIGNER_3); diff --git a/packages/contracts-communication/test/modules/SynapseModule.Destination.t.sol b/packages/contracts-communication/test/modules/SynapseModule.Destination.t.sol index a20767042d..85c1700490 100644 --- a/packages/contracts-communication/test/modules/SynapseModule.Destination.t.sol +++ b/packages/contracts-communication/test/modules/SynapseModule.Destination.t.sol @@ -4,14 +4,17 @@ pragma solidity 0.8.20; import {InterchainModuleEvents} from "../../contracts/events/InterchainModuleEvents.sol"; import {SynapseModuleEvents} from "../../contracts/events/SynapseModuleEvents.sol"; import {IInterchainModule} from "../../contracts/interfaces/IInterchainModule.sol"; +import {InterchainEntry} from "../../contracts/libs/InterchainEntry.sol"; import {ThresholdECDSALib} from "../../contracts/libs/ThresholdECDSA.sol"; -import {SynapseModule, InterchainEntry, ISynapseModule} from "../../contracts/modules/SynapseModule.sol"; +import {SynapseModule} 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"; +// solhint-disable func-name-mixedcase +// solhint-disable ordering contract SynapseModuleDestinationTest is Test, InterchainModuleEvents, SynapseModuleEvents { SynapseModule public module; GasOracleMock public gasOracle; diff --git a/packages/contracts-communication/test/modules/SynapseModule.Management.t.sol b/packages/contracts-communication/test/modules/SynapseModule.Management.t.sol index b07db40423..7ab125ccdf 100644 --- a/packages/contracts-communication/test/modules/SynapseModule.Management.t.sol +++ b/packages/contracts-communication/test/modules/SynapseModule.Management.t.sol @@ -11,6 +11,8 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import {Test} from "forge-std/Test.sol"; +// solhint-disable func-name-mixedcase +// solhint-disable ordering contract SynapseModuleManagementTest is Test, SynapseModuleEvents { SynapseModule public module; GasOracleMock public gasOracle; @@ -23,9 +25,14 @@ contract SynapseModuleManagementTest is Test, SynapseModuleEvents { address public constant VERIFIER_2 = address(2); address public constant VERIFIER_3 = address(3); + address[] public allVerifiers; + function setUp() public { module = new SynapseModule(interchainDB, owner); gasOracle = new GasOracleMock(); + allVerifiers.push(VERIFIER_1); + allVerifiers.push(VERIFIER_2); + allVerifiers.push(VERIFIER_3); } function basicSetup() internal { @@ -55,41 +62,82 @@ contract SynapseModuleManagementTest is Test, SynapseModuleEvents { assertEq(verifiers[2], VERIFIER_3); } - function test_addSigner_addsSinger() public { + function test_addVerifier_addsVerifier() public { vm.prank(owner); module.addVerifier(VERIFIER_1); assertTrue(module.isVerifier(VERIFIER_1)); } - function test_addSigner_emitsEvent() public { + function test_addVerifier_emitsEvent() public { vm.expectEmit(address(module)); emit VerifierAdded(VERIFIER_1); vm.prank(owner); module.addVerifier(VERIFIER_1); } - function test_addSigner_revert_alreadyAdded() public { + function test_addVerifier_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 { + function test_addVerifier_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 { + function test_addVerifiers_addsVerifiers() public { + vm.prank(owner); + module.addVerifiers(allVerifiers); + assertTrue(module.isVerifier(VERIFIER_1)); + assertTrue(module.isVerifier(VERIFIER_2)); + assertTrue(module.isVerifier(VERIFIER_3)); + } + + function test_addVerifiers_emitsEvents() public { + vm.expectEmit(address(module)); + emit VerifierAdded(VERIFIER_1); + vm.expectEmit(address(module)); + emit VerifierAdded(VERIFIER_2); + vm.expectEmit(address(module)); + emit VerifierAdded(VERIFIER_3); + vm.prank(owner); + module.addVerifiers(allVerifiers); + } + + function test_addVerifiers_revert_alreadyAdded() public { + vm.prank(owner); + module.addVerifier(VERIFIER_2); + vm.expectRevert(abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__AlreadySigner.selector, VERIFIER_2)); + vm.prank(owner); + module.addVerifiers(allVerifiers); + } + + function test_addVerifiers_revert_notOwner(address notOwner) public { + vm.assume(notOwner != owner); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); + vm.prank(notOwner); + module.addVerifiers(allVerifiers); + } + + function test_addVerifiers_revert_containsZeroAddress() public { + allVerifiers[1] = address(0); + vm.expectRevert(ThresholdECDSALib.ThresholdECDSA__ZeroAddress.selector); + vm.prank(owner); + module.addVerifiers(allVerifiers); + } + + function test_removeVerifier_removesVerifier() public { basicSetup(); vm.prank(owner); module.removeVerifier(VERIFIER_1); assertFalse(module.isVerifier(VERIFIER_1)); } - function test_removeSigner_emitsEvent() public { + function test_removeVerifier_emitsEvent() public { basicSetup(); vm.expectEmit(address(module)); emit VerifierRemoved(VERIFIER_1); @@ -97,19 +145,55 @@ contract SynapseModuleManagementTest is Test, SynapseModuleEvents { module.removeVerifier(VERIFIER_1); } - function test_removeSigner_revert_notAdded() public { + function test_removeVerifier_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 { + function test_removeVerifier_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_removeVerifiers_removesVerifiers() public { + basicSetup(); + vm.prank(owner); + module.removeVerifiers(allVerifiers); + assertFalse(module.isVerifier(VERIFIER_1)); + assertFalse(module.isVerifier(VERIFIER_2)); + assertFalse(module.isVerifier(VERIFIER_3)); + } + + function test_removeVerifiers_emitsEvents() public { + basicSetup(); + vm.expectEmit(address(module)); + emit VerifierRemoved(VERIFIER_1); + vm.expectEmit(address(module)); + emit VerifierRemoved(VERIFIER_2); + vm.expectEmit(address(module)); + emit VerifierRemoved(VERIFIER_3); + vm.prank(owner); + module.removeVerifiers(allVerifiers); + } + + function test_removeVerifiers_revert_notAdded() public { + vm.prank(owner); + module.addVerifier(VERIFIER_2); + vm.expectRevert(abi.encodeWithSelector(ThresholdECDSALib.ThresholdECDSA__NotSigner.selector, VERIFIER_1)); + vm.prank(owner); + module.removeVerifiers(allVerifiers); + } + + function test_removeVerifiers_revert_notOwner(address notOwner) public { + vm.assume(notOwner != owner); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); + vm.prank(notOwner); + module.removeVerifiers(allVerifiers); + } + function test_setThreshold_setsThreshold() public { vm.prank(owner); module.setThreshold(3); @@ -156,6 +240,44 @@ contract SynapseModuleManagementTest is Test, SynapseModuleEvents { module.setFeeCollector(feeCollector); } + function test_setFeeFraction_setsFeeFraction() public { + vm.prank(owner); + module.setClaimFeeFraction(0.001e18); + assertEq(module.getClaimFeeFraction(), 0.001e18); + } + + function test_setFeeFraction_emitsEvent() public { + vm.expectEmit(address(module)); + emit ClaimFeeFractionChanged(0.001e18); + vm.prank(owner); + module.setClaimFeeFraction(0.001e18); + } + + function test_setFeeFraction_exactlyMax() public { + uint256 maxFeeFraction = 0.01e18; + vm.expectEmit(address(module)); + emit ClaimFeeFractionChanged(maxFeeFraction); + vm.prank(owner); + module.setClaimFeeFraction(maxFeeFraction); + assertEq(module.getClaimFeeFraction(), maxFeeFraction); + } + + function test_setFeeFraction_revert_exceedsMax() public { + uint256 fractionTooBig = 0.01e18 + 1; + vm.expectRevert( + abi.encodeWithSelector(ISynapseModule.SynapseModule__ClaimFeeFractionExceedsMax.selector, fractionTooBig) + ); + vm.prank(owner); + module.setClaimFeeFraction(fractionTooBig); + } + + function test_setFeeFraction_revert_notOwner(address notOwner) public { + vm.assume(notOwner != owner); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); + vm.prank(notOwner); + module.setClaimFeeFraction(0.001e18); + } + function test_setGasOracle_setsGasOracle() public { vm.prank(owner); module.setGasOracle(address(gasOracle)); @@ -179,11 +301,31 @@ contract SynapseModuleManagementTest is Test, SynapseModuleEvents { function test_setGasOracle_revert_notContract() public { address notContract = makeAddr("NotContract"); // Sanity check - require(notContract.code.length == 0); + assert(notContract.code.length == 0); vm.expectRevert( abi.encodeWithSelector(ISynapseModule.SynapseModule__GasOracleNotContract.selector, notContract) ); vm.prank(owner); module.setGasOracle(notContract); } + + function test_setVerifyGasLimit_setsVerifyGasLimit() public { + vm.prank(owner); + module.setVerifyGasLimit(1, 1000); + assertEq(module.getVerifyGasLimit(1), 1000); + } + + function test_setVerifyGasLimit_emitsEvent() public { + vm.expectEmit(address(module)); + emit VerifyGasLimitChanged(1, 1000); + vm.prank(owner); + module.setVerifyGasLimit(1, 1000); + } + + function test_setVerifyGasLimit_revert_notOwner(address notOwner) public { + vm.assume(notOwner != owner); + vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, notOwner)); + vm.prank(notOwner); + module.setVerifyGasLimit(1, 1000); + } } diff --git a/packages/contracts-communication/test/modules/SynapseModule.Source.t.sol b/packages/contracts-communication/test/modules/SynapseModule.Source.t.sol index 36c6774cb9..23a4ae7070 100644 --- a/packages/contracts-communication/test/modules/SynapseModule.Source.t.sol +++ b/packages/contracts-communication/test/modules/SynapseModule.Source.t.sol @@ -4,12 +4,15 @@ pragma solidity 0.8.20; import {InterchainModuleEvents} from "../../contracts/events/InterchainModuleEvents.sol"; import {SynapseModuleEvents} from "../../contracts/events/SynapseModuleEvents.sol"; import {IInterchainModule} from "../../contracts/interfaces/IInterchainModule.sol"; -import {SynapseModule, InterchainEntry, ISynapseModule} from "../../contracts/modules/SynapseModule.sol"; +import {InterchainEntry} from "../../contracts/libs/InterchainEntry.sol"; +import {SynapseModule, ISynapseModule} from "../../contracts/modules/SynapseModule.sol"; import {GasOracleMock} from "../mocks/GasOracleMock.sol"; import {Test} from "forge-std/Test.sol"; +// solhint-disable func-name-mixedcase +// solhint-disable ordering contract SynapseModuleSourceTest is Test, InterchainModuleEvents, SynapseModuleEvents { SynapseModule public module; GasOracleMock public gasOracle; @@ -17,12 +20,12 @@ contract SynapseModuleSourceTest is Test, InterchainModuleEvents, SynapseModuleE address public interchainDB = makeAddr("InterchainDB"); address public feeCollector = makeAddr("FeeCollector"); address public owner = makeAddr("Owner"); + address public claimer = makeAddr("Claimer"); 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 DEFAULT_GAS_LIMIT = 100_000; uint256 public constant FEE = 100; @@ -82,9 +85,10 @@ contract SynapseModuleSourceTest is Test, InterchainModuleEvents, SynapseModuleE mockRequestVerification(FEE, mockEntry); } - function test_requestVerification_transfersFeeToCollector() public { + function test_requestVerification_accumulatesFee() public { + deal(address(module), 5 ether); mockRequestVerification(FEE, mockEntry); - assertEq(feeCollector.balance, FEE); + assertEq(address(module).balance, 5 ether + FEE); } function test_requestVerification_feeAboveRequired_emitsEvent() public { @@ -94,9 +98,10 @@ contract SynapseModuleSourceTest is Test, InterchainModuleEvents, SynapseModuleE mockRequestVerification(FEE + 1, mockEntry); } - function test_requestVerification_feeAboveRequired_transfersFeeToCollector() public { + function test_requestVerification_feeAboveRequired_accumulatesFee() public { + deal(address(module), 5 ether); mockRequestVerification(FEE + 1, mockEntry); - assertEq(feeCollector.balance, FEE + 1); + assertEq(address(module).balance, 5 ether + FEE + 1); } function test_requestVerification_revert_feeBelowRequired() public { @@ -106,29 +111,154 @@ contract SynapseModuleSourceTest is Test, InterchainModuleEvents, SynapseModuleE mockRequestVerification(FEE - 1, mockEntry); } + function test_claimFees_zeroClaimFee_emitsEvent() public { + deal(address(module), 5 ether); + vm.expectEmit(address(module)); + emit FeesClaimed(feeCollector, 5 ether, claimer, 0); + vm.prank(claimer); + module.claimFees(); + } + + function test_claimFees_zeroClaimFee_distributesFees() public { + deal(address(module), 5 ether); + vm.prank(claimer); + module.claimFees(); + assertEq(feeCollector.balance, 5 ether); + assertEq(claimer.balance, 0); + } + + function test_claimFees_zeroClaimFee_revert_feeCollectorNotSet() public { + vm.prank(owner); + module.setFeeCollector(address(0)); + vm.expectRevert(abi.encodeWithSelector(ISynapseModule.SynapseModule__FeeCollectorNotSet.selector)); + vm.prank(claimer); + module.claimFees(); + } + + function test_claimFees_zeroClaimFee_revert_noFeesToClaim() public { + vm.expectRevert(abi.encodeWithSelector(ISynapseModule.SynapseModule__NoFeesToClaim.selector)); + vm.prank(claimer); + module.claimFees(); + } + + function test_claimFees_nonZeroClaimFee_emitsEvent() public { + // Set claim fee to 0.1% + vm.prank(owner); + module.setClaimFeeFraction(0.001e18); + deal(address(module), 5 ether); + vm.expectEmit(address(module)); + emit FeesClaimed(feeCollector, 4.995 ether, claimer, 0.005 ether); + vm.prank(claimer); + module.claimFees(); + } + + function test_claimFees_nonZeroClaimFee_distributesFees() public { + // Set claim fee to 0.1% + vm.prank(owner); + module.setClaimFeeFraction(0.001e18); + deal(address(module), 5 ether); + vm.prank(claimer); + module.claimFees(); + assertEq(feeCollector.balance, 4.995 ether); + assertEq(claimer.balance, 0.005 ether); + } + + function test_claimFees_nonZeroClaimFee_revert_feeCollectorNotSet() public { + // Set claim fee to 0.1% + vm.startPrank(owner); + module.setFeeCollector(address(0)); + module.setClaimFeeFraction(0.001e18); + vm.stopPrank(); + deal(address(module), 5 ether); + vm.expectRevert(abi.encodeWithSelector(ISynapseModule.SynapseModule__FeeCollectorNotSet.selector)); + vm.prank(claimer); + module.claimFees(); + } + + function test_claimFees_nonZeroClaimFee_revert_noFeesToClaim() public { + // Set claim fee to 0.1% + vm.prank(owner); + module.setClaimFeeFraction(0.001e18); + vm.expectRevert(abi.encodeWithSelector(ISynapseModule.SynapseModule__NoFeesToClaim.selector)); + vm.prank(claimer); + module.claimFees(); + } + + function test_getClaimFeeAmount_zeroFees_zeroClaimFee() public { + assertEq(module.getClaimFeeAmount(), 0); + } + + function test_getClaimFeeAmount_zeroFees_nonZeroClaimFee() public { + // Set claim fee to 0.1% + vm.prank(owner); + module.setClaimFeeFraction(0.001e18); + assertEq(module.getClaimFeeAmount(), 0); + } + + function test_getClaimFeeAmount_zeroClaimFee() public { + deal(address(module), 5 ether); + assertEq(module.getClaimFeeAmount(), 0); + } + + function test_getClaimFeeAmount_nonZeroClaimFee() public { + // Set claim fee to 0.1% + vm.prank(owner); + module.setClaimFeeFraction(0.001e18); + deal(address(module), 5 ether); + assertEq(module.getClaimFeeAmount(), 0.005 ether); + } + function test_getModuleFee_thresholdTwo() public { assertEq(module.getModuleFee(DST_CHAIN_ID), FEE); } - function test_getModuleFee_callsGasOracle_twoSigners() public { + function test_getModuleFee_callsGasOracle_gasLimitDefault_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) + gasOracle.estimateTxCostInLocalUnits, (DST_CHAIN_ID, DEFAULT_GAS_LIMIT, remoteCalldata.length) ); vm.expectCall(address(gasOracle), expectedCalldata); module.getModuleFee(DST_CHAIN_ID); } - function test_getModuleFee_callsGasOracle_threeSigners() public { + function test_getModuleFee_callsGasOracle_gasLimitDefault_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) + gasOracle.estimateTxCostInLocalUnits, (DST_CHAIN_ID, DEFAULT_GAS_LIMIT, remoteCalldata.length) ); vm.expectCall(address(gasOracle), expectedCalldata); module.getModuleFee(DST_CHAIN_ID); } + + function test_getModuleFee_callsGasOracle_gasLimitSet_twoSigners() public { + vm.prank(owner); + module.setVerifyGasLimit(DST_CHAIN_ID, 200_000); + 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, 200_000, remoteCalldata.length)); + vm.expectCall(address(gasOracle), expectedCalldata); + module.getModuleFee(DST_CHAIN_ID); + } + + function test_getModuleFee_callsGasOracle_gasLimitSet_threeSigners() public { + vm.prank(owner); + module.setThreshold(3); + vm.prank(owner); + module.setVerifyGasLimit(DST_CHAIN_ID, 200_000); + 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, 200_000, remoteCalldata.length)); + vm.expectCall(address(gasOracle), expectedCalldata); + module.getModuleFee(DST_CHAIN_ID); + } + + function test_getVerifyGasLimit_default() public { + assertEq(module.getVerifyGasLimit(DST_CHAIN_ID), DEFAULT_GAS_LIMIT); + } }