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

improve(Arbitrum_CustomGasToken_Adapter): Handle non-18 custom gas token precision #589

Merged
merged 7 commits into from
Sep 19, 2024

Conversation

nicholaspai
Copy link
Member

Based on this Inbox code which implies that the tokenTotalFeeAmount passed into the ERC20Inbox should be in the custom gas token's precision while the maxSubmissionCost, gasPrice, and gasLimit should all be same as for normal Arbitrum messages

…ken 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
@nicholaspai nicholaspai requested a review from mrice32 September 3, 2024 18:50
@nicholaspai nicholaspai marked this pull request as ready for review September 3, 2024 18:50
* @return amount of gas token that this contract needs to hold in order for relayMessage to succeed.
*/
function getL1CallValue(uint32 l2GasLimit) public view returns (uint256) {
return L2_MAX_SUBMISSION_COST + L2_GAS_PRICE * l2GasLimit;
return _from18ToNativeDecimals(L2_MAX_SUBMISSION_COST + L2_GAS_PRICE * l2GasLimit);
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 benefit of putting the scaling logic into this public function is the deployer can quickly sanity check post-deployment that the L1CallValue will be set correctly with the right precision

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! Is the L1 representation the only one that has non 18 decimals or would the native representation on the L2 also have non 18 decimals?

@nicholaspai
Copy link
Member Author

LGTM! Is the L1 representation the only one that has non 18 decimals or would the native representation on the L2 also have non 18 decimals?

I believe the L2 native token is hardcoded to be 18 based on the [Inbox](https://github.com/OffchainLabs/nitro-contracts/blob/fbbcef09c95f69decabaced3da683f987902f3e2/src/bridge/AbsInbox.sol#L237] and ERC20Inbox code

@nicholaspai nicholaspai requested a review from mrice32 September 10, 2024 13:24
@nicholaspai nicholaspai requested review from bmzig and pxrl September 11, 2024 20:48
@mrice32 mrice32 merged commit bf55da0 into master Sep 19, 2024
11 checks passed
@mrice32 mrice32 deleted the npai/custom-gas-token-precision branch September 19, 2024 05:09
pxrl pushed a commit that referenced this pull request Oct 8, 2024
…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
nicholaspai added a commit that referenced this pull request Nov 6, 2024
* improve(Arbitrum_CustomGasToken_Adapter): Handle non-18 custom gas token 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

* feat: WorldChain deployments (#634)

* fix: Support OP bridged USDC in WorldChain adapter (#646)

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: Use custom USDC bridge in WorldChain_SpokePool (#647)

* fix: Support OP bridged USDC in WorldChain adapter

Signed-off-by: bennett <[email protected]>

---------

Signed-off-by: bennett <[email protected]>
Co-authored-by: Paul <[email protected]>

* chore: Add WorldChain adapter deployment (#658)

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.

* chore: update World Chain deployment info with new implementation (#659)

This is the deployment info for the WorldChain_SpokePool deployed at the 
current audit-latest code.

Signed-off-by: bennett <[email protected]>

* fix: modify Arbitrum_CustomGasToken_Adapter for Aleph Zero

Signed-off-by: bennett <[email protected]>

* split gas token adapters

Signed-off-by: bennett <[email protected]>

* remove native token decimals variable

Signed-off-by: bennett <[email protected]>

* make sure files are as similar as possible

Signed-off-by: bennett <[email protected]>

* import interfaces from base

Signed-off-by: bennett <[email protected]>

* inherit

Signed-off-by: bennett <[email protected]>

* collapse adapters

Signed-off-by: bennett <[email protected]>

* remove virtual

Signed-off-by: bennett <[email protected]>

* fix comment

Signed-off-by: bennett <[email protected]>

* go back to 18 decimals

Signed-off-by: bennett <[email protected]>

* remove

Signed-off-by: bennett <[email protected]>

* fix

* add docstrings, decimal error, and scale l2CallValue

Signed-off-by: bennett <[email protected]>

* fix comment

Signed-off-by: bennett <[email protected]>

* add comment about rounding

Signed-off-by: bennett <[email protected]>

---------

Signed-off-by: bennett <[email protected]>
Co-authored-by: nicholaspai <[email protected]>
Co-authored-by: Paul <[email protected]>
Co-authored-by: nicholaspai <[email protected]>
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.

3 participants