From c513cbbb5eaaad91042425fe2630ad9360d3381d Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Sun, 25 Aug 2024 18:02:08 +0200 Subject: [PATCH 1/4] Improve PNA implementation --- contracts/src/AgentExecutor.sol | 27 +-- contracts/src/Assets.sol | 164 ++++++++++--------- contracts/src/Gateway.sol | 80 ++------- contracts/src/Params.sol | 12 +- contracts/src/{ERC20.sol => Token.sol} | 36 ++-- contracts/src/{ERC20Lib.sol => TokenLib.sol} | 145 +++++++--------- contracts/src/Types.sol | 8 +- contracts/src/interfaces/IERC20.sol | 4 +- contracts/src/interfaces/IERC20Permit.sol | 5 + contracts/src/interfaces/IGateway.sol | 2 +- contracts/src/storage/AssetsStorage.sol | 3 +- contracts/test/Gateway.t.sol | 89 ++++------ contracts/test/mocks/MockGateway.sol | 4 +- 13 files changed, 238 insertions(+), 341 deletions(-) rename contracts/src/{ERC20.sol => Token.sol} (91%) rename contracts/src/{ERC20Lib.sol => TokenLib.sol} (61%) diff --git a/contracts/src/AgentExecutor.sol b/contracts/src/AgentExecutor.sol index 8b5f56fedf..c3ed7e2f81 100644 --- a/contracts/src/AgentExecutor.sol +++ b/contracts/src/AgentExecutor.sol @@ -7,7 +7,6 @@ import {SubstrateTypes} from "./SubstrateTypes.sol"; import {IERC20} from "./interfaces/IERC20.sol"; import {SafeTokenTransfer, SafeNativeTransfer} from "./utils/SafeTransfer.sol"; -import {ERC20} from "./ERC20.sol"; import {Gateway} from "./Gateway.sol"; /// @title Code which will run within an `Agent` using `delegatecall`. @@ -16,17 +15,6 @@ contract AgentExecutor { using SafeTokenTransfer for IERC20; using SafeNativeTransfer for address payable; - /// @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. - /// - function execute(bytes memory data) external { - (AgentExecuteCommand command, bytes memory params) = abi.decode(data, (AgentExecuteCommand, bytes)); - if (command == AgentExecuteCommand.TransferToken) { - (address token, address recipient, uint128 amount) = abi.decode(params, (address, address, uint128)); - _transferToken(token, recipient, amount); - } - } - /// @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. /// @@ -34,22 +22,13 @@ contract AgentExecutor { 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); - } - /// @dev Transfer ERC20 to `recipient`. Only callable via `execute`. function transferToken(address token, address recipient, uint128 amount) external { _transferToken(token, recipient, amount); } - /// @dev Mint ERC20 token to `recipient`. - function mintToken(address token, address recipient, uint256 amount) external { - ERC20(token).mint(recipient, amount); - } - - function burnToken(address token, address sender, uint256 amount) external { - ERC20(token).burn(sender, 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/contracts/src/Assets.sol b/contracts/src/Assets.sol index 1183f72986..62f70b186c 100644 --- a/contracts/src/Assets.sol +++ b/contracts/src/Assets.sol @@ -16,7 +16,7 @@ import {Address} from "./utils/Address.sol"; import {AgentExecutor} from "./AgentExecutor.sol"; import {Agent} from "./Agent.sol"; import {Call} from "./utils/Call.sol"; -import {ERC20} from "./ERC20.sol"; +import {Token} from "./Token.sol"; /// @title Library for implementing Ethereum->Polkadot ERC20 transfers. library Assets { @@ -108,6 +108,34 @@ library Assets { ) external returns (Ticket memory ticket) { AssetsStorage.Layout storage $ = AssetsStorage.layout(); + TokenInfo storage info = $.tokenRegistry[token]; + + if (!info.isRegistered) { + revert TokenNotRegistered(); + } + + if (info.foreignID == bytes32(0)) { + return _sendNativeToken( + token, sender, destinationChain, destinationAddress, destinationChainFee, maxDestinationChainFee, amount + ); + } else { + return _sendForeignToken( + info.foreignID, token, sender, destinationChain, destinationAddress, destinationChainFee, amount + ); + } + } + + function _sendNativeToken( + address token, + address sender, + ParaID destinationChain, + MultiAddress calldata destinationAddress, + uint128 destinationChainFee, + uint128 maxDestinationChainFee, + uint128 amount + ) internal returns (Ticket memory ticket) { + AssetsStorage.Layout storage $ = AssetsStorage.layout(); + // Lock the funds into AssetHub's agent contract _transferToAgent($.assetHubAgent, token, sender, amount); @@ -159,6 +187,42 @@ library Assets { emit IGateway.TokenSent(token, sender, destinationChain, destinationAddress, amount); } + // @dev Transfer Polkadot-native tokens back to Polkadot + function _sendForeignToken( + bytes32 foreignID, + address token, + address sender, + ParaID destinationChain, + MultiAddress calldata destinationAddress, + uint128 destinationChainFee, + uint128 amount + ) internal returns (Ticket memory ticket) { + if (destinationChainFee == 0) { + revert InvalidDestinationFee(); + } + + Token(token).burn(sender, amount); + + ticket.dest = destinationChain; + ticket.costs = _sendForeignTokenCosts(destinationChainFee); + + if (destinationAddress.isAddress32()) { + // The receiver has a 32-byte account ID + ticket.payload = SubstrateTypes.SendForeignTokenToAddress32( + foreignID, destinationChain, destinationAddress.asAddress32(), destinationChainFee, amount + ); + } else if (destinationAddress.isAddress20()) { + // The receiver has a 20-byte account ID + ticket.payload = SubstrateTypes.SendForeignTokenToAddress20( + foreignID, destinationChain, destinationAddress.asAddress20(), destinationChainFee, amount + ); + } else { + revert Unsupported(); + } + + emit IGateway.TokenSent(token, sender, destinationChain, destinationAddress, amount); + } + function registerTokenCosts() external view returns (Costs memory costs) { return _registerTokenCosts(); } @@ -195,90 +259,36 @@ library Assets { emit IGateway.TokenRegistrationSent(token); } - // @dev Transfer polkadot native tokens back - function sendForeignToken( - address agent, - address executor, - TokenInfo storage info, - address sender, - ParaID destinationChain, - MultiAddress calldata destinationAddress, - uint128 destinationChainFee, - uint128 amount - ) external returns (Ticket memory ticket) { - if (destinationChainFee == 0) { - revert InvalidDestinationFee(); - } - // Polkadot-native token: burn wrapped token - _burnToken(executor, agent, info.token, sender, amount); - - ticket.dest = destinationChain; - ticket.costs = _sendForeignTokenCosts(destinationChainFee); - - if (destinationAddress.isAddress32()) { - // The receiver has a 32-byte account ID - ticket.payload = SubstrateTypes.SendForeignTokenToAddress32( - info.tokenID, destinationChain, destinationAddress.asAddress32(), destinationChainFee, amount - ); - } else if (destinationAddress.isAddress20()) { - // The receiver has a 20-byte account ID - ticket.payload = SubstrateTypes.SendForeignTokenToAddress20( - info.tokenID, destinationChain, destinationAddress.asAddress20(), destinationChainFee, amount - ); - } else { - revert Unsupported(); - } - - emit IGateway.TokenSent(info.token, sender, destinationChain, destinationAddress, amount); - } - - function _burnToken(address agentExecutor, address agent, address token, address sender, uint256 amount) internal { - bytes memory call = abi.encodeCall(AgentExecutor.burnToken, (token, sender, amount)); - (bool success, bytes memory returndata) = (Agent(payable(agent)).invoke(agentExecutor, call)); - Call.verifyResult(success, returndata); - } - function _sendForeignTokenCosts(uint128 destinationChainFee) internal pure returns (Costs memory costs) { costs.foreign = destinationChainFee; costs.native = 0; } // @dev Register a new fungible Polkadot token for an agent - function registerForeignToken( - bytes32 agentID, - address agent, - bytes32 tokenID, - string memory name, - string memory symbol, - uint8 decimals - ) external { + function registerForeignToken(bytes32 foreignTokenID, string memory name, string memory symbol, uint8 decimals) + external + { AssetsStorage.Layout storage $ = AssetsStorage.layout(); - if ($.tokenRegistryByID[tokenID].isRegistered == true) { + if ($.tokenAddressOf[foreignTokenID] != address(0)) { revert TokenAlreadyRegistered(); } - ERC20 foreignToken = new ERC20(agent, name, symbol, decimals); - address token = address(foreignToken); - TokenInfo memory info = - TokenInfo({isRegistered: true, isForeign: true, tokenID: tokenID, agentID: agentID, token: token}); - $.tokenRegistry[token] = info; - $.tokenRegistryByID[tokenID] = info; - emit IGateway.ForeignTokenRegistered(tokenID, agentID, token); + Token token = new Token(name, symbol, decimals); + TokenInfo memory info = TokenInfo({isRegistered: true, foreignID: foreignTokenID}); + + $.tokenAddressOf[foreignTokenID] = address(token); + $.tokenRegistry[address(token)] = info; + + emit IGateway.ForeignTokenRegistered(foreignTokenID, address(token)); } // @dev Mint foreign token from Polkadot - function mintForeignToken(address executor, address agent, bytes32 tokenID, address recipient, uint256 amount) - external - { - address token = _tokenAddressOf(tokenID); - bytes memory call = abi.encodeCall(AgentExecutor.mintToken, (token, recipient, amount)); - (bool success,) = Agent(payable(agent)).invoke(executor, call); - if (!success) { - revert TokenMintFailed(); - } + function mintForeignToken(bytes32 foreignTokenID, address recipient, uint256 amount) external { + address token = _ensureTokenAddressOf(foreignTokenID); + Token(token).mint(recipient, amount); } // @dev Transfer ERC20 to `recipient` - function transferToken(address executor, address agent, address token, address recipient, uint128 amount) + function transferNativeToken(address executor, address agent, address token, address recipient, uint128 amount) external { bytes memory call = abi.encodeCall(AgentExecutor.transferToken, (token, recipient, amount)); @@ -290,15 +300,21 @@ library Assets { // @dev Get token address by tokenID function tokenAddressOf(bytes32 tokenID) external view returns (address) { - return _tokenAddressOf(tokenID); + AssetsStorage.Layout storage $ = AssetsStorage.layout(); + return $.tokenAddressOf[tokenID]; } // @dev Get token address by tokenID - function _tokenAddressOf(bytes32 tokenID) internal view returns (address) { + function _ensureTokenAddressOf(bytes32 tokenID) internal view returns (address) { AssetsStorage.Layout storage $ = AssetsStorage.layout(); - if ($.tokenRegistryByID[tokenID].isRegistered == false) { + if ($.tokenAddressOf[tokenID] == address(0)) { revert TokenNotRegistered(); } - return $.tokenRegistryByID[tokenID].token; + return $.tokenAddressOf[tokenID]; + } + + function _isTokenRegistered(address token) internal view returns (bool) { + AssetsStorage.Layout storage $ = AssetsStorage.layout(); + return $.tokenRegistry[token].isRegistered; } } diff --git a/contracts/src/Gateway.sol b/contracts/src/Gateway.sol index 7a6d44a9dd..d4ac703db6 100644 --- a/contracts/src/Gateway.sol +++ b/contracts/src/Gateway.sol @@ -44,7 +44,7 @@ import { SetPricingParametersParams, RegisterForeignTokenParams, MintForeignTokenParams, - TransferTokenParams + TransferNativeTokenParams } from "./Params.sol"; import {CoreStorage} from "./storage/CoreStorage.sol"; @@ -228,8 +228,8 @@ contract Gateway is IGateway, IInitializable, IUpgradable { catch { success = false; } - } else if (message.command == Command.TransferToken) { - try Gateway(this).transferToken{gas: maxDispatchGas}(message.params) {} + } else if (message.command == Command.TransferNativeToken) { + try Gateway(this).transferNativeToken{gas: maxDispatchGas}(message.params) {} catch { success = false; } @@ -306,11 +306,11 @@ contract Gateway is IGateway, IInitializable, IUpgradable { revert InvalidAgentExecutionPayload(); } - bytes memory call = abi.encodeCall(AgentExecutor.execute, params.payload); - - (bool success, bytes memory returndata) = Agent(payable(agent)).invoke(AGENT_EXECUTOR, call); - if (!success) { - revert AgentExecutionFailed(returndata); + (AgentExecuteCommand command, bytes memory commandParams) = + abi.decode(params.payload, (AgentExecuteCommand, bytes)); + if (command == AgentExecuteCommand.TransferToken) { + (address token, address recipient, uint128 amount) = abi.decode(commandParams, (address, address, uint128)); + Assets.transferNativeToken(AGENT_EXECUTOR, agent, token, recipient, amount); } } @@ -419,22 +419,20 @@ contract Gateway is IGateway, IInitializable, IUpgradable { // @dev Register a new fungible Polkadot token for an agent function registerForeignToken(bytes calldata data) external onlySelf { RegisterForeignTokenParams memory params = abi.decode(data, (RegisterForeignTokenParams)); - address agent = _ensureAgent(params.agentID); - Assets.registerForeignToken(params.agentID, agent, params.tokenID, params.name, params.symbol, params.decimals); + Assets.registerForeignToken(params.foreignTokenID, params.name, params.symbol, params.decimals); } // @dev Mint foreign token from polkadot function mintForeignToken(bytes calldata data) external onlySelf { MintForeignTokenParams memory params = abi.decode(data, (MintForeignTokenParams)); - address agent = _ensureAgent(params.agentID); - Assets.mintForeignToken(AGENT_EXECUTOR, agent, params.tokenID, params.recipient, params.amount); + Assets.mintForeignToken(params.foreignTokenID, params.recipient, params.amount); } // @dev Transfer Ethereum native token back from polkadot - function transferToken(bytes calldata data) external onlySelf { - TransferTokenParams memory params = abi.decode(data, (TransferTokenParams)); + function transferNativeToken(bytes calldata data) external onlySelf { + TransferNativeTokenParams memory params = abi.decode(data, (TransferNativeTokenParams)); address agent = _ensureAgent(params.agentID); - Assets.transferToken(AGENT_EXECUTOR, agent, params.token, params.recipient, params.amount); + Assets.transferNativeToken(AGENT_EXECUTOR, agent, params.token, params.recipient, params.amount); } function isTokenRegistered(address token) external view returns (bool) { @@ -468,33 +466,11 @@ contract Gateway is IGateway, IInitializable, IUpgradable { uint128 destinationFee, uint128 amount ) external payable { - AssetsStorage.Layout storage $ = AssetsStorage.layout(); + Ticket memory ticket = Assets.sendToken( + token, msg.sender, destinationChain, destinationAddress, destinationFee, MAX_DESTINATION_FEE, amount + ); - TokenInfo storage tokenInfo = $.tokenRegistry[token]; - if (!tokenInfo.isRegistered) { - revert TokenNotRegistered(); - } - if (tokenInfo.isForeign) { - address agent = _ensureAgent(tokenInfo.agentID); - _submitOutbound( - Assets.sendForeignToken( - agent, - AGENT_EXECUTOR, - tokenInfo, - msg.sender, - destinationChain, - destinationAddress, - destinationFee, - amount - ) - ); - } else { - _submitOutbound( - Assets.sendToken( - token, msg.sender, destinationChain, destinationAddress, destinationFee, MAX_DESTINATION_FEE, amount - ) - ); - } + _submitOutbound(ticket); } // @dev Get token address by tokenID @@ -721,26 +697,4 @@ contract Gateway is IGateway, IInitializable, IUpgradable { OperatorStorage.Layout storage operatorStorage = OperatorStorage.layout(); operatorStorage.operator = config.rescueOperator; } - - /// @dev Temporary rescue ability for the initial bootstrapping phase of the bridge - function rescue(address impl, bytes32 implCodeHash, bytes calldata initializerParams) external { - OperatorStorage.Layout storage operatorStorage = OperatorStorage.layout(); - if (msg.sender != operatorStorage.operator) { - revert Unauthorized(); - } - Upgrade.upgrade(impl, implCodeHash, initializerParams); - } - - function dropRescueAbility() external { - OperatorStorage.Layout storage operatorStorage = OperatorStorage.layout(); - if (msg.sender != operatorStorage.operator) { - revert Unauthorized(); - } - operatorStorage.operator = address(0); - } - - function rescueOperator() external view returns (address) { - OperatorStorage.Layout storage operatorStorage = OperatorStorage.layout(); - return operatorStorage.operator; - } } diff --git a/contracts/src/Params.sol b/contracts/src/Params.sol index 869cc4e75c..882c2c7856 100644 --- a/contracts/src/Params.sol +++ b/contracts/src/Params.sol @@ -85,10 +85,8 @@ struct SetPricingParametersParams { // Payload for RegisterForeignToken struct RegisterForeignTokenParams { - /// @dev The agent ID of the consensus system - bytes32 agentID; - /// @dev The token ID - bytes32 tokenID; + /// @dev The token ID (hash of stable location id of token) + bytes32 foreignTokenID; /// @dev The name of the token string name; /// @dev The symbol of the token @@ -99,10 +97,8 @@ struct RegisterForeignTokenParams { // Payload for MintForeignToken struct MintForeignTokenParams { - /// @dev The agent ID of the consensus system - bytes32 agentID; /// @dev The token ID - bytes32 tokenID; + bytes32 foreignTokenID; /// @dev The address of the recipient address recipient; /// @dev The amount to mint with @@ -110,7 +106,7 @@ struct MintForeignTokenParams { } // Payload for TransferToken -struct TransferTokenParams { +struct TransferNativeTokenParams { /// @dev The agent ID of the consensus system bytes32 agentID; /// @dev The token address diff --git a/contracts/src/ERC20.sol b/contracts/src/Token.sol similarity index 91% rename from contracts/src/ERC20.sol rename to contracts/src/Token.sol index ee9c88c42a..3e035ccbec 100644 --- a/contracts/src/ERC20.sol +++ b/contracts/src/Token.sol @@ -6,7 +6,7 @@ pragma solidity 0.8.25; import {IERC20} from "./interfaces/IERC20.sol"; import {IERC20Permit} from "./interfaces/IERC20Permit.sol"; -import {ERC20Lib} from "./ERC20Lib.sol"; +import {TokenLib} from "./TokenLib.sol"; /** * @dev Implementation of the {IERC20} interface. @@ -28,33 +28,29 @@ import {ERC20Lib} from "./ERC20Lib.sol"; * functions have been added to mitigate the well-known issues around setting * allowances. See {IERC20-approve}. */ -contract ERC20 is IERC20, IERC20Permit { - using ERC20Lib for ERC20Lib.TokenStorage; +contract Token is IERC20, IERC20Permit { + using TokenLib for TokenLib.Token; - error Unauthorized(); - - ERC20Lib.TokenStorage token; - - address public immutable OWNER; - - uint8 public immutable decimals; + TokenLib.Token token; + address public immutable GATEWAY; string public name; string public symbol; + uint8 public immutable decimals; /** * @dev Sets the values for {name}, {symbol}, and {decimals}. */ - constructor(address _owner, string memory name_, string memory symbol_, uint8 decimals_) { - OWNER = _owner; - name = name_; - symbol = symbol_; - decimals = decimals_; - token.init(name_); + constructor(string memory _name, string memory _symbol, uint8 _decimals) { + name = _name; + symbol = _symbol; + decimals = _decimals; + GATEWAY = msg.sender; + token.initialize(_name); } - modifier onlyOwner() { - if (msg.sender != OWNER) { + modifier onlyGateway() { + if (msg.sender != GATEWAY) { revert Unauthorized(); } _; @@ -70,14 +66,14 @@ contract ERC20 is IERC20, IERC20Permit { * * - `account` cannot be the zero address. */ - function mint(address account, uint256 amount) external virtual onlyOwner { + function mint(address account, uint256 amount) external virtual onlyGateway { token.mint(account, amount); } /** * @dev Destroys `amount` tokens from the account. */ - function burn(address account, uint256 amount) external virtual onlyOwner { + function burn(address account, uint256 amount) external virtual onlyGateway { token.burn(account, amount); } diff --git a/contracts/src/ERC20Lib.sol b/contracts/src/TokenLib.sol similarity index 61% rename from contracts/src/ERC20Lib.sol rename to contracts/src/TokenLib.sol index f67863cbc0..036ffa9fe5 100644 --- a/contracts/src/ERC20Lib.sol +++ b/contracts/src/TokenLib.sol @@ -7,7 +7,7 @@ pragma solidity 0.8.25; import {IERC20} from "./interfaces/IERC20.sol"; import {IERC20Permit} from "./interfaces/IERC20Permit.sol"; -library ERC20Lib { +library TokenLib { // keccak256('EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)') bytes32 internal constant DOMAIN_TYPE_SIGNATURE_HASH = bytes32(0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f); @@ -18,27 +18,18 @@ library ERC20Lib { string internal constant EIP191_PREFIX_FOR_EIP712_STRUCTURED_DATA = "\x19\x01"; - error InvalidAccount(); - error PermitExpired(); - error InvalidS(); - error InvalidV(); - error InvalidSignature(); - error ERC20InsufficientBalance(address sender, uint256 balance, uint256 needed); - error ERC20InsufficientAllowance(address spender, uint256 allowance, uint256 needed); - error OwnableInvalidOwner(address owner); - - struct TokenStorage { - mapping(address => uint256) balanceOf; - mapping(address => mapping(address => uint256)) allowance; - mapping(address => uint256) nonces; + struct Token { + mapping(address account => uint256) balance; + mapping(address account => mapping(address spender => uint256)) allowance; + mapping(address token => uint256) nonces; uint256 totalSupply; bytes32 domainSeparator; } - function init(TokenStorage storage self, string memory name_) internal { - self.domainSeparator = keccak256( + function initialize(Token storage token, string memory name) internal { + token.domainSeparator = keccak256( abi.encode( - DOMAIN_TYPE_SIGNATURE_HASH, keccak256(bytes(name_)), keccak256(bytes("1")), block.chainid, address(this) + DOMAIN_TYPE_SIGNATURE_HASH, keccak256(bytes(name)), keccak256(bytes("1")), block.chainid, address(this) ) ); } @@ -51,11 +42,8 @@ library ERC20Lib { * - `recipient` cannot be the zero address. * - the caller must have a balance of at least `amount`. */ - function transfer(TokenStorage storage self, address sender, address recipient, uint256 amount) - external - returns (bool) - { - _transfer(self, sender, recipient, amount); + function transfer(Token storage token, address sender, address recipient, uint256 amount) external returns (bool) { + _transfer(token, sender, recipient, amount); return true; } @@ -69,10 +57,10 @@ library ERC20Lib { * * - `to` cannot be the zero address. */ - function mint(TokenStorage storage self, address account, uint256 amount) external { - if (account == address(0)) revert InvalidAccount(); + function mint(Token storage token, address account, uint256 amount) external { + if (account == address(0)) revert IERC20.Unauthorized(); - _update(self, address(0), account, amount); + _update(token, address(0), account, amount); } /** @@ -86,10 +74,10 @@ library ERC20Lib { * - `account` cannot be the zero address. * - `account` must have at least `amount` tokens. */ - function burn(TokenStorage storage self, address account, uint256 amount) external { - if (account == address(0)) revert InvalidAccount(); + function burn(Token storage token, address account, uint256 amount) external { + if (account == address(0)) revert IERC20.Unauthorized(); - _update(self, account, address(0), amount); + _update(token, account, address(0), amount); } /** @@ -107,11 +95,8 @@ library ERC20Lib { * * - `spender` cannot be the zero address. */ - function approve(TokenStorage storage self, address owner, address spender, uint256 amount) - external - returns (bool) - { - _approve(self, owner, spender, amount); + function approve(Token storage token, address owner, address spender, uint256 amount) external returns (bool) { + _approve(token, owner, spender, amount); return true; } @@ -128,22 +113,22 @@ library ERC20Lib { * - the caller must have allowance for ``sender``'s tokens of at least * `amount`. */ - function transferFrom(TokenStorage storage self, address sender, address recipient, uint256 amount) + function transferFrom(Token storage token, address sender, address recipient, uint256 amount) external returns (bool) { - uint256 _allowance = self.allowance[sender][msg.sender]; + uint256 _allowance = token.allowance[sender][msg.sender]; if (_allowance != type(uint256).max) { if (_allowance < amount) { - revert ERC20InsufficientAllowance(msg.sender, _allowance, amount); + revert IERC20.InsufficientAllowance(msg.sender, _allowance, amount); } unchecked { - _approve(self, sender, msg.sender, _allowance - amount); + _approve(token, sender, msg.sender, _allowance - amount); } } - _transfer(self, sender, recipient, amount); + _transfer(token, sender, recipient, amount); return true; } @@ -160,13 +145,10 @@ library ERC20Lib { * * - `spender` cannot be the zero address. */ - function increaseAllowance(TokenStorage storage self, address spender, uint256 addedValue) - external - returns (bool) - { - uint256 _allowance = self.allowance[msg.sender][spender]; + function increaseAllowance(Token storage token, address spender, uint256 addedValue) external returns (bool) { + uint256 _allowance = token.allowance[msg.sender][spender]; if (_allowance != type(uint256).max) { - _approve(self, msg.sender, spender, _allowance + addedValue); + _approve(token, msg.sender, spender, _allowance + addedValue); } return true; } @@ -185,24 +167,21 @@ library ERC20Lib { * - `spender` must have allowance for the caller of at least * `subtractedValue`. */ - function decreaseAllowance(TokenStorage storage self, address spender, uint256 subtractedValue) - external - returns (bool) - { - uint256 _allowance = self.allowance[msg.sender][spender]; + function decreaseAllowance(Token storage token, address spender, uint256 subtractedValue) external returns (bool) { + uint256 _allowance = token.allowance[msg.sender][spender]; if (_allowance != type(uint256).max) { if (_allowance < subtractedValue) { - revert ERC20InsufficientAllowance(msg.sender, _allowance, subtractedValue); + revert IERC20.InsufficientAllowance(msg.sender, _allowance, subtractedValue); } unchecked { - _approve(self, msg.sender, spender, _allowance - subtractedValue); + _approve(token, msg.sender, spender, _allowance - subtractedValue); } } return true; } function permit( - TokenStorage storage self, + Token storage token, address issuer, address spender, uint256 value, @@ -211,46 +190,48 @@ library ERC20Lib { bytes32 r, bytes32 s ) external { - if (block.timestamp > deadline) revert PermitExpired(); + if (block.timestamp > deadline) revert IERC20Permit.PermitExpired(); - if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) revert InvalidS(); + if (uint256(s) > 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0) { + revert IERC20Permit.InvalidS(); + } - if (v != 27 && v != 28) revert InvalidV(); + if (v != 27 && v != 28) revert IERC20Permit.InvalidV(); bytes32 digest = keccak256( abi.encodePacked( EIP191_PREFIX_FOR_EIP712_STRUCTURED_DATA, - self.domainSeparator, - keccak256(abi.encode(PERMIT_SIGNATURE_HASH, issuer, spender, value, self.nonces[issuer]++, deadline)) + token.domainSeparator, + keccak256(abi.encode(PERMIT_SIGNATURE_HASH, issuer, spender, value, token.nonces[issuer]++, deadline)) ) ); address recoveredAddress = ecrecover(digest, v, r, s); - if (recoveredAddress != issuer) revert InvalidSignature(); + if (recoveredAddress != issuer) revert IERC20Permit.InvalidSignature(); // _approve will revert if issuer is address(0x0) - _approve(self, issuer, spender, value); + _approve(token, issuer, spender, value); } - function balancesOf(TokenStorage storage self, address account) internal view returns (uint256) { - return self.balanceOf[account]; + function balancesOf(Token storage token, address account) internal view returns (uint256) { + return token.balance[account]; } - function noncesOf(TokenStorage storage self, address account) external view returns (uint256) { - return self.nonces[account]; + function noncesOf(Token storage token, address account) external view returns (uint256) { + return token.nonces[account]; } - function totalSupplyOf(TokenStorage storage self) external view returns (uint256) { - return self.totalSupply; + function totalSupplyOf(Token storage token) external view returns (uint256) { + return token.totalSupply; } - function allowanceOf(TokenStorage storage self, address owner, address spender) external view returns (uint256) { - return self.allowance[owner][spender]; + function allowanceOf(Token storage token, address owner, address spender) external view returns (uint256) { + return token.allowance[owner][spender]; } - function domainSeparatorOf(TokenStorage storage self) external view returns (bytes32) { - return self.domainSeparator; + function domainSeparatorOf(Token storage token) external view returns (bytes32) { + return token.domainSeparator; } /** @@ -267,10 +248,10 @@ library ERC20Lib { * - `recipient` cannot be the zero address. * - `sender` must have a balance of at least `amount`. */ - function _transfer(TokenStorage storage self, address sender, address recipient, uint256 amount) internal { - if (sender == address(0) || recipient == address(0)) revert InvalidAccount(); + function _transfer(Token storage token, address sender, address recipient, uint256 amount) internal { + if (sender == address(0) || recipient == address(0)) revert IERC20.Unauthorized(); - _update(self, sender, recipient, amount); + _update(token, sender, recipient, amount); } /** @@ -286,10 +267,10 @@ library ERC20Lib { * - `owner` cannot be the zero address. * - `spender` cannot be the zero address. */ - function _approve(TokenStorage storage self, address owner, address spender, uint256 amount) internal { - if (owner == address(0) || spender == address(0)) revert InvalidAccount(); + function _approve(Token storage token, address owner, address spender, uint256 amount) internal { + if (owner == address(0) || spender == address(0)) revert IERC20.Unauthorized(); - self.allowance[owner][spender] = amount; + token.allowance[owner][spender] = amount; emit IERC20.Approval(owner, spender, amount); } @@ -300,30 +281,30 @@ library ERC20Lib { * * Emits a {Transfer} event. */ - function _update(TokenStorage storage self, address from, address to, uint256 value) internal { + function _update(Token storage token, address from, address to, uint256 value) internal { if (from == address(0)) { // Overflow check required: The rest of the code assumes that totalSupply never overflows - self.totalSupply += value; + token.totalSupply += value; } else { - uint256 fromBalance = self.balanceOf[from]; + uint256 fromBalance = token.balance[from]; if (fromBalance < value) { - revert ERC20InsufficientBalance(from, fromBalance, value); + revert IERC20.InsufficientBalance(from, fromBalance, value); } unchecked { // Overflow not possible: value <= fromBalance <= totalSupply. - self.balanceOf[from] = fromBalance - value; + token.balance[from] = fromBalance - value; } } if (to == address(0)) { unchecked { // Overflow not possible: value <= totalSupply or value <= fromBalance <= totalSupply. - self.totalSupply -= value; + token.totalSupply -= value; } } else { unchecked { // Overflow not possible: balance + value is at most totalSupply, which we know fits into a uint256. - self.balanceOf[to] += value; + token.balance[to] += value; } } diff --git a/contracts/src/Types.sol b/contracts/src/Types.sol index 9cb32b7a79..02e8bd0acc 100644 --- a/contracts/src/Types.sol +++ b/contracts/src/Types.sol @@ -85,11 +85,12 @@ enum Command { TransferNativeFromAgent, SetTokenTransferFees, SetPricingParameters, - TransferToken, + TransferNativeToken, RegisterForeignToken, MintForeignToken } +/// @dev DEPRECATED enum AgentExecuteCommand { TransferToken } @@ -110,8 +111,5 @@ struct Ticket { struct TokenInfo { bool isRegistered; - bool isForeign; - bytes32 tokenID; - bytes32 agentID; - address token; + bytes32 foreignID; } diff --git a/contracts/src/interfaces/IERC20.sol b/contracts/src/interfaces/IERC20.sol index 7cbdbe64a8..b11e957b6b 100644 --- a/contracts/src/interfaces/IERC20.sol +++ b/contracts/src/interfaces/IERC20.sol @@ -8,7 +8,9 @@ pragma solidity 0.8.25; * @dev Interface of the ERC20 standard as defined in the EIP. */ interface IERC20 { - error InvalidAccount(); + error Unauthorized(); + error InsufficientBalance(address sender, uint256 balance, uint256 needed); + error InsufficientAllowance(address spender, uint256 allowance, uint256 needed); /** * @dev Returns the amount of tokens in existence. diff --git a/contracts/src/interfaces/IERC20Permit.sol b/contracts/src/interfaces/IERC20Permit.sol index 8a91e55cfd..bcd8163669 100644 --- a/contracts/src/interfaces/IERC20Permit.sol +++ b/contracts/src/interfaces/IERC20Permit.sol @@ -5,6 +5,11 @@ pragma solidity 0.8.25; interface IERC20Permit { + error PermitExpired(); + error InvalidS(); + error InvalidV(); + error InvalidSignature(); + function DOMAIN_SEPARATOR() external view returns (bytes32); function nonces(address account) external view returns (uint256); diff --git a/contracts/src/interfaces/IGateway.sol b/contracts/src/interfaces/IGateway.sol index 342cd7dbe8..37e725727a 100644 --- a/contracts/src/interfaces/IGateway.sol +++ b/contracts/src/interfaces/IGateway.sol @@ -36,7 +36,7 @@ interface IGateway { event AgentFundsWithdrawn(bytes32 indexed agentID, address indexed recipient, uint256 amount); // Emitted when foreign token from polkadot registed - event ForeignTokenRegistered(bytes32 indexed tokenID, bytes32 agentID, address token); + event ForeignTokenRegistered(bytes32 indexed tokenID, address token); /** * Getters diff --git a/contracts/src/storage/AssetsStorage.sol b/contracts/src/storage/AssetsStorage.sol index 8585555aeb..252c8a0247 100644 --- a/contracts/src/storage/AssetsStorage.sol +++ b/contracts/src/storage/AssetsStorage.sol @@ -16,8 +16,7 @@ library AssetsStorage { uint256 registerTokenFee; // Token registry by token address mapping(address token => TokenInfo) tokenRegistry; - // Token registry by tokenID - mapping(bytes32 tokenID => TokenInfo) tokenRegistryByID; + mapping(bytes32 foreignID => address) tokenAddressOf; } bytes32 internal constant SLOT = keccak256("org.snowbridge.storage.assets.v2"); diff --git a/contracts/test/Gateway.t.sol b/contracts/test/Gateway.t.sol index 2da9da986e..22776dc47e 100644 --- a/contracts/test/Gateway.t.sol +++ b/contracts/test/Gateway.t.sol @@ -25,7 +25,9 @@ import {Channel, InboundMessage, OperatingMode, ParaID, Command, ChannelID, Mult import {NativeTransferFailed} from "../src/utils/SafeTransfer.sol"; import {PricingStorage} from "../src/storage/PricingStorage.sol"; -import {ERC20Lib} from "../src/ERC20Lib.sol"; +import {IERC20} from "../src/interfaces/IERC20.sol"; +import {TokenLib} from "../src/TokenLib.sol"; +import {Token} from "../src/Token.sol"; import { UpgradeParams, @@ -38,7 +40,7 @@ import { SetTokenTransferFeesParams, SetPricingParametersParams, RegisterForeignTokenParams, - TransferTokenParams, + TransferNativeTokenParams, MintForeignTokenParams } from "../src/Params.sol"; @@ -54,7 +56,6 @@ import { import {WETH9} from "canonical-weth/WETH9.sol"; import {UD60x18, ud60x18, convert} from "prb/math/src/UD60x18.sol"; -import {ERC20} from "../src/ERC20.sol"; contract GatewayTest is Test { // Emitted when token minted/burnt/transfered @@ -359,11 +360,15 @@ contract GatewayTest is Test { function testAgentExecution() public { token.transfer(address(assetHubAgent), 200); - TransferTokenParams memory params = - TransferTokenParams({agentID: assetHubAgentID, token: address(token), recipient: account2, amount: 10}); + TransferNativeTokenParams memory params = TransferNativeTokenParams({ + agentID: assetHubAgentID, + token: address(token), + recipient: account2, + amount: 10 + }); bytes memory encodedParams = abi.encode(params); - MockGateway(address(gateway)).transferTokenPublic(encodedParams); + MockGateway(address(gateway)).transferNativeTokenPublic(encodedParams); } function testAgentExecutionBadOrigin() public { @@ -864,64 +869,34 @@ contract GatewayTest is Test { ); } - function testAgentRegisterToken() public { - RegisterForeignTokenParams memory params = RegisterForeignTokenParams({ - agentID: assetHubAgentID, - tokenID: dotTokenID, - name: "DOT", - symbol: "DOT", - decimals: 10 - }); + function testRegisterForeignToken() public { + RegisterForeignTokenParams memory params = + RegisterForeignTokenParams({foreignTokenID: dotTokenID, name: "DOT", symbol: "DOT", decimals: 10}); vm.expectEmit(true, true, false, false); - emit IGateway.ForeignTokenRegistered(bytes32(uint256(1)), assetHubAgentID, address(0)); - - MockGateway(address(gateway)).registerForeignTokenPublic(abi.encode(params)); - } - - function testAgentRegisterTokenWithAgentIDNotExistWillFail() public { - testAgentRegisterToken(); - - RegisterForeignTokenParams memory params = RegisterForeignTokenParams({ - agentID: bytes32(0), - tokenID: dotTokenID, - name: "DOT", - symbol: "DOT", - decimals: 10 - }); - - vm.expectRevert(Gateway.AgentDoesNotExist.selector); + emit IGateway.ForeignTokenRegistered(bytes32(uint256(1)), address(0)); MockGateway(address(gateway)).registerForeignTokenPublic(abi.encode(params)); } - function testAgentRegisterSameTokenAgainWillFail() public { - testAgentRegisterToken(); + function testRegisterForeignTokenDuplicateFail() public { + testRegisterForeignToken(); - RegisterForeignTokenParams memory params = RegisterForeignTokenParams({ - agentID: assetHubAgentID, - tokenID: dotTokenID, - name: "DOT", - symbol: "DOT", - decimals: 10 - }); + RegisterForeignTokenParams memory params = + RegisterForeignTokenParams({foreignTokenID: dotTokenID, name: "DOT", symbol: "DOT", decimals: 10}); vm.expectRevert(Assets.TokenAlreadyRegistered.selector); MockGateway(address(gateway)).registerForeignTokenPublic(abi.encode(params)); } - function testAgentMintToken() public { - testAgentRegisterToken(); + function testMintForeignToken() public { + testRegisterForeignToken(); uint256 amount = 1000; - MintForeignTokenParams memory params = MintForeignTokenParams({ - agentID: assetHubAgentID, - tokenID: bytes32(uint256(1)), - recipient: account1, - amount: amount - }); + MintForeignTokenParams memory params = + MintForeignTokenParams({foreignTokenID: bytes32(uint256(1)), recipient: account1, amount: amount}); vm.expectEmit(true, true, false, false); emit Transfer(address(0), account1, 1000); @@ -930,18 +905,14 @@ contract GatewayTest is Test { address dotToken = MockGateway(address(gateway)).tokenAddressOf(dotTokenID); - uint256 balance = ERC20(dotToken).balanceOf(account1); + uint256 balance = Token(dotToken).balanceOf(account1); assertEq(balance, amount); } - function testAgentMintNotRegisteredTokenWillFail() public { - MintForeignTokenParams memory params = MintForeignTokenParams({ - agentID: assetHubAgentID, - tokenID: bytes32(uint256(1)), - recipient: account1, - amount: 1000 - }); + function testMintNotRegisteredTokenWillFail() public { + MintForeignTokenParams memory params = + MintForeignTokenParams({foreignTokenID: bytes32(uint256(1)), recipient: account1, amount: 1000}); vm.expectRevert(Assets.TokenNotRegistered.selector); @@ -950,7 +921,7 @@ contract GatewayTest is Test { function testSendRelayTokenToAssetHub() public { // Register and then mint some DOT to account1 - testAgentMintToken(); + testMintForeignToken(); address dotToken = MockGateway(address(gateway)).tokenAddressOf(dotTokenID); @@ -977,7 +948,7 @@ contract GatewayTest is Test { } function testSendTokenFromNotMintedAccountWillFail() public { - testAgentRegisterToken(); + testRegisterForeignToken(); address dotToken = MockGateway(address(gateway)).tokenAddressOf(dotTokenID); @@ -985,7 +956,7 @@ contract GatewayTest is Test { vm.prank(account1); - vm.expectRevert(abi.encodeWithSelector(ERC20Lib.ERC20InsufficientBalance.selector, account1, 0, 1)); + vm.expectRevert(abi.encodeWithSelector(IERC20.InsufficientBalance.selector, account1, 0, 1)); IGateway(address(gateway)).sendToken{value: 0.1 ether}(address(dotToken), destPara, recipientAddress32, 1, 1); } diff --git a/contracts/test/mocks/MockGateway.sol b/contracts/test/mocks/MockGateway.sol index 01d6970416..4356bb9876 100644 --- a/contracts/test/mocks/MockGateway.sol +++ b/contracts/test/mocks/MockGateway.sol @@ -85,7 +85,7 @@ contract MockGateway is Gateway { this.mintForeignToken(params); } - function transferTokenPublic(bytes calldata params) external { - this.transferToken(params); + function transferNativeTokenPublic(bytes calldata params) external { + this.transferNativeToken(params); } } From 53608daeb8c6d6a55ae49f2cfa7071b0d2cbac61 Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Tue, 27 Aug 2024 19:04:29 +0200 Subject: [PATCH 2/4] More improvents to ERC20 token --- contracts/src/Token.sol | 47 ++++++++++++++++------------ contracts/src/TokenLib.sol | 48 +++++++++-------------------- contracts/src/interfaces/IERC20.sol | 2 +- 3 files changed, 42 insertions(+), 55 deletions(-) diff --git a/contracts/src/Token.sol b/contracts/src/Token.sol index 3e035ccbec..f66a9e34be 100644 --- a/contracts/src/Token.sol +++ b/contracts/src/Token.sol @@ -31,12 +31,15 @@ import {TokenLib} from "./TokenLib.sol"; contract Token is IERC20, IERC20Permit { using TokenLib for TokenLib.Token; - TokenLib.Token token; - address public immutable GATEWAY; + bytes32 public immutable DOMAIN_SEPARATOR; + uint8 public immutable decimals; + string public name; string public symbol; - uint8 public immutable decimals; + TokenLib.Token token; + + error Unauthorized(); /** * @dev Sets the values for {name}, {symbol}, and {decimals}. @@ -46,7 +49,15 @@ contract Token is IERC20, IERC20Permit { symbol = _symbol; decimals = _decimals; GATEWAY = msg.sender; - token.initialize(_name); + DOMAIN_SEPARATOR = keccak256( + abi.encode( + TokenLib.DOMAIN_TYPE_SIGNATURE_HASH, + keccak256(bytes(_name)), + keccak256(bytes("1")), + block.chainid, + address(this) + ) + ); } modifier onlyGateway() { @@ -66,14 +77,14 @@ contract Token is IERC20, IERC20Permit { * * - `account` cannot be the zero address. */ - function mint(address account, uint256 amount) external virtual onlyGateway { + function mint(address account, uint256 amount) external onlyGateway { token.mint(account, amount); } /** * @dev Destroys `amount` tokens from the account. */ - function burn(address account, uint256 amount) external virtual onlyGateway { + function burn(address account, uint256 amount) external onlyGateway { token.burn(account, amount); } @@ -85,7 +96,7 @@ contract Token is IERC20, IERC20Permit { * - `recipient` cannot be the zero address. * - the caller must have a balance of at least `amount`. */ - function transfer(address recipient, uint256 amount) external virtual override returns (bool) { + function transfer(address recipient, uint256 amount) external returns (bool) { return token.transfer(msg.sender, recipient, amount); } @@ -104,7 +115,7 @@ contract Token is IERC20, IERC20Permit { * * - `spender` cannot be the zero address. */ - function approve(address spender, uint256 amount) external virtual override returns (bool) { + function approve(address spender, uint256 amount) external returns (bool) { return token.approve(msg.sender, spender, amount); } @@ -121,7 +132,7 @@ contract Token is IERC20, IERC20Permit { * - the caller must have allowance for ``sender``'s tokens of at least * `amount`. */ - function transferFrom(address sender, address recipient, uint256 amount) external virtual override returns (bool) { + function transferFrom(address sender, address recipient, uint256 amount) external returns (bool) { return token.transferFrom(sender, recipient, amount); } @@ -137,7 +148,7 @@ contract Token is IERC20, IERC20Permit { * * - `spender` cannot be the zero address. */ - function increaseAllowance(address spender, uint256 addedValue) external virtual returns (bool) { + function increaseAllowance(address spender, uint256 addedValue) external returns (bool) { return token.increaseAllowance(spender, addedValue); } @@ -155,33 +166,29 @@ contract Token is IERC20, IERC20Permit { * - `spender` must have allowance for the caller of at least * `subtractedValue`. */ - function decreaseAllowance(address spender, uint256 subtractedValue) external virtual returns (bool) { + function decreaseAllowance(address spender, uint256 subtractedValue) external returns (bool) { return token.decreaseAllowance(spender, subtractedValue); } function permit(address issuer, address spender, uint256 value, uint256 deadline, uint8 v, bytes32 r, bytes32 s) external { - token.permit(issuer, spender, value, deadline, v, r, s); + token.permit(DOMAIN_SEPARATOR, issuer, spender, value, deadline, v, r, s); } function balanceOf(address account) external view returns (uint256) { - return token.balancesOf(account); + return token.balance[account]; } function nonces(address account) external view returns (uint256) { - return token.noncesOf(account); + return token.nonces[account]; } function totalSupply() external view returns (uint256) { - return token.totalSupplyOf(); + return token.totalSupply; } function allowance(address owner, address spender) external view returns (uint256) { - return token.allowanceOf(owner, spender); - } - - function DOMAIN_SEPARATOR() external view returns (bytes32) { - return token.domainSeparatorOf(); + return token.allowance[owner][spender]; } } diff --git a/contracts/src/TokenLib.sol b/contracts/src/TokenLib.sol index 036ffa9fe5..de21ea679f 100644 --- a/contracts/src/TokenLib.sol +++ b/contracts/src/TokenLib.sol @@ -23,15 +23,6 @@ library TokenLib { mapping(address account => mapping(address spender => uint256)) allowance; mapping(address token => uint256) nonces; uint256 totalSupply; - bytes32 domainSeparator; - } - - function initialize(Token storage token, string memory name) internal { - token.domainSeparator = keccak256( - abi.encode( - DOMAIN_TYPE_SIGNATURE_HASH, keccak256(bytes(name)), keccak256(bytes("1")), block.chainid, address(this) - ) - ); } /** @@ -58,7 +49,9 @@ library TokenLib { * - `to` cannot be the zero address. */ function mint(Token storage token, address account, uint256 amount) external { - if (account == address(0)) revert IERC20.Unauthorized(); + if (account == address(0)) { + revert IERC20.InvalidAccount(); + } _update(token, address(0), account, amount); } @@ -75,7 +68,9 @@ library TokenLib { * - `account` must have at least `amount` tokens. */ function burn(Token storage token, address account, uint256 amount) external { - if (account == address(0)) revert IERC20.Unauthorized(); + if (account == address(0)) { + revert IERC20.InvalidAccount(); + } _update(token, account, address(0), amount); } @@ -182,6 +177,7 @@ library TokenLib { function permit( Token storage token, + bytes32 domainSeparator, address issuer, address spender, uint256 value, @@ -201,7 +197,7 @@ library TokenLib { bytes32 digest = keccak256( abi.encodePacked( EIP191_PREFIX_FOR_EIP712_STRUCTURED_DATA, - token.domainSeparator, + domainSeparator, keccak256(abi.encode(PERMIT_SIGNATURE_HASH, issuer, spender, value, token.nonces[issuer]++, deadline)) ) ); @@ -214,26 +210,6 @@ library TokenLib { _approve(token, issuer, spender, value); } - function balancesOf(Token storage token, address account) internal view returns (uint256) { - return token.balance[account]; - } - - function noncesOf(Token storage token, address account) external view returns (uint256) { - return token.nonces[account]; - } - - function totalSupplyOf(Token storage token) external view returns (uint256) { - return token.totalSupply; - } - - function allowanceOf(Token storage token, address owner, address spender) external view returns (uint256) { - return token.allowance[owner][spender]; - } - - function domainSeparatorOf(Token storage token) external view returns (bytes32) { - return token.domainSeparator; - } - /** * @dev Moves tokens `amount` from `sender` to `recipient`. * @@ -249,7 +225,9 @@ library TokenLib { * - `sender` must have a balance of at least `amount`. */ function _transfer(Token storage token, address sender, address recipient, uint256 amount) internal { - if (sender == address(0) || recipient == address(0)) revert IERC20.Unauthorized(); + if (sender == address(0) || recipient == address(0)) { + revert IERC20.InvalidAccount(); + } _update(token, sender, recipient, amount); } @@ -268,7 +246,9 @@ library TokenLib { * - `spender` cannot be the zero address. */ function _approve(Token storage token, address owner, address spender, uint256 amount) internal { - if (owner == address(0) || spender == address(0)) revert IERC20.Unauthorized(); + if (owner == address(0) || spender == address(0)) { + revert IERC20.InvalidAccount(); + } token.allowance[owner][spender] = amount; emit IERC20.Approval(owner, spender, amount); diff --git a/contracts/src/interfaces/IERC20.sol b/contracts/src/interfaces/IERC20.sol index b11e957b6b..5e921d62db 100644 --- a/contracts/src/interfaces/IERC20.sol +++ b/contracts/src/interfaces/IERC20.sol @@ -8,7 +8,7 @@ pragma solidity 0.8.25; * @dev Interface of the ERC20 standard as defined in the EIP. */ interface IERC20 { - error Unauthorized(); + error InvalidAccount(); error InsufficientBalance(address sender, uint256 balance, uint256 needed); error InsufficientAllowance(address spender, uint256 allowance, uint256 needed); From e1bd6c52d4d25a286208393e4e955f1b60a7550e Mon Sep 17 00:00:00 2001 From: Vincent Geddes <117534+vgeddes@users.noreply.github.com> Date: Tue, 27 Aug 2024 19:40:46 +0200 Subject: [PATCH 3/4] Fix implementation of Assets._sendForeignToken --- contracts/src/Assets.sol | 92 ++++++++++++++++++++++++-------- contracts/src/SubstrateTypes.sol | 17 ++++++ 2 files changed, 87 insertions(+), 22 deletions(-) diff --git a/contracts/src/Assets.sol b/contracts/src/Assets.sol index 62f70b186c..d501f6e369 100644 --- a/contracts/src/Assets.sol +++ b/contracts/src/Assets.sol @@ -120,7 +120,14 @@ library Assets { ); } else { return _sendForeignToken( - info.foreignID, token, sender, destinationChain, destinationAddress, destinationChainFee, amount + info.foreignID, + token, + sender, + destinationChain, + destinationAddress, + destinationChainFee, + maxDestinationChainFee, + amount ); } } @@ -187,6 +194,36 @@ library Assets { emit IGateway.TokenSent(token, sender, destinationChain, destinationAddress, amount); } + function _sendForeignTokenCosts( + ParaID destinationChain, + uint128 destinationChainFee, + uint128 maxDestinationChainFee + ) internal view returns (Costs memory costs) { + AssetsStorage.Layout storage $ = AssetsStorage.layout(); + if ($.assetHubParaID == destinationChain) { + costs.foreign = $.assetHubReserveTransferFee; + } else { + // Reduce the ability for users to perform arbitrage by exploiting a + // favourable exchange rate. For example supplying Ether + // and gaining a more valuable amount of DOT on the destination chain. + // + // Also prevents users from mistakenly sending more fees than would be required + // which has negative effects like draining AssetHub's sovereign account. + // + // For safety, `maxDestinationChainFee` should be less valuable + // than the gas cost to send tokens. + if (destinationChainFee > maxDestinationChainFee) { + revert InvalidDestinationFee(); + } + + // If the final destination chain is not AssetHub, then the fee needs to additionally + // include the cost of executing an XCM on the final destination parachain. + costs.foreign = $.assetHubReserveTransferFee + destinationChainFee; + } + // We don't charge any extra fees beyond delivery costs + costs.native = 0; + } + // @dev Transfer Polkadot-native tokens back to Polkadot function _sendForeignToken( bytes32 foreignID, @@ -195,29 +232,45 @@ library Assets { ParaID destinationChain, MultiAddress calldata destinationAddress, uint128 destinationChainFee, + uint128 maxDestinationChainFee, uint128 amount ) internal returns (Ticket memory ticket) { - if (destinationChainFee == 0) { - revert InvalidDestinationFee(); - } + AssetsStorage.Layout storage $ = AssetsStorage.layout(); Token(token).burn(sender, amount); - ticket.dest = destinationChain; - ticket.costs = _sendForeignTokenCosts(destinationChainFee); + ticket.dest = $.assetHubParaID; + ticket.costs = _sendForeignTokenCosts(destinationChain, destinationChainFee, maxDestinationChainFee); - if (destinationAddress.isAddress32()) { - // The receiver has a 32-byte account ID - ticket.payload = SubstrateTypes.SendForeignTokenToAddress32( - foreignID, destinationChain, destinationAddress.asAddress32(), destinationChainFee, amount - ); - } else if (destinationAddress.isAddress20()) { - // The receiver has a 20-byte account ID - ticket.payload = SubstrateTypes.SendForeignTokenToAddress20( - foreignID, destinationChain, destinationAddress.asAddress20(), destinationChainFee, amount - ); + // Construct a message payload + if (destinationChain == $.assetHubParaID) { + // The funds will be minted into the receiver's account on AssetHub + if (destinationAddress.isAddress32()) { + // The receiver has a 32-byte account ID + ticket.payload = SubstrateTypes.SendTokenToAssetHubAddress32( + token, destinationAddress.asAddress32(), $.assetHubReserveTransferFee, amount + ); + } else { + // AssetHub does not support 20-byte account IDs + revert Unsupported(); + } } else { - revert Unsupported(); + if (destinationChainFee == 0) { + revert InvalidDestinationFee(); + } + if (destinationAddress.isAddress32()) { + // The receiver has a 32-byte account ID + ticket.payload = SubstrateTypes.SendForeignTokenToAddress32( + foreignID, destinationChain, destinationAddress.asAddress32(), destinationChainFee, amount + ); + } else if (destinationAddress.isAddress20()) { + // The receiver has a 20-byte account ID + ticket.payload = SubstrateTypes.SendForeignTokenToAddress20( + foreignID, destinationChain, destinationAddress.asAddress20(), destinationChainFee, amount + ); + } else { + revert Unsupported(); + } } emit IGateway.TokenSent(token, sender, destinationChain, destinationAddress, amount); @@ -259,11 +312,6 @@ library Assets { emit IGateway.TokenRegistrationSent(token); } - function _sendForeignTokenCosts(uint128 destinationChainFee) internal pure returns (Costs memory costs) { - costs.foreign = destinationChainFee; - costs.native = 0; - } - // @dev Register a new fungible Polkadot token for an agent function registerForeignToken(bytes32 foreignTokenID, string memory name, string memory symbol, uint8 decimals) external diff --git a/contracts/src/SubstrateTypes.sol b/contracts/src/SubstrateTypes.sol index 61bbb29933..0040297166 100644 --- a/contracts/src/SubstrateTypes.sol +++ b/contracts/src/SubstrateTypes.sol @@ -134,6 +134,23 @@ library SubstrateTypes { ); } + function SendForeignTokenToAssetHubAddress32(address token, bytes32 recipient, uint128 xcmFee, uint128 amount) + internal + view + returns (bytes memory) + { + return bytes.concat( + bytes1(0x00), + ScaleCodec.encodeU64(uint64(block.chainid)), + bytes1(0x02), + SubstrateTypes.H160(token), + bytes1(0x00), + recipient, + ScaleCodec.encodeU128(amount), + ScaleCodec.encodeU128(xcmFee) + ); + } + // destination is AccountID32 address function SendForeignTokenToAddress32( bytes32 tokenID, From 57fa08c1ad101f46a12fc469b75d30bd76658290 Mon Sep 17 00:00:00 2001 From: ron Date: Wed, 28 Aug 2024 13:37:47 +0800 Subject: [PATCH 4/4] Fix build payload for foreign token --- contracts/src/Assets.sol | 18 ++++++++++++++---- contracts/src/SubstrateTypes.sol | 16 ++++++++++------ 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/contracts/src/Assets.sol b/contracts/src/Assets.sol index d501f6e369..c88ead8846 100644 --- a/contracts/src/Assets.sol +++ b/contracts/src/Assets.sol @@ -247,8 +247,8 @@ library Assets { // The funds will be minted into the receiver's account on AssetHub if (destinationAddress.isAddress32()) { // The receiver has a 32-byte account ID - ticket.payload = SubstrateTypes.SendTokenToAssetHubAddress32( - token, destinationAddress.asAddress32(), $.assetHubReserveTransferFee, amount + ticket.payload = SubstrateTypes.SendForeignTokenToAssetHubAddress32( + foreignID, destinationAddress.asAddress32(), $.assetHubReserveTransferFee, amount ); } else { // AssetHub does not support 20-byte account IDs @@ -261,12 +261,22 @@ library Assets { if (destinationAddress.isAddress32()) { // The receiver has a 32-byte account ID ticket.payload = SubstrateTypes.SendForeignTokenToAddress32( - foreignID, destinationChain, destinationAddress.asAddress32(), destinationChainFee, amount + foreignID, + destinationChain, + destinationAddress.asAddress32(), + $.assetHubReserveTransferFee, + destinationChainFee, + amount ); } else if (destinationAddress.isAddress20()) { // The receiver has a 20-byte account ID ticket.payload = SubstrateTypes.SendForeignTokenToAddress20( - foreignID, destinationChain, destinationAddress.asAddress20(), destinationChainFee, amount + foreignID, + destinationChain, + destinationAddress.asAddress20(), + $.assetHubReserveTransferFee, + destinationChainFee, + amount ); } else { revert Unsupported(); diff --git a/contracts/src/SubstrateTypes.sol b/contracts/src/SubstrateTypes.sol index 0040297166..296f32ce57 100644 --- a/contracts/src/SubstrateTypes.sol +++ b/contracts/src/SubstrateTypes.sol @@ -134,7 +134,7 @@ library SubstrateTypes { ); } - function SendForeignTokenToAssetHubAddress32(address token, bytes32 recipient, uint128 xcmFee, uint128 amount) + function SendForeignTokenToAssetHubAddress32(bytes32 tokenID, bytes32 recipient, uint128 xcmFee, uint128 amount) internal view returns (bytes memory) @@ -143,7 +143,7 @@ library SubstrateTypes { bytes1(0x00), ScaleCodec.encodeU64(uint64(block.chainid)), bytes1(0x02), - SubstrateTypes.H160(token), + tokenID, bytes1(0x00), recipient, ScaleCodec.encodeU128(amount), @@ -157,6 +157,7 @@ library SubstrateTypes { ParaID paraID, bytes32 recipient, uint128 xcmFee, + uint128 destinationXcmFee, uint128 amount ) internal view returns (bytes memory) { return bytes.concat( @@ -167,8 +168,9 @@ library SubstrateTypes { bytes1(0x01), ScaleCodec.encodeU32(uint32(ParaID.unwrap(paraID))), recipient, - ScaleCodec.encodeU128(xcmFee), - ScaleCodec.encodeU128(amount) + ScaleCodec.encodeU128(destinationXcmFee), + ScaleCodec.encodeU128(amount), + ScaleCodec.encodeU128(xcmFee) ); } @@ -178,6 +180,7 @@ library SubstrateTypes { ParaID paraID, bytes20 recipient, uint128 xcmFee, + uint128 destinationXcmFee, uint128 amount ) internal view returns (bytes memory) { return bytes.concat( @@ -188,8 +191,9 @@ library SubstrateTypes { bytes1(0x02), ScaleCodec.encodeU32(uint32(ParaID.unwrap(paraID))), recipient, - ScaleCodec.encodeU128(xcmFee), - ScaleCodec.encodeU128(amount) + ScaleCodec.encodeU128(destinationXcmFee), + ScaleCodec.encodeU128(amount), + ScaleCodec.encodeU128(xcmFee) ); } }