Skip to content
This repository has been archived by the owner on Nov 5, 2023. It is now read-only.

[wip] feat: state function changes #6

Conversation

merklefruit
Copy link
Collaborator

@merklefruit merklefruit commented Jun 12, 2023

Work in progress:

  • add L1 cost calculation and logic
  • add receipts deposit nonce
  • add deposit minting support
  • handle gas refund for deposits
  • send base_fee and l1_fee to the fee vaults
  • add tests
  • add docs

@merklefruit merklefruit changed the title feat: state function changes [wip] feat: state function changes Jun 12, 2023
}

let new_sender_info = to_reth_acc(&sender_account.info);
post_state.change_account(sender, old_sender_info, new_sender_info);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Potential problem: when changing the post_state before the tx execution like we do here, does this create a clash with the state object returned by the execution? hopefully not, but have to test this

Comment on lines +486 to +501
// Route the l1 cost and base fee to the appropriate optimism vaults
self.increment_account_balance(
optimism::l1_cost_recipient(),
l1_cost,
&mut post_state,
)?;
self.increment_account_balance(
optimism::base_fee_recipient(),
U256::from(
block
.base_fee_per_gas
.unwrap_or_default()
.saturating_mul(result.gas_used()),
),
&mut post_state,
)?;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

can probably be refactored to be prettier

crates/executor/src/optimism.rs Outdated Show resolved Hide resolved
crates/executor/src/optimism.rs Outdated Show resolved Hide resolved
crates/revm/revm-primitives/src/env.rs Outdated Show resolved Hide resolved
crates/transaction-pool/src/traits.rs Show resolved Hide resolved
Comment on lines +416 to +421
if let Some(m) = transaction.mint() {
// Add balance to the caller account equal to the minted amount.
// Note: this is unconditional, and will not be reverted if the tx fails
// (unless the block can't be built at all due to gas limit constraints)
sender_account.info.balance += U256::from(m);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This could have been addressed in Revm's transact() function in a more clean way. In fact I have a local branch of what this change would look like here: https://github.com/merklefruit/revm/pull/1/files#diff-1d478ba44ccc56e3b1142bd3723bf97f3e254c25dd18323481aedadce0803e91R130

However that solution has some cons:

  1. we are not able to revert the deposit if the block runs out of gas, because revm's transact() context doesn't know about the outstanding block gas used (called cumulative_gas_used here)
  2. we can't cache the values to calculate l1_cost for the entire block, and would have to access the DB for each transaction in the block instead. This is the main reason why I wanted to keep all the diffs on Op-reth if possible

@merklefruit merklefruit marked this pull request as ready for review June 24, 2023 11:14
@clabby clabby mentioned this pull request Jul 8, 2023
@merklefruit
Copy link
Collaborator Author

Changes rebased in #8

@merklefruit merklefruit closed this Jul 9, 2023
@merklefruit merklefruit mentioned this pull request Jul 13, 2023
clabby added a commit that referenced this pull request Aug 12, 2023
clabby added a commit that referenced this pull request Aug 13, 2023
Co-authored-by: Brian Bland <[email protected]>
Co-authored-by: refcell <[email protected]>
Co-authored-by: nicolas <[email protected]>
Co-authored-by: Roberto <[email protected]>

Resolution checkpoint

Resolution checkpoint #2

Resolution checkpoint #3

x

Resolution checkpoint #4

Resolution checkpoint #5

Resolution checkpoint #6

Resolution checkpoint #7

Resolution checkpoint #8

Resolve checkpoint #9 (transaction primitive)

Resolve checkpoint #10 (rpc api transactions)

Resolve checkpoint #11 (building w/o feature flag)

Start review

Compiling with and without `optimism` feature flag

Remove `DepositTx` from txpool mock tests, they never go into the txpool

fmt

code lint

fix signature tests

Co-authored-by: nicolas <[email protected]>

Use free CI runners (revert before upstream)

Co-authored-by: refcell <[email protected]>

Signature test fixes

Co-authored-by refcell <[email protected]>

Fix Receipt proptest

Co-authored-by BB <[email protected]>

lint

Fix variable-length compact for txtype/transaction

Co-authored-by: Brian Bland <[email protected]>

Fix basefee tests

Remove unnecessary rpc deps
clabby added a commit that referenced this pull request Aug 13, 2023
Resolution checkpoint

Resolution checkpoint #2

Resolution checkpoint #3

x

Resolution checkpoint #4

Resolution checkpoint #5

Resolution checkpoint #6

Resolution checkpoint #7

Resolution checkpoint #8

Resolve checkpoint #9 (transaction primitive)

Resolve checkpoint #10 (rpc api transactions)

Resolve checkpoint #11 (building w/o feature flag)

Start review

Compiling with and without `optimism` feature flag

Remove `DepositTx` from txpool mock tests, they never go into the txpool

fmt

code lint

fix signature tests

Co-authored-by: nicolas <[email protected]>

Use free CI runners (revert before upstream)

Co-authored-by: refcell <[email protected]>

Signature test fixes

Co-authored-by refcell <[email protected]>

Fix Receipt proptest

Co-authored-by BB <[email protected]>

lint

Fix variable-length compact for txtype/transaction

Co-authored-by: Brian Bland <[email protected]>

Fix basefee tests

Remove unnecessary rpc deps

Co-authored-by: Brian Bland <[email protected]>
Co-authored-by: refcell <[email protected]>
Co-authored-by: nicolas <[email protected]>
Co-authored-by: Roberto <[email protected]>
clabby added a commit that referenced this pull request Aug 13, 2023
Resolution checkpoint

Resolution checkpoint #2

Resolution checkpoint #3

x

Resolution checkpoint #4

Resolution checkpoint #5

Resolution checkpoint #6

Resolution checkpoint #7

Resolution checkpoint #8

Resolve checkpoint #9 (transaction primitive)

Resolve checkpoint #10 (rpc api transactions)

Resolve checkpoint #11 (building w/o feature flag)

Start review

Compiling with and without `optimism` feature flag

Remove `DepositTx` from txpool mock tests, they never go into the txpool

fmt

code lint

fix signature tests

Co-authored-by: nicolas <[email protected]>

Use free CI runners (revert before upstream)

Co-authored-by: refcell <[email protected]>

Signature test fixes

Co-authored-by refcell <[email protected]>

Fix Receipt proptest

Co-authored-by BB <[email protected]>

lint

Fix variable-length compact for txtype/transaction

Co-authored-by: Brian Bland <[email protected]>

Fix basefee tests

Remove unnecessary rpc deps

Co-authored-by: Brian Bland <[email protected]>
Co-authored-by: refcell <[email protected]>
Co-authored-by: nicolas <[email protected]>
Co-authored-by: Roberto <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant