From 6e6808d4eeb6d9cb83c8300c2066e6c6741abdea Mon Sep 17 00:00:00 2001 From: Massil Achab Date: Thu, 7 Mar 2024 10:15:56 +0100 Subject: [PATCH] feat: make StarknetTokenBridge ERC2771 compliant --- README.md | 4 ++ src/solidity/ERC2771Recipient.sol | 63 ++++++++++++++++++++++++++++ src/solidity/IERC2771Recipient.sol | 36 ++++++++++++++++ src/solidity/StarknetTokenBridge.sol | 32 +++++++------- 4 files changed, 120 insertions(+), 15 deletions(-) create mode 100644 src/solidity/ERC2771Recipient.sol create mode 100644 src/solidity/IERC2771Recipient.sol diff --git a/README.md b/README.md index b27f218..238c9bd 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,10 @@ Note: the frontend implementation of the bridges, can be found [here](https://gi This project contains scripts written in Python 3.9. +## EIP2771 compliance + +We made the L1 bridge EIP2771 compliant by inheriting from [ERC2771Recipient.sol](https://github.com/opengsn/gsn/blob/master/packages/contracts/src/ERC2771Recipient.sol). + ## Getting Started diff --git a/src/solidity/ERC2771Recipient.sol b/src/solidity/ERC2771Recipient.sol new file mode 100644 index 0000000..5e674ee --- /dev/null +++ b/src/solidity/ERC2771Recipient.sol @@ -0,0 +1,63 @@ +// SPDX-License-Identifier: MIT +// solhint-disable no-inline-assembly +pragma solidity >=0.6.9; + +import "./IERC2771Recipient.sol"; + +/** + * @title The ERC-2771 Recipient Base Abstract Class - Implementation + * + * @notice Note that this contract was called `BaseRelayRecipient` in the previous revision of the GSN. + * + * @notice A base contract to be inherited by any contract that want to receive relayed transactions. + * + * @notice A subclass must use `_msgSender()` instead of `msg.sender`. + */ +abstract contract ERC2771Recipient is IERC2771Recipient { + + /* + * Forwarder singleton we accept calls from + */ + address private _trustedForwarder; + + /** + * :warning: **Warning** :warning: The Forwarder can have a full control over your Recipient. Only trust verified Forwarder. + * @notice Method is not a required method to allow Recipients to trust multiple Forwarders. Not recommended yet. + * @return forwarder The address of the Forwarder contract that is being used. + */ + function getTrustedForwarder() public virtual view returns (address forwarder){ + return _trustedForwarder; + } + + function _setTrustedForwarder(address _forwarder) internal { + _trustedForwarder = _forwarder; + } + + /// @inheritdoc IERC2771Recipient + function isTrustedForwarder(address forwarder) public virtual override view returns(bool) { + return forwarder == _trustedForwarder; + } + + /// @inheritdoc IERC2771Recipient + function _msgSender() internal override virtual view returns (address ret) { + if (msg.data.length >= 20 && isTrustedForwarder(msg.sender)) { + // At this point we know that the sender is a trusted forwarder, + // so we trust that the last bytes of msg.data are the verified sender address. + // extract sender address from the end of msg.data + assembly { + ret := shr(96,calldataload(sub(calldatasize(),20))) + } + } else { + ret = msg.sender; + } + } + + /// @inheritdoc IERC2771Recipient + function _msgData() internal override virtual view returns (bytes calldata ret) { + if (msg.data.length >= 20 && isTrustedForwarder(msg.sender)) { + return msg.data[0:msg.data.length-20]; + } else { + return msg.data; + } + } +} diff --git a/src/solidity/IERC2771Recipient.sol b/src/solidity/IERC2771Recipient.sol new file mode 100644 index 0000000..535909c --- /dev/null +++ b/src/solidity/IERC2771Recipient.sol @@ -0,0 +1,36 @@ +// SPDX-License-Identifier: MIT +pragma solidity >=0.6.0; + +/** + * @title The ERC-2771 Recipient Base Abstract Class - Declarations + * + * @notice A contract must implement this interface in order to support relayed transaction. + * + * @notice It is recommended that your contract inherits from the ERC2771Recipient contract. + */ +abstract contract IERC2771Recipient { + + /** + * :warning: **Warning** :warning: The Forwarder can have a full control over your Recipient. Only trust verified Forwarder. + * @param forwarder The address of the Forwarder contract that is being used. + * @return isTrustedForwarder `true` if the Forwarder is trusted to forward relayed transactions by this Recipient. + */ + function isTrustedForwarder(address forwarder) public virtual view returns(bool); + + /** + * @notice Use this method the contract anywhere instead of msg.sender to support relayed transactions. + * @return sender The real sender of this call. + * For a call that came through the Forwarder the real sender is extracted from the last 20 bytes of the `msg.data`. + * Otherwise simply returns `msg.sender`. + */ + function _msgSender() internal virtual view returns (address); + + /** + * @notice Use this method in the contract instead of `msg.data` when difference matters (hashing, signature, etc.) + * @return data The real `msg.data` of this call. + * For a call that came through the Forwarder, the real sender address was appended as the last 20 bytes + * of the `msg.data` - so this method will strip those 20 bytes off. + * Otherwise (if the call was made directly and not through the forwarder) simply returns `msg.data`. + */ + function _msgData() internal virtual view returns (bytes calldata); +} diff --git a/src/solidity/StarknetTokenBridge.sol b/src/solidity/StarknetTokenBridge.sol index dbe244b..0d33383 100644 --- a/src/solidity/StarknetTokenBridge.sol +++ b/src/solidity/StarknetTokenBridge.sol @@ -9,6 +9,7 @@ import "starkware/solidity/libraries/Transfers.sol"; import "starkware/solidity/tokens/ERC20/IERC20.sol"; import "starkware/solidity/tokens/ERC20/IERC20Metadata.sol"; import "starkware/starknet/solidity/IStarknetMessaging.sol"; +import "src/solidity/ERC2771Recipient.sol"; import "src/solidity/Fees.sol"; import "src/solidity/IStarkgateBridge.sol"; import "src/solidity/IStarkgateManager.sol"; @@ -25,7 +26,8 @@ contract StarknetTokenBridge is IStarkgateService, Identity, StarknetTokenStorage, - ProxySupport + ProxySupport, + ERC2771Recipient { using Addresses for address; using Felt252 for string; @@ -125,7 +127,7 @@ contract StarknetTokenBridge is } modifier onlyManager() { - require(manager() == msg.sender, "ONLY_MANAGER"); + require(manager() == _msgSender(), "ONLY_MANAGER"); _; } @@ -152,7 +154,7 @@ contract StarknetTokenBridge is Fees.checkDepositFee(msg.value); uint256 currentBalance = IERC20(token).balanceOf(address(this)); require(currentBalance + amount <= getMaxTotalBalance(token), "MAX_BALANCE_EXCEEDED"); - Transfers.transferIn(token, msg.sender, amount); + Transfers.transferIn(token, _msgSender(), amount); return msg.value; } @@ -320,10 +322,10 @@ contract StarknetTokenBridge is uint256 fee ) internal { if (selector == HANDLE_TOKEN_DEPOSIT_SELECTOR) { - emit Deposit(msg.sender, token, amount, l2Recipient, nonce, fee); + emit Deposit(_msgSender(), token, amount, l2Recipient, nonce, fee); } else { require(selector == HANDLE_DEPOSIT_WITH_MESSAGE_SELECTOR, "UNKNOWN_SELECTOR"); - emit DepositWithMessage(msg.sender, token, amount, l2Recipient, message, nonce, fee); + emit DepositWithMessage(_msgSender(), token, amount, l2Recipient, message, nonce, fee); } } @@ -339,7 +341,7 @@ contract StarknetTokenBridge is */ function enableWithdrawalLimit(address token) external onlySecurityAgent { tokenSettings()[token].withdrawalLimitApplied = true; - emit WithdrawalLimitEnabled(msg.sender, token); + emit WithdrawalLimitEnabled(_msgSender(), token); } /** @@ -347,7 +349,7 @@ contract StarknetTokenBridge is */ function disableWithdrawalLimit(address token) external onlySecurityAdmin { tokenSettings()[token].withdrawalLimitApplied = false; - emit WithdrawalLimitDisabled(msg.sender, token); + emit WithdrawalLimitDisabled(_msgSender(), token); } /** @@ -397,7 +399,7 @@ contract StarknetTokenBridge is : N_DEPOSIT_PAYLOAD_ARGS; uint256[] memory payload = new uint256[](MESSAGE_OFFSET + message.length); payload[0] = uint256(uint160(token)); - payload[1] = uint256(uint160(msg.sender)); + payload[1] = uint256(uint160(_msgSender())); payload[2] = l2Recipient; payload[3] = amount & (UINT256_PART_SIZE - 1); payload[4] = amount >> UINT256_PART_SIZE_BITS; @@ -499,7 +501,7 @@ contract StarknetTokenBridge is } function withdraw(address token, uint256 amount) external { - withdraw(token, amount, msg.sender); + withdraw(token, amount, _msgSender()); } /* @@ -525,7 +527,7 @@ contract StarknetTokenBridge is nonce ); - emit DepositCancelRequest(msg.sender, token, amount, l2Recipient, nonce); + emit DepositCancelRequest(_msgSender(), token, amount, l2Recipient, nonce); } /* @@ -552,7 +554,7 @@ contract StarknetTokenBridge is ); emit DepositWithMessageCancelRequest( - msg.sender, + _msgSender(), token, amount, l2Recipient, @@ -581,8 +583,8 @@ contract StarknetTokenBridge is nonce ); - transferOutFunds(token, amount, msg.sender); - emit DepositWithMessageReclaimed(msg.sender, token, amount, l2Recipient, message, nonce); + transferOutFunds(token, amount, _msgSender()); + emit DepositWithMessageReclaimed(_msgSender(), token, amount, l2Recipient, message, nonce); } function depositReclaim( @@ -598,7 +600,7 @@ contract StarknetTokenBridge is nonce ); - transferOutFunds(token, amount, msg.sender); - emit DepositReclaimed(msg.sender, token, amount, l2Recipient, nonce); + transferOutFunds(token, amount, _msgSender()); + emit DepositReclaimed(_msgSender(), token, amount, l2Recipient, nonce); } }