fix: modify Arbitrum_CustomGasToken_Adapter for Aleph Zero #699
Conversation
…ken precision (#589) * improve(Arbitrum_CustomGasToken_Adapter): Handle non-18 custom gas token precision Based on this [Inbox](https://github.com/OffchainLabs/nitro-contracts/blob/fbbcef09c95f69decabaced3da683f987902f3e2/src/bridge/AbsInbox.sol#L237) code which implies that the `tokenTotalFeeAmount` passed into the [ERC20Inbox](https://github.com/OffchainLabs/nitro-contracts/blob/fbbcef09c95f69decabaced3da683f987902f3e2/src/bridge/ERC20Inbox.sol#L85) should be in the custom gas token's precision while the `maxSubmissionCost`, `gasPrice`, and `gasLimit` should all be same as for normal Arbitrum messages * Update contracts/chain-adapters/Arbitrum_CustomGasToken_Adapter.sol * Update contracts/chain-adapters/Arbitrum_CustomGasToken_Adapter.sol * Update Arbitrum_CustomGasToken_Adapter.sol * support deecimals > 18 * divCeil instead of divFLoor
World Chain implements Circle's bridged USDC standard. Deposits & withdrawals cannot be initiated via the L1StandardBridge and L2StandardBridge contracts respectively. Instead they must be sent to World Chain's own implementations of Circle's bridged USDC standard. In recognition of the fact that this will likely be a repeating pattern for new deployments, restyle the World Chain adapter as a generic OP adapter and instead rely on deployment scripts to configure it with the correct bridges. Circle's standard is described here: https://github.com/circlefin/stablecoin-evm/blob/master/doc/bridged_USDC_standard.md The process for bridged USDC deposits & withdrawals is here: https://github.com/defi-wonderland/opUSDC/tree/main?tab=readme-ov-file#l1--l2-usdc-canonical-bridging
* fix: Support OP bridged USDC in WorldChain adapter Signed-off-by: bennett <bennett@umaproject.org> --------- Signed-off-by: bennett <bennett@umaproject.org> Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com>
These are recorded under the OP adapter due to the name of the contract that was deployed. Some or all of this will be overwritten on subsequent OP adapter deployments.
This is the deployment info for the WorldChain_SpokePool deployed at the current audit-latest code. Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
|
Is this ready to go in? cc @melisaguevara @bmzig @pxrl |
Signed-off-by: bennett <bennett@umaproject.org>
contracts/chain-adapters/Arbitrum_LegacyCustomGasToken_Adapter.sol
Outdated
Show resolved
Hide resolved
Signed-off-by: bennett <bennett@umaproject.org>
contracts/chain-adapters/Arbitrum_LegacyCustomGasToken_Adapter.sol
Outdated
Show resolved
Hide resolved
Signed-off-by: bennett <bennett@umaproject.org>
contracts/chain-adapters/Arbitrum_LegacyCustomGasToken_Adapter.sol
Outdated
Show resolved
Hide resolved
| * - The token must have a max of 2^256 - 1 wei total supply unscaled | ||
| * - The token must have a max of 2^256 - 1 wei total supply when scaled to 18 decimals | ||
| */ | ||
| interface ArbitrumL1ERC20Bridge { |
There was a problem hiding this comment.
should these interfaces be read from the base adapter file?
There was a problem hiding this comment.
Nice. Yeah it should: 897a339
Signed-off-by: bennett <bennett@umaproject.org>
contracts/chain-adapters/Arbitrum_LegacyCustomGasToken_Adapter.sol
Outdated
Show resolved
Hide resolved
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
| } | ||
| } | ||
|
|
||
| function _fromNativeTo18Decimals(uint256 amount) internal view returns (uint256) { |
There was a problem hiding this comment.
So just to note, in the case where NATIVE_TOKEN_DECIMALS > 18, we'll be dividing and effectively rounding down the result. Is that OK when passing in this rounded down value into the Inbox.createRetryableTicket?
There was a problem hiding this comment.
Added a small comment about this here: 7082475
Signed-off-by: bennett <bennett@umaproject.org>
| L1_INBOX.createRetryableTicket( | ||
| to, // destAddr destination L2 contract address | ||
| L2_CALL_VALUE, // l2CallValue call value for retryable L2 message | ||
| _fromNativeTo18Decimals(amount), // l2CallValue call value for retryable L2 message |
There was a problem hiding this comment.
This isn't just a decimal change, this seems to be a deeper functional change.
Previously, the second arg looks like it was always set to 0. Now it appears that we're passing in the amount (decimal converted).
Was the previous code wrong in setting this field to a 0 constant? Or is the new code doing something more subtle that I'm failing to understand?
There was a problem hiding this comment.
For (hopefully) full context:
Setting this to 0 was incorrect. This function is called only when trying to bridge the AZERO token. When we set this to zero, the aliased L1 sender address will receive amountToBridge, i.e. amount+bridge fees, and then call the target with 0 value (in this context, 0 value means no L2 native token transfer). It then only refunds excess bridge gas costs to the L2 target. This means that amount is stuck in the aliased address on L2 (for example, we can see this happened with the aliased dev wallet and the aliased adapter). While it's not loss of funds, since we effectively have control over the aliased address by nudging the bridge from L1, we obviously didn't want this.
The first change was to just put in amount as the l2CallValue field here, which works for AZERO, since AZERO happens to have 18 decimals, but for a token which has non-18 decimal precision, we would be passing in a value with native token precision, when the contracts assume l2Callvalue is in 18 decimal precision. This lead to the second change, seen above, where we convert l2CallValue from native token decimals to 18.
Signed-off-by: bennett <bennett@umaproject.org>
Summary
Adding backwards and forwards compatibility
When attempting to deploy the Arbitrum_CustomGasTokens_Adapter to support Aleph Zero, we realized that the deployed ERC20BridgeContract does not actually contain the
nativeTokenDecimals()function as the contracts would suggest. More critically, the amount to be bridged is not scaled in the legacy AbsInbox but it is in the new one.It turns out the issue is that Aleph Zero contracts were deployed using an outdated version of Arbitrum contracts. The contract has been modified to work with legacy and new versions of the Inbox contract:
_from18ToNativeDecimals()conversion function in the relayTokens/relayMessage logicOther changes
The following opportunistic fixes were also made to the Arbitrum_CustomGasTokens_Adapter contract in this PR:
Notes
Re-implementation of #690