diff --git a/core/packages/contracts/src/Agent.sol b/core/packages/contracts/src/Agent.sol index 6cea71dede..fedaa5d972 100644 --- a/core/packages/contracts/src/Agent.sol +++ b/core/packages/contracts/src/Agent.sol @@ -2,13 +2,16 @@ // SPDX-FileCopyrightText: 2023 Snowfork pragma solidity 0.8.20; +/// @title An agent contract that acts on behalf of a consensus system on Polkadot +/// @dev Instances of this contract act as an agents for arbitrary consensus systems on Polkadot. These consensus systems +/// can include toplevel parachains as as well as nested consensus systems within a parachain. contract Agent { error Unauthorized(); - // The unique ID for this agent, derived from the MultiLocation of the corresponding consensus system on Polkadot + /// @dev The unique ID for this agent, derived from the MultiLocation of the corresponding consensus system on Polkadot bytes32 public immutable AGENT_ID; - // The gateway contract owning this agent + /// @dev The gateway contract controlling this agent address public immutable GATEWAY; constructor(bytes32 agentID) { @@ -16,8 +19,13 @@ contract Agent { GATEWAY = msg.sender; } + /// @dev Agents can receive ether permissionlessly. + /// This is important, as agents for top-level parachains also act as sovereign accounts from which message relayers + /// are rewarded. receive() external payable {} + /// @dev Allow the gateway to invoke some code within the context of this agent + /// using `delegatecall`. Typically this code will be provided by `AgentExecutor.sol`. function invoke(address executor, bytes calldata data) external returns (bool, bytes memory) { if (msg.sender != GATEWAY) { revert Unauthorized(); diff --git a/core/packages/contracts/src/AgentExecutor.sol b/core/packages/contracts/src/AgentExecutor.sol index 51fe29c9a7..bd7005f9f2 100644 --- a/core/packages/contracts/src/AgentExecutor.sol +++ b/core/packages/contracts/src/AgentExecutor.sol @@ -8,12 +8,18 @@ import {SubstrateTypes} from "./SubstrateTypes.sol"; import {IERC20} from "./interfaces/IERC20.sol"; import {SafeTokenTransfer, SafeNativeTransfer} from "./utils/SafeTransfer.sol"; +/// @title Code which will run within an `Agent` using `delegatecall`. +/// @dev This is a singleton contract, meaning that all agents will execute the same code. contract AgentExecutor { using SafeTokenTransfer for IERC20; using SafeNativeTransfer for address payable; bytes32 internal constant COMMAND_TRANSFER_TOKEN = keccak256("transferToken"); + /// @dev Execute a message which originated from the Polkadot side of the bridge. In other terms, + /// the `data` parameter is constructed by the BridgeHub parachain. + /// + /// NOTE: There are no authorization checks here. Since this contract is only used for its code. function execute(address, bytes memory data) external { (bytes32 command, bytes memory params) = abi.decode(data, (bytes32, bytes)); if (command == COMMAND_TRANSFER_TOKEN) { @@ -22,10 +28,15 @@ contract AgentExecutor { } } + /// @dev Transfer ether to `recipient`. Unlike `_transferToken` This logic is not nested within `execute`, + /// as the gateway needs to control an agent's ether balance directly. + /// + /// NOTE: There are no authorization checks here. Since this contract is only used for its code. function transferNative(address payable recipient, uint256 amount) external { recipient.safeNativeTransfer(amount); } + /// @dev Transfer ERC20 to `recipient`. Only callable via `execute`. function _transferToken(address token, address recipient, uint128 amount) internal { IERC20(token).safeTransfer(recipient, amount); } diff --git a/core/packages/contracts/src/Assets.sol b/core/packages/contracts/src/Assets.sol index 0347ab67d1..6b2bf1c838 100644 --- a/core/packages/contracts/src/Assets.sol +++ b/core/packages/contracts/src/Assets.sol @@ -12,6 +12,7 @@ import {SubstrateTypes} from "./SubstrateTypes.sol"; import {ParaID, Config} from "./Types.sol"; import {Address} from "./utils/Address.sol"; +/// @title Library for implementing Ethereum->Polkadot ERC20 transfers. library Assets { using Address for address; using SafeTokenTransferFrom for IERC20; @@ -23,11 +24,11 @@ library Assets { event TokenRegistrationSent(address token); /* Errors */ - error InvalidToken(); error InvalidAmount(); error InvalidDestination(); + // This library requires state which must be initialized in the gateway's storage. function initialize(uint256 registerTokenFee, uint256 sendTokenFee) external { AssetsStorage.Layout storage $ = AssetsStorage.layout(); @@ -79,6 +80,7 @@ library Assets { emit TokenSent(sender, token, destinationChain, abi.encodePacked(destinationAddress), amount); } + /// @dev transfer tokens from the sender to the specified function _transferToAgent(address assetHubAgent, address token, address sender, uint128 amount) internal { if (!token.isContract()) { revert InvalidToken(); diff --git a/core/packages/contracts/src/Gateway.sol b/core/packages/contracts/src/Gateway.sol index f91abcf2d6..6d471b9a47 100644 --- a/core/packages/contracts/src/Gateway.sol +++ b/core/packages/contracts/src/Gateway.sol @@ -92,6 +92,10 @@ contract Gateway is IGateway { CREATE_TOKEN_CALL_ID = createTokenCallID; } + /// @dev Submit a message from Polkadot for verification and dispatch + /// @param message A message produced by the OutboundQueue pallet on BridgeHub + /// @param leafProof A message proof used to verify that the message is in the merkle tree committed by the OutboundQueue pallet + /// @param headerProof A proof used to verify that the commitment was included in a BridgeHub header that was finalized by BEEFY. function submitInbound( InboundMessage calldata message, bytes32[] calldata leafProof, @@ -99,37 +103,43 @@ contract Gateway is IGateway { ) external { Channel storage channel = _ensureChannel(message.origin); + // 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); + // Verify that the commitment is included in a parachain header finalized by BEEFY. if (!verifyCommitment(commitment, headerProof)) { revert InvalidProof(); } + // Ensure this message is not being replayed if (message.nonce != channel.inboundNonce + 1) { revert InvalidNonce(); } - // Increment nonce for origin. Together with the above nonce check, this should prevent reentrancy. + // Increment nonce for origin. + // This also prevents the re-entrancy case in which a malicous party tries to re-enter by calling `submitInbound` + // again with the same (message, leafProof, headerProof) arguments. channel.inboundNonce++; - // reward the relayer from the agent contract - // Should revert if there are not enough funds. In which case, the origin - // should top up the funds and have a relayer resend the message. + // 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); } // Ensure relayers pass enough gas for message to execute. - // Otherwise malicious relayers can break the bridge by allowing handlers to run out gas. - // Resubmission of the message by honest relayers will fail as the tracked nonce - // has already been updated. + // 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) { revert NotEnoughGas(); } bool success = true; + // Dispatch message to a handler if (message.command == Command.AgentExecute) { try Gateway(this).agentExecute{gas: DISPATCH_GAS}(message.params) {} catch { @@ -170,15 +180,6 @@ contract Gateway is IGateway { emit IGateway.InboundMessageDispatched(message.origin, message.nonce, success); } - function verifyCommitment(bytes32 commitment, Verification.Proof calldata proof) internal view returns (bool) { - if (BEEFY_CLIENT != address(0)) { - return Verification.verifyCommitment(BEEFY_CLIENT, BRIDGE_HUB_PARA_ID_ENCODED, commitment, proof); - } else { - // for unit tests, verification is bypassed - return true; - } - } - /** * Getters */ @@ -244,30 +245,35 @@ contract Gateway is IGateway { } struct CreateAgentParams { + /// @dev The agent ID of the consensus system bytes32 agentID; } - // Create an agent for a consensus system on Polkadot + /// @dev Create an agent for a consensus system on Polkadot function createAgent(bytes calldata data) external onlySelf { CoreStorage.Layout storage $ = CoreStorage.layout(); CreateAgentParams memory params = abi.decode(data, (CreateAgentParams)); + // Ensure we don't overwrite an existing agent if (address($.agents[params.agentID]) != address(0)) { revert AgentAlreadyCreated(); } - address payable agent = payable(new Agent(params.agentID)); + address payable agent = payable(new Agent(params.agentID)); $.agents[params.agentID] = agent; + emit AgentCreated(params.agentID, agent); } struct CreateChannelParams { + /// @dev The parachain which will own the newly created channel ParaID paraID; + /// @dev The agent ID of the parachain bytes32 agentID; } - // Create a messaging channel to a Polkadot parachain + /// @dev Create a messaging channel for a Polkadot parachain function createChannel(bytes calldata data) external onlySelf { CoreStorage.Layout storage $ = CoreStorage.layout(); @@ -296,13 +302,17 @@ contract Gateway is IGateway { } struct UpdateChannelParams { + /// @dev The parachain used to identify the channel to update ParaID paraID; + /// @dev The new operating mode OperatingMode mode; + /// @dev The new fee for accepting outbound messages uint256 fee; + /// @dev The new reward to be given to relayers for submitting inbound messages uint256 reward; } - // Update parameters for a channel + /// @dev Update the configuration for a channel function updateChannel(bytes calldata data) external onlySelf { UpdateChannelParams memory params = abi.decode(data, (UpdateChannelParams)); @@ -325,18 +335,35 @@ contract Gateway is IGateway { } struct UpgradeParams { + /// @dev The address of the implementation contract address impl; + /// @dev the codehash of the new implementation contract. + /// Used to ensure the implementation isn't updated while + /// the upgrade is in flight bytes32 implCodeHash; + /// @dev parameters used to upgrade storage of the gateway bytes initParams; } - // Perform an upgrade + /// @dev Perform an upgrade of the gateway function upgrade(bytes calldata data) external onlySelf { UpgradeParams memory params = abi.decode(data, (UpgradeParams)); - if (params.impl.isContract() && params.impl.codehash != params.implCodeHash) { + + // Verify that the implementation is actually a contract + if (!params.impl.isContract()) { revert InvalidCodeHash(); } + + // Verify that the code in the implementation contract was not changed + // after the upgrade initiated on BridgeHub parachain. + if (params.impl.codehash != params.implCodeHash) { + revert InvalidCodeHash(); + } + + // Update the proxy with the address of the new implementation ERC1967.store(params.impl); + + // Apply the initialization function of the implementation only if params were provided if (params.initParams.length > 0) { (bool success,) = params.impl.delegatecall(abi.encodeWithSelector(this.initialize.selector, params.initParams)); @@ -344,14 +371,16 @@ contract Gateway is IGateway { revert SetupFailed(); } } + emit Upgraded(params.impl); } struct SetOperatingModeParams { + /// @dev The new operating mode OperatingMode mode; } - // Set the operating mode + // @dev Set the operating mode of the gateway function setOperatingMode(bytes calldata data) external onlySelf { CoreStorage.Layout storage $ = CoreStorage.layout(); SetOperatingModeParams memory params = abi.decode(data, (SetOperatingModeParams)); @@ -359,12 +388,15 @@ contract Gateway is IGateway { } struct TransferNativeFromAgentParams { + /// @dev The ID of the agent to transfer funds from bytes32 agentID; + /// @dev The recipient of the funds address recipient; + /// @dev The amount to transfer uint256 amount; } - // Withdraw funds from an agent to a recipient account + // @dev Transfer funds from an agent to a recipient account function transferNativeFromAgent(bytes calldata data) external onlySelf { CoreStorage.Layout storage $ = CoreStorage.layout(); @@ -424,10 +456,24 @@ contract Gateway is IGateway { /* Internal functions */ + // Verify that a message commitment is considered finalized by our BEEFY light client. + function verifyCommitment(bytes32 commitment, Verification.Proof calldata proof) internal view returns (bool) { + if (BEEFY_CLIENT != address(0)) { + return Verification.verifyCommitment(BEEFY_CLIENT, BRIDGE_HUB_PARA_ID_ENCODED, commitment, proof); + } else { + // for unit tests, verification is bypassed + return true; + } + } + + // Submit an outbound message to Polkadot function _submitOutbound(ParaID dest, bytes memory payload, uint256 extraFee) internal { Channel storage channel = _ensureChannel(dest); + + // Ensure outbound messaging is allowed _ensureOutboundMessagingEnabled(channel); + // Ensure the user has enough funds for this message to be accepted if (msg.value < channel.fee + extraFee) { revert FeePaymentToLow(); } @@ -440,6 +486,7 @@ contract Gateway is IGateway { emit IGateway.OutboundMessageAccepted(dest, channel.outboundNonce, payload); } + /// @dev Outbound message can be disabled globally or on a per-channel basis. function _ensureOutboundMessagingEnabled(Channel storage ch) internal view { CoreStorage.Layout storage $ = CoreStorage.layout(); if ($.mode != OperatingMode.Normal || ch.mode != OperatingMode.Normal) { @@ -447,6 +494,7 @@ contract Gateway is IGateway { } } + /// @dev Ensure that the specified parachain has a channel allocated function _ensureChannel(ParaID paraID) internal view returns (Channel storage ch) { ch = CoreStorage.layout().channels[paraID]; // A channel always has an agent specified. @@ -455,11 +503,13 @@ contract Gateway is IGateway { } } + /// @dev Invoke some code within an agent function _invokeOnAgent(address agent, bytes memory data) internal returns (bytes memory) { (bool success, bytes memory returndata) = (Agent(payable(agent)).invoke(AGENT_EXECUTOR, data)); return Call.verifyResult(success, returndata); } + /// @dev Transfer ether from an agent function _transferNativeFromAgent(address agent, address payable recipient, uint256 amount) internal { bytes memory call = abi.encodeCall(AgentExecutor.transferNative, (recipient, amount)); _invokeOnAgent(agent, call); @@ -469,7 +519,8 @@ contract Gateway is IGateway { * Upgrades */ - /// @dev Not externally accessible as overshadowed in the proxy + /// @dev Initialize storage in the gateway + /// NOTE: This is not externally accessible as this function selector is overshadowed in the proxy function initialize(bytes memory data) external { // Prevent initialization of storage in implementation contract if (ERC1967.load() == address(0)) { diff --git a/core/packages/contracts/src/GatewayProxy.sol b/core/packages/contracts/src/GatewayProxy.sol index 104ac5a571..16c44b545d 100644 --- a/core/packages/contracts/src/GatewayProxy.sol +++ b/core/packages/contracts/src/GatewayProxy.sol @@ -10,14 +10,17 @@ contract GatewayProxy { error NativeCurrencyNotAccepted(); constructor(address implementation, bytes memory params) { + // Store the address of the implementation contract ERC1967.store(implementation); + // Initialize storage by calling the implementation's `initialize(bytes)` function + // using `delegatecall`. (bool success,) = implementation.delegatecall(abi.encodeCall(GatewayProxy.initialize, params)); if (!success) { revert InitializationFailed(); } } - // Prevent fallback() from calling `initialize(bytes)` on implementation contract + // Prevent fallback() from calling `initialize(bytes)` on the implementation contract function initialize(bytes calldata) external pure { revert Unauthorized(); } @@ -34,6 +37,8 @@ contract GatewayProxy { } } + // Prevent users from unwittingly sending ether to the gateway, as these funds + // would otherwise be lost forever. receive() external payable { revert NativeCurrencyNotAccepted(); } diff --git a/core/packages/contracts/src/Types.sol b/core/packages/contracts/src/Types.sol index e8ef5497a9..f50a9ce7af 100644 --- a/core/packages/contracts/src/Types.sol +++ b/core/packages/contracts/src/Types.sol @@ -14,20 +14,32 @@ function ParaIDNe(ParaID a, ParaID b) pure returns (bool) { return !ParaIDEq(a, b); } +/// @dev A messaging channel for a Polkadot parachain struct Channel { + /// @dev The operating mode for this channel. Can be used to + /// disable messaging on a per-channel basis. OperatingMode mode; + /// @dev The current nonce for the inbound lane uint64 inboundNonce; + /// @dev The current node for the outbound lane uint64 outboundNonce; + /// @dev The address of the agent of the parachain owning this 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; } -// Inbound message from a Polkadot parachain (via BridgeHub) +/// @dev Inbound message from a Polkadot parachain (via BridgeHub) struct InboundMessage { + /// @dev The parachain from which this message originated ParaID origin; + /// @dev The channel nonce uint64 nonce; + /// @dev The command to execute Command command; + /// @dev The Parameters for the command bytes params; } @@ -38,14 +50,17 @@ enum OperatingMode { // Initial configuration for bridge struct Config { - // default fee/reward parameters + /// @dev The default fee charged to users for submitting outbound messages. uint256 fee; + /// @dev The default reward disbursed to message relayers for submitting inbound messages. uint256 reward; - // Assets + /// @dev The extra fee charged for registering tokens. uint256 registerNativeTokenFee; + /// @dev The extra fee charged for sending tokens. uint256 sendNativeTokenFee; } +/// @dev Messages from Polkadot take the form of these commands. enum Command { AgentExecute, Upgrade, diff --git a/core/packages/contracts/src/Verification.sol b/core/packages/contracts/src/Verification.sol index c41e787857..a91099be45 100644 --- a/core/packages/contracts/src/Verification.sol +++ b/core/packages/contracts/src/Verification.sol @@ -8,12 +8,19 @@ import {ScaleCodec} from "./utils/ScaleCodec.sol"; import {SubstrateTypes} from "./SubstrateTypes.sol"; library Verification { + /// @dev Merkle proof for parachain header finalized by BEEFY + /// Reference: https://github.com/paritytech/polkadot/blob/09b61286da11921a3dda0a8e4015ceb9ef9cffca/runtime/rococo/src/lib.rs#L1312 struct HeadProof { + /// @dev The leaf index of the parachain being proven uint256 pos; + /// @dev The number of leaves in the merkle tree uint256 width; + /// @dev The proof items bytes32[] proof; } + /// @dev An MMRLeaf without the `leaf_extra` field. + /// Reference: https://github.com/paritytech/substrate/blob/14e0a0b628f9154c5a2c870062c3aac7df8983ed/primitives/consensus/beefy/src/mmr.rs#L52 struct MMRLeafPartial { uint8 version; uint32 parentNumber; @@ -23,18 +30,10 @@ library Verification { bytes32 nextAuthoritySetRoot; } - uint256 public constant DIGEST_ITEM_PRERUNTIME = 6; - uint256 public constant DIGEST_ITEM_CONSENSUS = 4; - uint256 public constant DIGEST_ITEM_SEAL = 5; - uint256 public constant DIGEST_ITEM_OTHER = 0; - uint256 public constant DIGEST_ITEM_RUNTIME_ENVIRONMENT_UPDATED = 8; - - struct DigestItem { - uint256 kind; - bytes4 consensusEngineID; - bytes data; - } - + /// @dev Parachain header + /// References: + /// * https://paritytech.github.io/substrate/master/sp_runtime/generic/struct.Header.html + /// * https://github.com/paritytech/substrate/blob/14e0a0b628f9154c5a2c870062c3aac7df8983ed/primitives/runtime/src/generic/header.rs#L41 struct ParachainHeader { bytes32 parentHash; uint256 number; @@ -43,16 +42,60 @@ library Verification { DigestItem[] digestItems; } + /// @dev Represents a digest item within a parachain header. + /// References: + /// * https://paritytech.github.io/substrate/master/sp_runtime/generic/enum.DigestItem.html + /// * https://github.com/paritytech/substrate/blob/14e0a0b628f9154c5a2c870062c3aac7df8983ed/primitives/runtime/src/generic/digest.rs#L75 + struct DigestItem { + uint256 kind; + bytes4 consensusEngineID; + bytes data; + } + + /// @dev A chain of proofs struct Proof { + /// @dev The parachain header containing the message commitment as a digest item ParachainHeader header; + /// @dev The proof used to generate a merkle root of parachain heads HeadProof headProof; + /// @dev The MMR leaf to be proven MMRLeafPartial leafPartial; + /// @dev The MMR leaf prove bytes32[] leafProof; + /// @dev The order in which proof items should be combined uint256 leafProofOrder; } error InvalidParachainHeader(); + /// @dev IDs of enum variants of DigestItem + /// Reference: https://github.com/paritytech/substrate/blob/14e0a0b628f9154c5a2c870062c3aac7df8983ed/primitives/runtime/src/generic/digest.rs#L201 + uint256 public constant DIGEST_ITEM_OTHER = 0; + uint256 public constant DIGEST_ITEM_CONSENSUS = 4; + uint256 public constant DIGEST_ITEM_SEAL = 5; + uint256 public constant DIGEST_ITEM_PRERUNTIME = 6; + uint256 public constant DIGEST_ITEM_RUNTIME_ENVIRONMENT_UPDATED = 8; + + /// @dev Verify the message commitment by applying several proofs + /// + /// 1. First check that the commitment is included in the digest items of the parachain header + /// 2. Generate the root of the parachain heads merkle tree + /// 3. Construct an MMR leaf containing the parachain heads root. + /// 4. Verify that the MMR leaf is included in the MMR maintained by the BEEFY light client. + /// + /// Background info: + /// + /// In the Polkadot relay chain, for every block: + /// 1. A merkle root of finalized parachain headers is constructed: + /// https://github.com/paritytech/polkadot/blob/09b61286da11921a3dda0a8e4015ceb9ef9cffca/runtime/rococo/src/lib.rs#L1312. + /// 2. An MMR leaf is produced, containing this parachain headers root, and is then inserted into the + /// MMR maintained by the `merkle-mountain-range` pallet: + /// https://github.com/paritytech/substrate/tree/master/frame/merkle-mountain-range + /// + /// @param beefyClient The address of the BEEFY light client + /// @param encodedParaID The SCALE-encoded parachain ID of BridgeHub + /// @param commitment The message commitment root expected to be contained within the + /// digest of BridgeHub parachain header. function verifyCommitment(address beefyClient, bytes4 encodedParaID, bytes32 commitment, Proof calldata proof) external view @@ -67,6 +110,8 @@ library Verification { bytes32 parachainHeadHash = createParachainHeaderMerkleLeaf(encodedParaID, proof.header); // Compute the merkle root hash of all parachain heads + // + // For reference, in the polkadot relay chain, this is where the merkle tree root is constructed: if (proof.headProof.pos >= proof.headProof.width) { return false; } @@ -75,6 +120,8 @@ library Verification { ); bytes32 leafHash = createMMRLeaf(proof.leafPartial, parachainHeadsRoot); + + // Verify that the MMR leaf is part of the MMR maintained by the BEEFY light client return BeefyClient(beefyClient).verifyMMRLeafProof(leafHash, proof.leafProof, proof.leafProofOrder); } @@ -93,14 +140,15 @@ library Verification { return false; } - // encodes Vec + // SCALE-Encodes: Vec + // Reference: https://github.com/paritytech/substrate/blob/14e0a0b628f9154c5a2c870062c3aac7df8983ed/primitives/runtime/src/generic/digest.rs#L40 function encodeDigestItems(DigestItem[] calldata digestItems) internal pure returns (bytes memory) { // encode all digest items into a buffer bytes memory accum = hex""; for (uint256 i = 0; i < digestItems.length; i++) { accum = bytes.concat(accum, encodeDigestItem(digestItems[i])); } - // encode number of digest items, followed by encoded digest items + // Encode number of digest items, followed by encoded digest items return bytes.concat(ScaleCodec.checkedEncodeCompactU32(uint32(digestItems.length)), accum); } @@ -199,6 +247,8 @@ library Verification { ); } + // SCALE-encode: MMRLeaf + // Reference: https://github.com/paritytech/substrate/blob/14e0a0b628f9154c5a2c870062c3aac7df8983ed/primitives/consensus/beefy/src/mmr.rs#L52 function createMMRLeaf(MMRLeafPartial memory leaf, bytes32 parachainHeadsRoot) internal pure returns (bytes32) { bytes memory encodedLeaf = bytes.concat( ScaleCodec.encodeU8(leaf.version),