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

Macro Audit Findings Fixes #17

Merged
merged 6 commits into from
Jul 10, 2024
Merged
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
2 changes: 1 addition & 1 deletion .prettierrc.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
bracketSpacing: true
printWidth: 120
printWidth: 121
proseWrap: "always"
singleQuote: false
tabWidth: 2
Expand Down
2 changes: 1 addition & 1 deletion .solhint.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"compiler-version": ["error", ">=0.8.25"],
"func-name-mixedcase": "off",
"func-visibility": ["error", { "ignoreConstructors": true }],
"max-line-length": ["error", 120],
"max-line-length": ["error", 121],
"named-parameters-mapping": "warn",
"no-console": "off",
"not-rely-on-time": "off",
Expand Down
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ order for Renzo to enable bridging the ezETH token it would need to:

## An XERC20 token on Ethereum with a XERC20 counterpart token on Arbitrum

There are a few prerequisites to keep in mind for registering a token in the router associating it to a specific
gateway.
There are a few prerequisites to keep in mind for registering a token in the router associating it to a specific gateway.

First of all, the L1 counterpart of the token must conform to the ICustomToken interface. This means that:

Expand All @@ -50,15 +49,16 @@ In order to be able to use this approach it would be required to:

- BootNode's UI PR to be merged (TODO add PR link)

**Anyone**
**Some trusted entity**

- Deploy both `L1XERC20Gateway` and `L1XERC20Gateway`

**XERC20 Token Issuer**

1. Deploy an `L1XERC20Adapter` if the XERC20 token
2. Call the `registerTokenOnL2` function on the deployed `L1XERC20Adapter`
3. Set the deployed `L1XERC20Adapter` as a `bridge` on the XERC20 token
3. Set the `L1XERC20Gateway` as a `bridge` on the XERC20 token. **WARNING:** Token issuer must ensure the previous step
was properly executed before this one.
4. Make a PR to [Arbitrum's UI repository](https://github.com/OffchainLabs/arbitrum-token-bridge) adding the L2 XERC20
token to
[L2ApprovalUtils](https://github.com/OffchainLabs/arbitrum-token-bridge/blob/master/packages/arb-token-bridge-ui/src/util/L2ApprovalUtils.ts)
Expand Down
Binary file modified bun.lockb
Binary file not shown.
20 changes: 10 additions & 10 deletions docs/deployment.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ Notice that the values used for `GATEWAY_SALT` can be used only once on each net

**L1XERC20Gateway** Required environment variables:

- `GATEWAY_SALT`: Salt used for deploying the contract through `CREATE3Factory`. **IMPORTANT:** make sure to use the
same address for `L2XERC20Gateway`
- `GATEWAY_SALT`: Salt used for deploying the contract through `CREATE3Factory`. **IMPORTANT:** make sure to use the same
address for `L2XERC20Gateway`
- `DEPLOYER_PK`: Your deployer address private key. **IMPORTANT:** make sure to use the same address for
`L2XERC20Gateway`
- `CREATE3_FACTORY`: Address of the `CREATE3Factory`. **IMPORTANT:** make sure to use the same address for
Expand All @@ -52,8 +52,8 @@ $ yarn deploy:gateway

**L2XER20Gateway** Required environment variables:

- `GATEWAY_SALT`: Salt used for deploying the contract through `CREATE3Factory`. **IMPORTANT:** make sure to use the
same address for `L1XERC20Gateway`
- `GATEWAY_SALT`: Salt used for deploying the contract through `CREATE3Factory`. **IMPORTANT:** make sure to use the same
address for `L1XERC20Gateway`
- `DEPLOYER_PK`: Your deployer address private key. **IMPORTANT:** make sure to use the same address for
`L1XERC20Gateway`
- `CREATE3_FACTORY`: Address of the `CREATE3Factory`. **IMPORTANT:** make sure to use the same address for
Expand Down Expand Up @@ -82,8 +82,8 @@ Notice that the values used for `GATEWAY_SALT` can be used only once on each net

**L1LockboxGateway** Required environment variables:

- `GATEWAY_SALT`: Salt used for deploying the contract through `CREATE3Factory`. **IMPORTANT:** make sure to use the
same address for `L2XERC20Gateway`
- `GATEWAY_SALT`: Salt used for deploying the contract through `CREATE3Factory`. **IMPORTANT:** make sure to use the same
address for `L2XERC20Gateway`
- `DEPLOYER_PK`: Your deployer address private key. **IMPORTANT:** make sure to use the same address for
`L2XERC20Gateway`
- `CREATE3_FACTORY`: Address of the `CREATE3Factory`. **IMPORTANT:** make sure to use the same address for
Expand All @@ -108,8 +108,8 @@ $ yarn deploy:lockboxGateway

**L2LockboxGateway** Required environment variables:

- `GATEWAY_SALT`: Salt used for deploying the contract through `CREATE3Factory`. **IMPORTANT:** make sure to use the
same address for `L1XERC20Gateway`
- `GATEWAY_SALT`: Salt used for deploying the contract through `CREATE3Factory`. **IMPORTANT:** make sure to use the same
address for `L1XERC20Gateway`
- `DEPLOYER_PK`: Your deployer address private key. **IMPORTANT:** make sure to use the same address for
`L1XERC20Gateway`
- `CREATE3_FACTORY`: Address of the `CREATE3Factory`. **IMPORTANT:** make sure to use the same address for
Expand All @@ -135,8 +135,8 @@ $ yarn deploy:lockboxGateway:arb

**XERC20**

The XERC20 token is deployed using a `XERC20Factory` contract. In order to have the token deployed on the same address
on both networks you must to use the same `DEPLOYER_PK`, `XERC20_FACTORY`, `XERC20_NAME` and `XERC20_SYMBOL` values for
The XERC20 token is deployed using a `XERC20Factory` contract. In order to have the token deployed on the same address on
both networks you must to use the same `DEPLOYER_PK`, `XERC20_FACTORY`, `XERC20_NAME` and `XERC20_SYMBOL` values for
running bot scripts.

Required environment variables:
Expand Down
2 changes: 1 addition & 1 deletion foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
[fmt]
bracket_spacing = true
int_types = "long"
line_length = 120
line_length = 121
multiline_func_header = "all"
number_underscore = "thousands"
quote_style = "double"
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"sol:fmt": "forge fmt",
"prettier:check": "prettier --check \"**/*.{json,md,yml}\" --ignore-path \".prettierignore\"",
"prettier:write": "prettier --write \"**/*.{json,md,yml}\" --ignore-path \".prettierignore\"",
"test": "forge test",
"test": "forge test -vvv",
"test:coverage": "forge coverage",
"test:coverage:report": "forge coverage --report lcov && genhtml lcov.info --branch-coverage --output-dir coverage",
"factory:sepolia": "forge script script/XERC20/XERC20Factory.s.sol:XERC20FactoryDeploy -f sepolia --broadcast --verify --etherscan-api-key $ETHERSCAN_API_KEY --chain sepolia -vvvv",
Expand Down
1 change: 1 addition & 0 deletions src/L1ArbitrumEnabledXERC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ contract L1ArbitrumEnabledXERC20 is XERC20, L1ArbitrumEnabled {
payable
override
onlyOwner
checkValue(valueForGateway + valueForRouter)
{
_registerTokenOnL2(
l2TokenAddress,
Expand Down
13 changes: 1 addition & 12 deletions src/L1LockboxGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,7 @@ contract L1LockboxGateway is XERC20BaseGateway, L1CustomGateway {
/**
* @dev Not implemented since L1/L2 token relation is being built upon deployment.
*/
function registerTokenToL2(
address,
uint256,
uint256,
uint256
)
external
payable
virtual
override
returns (uint256)
{
function registerTokenToL2(address, uint256, uint256, uint256) external payable virtual override returns (uint256) {
revert NotImplementedFunction();
}

Expand Down
10 changes: 7 additions & 3 deletions src/L1XERC20Adapter.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;

import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
import { XERC20BaseAdapter } from "src/XERC20BaseAdapter.sol";
import { L1ArbitrumEnabled } from "src/libraries/L1ArbitrumEnabled.sol";

Expand All @@ -14,7 +15,7 @@ import { L1ArbitrumEnabled } from "src/libraries/L1ArbitrumEnabled.sol";
*
* @author BootNode
*/
contract L1XERC20Adapter is XERC20BaseAdapter, L1ArbitrumEnabled {
contract L1XERC20Adapter is XERC20BaseAdapter, L1ArbitrumEnabled, Ownable {
/**
* @dev Sets the XERC20 token, the gateway and the owner.
*/
Expand All @@ -23,9 +24,11 @@ contract L1XERC20Adapter is XERC20BaseAdapter, L1ArbitrumEnabled {
address _gatewayAddress,
address _owner
)
XERC20BaseAdapter(_xerc20, _owner)
XERC20BaseAdapter(_xerc20)
L1ArbitrumEnabled(_gatewayAddress)
{ }
{
_transferOwnership(_owner);
}

/**
* @dev Sets the token/gateway relation on Arbitrum Router and registers the token on L2 counterpart gateway.
Expand Down Expand Up @@ -55,6 +58,7 @@ contract L1XERC20Adapter is XERC20BaseAdapter, L1ArbitrumEnabled {
payable
override
onlyOwner
checkValue(valueForGateway + valueForRouter)
{
_registerTokenOnL2(
l2TokenAddress,
Expand Down
108 changes: 105 additions & 3 deletions src/L1XERC20Gateway.sol
Original file line number Diff line number Diff line change
@@ -1,20 +1,41 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;

import { ERC165Checker } from "@openzeppelin/contracts/utils/introspection/ERC165Checker.sol";

import { L1CustomGateway } from "@arbitrum/tokenbridge/ethereum/gateway/L1CustomGateway.sol";

import { XERC20BaseGateway } from "src/XERC20BaseGateway.sol";

import { IXERC20Adapter } from "src/interfaces/IXERC20Adapter.sol";

/**
* @title L1XERC20Gateway
* @dev A Custom gateway that allows XERC20 tokens to be bridged through the Arbitrum canonical bridge.
* @notice A Custom gateway that allows XERC20 tokens to be bridged through the Arbitrum canonical bridge.
* This gateway should be set as a bridge for the XERC20 token, and the user should previously grant approval to this
* contract before sending the tokens to Arbitrum.
* Burns L1 XERC20 when sending to Arbitrum and mints it when the user sends tokens from Arbitrum.
*
* This contract facilitates the bridging of multiple token pairs, enhancing the user experience for token issuers by
* allowing them to deploy just one smart contract (L1XERC20Adapter) rather than three (L1XERC20Adapter, L1XERC20Gateway
* and L2XERC20Gateway). The primary trade-off is that issuers of XERC20 tokens must ensure the `registerTokenToL2`
* function is properly executed by their L1XERC20Adapter before they set minting and burning limits for this contract.
*
* @author BootNode
*/
contract L1XERC20Gateway is XERC20BaseGateway, L1CustomGateway {
using ERC165Checker for address;

// stores addresses of registered tokens
mapping(address => address) public adapterToToken;
mapping(address => bool) public l1RegisteredTokens;
mapping(address => bool) public l2RegisteredTokens;

error AlreadyRegisteredL1Token();
error AlreadyRegisteredL2Token();
error InvalidLengths();
error NotRegisteredToken();

/**
* @dev Sets the arbitrum router, inbox and the owner of this contract.
*/
Expand All @@ -23,6 +44,79 @@ contract L1XERC20Gateway is XERC20BaseGateway, L1CustomGateway {
initialize(_l2Counterpart, _l1Router, _inbox, _owner);
}

function addressIsAdapter(address _tokenOrAdapter) public view returns (bool) {
return _tokenOrAdapter.supportsInterface(type(IXERC20Adapter).interfaceId);
}

/**
* @notice Override L1CustomGateway function for validating already registered tokens.
*/
function registerTokenToL2(
address _l2Address,
uint256 _maxGas,
uint256 _gasPriceBid,
uint256 _maxSubmissionCost,
address _creditBackAddress
)
public
payable
virtual
override
returns (uint256)
{
address _l1Token = msg.sender;

if (addressIsAdapter(msg.sender)) {
_l1Token = IXERC20Adapter(_l1Token).getXERC20();
}

if (l1RegisteredTokens[_l1Token]) revert AlreadyRegisteredL1Token();
if (l2RegisteredTokens[_l2Address]) revert AlreadyRegisteredL2Token();

adapterToToken[msg.sender] = _l1Token;
l1RegisteredTokens[_l1Token] = true;
l2RegisteredTokens[_l2Address] = true;

return _registerTokenToL2(_l2Address, _maxGas, _gasPriceBid, _maxSubmissionCost, _creditBackAddress, msg.value);
}

/**
* @notice Override L1CustomGateway function setting l1RegisteredTokens and l2RegisteredTokens.
* It assumes the owner checked both addresses offchain before force registering. Multi-sig or governance is
* recommended be used as the owner
*/
function forceRegisterTokenToL2(
address[] calldata _l1Addresses,
address[] calldata _l2Addresses,
uint256 _maxGas,
uint256 _gasPriceBid,
uint256 _maxSubmissionCost
)
external
payable
virtual
override
onlyOwner
returns (uint256)
{
if (_l1Addresses.length != _l2Addresses.length) revert InvalidLengths();

address _l1Token;
for (uint256 i = 0; i < _l1Addresses.length; i++) {
_l1Token = _l1Addresses[i];

if (addressIsAdapter(_l1Addresses[i])) {
_l1Token = IXERC20Adapter(_l1Token).getXERC20();
}

adapterToToken[_l1Addresses[i]] = _l1Token;
l1RegisteredTokens[_l1Token] = true;
l2RegisteredTokens[_l2Addresses[i]] = true;
}

return _forceRegisterTokenToL2(_l1Addresses, _l2Addresses, _maxGas, _gasPriceBid, _maxSubmissionCost, msg.value);
}

/**
* @dev This function is called inside the `outboundTransferCustomRefund` when the token is being bridged from
* Ethereum to Arbitrum.
Expand All @@ -40,7 +134,11 @@ contract L1XERC20Gateway is XERC20BaseGateway, L1CustomGateway {
override
returns (uint256 amountReceived)
{
return _outboundEscrowTransfer(_l1TokenOrAdapter, _from, _amount);
address _token = adapterToToken[_l1TokenOrAdapter];

if (_token == address(0)) revert NotRegisteredToken();

return _outboundEscrowTransfer(_token, _from, _amount);
}

/**
Expand All @@ -51,6 +149,10 @@ contract L1XERC20Gateway is XERC20BaseGateway, L1CustomGateway {
* @param _amount Amount of tokens
*/
function inboundEscrowTransfer(address _l1TokenOrAdapter, address _dest, uint256 _amount) internal override {
_inboundEscrowTransfer(_l1TokenOrAdapter, _dest, _amount);
address _token = adapterToToken[_l1TokenOrAdapter];

if (_token == address(0)) revert NotRegisteredToken();

_inboundEscrowTransfer(_token, _dest, _amount);
}
}
18 changes: 4 additions & 14 deletions src/L2XERC20Gateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -125,40 +125,30 @@ contract L2XERC20Gateway is XERC20BaseGateway, L2CustomGateway {

/**
* @notice Mint on L2 upon L1 deposit.
* If token not yet deployed and symbol/name/decimal data is included, deploys StandardArbERC20
* @dev Callable only by the L1ERC20Gateway.outboundTransfer method. For initial deployments of a token the L1
* L1ERC20Gateway
* is expected to include the deployData. If not a L1 withdrawal is automatically triggered for the user
* @dev Callable only by the L1ERC20Gateway.outboundTransfer method. If L2Token is not yet deployed a L1 withdrawal
* is automatically triggered for the user.
* @param _token L1 address of ERC20
* @param _from account that initiated the deposit in the L1
* @param _to account to be credited with the tokens in the L2 (can be the user's L2 account or a contract)
* @param _amount token amount to be minted to the user
* @param _data encoded symbol/name/decimal data for deploy, in addition to any additional callhook data
*/
function finalizeInboundTransfer(
address _token,
address _from,
address _to,
uint256 _amount,
bytes calldata _data
bytes calldata
)
external
payable
virtual
override
onlyCounterpartGateway
{
(bytes memory gatewayData, bytes memory callHookData) = GatewayMessageHandler.parseFromL1GatewayMsg(_data);

if (callHookData.length != 0) {
// callHookData should always be 0 since inboundEscrowAndCall is disabled
callHookData = bytes("");
}

address expectedAddress = calculateL2TokenAddress(_token);

if (!expectedAddress.isContract()) {
bool shouldHalt = handleNoContract(_token, expectedAddress, _from, _to, _amount, gatewayData);
bool shouldHalt = handleNoContract(_token, expectedAddress, _from, _to, _amount, bytes(""));
if (shouldHalt) return;
}
// ignores gatewayData if token already deployed
Expand Down
Loading