Improved fees and rewards logic for messages to Ethereum (paritytech#968

)

Co-authored-by: ron <yrong1997@gmail.com>
Co-authored-by: Clara van Staden <claravanstaden64@gmail.com>

.github/workflows/parachain.yml

@@ -113,14 +113,12 @@ jobs:
         run: rustup show
       - name: run coverage test
         run: >
-          cargo install cargo-tarpaulin &&
+          cargo install cargo-tarpaulin@0.27.0 &&
           cargo tarpaulin
           --manifest-path parachain/Cargo.toml
           --workspace
           --engine llvm
-          --out Xml
-          --exclude substrate-call-index
-          --exclude snowbridge-query-events
+          --out xml
       - name: Upload coverage reports to Codecov with GitHub Action
         uses: codecov/codecov-action@v3
         with:
@@ -133,7 +131,7 @@ jobs:
     steps:
       - uses: actions/checkout@v2
         with:
-          submodules: 'true'
+          submodules: "true"
       - uses: arduino/setup-protoc@v2
         with:
           repo-token: ${{ secrets.GITHUB_TOKEN }}
@@ -147,6 +145,7 @@ jobs:
           --package bridge-hub-rococo-runtime
 
   beacon-fuzz:
+    if: false
     needs: test
     runs-on: snowbridge-runner
     env:

contracts/src/DeployScript.sol

@@ -50,7 +50,6 @@ contract DeployScript is Script {
         Gateway gatewayLogic = new Gateway(
             address(beefyClient),
             address(executor),
-            vm.envUint("DISPATCH_GAS"),
             bridgeHubParaID,
             bridgeHubAgentID,
             assetHubParaID,
@@ -59,10 +58,7 @@ contract DeployScript is Script {
         );
 
         bytes memory initParams = abi.encode(
-            vm.envUint("DEFAULT_FEE"),
-            vm.envUint("DEFAULT_REWARD"),
-            vm.envUint("REGISTER_NATIVE_TOKEN_FEE"),
-            vm.envUint("SEND_NATIVE_TOKEN_FEE")
+            vm.envUint("DEFAULT_FEE"), vm.envUint("REGISTER_NATIVE_TOKEN_FEE"), vm.envUint("SEND_NATIVE_TOKEN_FEE")
         );
 
         GatewayProxy gateway = new GatewayProxy(address(gatewayLogic), initParams);

contracts/src/FundAgent.sol

@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: Apache-2.0
+// SPDX-FileCopyrightText: 2023 Snowfork <hello@snowfork.com>
+pragma solidity 0.8.20;
+
+import {WETH9} from "canonical-weth/WETH9.sol";
+import {Script} from "forge-std/Script.sol";
+import {BeefyClient} from "./BeefyClient.sol";
+
+import {IGateway} from "./interfaces/IGateway.sol";
+import {GatewayProxy} from "./GatewayProxy.sol";
+import {Gateway} from "./Gateway.sol";
+import {GatewayUpgradeMock} from "../test/mocks/GatewayUpgradeMock.sol";
+import {Agent} from "./Agent.sol";
+import {AgentExecutor} from "./AgentExecutor.sol";
+import {ParaID, Config} from "./Types.sol";
+import {SafeNativeTransfer} from "./utils/SafeTransfer.sol";
+import {stdJson} from "forge-std/StdJson.sol";
+
+contract FundAgent is Script {
+    using SafeNativeTransfer for address payable;
+    using stdJson for string;
+
+    function setUp() public {}
+
+    function run() public {
+        uint256 privateKey = vm.envUint("PRIVATE_KEY");
+        address deployer = vm.rememberKey(privateKey);
+        vm.startBroadcast(deployer);
+
+        uint256 initialDeposit = vm.envUint("BRIDGE_HUB_INITIAL_DEPOSIT");
+        address gatewayAddress = vm.envAddress("GATEWAY_PROXY_CONTRACT");
+
+        bytes32 bridgeHubAgentID = vm.envBytes32("BRIDGE_HUB_AGENT_ID");
+        bytes32 assetHubAgentID = vm.envBytes32("ASSET_HUB_AGENT_ID");
+
+        address bridgeHubAgent = IGateway(gatewayAddress).agentOf(bridgeHubAgentID);
+        address assetHubAgent = IGateway(gatewayAddress).agentOf(assetHubAgentID);
+
+        payable(bridgeHubAgent).safeNativeTransfer(initialDeposit);
+        payable(assetHubAgent).safeNativeTransfer(initialDeposit);
+
+        vm.stopBroadcast();
+    }
+}

contracts/src/Gateway.sol

@@ -15,6 +15,7 @@ import {ERC1967} from "./utils/ERC1967.sol";
 import {Address} from "./utils/Address.sol";
 import {SafeNativeTransfer} from "./utils/SafeTransfer.sol";
 import {Call} from "./utils/Call.sol";
+import {Math} from "./utils/Math.sol";
 import {ScaleCodec} from "./utils/ScaleCodec.sol";
 
 import {CoreStorage} from "./storage/CoreStorage.sol";
@@ -25,8 +26,7 @@ contract Gateway is IGateway, IInitializable {
     using SafeNativeTransfer for address payable;
 
     // After message dispatch, there should be some gas left over for post dispatch logic
-    uint256 internal constant BUFFER_GAS = 32_000;
-    uint256 internal immutable DISPATCH_GAS;
+    uint256 internal constant BUFFER_GAS = 48_000;
     address internal immutable AGENT_EXECUTOR;
 
     // Verification state
@@ -42,6 +42,9 @@ contract Gateway is IGateway, IInitializable {
     bytes32 internal immutable ASSET_HUB_AGENT_ID;
     bytes2 internal immutable CREATE_TOKEN_CALL_ID;
 
+    // Fixed amount of gas used outside the gas metering in submitInbound
+    uint256 BASE_GAS_USED = 31000;
+
     error InvalidProof();
     error InvalidNonce();
     error NotEnoughGas();
@@ -69,24 +72,21 @@ contract Gateway is IGateway, IInitializable {
     constructor(
         address beefyClient,
         address agentExecutor,
-        uint256 dispatchGas,
         ParaID bridgeHubParaID,
         bytes32 bridgeHubAgentID,
         ParaID assetHubParaID,
         bytes32 assetHubAgentID,
         bytes2 createTokenCallID
     ) {
         if (
-            dispatchGas == 0 || bridgeHubParaID == ParaID.wrap(0) || bridgeHubAgentID == 0
-                || assetHubParaID == ParaID.wrap(0) || assetHubAgentID == 0 || bridgeHubParaID == assetHubParaID
-                || bridgeHubAgentID == assetHubAgentID
+            bridgeHubParaID == ParaID.wrap(0) || bridgeHubAgentID == 0 || assetHubParaID == ParaID.wrap(0)
+                || assetHubAgentID == 0 || bridgeHubParaID == assetHubParaID || bridgeHubAgentID == assetHubAgentID
         ) {
             revert InvalidConstructorParams();
         }
 
         BEEFY_CLIENT = beefyClient;
         AGENT_EXECUTOR = agentExecutor;
-        DISPATCH_GAS = dispatchGas;
         BRIDGE_HUB_PARA_ID_ENCODED = ScaleCodec.encodeU32(uint32(ParaID.unwrap(bridgeHubParaID)));
         BRIDGE_HUB_PARA_ID = bridgeHubParaID;
         BRIDGE_HUB_AGENT_ID = bridgeHubAgentID;
@@ -104,6 +104,8 @@ contract Gateway is IGateway, IInitializable {
         bytes32[] calldata leafProof,
         Verification.Proof calldata headerProof
     ) external {
+        uint256 startGas = gasleft();
+
         Channel storage channel = _ensureChannel(message.origin);
 
         // Ensure this message is not being replayed
@@ -116,13 +118,6 @@ contract Gateway is IGateway, IInitializable {
         // again with the same (message, leafProof, headerProof) arguments.
         channel.inboundNonce++;
 
-        // Reward the relayer from the agent contract
-        // Expected to revert if the agent for the message origin does not have enough funds to reward the relayer.
-        // In that case, the origin should top up the funds of their agent.
-        if (channel.reward > 0) {
-            _transferNativeFromAgent(channel.agent, payable(msg.sender), channel.reward);
-        }
-
         // Produce the commitment (message root) by applying the leaf proof to the message leaf
         bytes32 leafHash = keccak256(abi.encode(message));
         bytes32 commitment = MerkleProof.processProof(leafProof, leafHash);
@@ -136,50 +131,71 @@ contract Gateway is IGateway, IInitializable {
         // Otherwise malicious relayers can break the bridge by allowing the message handlers below to run out gas and fail silently.
         // In this scenario case, the channel's state would have been updated to accept the message (by virtue of the nonce increment), yet the actual message
         // dispatch would have failed
-        if (gasleft() < DISPATCH_GAS + BUFFER_GAS) {
+        uint256 maxDispatchGas = message.maxDispatchGas;
+        if (gasleft() < maxDispatchGas + BUFFER_GAS) {
             revert NotEnoughGas();
         }
 
         bool success = true;
 
         // Dispatch message to a handler
         if (message.command == Command.AgentExecute) {
-            try Gateway(this).agentExecute{gas: DISPATCH_GAS}(message.params) {}
+            try Gateway(this).agentExecute{gas: maxDispatchGas}(message.params) {}
             catch {
                 success = false;
             }
         } else if (message.command == Command.CreateAgent) {
-            try Gateway(this).createAgent{gas: DISPATCH_GAS}(message.params) {}
+            try Gateway(this).createAgent{gas: maxDispatchGas}(message.params) {}
             catch {
                 success = false;
             }
         } else if (message.command == Command.CreateChannel) {
-            try Gateway(this).createChannel{gas: DISPATCH_GAS}(message.params) {}
+            try Gateway(this).createChannel{gas: maxDispatchGas}(message.params) {}
             catch {
                 success = false;
             }
         } else if (message.command == Command.UpdateChannel) {
-            try Gateway(this).updateChannel{gas: DISPATCH_GAS}(message.params) {}
+            try Gateway(this).updateChannel{gas: maxDispatchGas}(message.params) {}
             catch {
                 success = false;
             }
         } else if (message.command == Command.SetOperatingMode) {
-            try Gateway(this).setOperatingMode{gas: DISPATCH_GAS}(message.params) {}
+            try Gateway(this).setOperatingMode{gas: maxDispatchGas}(message.params) {}
             catch {
                 success = false;
             }
         } else if (message.command == Command.TransferNativeFromAgent) {
-            try Gateway(this).transferNativeFromAgent{gas: DISPATCH_GAS}(message.params) {}
+            try Gateway(this).transferNativeFromAgent{gas: maxDispatchGas}(message.params) {}
             catch {
                 success = false;
             }
         } else if (message.command == Command.Upgrade) {
-            try Gateway(this).upgrade{gas: DISPATCH_GAS}(message.params) {}
+            try Gateway(this).upgrade{gas: maxDispatchGas}(message.params) {}
             catch {
                 success = false;
             }
         }
 
+        // Calculate the actual cost of executing this message
+        uint256 gasUsed = startGas - gasleft() + BASE_GAS_USED;
+        uint256 calculatedRefund = gasUsed * tx.gasprice;
+
+        // If the actual refund amount is less than the estimated maximum refund, then
+        // reduce the amount paid out accordingly
+        uint256 amount = message.maxRefund;
+        if (message.maxRefund > calculatedRefund) {
+            amount = calculatedRefund;
+        }
+
+        // Add the reward to the refund amount. If the sum is more than the funds available
+        // in the channel agent, then reduce the total amount
+        amount = Math.min(amount + message.reward, address(channel.agent).balance);
+
+        // Do the payment if there funds available in the agent
+        if (amount > dustThreshold()) {
+            _transferNativeFromAgent(channel.agent, payable(msg.sender), amount);
+        }
+
         emit IGateway.InboundMessageDispatched(message.origin, message.nonce, success);
     }
 
@@ -201,9 +217,9 @@ contract Gateway is IGateway, IInitializable {
         return (ch.inboundNonce, ch.outboundNonce);
     }
 
-    function channelFeeRewardOf(ParaID paraID) external view returns (uint256, uint256) {
+    function channelFeeOf(ParaID paraID) external view returns (uint256) {
         Channel storage ch = _ensureChannel(paraID);
-        return (ch.fee, ch.reward);
+        return ch.fee;
     }
 
     function agentOf(bytes32 agentID) external view returns (address) {
@@ -291,7 +307,6 @@ contract Gateway is IGateway, IInitializable {
         ch.inboundNonce = 0;
         ch.outboundNonce = 0;
         ch.fee = $.defaultFee;
-        ch.reward = $.defaultReward;
 
         emit ChannelCreated(params.paraID);
     }
@@ -313,18 +328,13 @@ contract Gateway is IGateway, IInitializable {
 
         Channel storage ch = _ensureChannel(params.paraID);
 
-        // Extra sanity checks when updating the BridgeHub channel. For example, a huge reward could
-        // effectively brick the bridge permanently.
-        if (
-            params.paraID == BRIDGE_HUB_PARA_ID
-                && (params.mode != OperatingMode.Normal || params.fee > 1 ether || params.reward > 1 ether)
-        ) {
+        // Extra sanity checks when updating the BridgeHub channel, which should never be paused.
+        if (params.paraID == BRIDGE_HUB_PARA_ID && (params.mode != OperatingMode.Normal || params.fee > 1 ether)) {
             revert InvalidChannelUpdate();
         }
 
         ch.mode = params.mode;
         ch.fee = params.fee;
-        ch.reward = params.reward;
 
         emit ChannelUpdated(params.paraID);
     }
@@ -517,6 +527,11 @@ contract Gateway is IGateway, IInitializable {
         _invokeOnAgent(agent, call);
     }
 
+    /// @dev Define the dust threshold as the minimum cost to transfer ether between accounts
+    function dustThreshold() view internal returns (uint256) {
+        return 21000 * tx.gasprice;
+    }
+
     /**
      * Upgrades
      */
@@ -529,14 +544,13 @@ contract Gateway is IGateway, IInitializable {
             revert Unauthorized();
         }
 
-        (uint256 defaultFee, uint256 defaultReward, uint256 registerTokenFee, uint256 sendTokenFee) =
-            abi.decode(data, (uint256, uint256, uint256, uint256));
+        (uint256 defaultFee, uint256 registerTokenFee, uint256 sendTokenFee) =
+            abi.decode(data, (uint256, uint256, uint256));
 
         CoreStorage.Layout storage $ = CoreStorage.layout();
 
         $.mode = OperatingMode.Normal;
         $.defaultFee = defaultFee;
-        $.defaultReward = defaultReward;
 
         // Initialize an agent & channel for BridgeHub
         address bridgeHubAgent = address(new Agent(BRIDGE_HUB_AGENT_ID));
@@ -546,8 +560,7 @@ contract Gateway is IGateway, IInitializable {
             agent: bridgeHubAgent,
             inboundNonce: 0,
             outboundNonce: 0,
-            fee: defaultFee,
-            reward: defaultReward
+            fee: defaultFee
         });
 
         // Initialize an agent & channel for AssetHub
@@ -558,8 +571,7 @@ contract Gateway is IGateway, IInitializable {
             agent: assetHubAgent,
             inboundNonce: 0,
             outboundNonce: 0,
-            fee: defaultFee,
-            reward: defaultReward
+            fee: defaultFee
         });
 
         Assets.initialize(registerTokenFee, sendTokenFee);

contracts/src/Types.sol

@@ -27,8 +27,6 @@ struct Channel {
     address agent;
     /// @dev The fee charged to users for submitting outbound messages
     uint256 fee;
-    /// @dev The reward disbursed to message relayers for submitting inbound messages
-    uint256 reward;
 }
 
 /// @dev Inbound message from a Polkadot parachain (via BridgeHub)
@@ -41,6 +39,12 @@ struct InboundMessage {
     Command command;
     /// @dev The Parameters for the command
     bytes params;
+    /// @dev The maximum gas allowed for message dispatch
+    uint256 maxDispatchGas;
+    /// @dev The maximum gas refund for message submission
+    uint256 maxRefund;
+    /// @dev The reward for message submission
+    uint256 reward;
 }
 
 enum OperatingMode {

contracts/src/interfaces/IGateway.sol

@@ -40,7 +40,7 @@ interface IGateway {
 
     function operatingMode() external view returns (OperatingMode);
     function channelOperatingModeOf(ParaID paraID) external view returns (OperatingMode);
-    function channelFeeRewardOf(ParaID paraID) external view returns (uint256, uint256);
+    function channelFeeOf(ParaID paraID) external view returns (uint256);
     function channelNoncesOf(ParaID paraID) external view returns (uint64, uint64);
     function agentOf(bytes32 agentID) external view returns (address);
     function implementation() external view returns (address);

contracts/src/storage/CoreStorage.sol

@@ -14,8 +14,6 @@ library CoreStorage {
         mapping(bytes32 agentID => address) agents;
         // The default fee charged to users for submitting outbound message to Polkadot
         uint256 defaultFee;
-        // The default reward given to relayers for submitting inbound messages from Polkadot
-        uint256 defaultReward;
     }
 
     bytes32 internal constant SLOT = keccak256("org.snowbridge.storage.core");