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

Support for rollup mode when using fee token #252

Draft
wants to merge 59 commits into
base: develop
Choose a base branch
from

Conversation

gvladika
Copy link
Contributor

@gvladika gvladika commented Sep 19, 2024

This feature adds support to run fee token based Orbit chains in rollup mode. Previously only Anytrust mode was supported when fee token is used. This is because ArbO assumes that the asset used to pay for batch posting on parent chain is the same asset which is used as native asset on the child chain. If batch poster spends 0.01 ETH to post batch on the parent chain, it will be reimbursed 0.01 ETH on the child chain. However this approach is not valid when fee token is used. Reimbursing batch poster with 0.01 fee token after it has spent 0.01 ETH would be fine only if exchange rate between ETH and fee token was 1:1. In practice this will not be the case, so we’re introducing a way to get the actual exchange rate on-chain and use it to correctly reimburse the batch poster.

This PR enables SequencerInbox to fetch the feeToken:nativeToken exchange rate from the external source called feeTokenPricer which is set by the chain owner. Pricer needs to implement a single function getExchangeRate() returns(uint256). In practice, pricer can use Chainlink, amm twap, constant price, or any other approach to report exchange rate.

@cla-bot cla-bot bot added the s label Sep 19, 2024
* @dev For example, parent chain's native token is ETH, fee token is DAI. If price of 1ETH = 2000DAI, then function should return 2000*1e18.
* If fee token is USDC instead and price of 1ETH = 2000USDC, function should still return 2000*1e18, no matter that USDC uses 6 decimals.
*/
function getExchangeRate() external returns (uint256);
Copy link
Member

Choose a reason for hiding this comment

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

instead of a getter that return spot rate, I think it make more sense to have a mutable method spendingInEth(uint256 wei, address spender) that take the number of eth spent as input and return the number of token spent. This allow the pricer to do more complex internal accounting if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we discussed, we can probably satisfy all use-cases with getExchangeRate(). It's mutable so it enables internal accounting. Gas reporting hooks can be used to update state and account for which state is updated can be tx.origin or 'spender' as reported by the hook.

@gzeoneth gzeoneth mentioned this pull request Sep 30, 2024
@yahgwai yahgwai changed the base branch from develop to bold-merge November 29, 2024 14:04
Copy link
Member

@gzeoneth gzeoneth left a comment

Choose a reason for hiding this comment

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

LGTM

@gzeoneth gzeoneth self-requested a review November 29, 2024 17:11
Copy link
Member

@gzeoneth gzeoneth left a comment

Choose a reason for hiding this comment

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

some test are failing, e.g. test/contract/sequencerInboxDelayBufferable.spec.ts

Base automatically changed from bold-merge to develop December 11, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants