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 48 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
36e3996
Start of custom fee tokens for rollups
yahgwai Sep 18, 2024
84ef567
Do not exclude batch reports when using fee token
gvladika Sep 19, 2024
8125656
Add condition for submitting the batch report, and do the scaling
gvladika Sep 19, 2024
74f9939
Add IFeeTokenPricer
gvladika Sep 19, 2024
504dbc0
Add getter and setter for pricer to the interface
gvladika Sep 19, 2024
44f3986
Add setter for feeTokenPricer
gvladika Sep 19, 2024
ef4f1fb
Add missing param
gvladika Sep 19, 2024
a6a3f55
Use new SeqInbox init param
gvladika Sep 19, 2024
f3eb844
Make existing tests work with feeTokenPricer
gvladika Sep 20, 2024
2897ce6
Format
gvladika Sep 20, 2024
70f6ad4
Add setFeeTokenPricer tests
gvladika Sep 20, 2024
213039f
Fee token pricing deployment
yahgwai Sep 20, 2024
4c643e1
Merge branch 'custom-fee-rollup' of https://github.com/OffchainLabs/n…
yahgwai Sep 20, 2024
57f46ed
Revert if trying to set fee token pricer when fee token is not used
gvladika Sep 20, 2024
33420d4
Add fuzz test for different exchange rates
gvladika Sep 23, 2024
8a1dfdd
Handle overflow case due to too high exchange rate
gvladika Sep 23, 2024
252f599
Updated erc20rollupeventinbox to do gas price scaling
yahgwai Sep 25, 2024
e852b6a
Merge branch 'custom-fee-rollup' of https://github.com/OffchainLabs/n…
yahgwai Sep 25, 2024
a702a08
Added integration test for batch poster reimbursement
yahgwai Sep 26, 2024
86c5a94
Update rollupInitialized test
gvladika Sep 26, 2024
1bcf1aa
Refactor tests and check initial cost is properly reported
gvladika Sep 26, 2024
e752cb7
Merge branch 'custom-fee-rollup' of https://github.com/OffchainLabs/n…
yahgwai Sep 26, 2024
14297dc
Added fee token pricer arg
yahgwai Sep 26, 2024
e3217ed
Use token fee pricer testnode ref
yahgwai Sep 26, 2024
cf42d00
Updated int fee token tests
yahgwai Sep 26, 2024
f64df33
Formatting
yahgwai Sep 26, 2024
70c19e8
Updated comments
yahgwai Sep 26, 2024
e3a7f9b
Formatting
yahgwai Sep 26, 2024
fcb4950
Try tests with init
yahgwai Sep 26, 2024
483171b
Added separate job for e2e tests
yahgwai Sep 26, 2024
913bf45
Missed test name change
yahgwai Sep 26, 2024
2956969
Disable while true condition linting
yahgwai Sep 26, 2024
1f33876
Fixed 4844 tests
yahgwai Sep 26, 2024
f22ec8c
Added fee token pricer address zero to arb rollup spec
yahgwai Sep 26, 2024
2b57aac
Formatting
yahgwai Sep 26, 2024
e6b0beb
Updated orbit ame
yahgwai Sep 26, 2024
2f876cf
Updated sigs and storage
yahgwai Sep 26, 2024
fde0833
Added estimate gas
yahgwai Sep 26, 2024
14a10ef
L1 wal balance console
yahgwai Sep 26, 2024
20ebdbe
Console logging
yahgwai Sep 26, 2024
782c792
Added gas price estimation
yahgwai Sep 26, 2024
9f17bcb
Update Slither db, no new findings
gvladika Sep 27, 2024
7f4161f
Add fuzz test for rollupInitialized
gvladika Sep 27, 2024
96d2592
Merge branch 'develop' into custom-fee-rollup
gzeoneth Oct 1, 2024
bf402da
Merge branch 'develop' into custom-fee-rollup
gvladika Oct 9, 2024
e071dde
Clean up
gvladika Oct 9, 2024
f050fdb
Update slither db
gvladika Oct 9, 2024
c015aaf
Update audit-ci.jsonc
gvladika Oct 9, 2024
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
1 change: 1 addition & 0 deletions .env.sample.goerli
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ DEVNET_PRIVKEY=""

## optional - address of already deployed ERC20 token which shall be used as rollup's fee token
FEE_TOKEN_ADDRESS=""
FEE_TOKEN_PRICER_ADDRESS=""
22 changes: 15 additions & 7 deletions .github/workflows/contract-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,8 @@ jobs:
- name: Compile contracts
run: yarn build

- name: Run e2e tests
run: yarn test:e2e
- name: Run e2e orbit tests
run: yarn test:e2e:orbit
test-e2e-custom-fee-token:
name: Test e2e custom fee token
runs-on: ubuntu-latest
Expand All @@ -188,9 +188,10 @@ jobs:
- uses: OffchainLabs/actions/run-nitro-test-node@main
with:
l3-node: true
args: --l3-fee-token
args: --l3-fee-token --l3-fee-token-pricer
no-token-bridge: true
no-l3-token-bridge: true
nitro-testnode-ref: fee-token-pricer
nitro-contracts-branch: '${{ github.event.pull_request.head.sha || github.sha }}'

- name: Setup node/yarn
Expand All @@ -206,8 +207,11 @@ jobs:
- name: Compile contracts
run: yarn build

- name: Run e2e tests
run: yarn test:e2e
- name: Run e2e orbit tests
run: yarn test:e2e:orbit

- name: Run e2e orbit custom fee token rollup tests
run: yarn test:e2e:orbit-fee-token-rollup
test-e2e-fee-token-6-decimals:
name: Test e2e fee token with 6 decimals
runs-on: ubuntu-latest
Expand All @@ -219,9 +223,10 @@ jobs:
- uses: OffchainLabs/actions/run-nitro-test-node@main
with:
l3-node: true
args: --l3-fee-token --l3-fee-token-decimals 6
args: --l3-fee-token --l3-fee-token-pricer --l3-fee-token-decimals 6
no-token-bridge: true
no-l3-token-bridge: true
nitro-testnode-ref: fee-token-pricer
nitro-contracts-branch: '${{ github.event.pull_request.head.sha || github.sha }}'

- name: Setup node/yarn
Expand All @@ -238,4 +243,7 @@ jobs:
run: yarn build

- name: Run e2e tests
run: yarn test:e2e
run: yarn test:e2e:orbit

- name: Run e2e orbit custom fee token rollup tests
run: yarn test:e2e:orbit-fee-token-rollup
4 changes: 3 additions & 1 deletion audit-ci.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@
// Server-Side Request Forgery in axios
"GHSA-8hc4-vh64-cxmj",
// Regular Expression Denial of Service (ReDoS) in micromatch
"GHSA-952p-6rrq-rcjv"
"GHSA-952p-6rrq-rcjv",
// cookie accepts cookie name, path, and domain with out of bounds characters
"GHSA-pxg6-pf52-xh8x"
]
}
4 changes: 4 additions & 0 deletions foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,8 @@ auto_detect_remappings = false
[fmt]
number_underscore = 'thousands'
line_length = 100

[fuzz]
runs = 5000

# See more config options https://github.com/foundry-rs/foundry/tree/master/config
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,16 @@
"lint:test": "eslint ./test",
"solhint": "solhint -f table src/**/*.sol",
"prettier:solidity": "prettier --write src/**/*.sol",
"format": "prettier './**/*.{js,json,ts,yml,sol}' --write && yarn run lint:test --fix",
"format": "prettier './test/e2e/**/*.{js,json,ts,yml,sol}' --write && yarn run lint:test --fix",
"build:0.6": "INTERFACE_TESTER_SOLC_VERSION=0.6.9 yarn run build",
"build:0.7": "INTERFACE_TESTER_SOLC_VERSION=0.7.0 yarn run build",
"test": "DISABLE_GAS_REPORTER=true hardhat --network hardhat test test/contract/*.spec.ts",
"test:4844": "DISABLE_GAS_REPORTER=true hardhat --network hardhat test test/contract/*.spec.4844.ts",
"test:compatibility": "yarn run build:0.6 && yarn run build:0.7",
"test:storage": "./test/storage/test.bash",
"test:signatures": "./test/signatures/test-sigs.bash",
"test:e2e": "hardhat test test/e2e/*.ts",
"test:e2e:orbit": "hardhat test test/e2e/orbitChain.ts",
"test:e2e:orbit-fee-token-rollup": "hardhat test test/e2e/customFeeRollup.ts",
"test:update": "yarn run test:signatures || yarn run test:storage",
"metadatahash": "yarn build:all && hardhat run scripts/printMetadataHashes.ts",
"upload-4bytes": "forge build && find ./out -type f -name \"*.json\" -exec cast upload-signature {} + | grep -v Duplicated:",
Expand Down
10 changes: 8 additions & 2 deletions scripts/createERC20Rollup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,18 @@ async function main() {
throw new Error('ROLLUP_CREATOR_ADDRESS not set')
}

console.log('Creating new rollup with', customFeeTokenAddress, 'as fee token')
let feeTokenPricer = process.env.FEE_TOKEN_PRICER_ADDRESS
if(!feeTokenPricer) {
feeTokenPricer = ethers.constants.AddressZero
}

console.log('Creating new rollup with', customFeeTokenAddress, 'as fee token and', feeTokenPricer, 'as fee token pricer')
await createRollup(
deployer,
false,
rollupCreatorAddress,
customFeeTokenAddress
customFeeTokenAddress,
feeTokenPricer
)
}

Expand Down
3 changes: 2 additions & 1 deletion scripts/createEthRollup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@ import { createRollup } from './rollupCreation'

async function main() {
const feeToken = ethers.constants.AddressZero
const feeTokenPricer = ethers.constants.AddressZero
const rollupCreatorAddress = process.env.ROLLUP_CREATOR_ADDRESS
if (!rollupCreatorAddress) {
throw new Error('ROLLUP_CREATOR_ADDRESS not set')
}

const [signer] = await ethers.getSigners()

await createRollup(signer, false, rollupCreatorAddress, feeToken)
await createRollup(signer, false, rollupCreatorAddress, feeToken, feeTokenPricer)
}

main()
Expand Down
7 changes: 6 additions & 1 deletion scripts/local-deployment/deployCreatorAndCreateRollup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ async function main() {
if (!feeToken) {
feeToken = ethers.constants.AddressZero
}
let feeTokenPricer = process.env.FEE_TOKEN_PRICER_ADDRESS as string
if (!feeTokenPricer) {
feeTokenPricer = ethers.constants.AddressZero
}

/// deploy templates and rollup creator
console.log('Deploy RollupCreator')
Expand Down Expand Up @@ -74,7 +78,8 @@ async function main() {
deployerWallet,
true,
contracts.rollupCreator.address,
feeToken
feeToken,
feeTokenPricer
)

if (!result) {
Expand Down
16 changes: 10 additions & 6 deletions scripts/rollupCreation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import '@nomiclabs/hardhat-ethers'
import { run } from 'hardhat'
import { abi as rollupCreatorAbi } from '../build/contracts/src/rollup/RollupCreator.sol/RollupCreator.json'
import { config, maxDataSize } from './config'
import { BigNumber, Signer } from 'ethers'
import { ERC20, ERC20__factory, IERC20__factory } from '../build/types'
import { BigNumber, Event, Signer } from 'ethers'
import { ERC20, ERC20__factory, IERC20__factory, RollupCreator } from '../build/types'
import { sleep } from './testSetup'
import { promises as fs } from 'fs'
import { _isRunningOnArbitrum, verifyContract } from './deploymentUtils'
Expand Down Expand Up @@ -61,7 +61,8 @@ export async function createRollup(
signer: Signer,
isDevDeployment: boolean,
rollupCreatorAddress: string,
feeToken: string
feeToken: string,
feeTokenPricer: string
): Promise<{
rollupCreationResult: RollupCreationResult
chainInfo: ChainInfo
Expand All @@ -76,7 +77,7 @@ export async function createRollup(
rollupCreatorAddress,
rollupCreatorAbi,
signer
)
) as RollupCreator
const validatorWalletCreator = await rollupCreator.validatorWalletCreator()

try {
Expand All @@ -101,7 +102,7 @@ export async function createRollup(
// Call the createRollup function
console.log('Calling createRollup to generate a new rollup ...')
const deployParams = isDevDeployment
? await _getDevRollupConfig(feeToken, validatorWalletCreator)
? await _getDevRollupConfig(feeToken, feeTokenPricer, validatorWalletCreator)
: {
config: config.rollupConfig,
validators: config.validators,
Expand All @@ -111,6 +112,7 @@ export async function createRollup(
maxFeePerGasForRetryables: MAX_FER_PER_GAS,
batchPosters: config.batchPosters,
batchPosterManager: config.batchPosterManager,
feeTokenPricer: feeTokenPricer
}

const createRollupTx = await rollupCreator.createRollup(deployParams, {
Expand All @@ -119,7 +121,7 @@ export async function createRollup(
const createRollupReceipt = await createRollupTx.wait()

const rollupCreatedEvent = createRollupReceipt.events?.find(
(event: RollupCreatedEvent) =>
(event: Event): event is Event =>
event.event === 'RollupCreated' &&
event.address.toLowerCase() === rollupCreatorAddress.toLowerCase()
)
Expand Down Expand Up @@ -222,6 +224,7 @@ export async function createRollup(

async function _getDevRollupConfig(
feeToken: string,
feeTokenPricer: string,
validatorWalletCreator: string
) {
// set up owner address
Expand Down Expand Up @@ -321,6 +324,7 @@ async function _getDevRollupConfig(
maxFeePerGasForRetryables: MAX_FER_PER_GAS,
batchPosters: batchPosters,
batchPosterManager: batchPosterManager,
feeTokenPricer: feeTokenPricer
}

function _createValidatorAddress(
Expand Down
2 changes: 1 addition & 1 deletion slither.db.json

Large diffs are not rendered by default.

32 changes: 31 additions & 1 deletion src/bridge/ISequencerInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ interface ISequencerInbox is IDelayedMessageProvider {
/// This enables the batch poster to do key rotation
function batchPosterManager() external view returns (address);

/// @notice Pricer contract is used to get the exchange rate between child chain's fee token
/// and parent chain's native token. This is needed when child chain uses custom fee
/// token which is different from parent chain's native token. The exchange rate is
/// used to correctly report converted gas price in the batch spending reports, so
/// that batch poster can get properly reimbursed on the child chain. If chain uses
/// custom fee token, but pricer is not set, then batch poster reports won't be reported
/// and batch poster won't get reimbursed.
function feeTokenPricer() external view returns (IFeeTokenPricer);

struct DasKeySetInfo {
bool isValidKeyset;
uint64 creationBlock;
Expand Down Expand Up @@ -220,10 +229,31 @@ interface ISequencerInbox is IDelayedMessageProvider {
*/
function setBatchPosterManager(address newBatchPosterManager) external;

/**
* @notice Updates the fee token pricer, the contract which is used to get the exchange rate between child
* chain's fee token and parent chain's native token in rollups that use custom fee token.
* @param newFeeTokenPricer The new fee token pricer to be set
*/
function setFeeTokenPricer(IFeeTokenPricer newFeeTokenPricer) external;

/// @notice Allows the rollup owner to sync the rollup address
function updateRollupAddress() external;

// ---------- initializer ----------

function initialize(IBridge bridge_, MaxTimeVariation calldata maxTimeVariation_) external;
function initialize(
IBridge bridge_,
MaxTimeVariation calldata maxTimeVariation_,
IFeeTokenPricer feeTokenPricer_
) external;
}

interface IFeeTokenPricer {
/**
* @notice Get the number of child chain's fee tokens per 1 parent chain's native token. Exchange rate must be
* denominated in 18 decimals. Function is mutable so it allows the pricer to keep internal state.
* @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.

}
51 changes: 44 additions & 7 deletions src/bridge/SequencerInbox.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ import {
InvalidHeaderFlag,
NativeTokenMismatch,
BadMaxTimeVariation,
Deprecated
Deprecated,
CannotSetFeeTokenPricer
} from "../libraries/Error.sol";
import "./IBridge.sol";
import "./IInboxBase.sol";
Expand Down Expand Up @@ -117,6 +118,9 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
/// @inheritdoc ISequencerInbox
address public batchPosterManager;

/// @inheritdoc ISequencerInbox
IFeeTokenPricer public feeTokenPricer;

// On L1 this should be set to 117964: 90% of Geth's 128KB tx size limit, leaving ~13KB for proving
uint256 public immutable maxDataSize;
uint256 internal immutable deployTimeChainId = block.chainid;
Expand Down Expand Up @@ -178,7 +182,8 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox

function initialize(
IBridge bridge_,
ISequencerInbox.MaxTimeVariation calldata maxTimeVariation_
ISequencerInbox.MaxTimeVariation calldata maxTimeVariation_,
IFeeTokenPricer feeTokenPricer_
) external onlyDelegated {
if (bridge != IBridge(address(0))) revert AlreadyInit();
if (bridge_ == IBridge(address(0))) revert HadZeroInit();
Expand All @@ -199,6 +204,11 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
rollup = bridge_.rollup();

_setMaxTimeVariation(maxTimeVariation_);

if (!isUsingFeeToken && feeTokenPricer_ != IFeeTokenPricer(address(0))) {
revert CannotSetFeeTokenPricer();
}
feeTokenPricer = feeTokenPricer_;
}

/// @notice Allows the rollup owner to sync the rollup address
Expand Down Expand Up @@ -460,7 +470,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
// same as using calldata, we only submit spending report if the caller is the origin of the tx
// such that one cannot "double-claim" batch posting refund in the same tx
// solhint-disable-next-line avoid-tx-origin
if (msg.sender == tx.origin && !isUsingFeeToken) {
if (msg.sender == tx.origin) {
submitBatchSpendingReport(dataHash, seqMessageIndex, block.basefee, blobGas);
}
}
Expand Down Expand Up @@ -641,19 +651,37 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
uint256 gasPrice,
uint256 extraGas
) internal {
// When using fee token batch poster needs to be reimbursed on the child chain in the fee token. For that reason
// we need to get the exchange rate between the child chain fee token and the parent chain's native token. Pricer
// is required to get the exchange rate. If the pricer is not set, then we do not send batch reports and batch poster
// never gets reimbursed
IFeeTokenPricer _feeTokenPricer = feeTokenPricer;
if (isUsingFeeToken && address(_feeTokenPricer) == address(0)) {
return;
}

// report the account who paid the gas (tx.origin) for the tx as batch poster
// if msg.sender is used and is a contract, it might not be able to spend the refund on l2
// solhint-disable-next-line avoid-tx-origin
address batchPoster = tx.origin;

// this msg isn't included in the current sequencer batch, but instead added to
// the delayed messages queue that is yet to be included
if (hostChainIsArbitrum) {
// Include extra gas for the host chain's L1 gas charging
uint256 l1Fees = ArbGasInfo(address(0x6c)).getCurrentTxL1GasFees();
extraGas += l1Fees / block.basefee;
}
require(extraGas <= type(uint64).max, "EXTRA_GAS_NOT_UINT64");

if (isUsingFeeToken && address(_feeTokenPricer) != address(0)) {
// gasPrice is originally denominated in parent chain's native token and we want to scale it to child
// chain's fee token. For that we need the exchange rate which tells us how many child chain's fee tokens
// we get for 1 parent chain's native token. Exchange rate itself should be denominated in 18 decimals.
uint256 exchangeRate = _feeTokenPricer.getExchangeRate();
gasPrice = (gasPrice * exchangeRate) / 1e18;
}

// this msg isn't included in the current sequencer batch, but instead added to
// the delayed messages queue that is yet to be included
bytes memory spendingReportMsg = abi.encodePacked(
block.timestamp,
batchPoster,
Expand Down Expand Up @@ -698,8 +726,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox

totalDelayedMessagesRead = afterDelayedMessagesRead;

if (calldataLengthPosted > 0 && !isUsingFeeToken) {
// only report batch poster spendings if chain is using ETH as native currency
if (calldataLengthPosted > 0) {
submitBatchSpendingReport(dataHash, seqMessageIndex, block.basefee, 0);
}
}
Expand Down Expand Up @@ -792,6 +819,16 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
emit OwnerFunctionCalled(5);
}

/// @inheritdoc ISequencerInbox
function setFeeTokenPricer(IFeeTokenPricer feeTokenPricer_) external onlyRollupOwner {
if (!isUsingFeeToken) {
revert CannotSetFeeTokenPricer();
}

feeTokenPricer = feeTokenPricer_;
emit OwnerFunctionCalled(6);
}

function isValidKeysetHash(bytes32 ksHash) external view returns (bool) {
return dasKeySetInfo[ksHash].isValidKeyset;
}
Expand Down
Loading
Loading