Feat: synapse module cleanup

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);
 }

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);
 }

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);

packages/contracts-communication/contracts/modules/SynapseModule.sol

@@ -7,22 +7,28 @@ 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";
 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()
         });
     }

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;