Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make StarknetTokenBridge EIC2771 compliant #16

Open
wants to merge 2 commits into
base: cairo-1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
63 changes: 63 additions & 0 deletions src/solidity/ERC2771Recipient.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// SPDX-License-Identifier: MIT
// solhint-disable no-inline-assembly
pragma solidity >=0.6.9;

import "starkware/solidity/libraries/NamedStorage.sol";
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
*/
string internal constant TRUSTED_FORWARDER_TAG = "STARKNET_TOKEN_BRIDGE_TRUSTED_FORWARDER_TAG";

/**
* :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 view virtual returns (address forwarder) {
return NamedStorage.getAddressValue(TRUSTED_FORWARDER_TAG);
}

function _setTrustedForwarder(address _forwarder) internal {
NamedStorage.setAddressValueOnce(TRUSTED_FORWARDER_TAG, _forwarder);
}

/// @inheritdoc IERC2771Recipient
function isTrustedForwarder(address forwarder) public view virtual override returns (bool) {
return forwarder == NamedStorage.getAddressValue(TRUSTED_FORWARDER_TAG);
}

/// @inheritdoc IERC2771Recipient
function _msgSender() internal view virtual override 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 view virtual override returns (bytes calldata ret) {
if (msg.data.length >= 20 && isTrustedForwarder(msg.sender)) {
return msg.data[0:msg.data.length - 20];
} else {
return msg.data;
}
}
}
35 changes: 35 additions & 0 deletions src/solidity/IERC2771Recipient.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
// 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 view virtual 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 view virtual 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 view virtual returns (bytes calldata);
}
42 changes: 27 additions & 15 deletions src/solidity/StarknetTokenBridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -25,7 +26,8 @@ contract StarknetTokenBridge is
IStarkgateService,
Identity,
StarknetTokenStorage,
ProxySupport
ProxySupport,
ERC2771Recipient
{
using Addresses for address;
using Felt252 for string;
Expand Down Expand Up @@ -125,7 +127,7 @@ contract StarknetTokenBridge is
}

modifier onlyManager() {
require(manager() == msg.sender, "ONLY_MANAGER");
require(manager() == _msgSender(), "ONLY_MANAGER");
_;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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);
}
}

Expand All @@ -339,15 +341,15 @@ contract StarknetTokenBridge is
*/
function enableWithdrawalLimit(address token) external onlySecurityAgent {
tokenSettings()[token].withdrawalLimitApplied = true;
emit WithdrawalLimitEnabled(msg.sender, token);
emit WithdrawalLimitEnabled(_msgSender(), token);
}

/**
Unset withdrawal limit for a token.
*/
function disableWithdrawalLimit(address token) external onlySecurityAdmin {
tokenSettings()[token].withdrawalLimitApplied = false;
emit WithdrawalLimitDisabled(msg.sender, token);
emit WithdrawalLimitDisabled(_msgSender(), token);
}

/**
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -499,7 +501,7 @@ contract StarknetTokenBridge is
}

function withdraw(address token, uint256 amount) external {
withdraw(token, amount, msg.sender);
withdraw(token, amount, _msgSender());
}

/*
Expand All @@ -525,7 +527,7 @@ contract StarknetTokenBridge is
nonce
);

emit DepositCancelRequest(msg.sender, token, amount, l2Recipient, nonce);
emit DepositCancelRequest(_msgSender(), token, amount, l2Recipient, nonce);
}

/*
Expand All @@ -552,7 +554,7 @@ contract StarknetTokenBridge is
);

emit DepositWithMessageCancelRequest(
msg.sender,
_msgSender(),
token,
amount,
l2Recipient,
Expand Down Expand Up @@ -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(
Expand All @@ -598,7 +600,17 @@ 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);
}

// EIP2771 related functions

function setTrustedForwarder(address _forwarder) public onlyManager {
_setTrustedForwarder(_forwarder);
}

function versionRecipient() external pure returns (string memory) {
return "1";
}
}