Skip to content

Conversation

@nicholaspai
Copy link
Member

This merged PR is now live for most Arbitrum ERC20 gateways (except DAI), so we can use the new function outboundTransferCustomRefund to receive refunds at an EOA we control instead of the aliased hub pool address, which is the L1 sender of the tokens technically.

@nicholaspai nicholaspai requested a review from mrice32 August 10, 2022 16:07
@nicholaspai nicholaspai marked this pull request as ready for review August 10, 2022 16:28
@nicholaspai nicholaspai requested a review from kevinuma August 11, 2022 11:21
Copy link
Contributor

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

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

LGTM!

if (l1Token == 0x6B175474E89094C44Da98b954EedeAC495271d0F) {
// Note: outboundTransfer() will ultimately create a retryable ticket and set this contract's address as the
// refund address. This means that the excess ETH to pay for the L2 transaction will be sent to the aliased
// contract address on L2 and lost.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we don't lose the ETH, it's just hard to retrieve (we have a custom adapter for retrieving it now).

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

// Note: Legacy routers don't have the outboundTransferCustomRefund method, so default to using
// outboundTransfer(). Legacy routers are used for:
// - DAI
if (l1Token == 0x6B175474E89094C44Da98b954EedeAC495271d0F) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is DAI the only token this applies to? Or just the only token we currently support that this applies to?

Copy link
Member Author

Choose a reason for hiding this comment

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

The latter, DAI is the only token we currently support that this applies to, I'll clarify the comment

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

@nicholaspai nicholaspai requested a review from mrice32 August 15, 2022 16:38
// Note: Legacy routers don't have the outboundTransferCustomRefund method, so default to using
// outboundTransfer(). Legacy routers are used for:
// - DAI
if (l1Token == 0x6B175474E89094C44Da98b954EedeAC495271d0F) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good!

@nicholaspai nicholaspai merged commit b15ae29 into master Aug 16, 2022
@nicholaspai nicholaspai deleted the nick/acx-147-upgrade-arbitrum-adapter-to-set-custom branch August 16, 2022 14:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants