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.

Sign up for GitHub

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

fix: Refactor Arbitrum_Adapter #214

Merged
merged 13 commits into from
Dec 23, 2022
Merged

fix: Refactor Arbitrum_Adapter #214

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8ffa355
fix: Revert use of outboundTransferCustomRefund
nicholaspai Dec 21, 2022
62d870f
Update Arbitrum_Adapter.ts
nicholaspai Dec 21, 2022
1f9d204
fix tests
nicholaspai Dec 21, 2022
bb86aad
Fix
nicholaspai Dec 21, 2022
2fddeeb
Update Arbitrum_Adapter.sol
nicholaspai Dec 21, 2022
23ef0ef
Make vars constant
nicholaspai Dec 21, 2022
d4c4c9d
redeploy
nicholaspai Dec 21, 2022
01be5a0
Update Arbitrum_Adapter.ts
nicholaspai Dec 21, 2022
4a0b305
revert
nicholaspai Dec 22, 2022
5129c5e
update
nicholaspai Dec 22, 2022
b8ed841
Update Arbitrum_Adapter.ts
nicholaspai Dec 22, 2022
3821a1f
add deployment files
nicholaspai Dec 22, 2022
b680b74
add comments
nicholaspai Dec 23, 2022
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
30 changes: 13 additions & 17 deletions contracts/chain-adapters/Arbitrum_Adapter.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,16 @@ contract Arbitrum_Adapter is AdapterInterface {
// ticket’s calldata in the retry buffer. (current base submission fee is queryable via
// ArbRetryableTx.getSubmissionPrice). ArbRetryableTicket precompile interface exists at L2 address
// 0x000000000000000000000000000000000000006E.
uint256 public immutable l2MaxSubmissionCost = 0.01e18;
uint256 public constant l2MaxSubmissionCost = 0.01e18;

// L2 Gas price bid for immediate L2 execution attempt (queryable via standard eth*gasPrice RPC)
uint256 public immutable l2GasPrice = 5e9; // 5 gWei
uint256 public constant l2GasPrice = 5e9; // 5 gWei

// Gas limit for immediate L2 execution attempt (can be estimated via NodeInterface.estimateRetryableTicket).
// NodeInterface precompile interface exists at L2 address 0x00000000000000000000000000000000000000C8
uint32 public immutable l2GasLimit = 2_000_000;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gas limit for sending tokens should be a lot lower than for sending messages

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: any reason not to make these both constants rather than hardcoding them?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean instead of uint256 public immutable do uint256 public constant? Is that a gas savings?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, yeah, that too (constant is better than immutable!), but I just meant giving them names, which you did!

uint32 public constant RELAY_TOKENS_L2_GAS_LIMIT = 300_000;
uint32 public constant RELAY_MESSAGE_L2_GAS_LIMIT = 2_000_000;

// This address on L2 receives extra ETH that is left over after relaying a message via the inbox.
address public immutable l2RefundL2Address;
address public constant l2RefundL2Address = 0x428AB2BA90Eba0a4Be7aF34C9Ac451ab061AC010;

ArbitrumL1InboxLike public immutable l1Inbox;

Expand All @@ -83,8 +82,6 @@ contract Arbitrum_Adapter is AdapterInterface {
constructor(ArbitrumL1InboxLike _l1ArbitrumInbox, ArbitrumL1ERC20GatewayLike _l1ERC20GatewayRouter) {
l1Inbox = _l1ArbitrumInbox;
l1ERC20GatewayRouter = _l1ERC20GatewayRouter;

l2RefundL2Address = msg.sender;
}

/**
Expand All @@ -95,15 +92,15 @@ contract Arbitrum_Adapter is AdapterInterface {
* @param message Data to send to target.
*/
function relayMessage(address target, bytes memory message) external payable override {
uint256 requiredL1CallValue = _contractHasSufficientEthBalance();
uint256 requiredL1CallValue = _contractHasSufficientEthBalance(RELAY_MESSAGE_L2_GAS_LIMIT);

l1Inbox.createRetryableTicket{ value: requiredL1CallValue }(
target, // destAddr destination L2 contract address
0, // l2CallValue call value for retryable L2 message
l2MaxSubmissionCost, // maxSubmissionCost Max gas deducted from user's L2 balance to cover base fee
l2RefundL2Address, // excessFeeRefundAddress maxgas * gasprice - execution cost gets credited here on L2
l2RefundL2Address, // callValueRefundAddress l2Callvalue gets credited here on L2 if retryable txn times out or gets cancelled
l2GasLimit, // maxGas Max gas deducted from user's L2 balance to cover L2 execution
RELAY_MESSAGE_L2_GAS_LIMIT, // maxGas Max gas deducted from user's L2 balance to cover L2 execution
l2GasPrice, // gasPriceBid price bid for L2 execution
message // data ABI encoded data of L2 message
);
Expand All @@ -126,7 +123,7 @@ contract Arbitrum_Adapter is AdapterInterface {
uint256 amount,
address to
) external payable override {
uint256 requiredL1CallValue = _contractHasSufficientEthBalance();
uint256 requiredL1CallValue = _contractHasSufficientEthBalance(RELAY_TOKENS_L2_GAS_LIMIT);

// Approve the gateway, not the router, to spend the hub pool's balance. The gateway, which is different
// per L1 token, will temporarily escrow the tokens to be bridged and pull them from this contract.
Expand All @@ -149,7 +146,7 @@ contract Arbitrum_Adapter is AdapterInterface {
l1Token,
to,
amount,
l2GasLimit,
RELAY_TOKENS_L2_GAS_LIMIT,
l2GasPrice,
data
);
Expand All @@ -159,25 +156,24 @@ contract Arbitrum_Adapter is AdapterInterface {
l2RefundL2Address,
to,
amount,
l2GasLimit,
RELAY_TOKENS_L2_GAS_LIMIT,
l2GasPrice,
data
);
}

emit TokensRelayed(l1Token, l2Token, amount, to);
}

/**
* @notice Returns required amount of ETH to send a message via the Inbox.
* @return amount of ETH that this contract needs to hold in order for relayMessage to succeed.
*/
function getL1CallValue() public pure returns (uint256) {
function getL1CallValue(uint32 l2GasLimit) public pure returns (uint256) {
return l2MaxSubmissionCost + l2GasPrice * l2GasLimit;
}

function _contractHasSufficientEthBalance() internal view returns (uint256 requiredL1CallValue) {
requiredL1CallValue = getL1CallValue();
function _contractHasSufficientEthBalance(uint32 l2GasLimit) internal view returns (uint256 requiredL1CallValue) {
requiredL1CallValue = getL1CallValue(l2GasLimit);
require(address(this).balance >= requiredL1CallValue, "Insufficient ETH balance");
}
}
285 changes: 0 additions & 285 deletions deployments/mainnet/Arbitrum_Adapter.json
View file
Open in desktop

This file was deleted.

12 changes: 6 additions & 6 deletions test/chain-adapters/Arbitrum_Adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ describe("Arbitrum Chain Adapter", function () {
mockSpoke.address,
0,
consts.sampleL2MaxSubmissionCost,
owner.address,
owner.address,
"0x428AB2BA90Eba0a4Be7aF34C9Ac451ab061AC010",
"0x428AB2BA90Eba0a4Be7aF34C9Ac451ab061AC010",
consts.sampleL2Gas,
consts.sampleL2GasPrice,
functionCallData
Expand Down Expand Up @@ -99,10 +99,10 @@ describe("Arbitrum Chain Adapter", function () {
const message = defaultAbiCoder.encode(["uint256", "bytes"], [consts.sampleL2MaxSubmissionCost, "0x"]);
expect(l1ERC20GatewayRouter.outboundTransferCustomRefund).to.have.been.calledWith(
dai.address,
owner.address,
"0x428AB2BA90Eba0a4Be7aF34C9Ac451ab061AC010",
mockSpoke.address,
tokensSendToL2,
consts.sampleL2Gas,
consts.sampleL2GasSendTokens,
consts.sampleL2GasPrice,
message
);
Expand All @@ -111,8 +111,8 @@ describe("Arbitrum Chain Adapter", function () {
mockSpoke.address,
0,
consts.sampleL2MaxSubmissionCost,
owner.address,
owner.address,
"0x428AB2BA90Eba0a4Be7aF34C9Ac451ab061AC010",
"0x428AB2BA90Eba0a4Be7aF34C9Ac451ab061AC010",
consts.sampleL2Gas,
consts.sampleL2GasPrice,
mockSpoke.interface.encodeFunctionData("relayRootBundle", [
Expand Down
3 changes: 2 additions & 1 deletion test/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@ export const amountToReturn = toWei("1");

export const mockTreeRoot = createRandomBytes32();

// Following should match variables set in Arbitrum_Adapter
// Following should match variables set in Arbitrum_Adapter and Optimism_Adapter.
export const sampleL2Gas = 2000000;
export const sampleL2GasSendTokens = 300000;

export const sampleL2MaxSubmissionCost = toWei("0.01");

Expand Down