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

feat(optimism): Implement new L1 cost function for Fjord #1420

Merged
merged 15 commits into from
May 28, 2024

Conversation

BrianBland
Copy link
Contributor

@BrianBland BrianBland commented May 14, 2024

  • Add new Optimism Fjord hardfork spec
  • Implement the Fjord L1 Cost function according to this spec

Note 1: This adds a length-only FastLZ compression implementation under the optimism module. One alternative approach would be to depend on a battle-tested full FastLZ compression implementation, but this would be more computationally expensive and may require the use of C bindings. This could also be a test-only dependency for testing behavioral equivalence.

Note 2: The FastLZ size estimator is called in L1BlockInfo.data_gas, which itself is called within L1BlockInfo.calculate_tx_l1_cost. Callers may often need to perform this pseudo-compression twice per transaction unless we refactor these methods to reuse this computation.

let abi = BaseContract::from(
parse_abi(&["function fastLz(bytes) external view returns (uint256)"]).unwrap(),
);
let contract_bytecode = Bytecode::new_raw(bytes!("608060405234801561001057600080fd5b506004361061002b5760003560e01c8063920a769114610030575b600080fd5b61004361003e366004610374565b610055565b60405190815260200160405180910390f35b600061006082610067565b5192915050565b60606101e0565b818153600101919050565b600082840393505b838110156100a25782810151828201511860001a1590930292600101610081565b9392505050565b825b602082106100d75782516100c0601f8361006e565b5260209290920191601f19909101906021016100ab565b81156100a25782516100ec600184038361006e565b520160010192915050565b60006001830392505b61010782106101385761012a8360ff1661012560fd6101258760081c60e0018961006e565b61006e565b935061010682039150610100565b600782106101655761015e8360ff16610125600785036101258760081c60e0018961006e565b90506100a2565b61017e8360ff166101258560081c8560051b018761006e565b949350505050565b80516101d890838303906101bc90600081901a600182901a60081b1760029190911a60101b17639e3779b90260131c611fff1690565b8060021b6040510182815160e01c1860e01b8151188152505050565b600101919050565b5060405161800038823961800081016020830180600d8551820103826002015b81811015610313576000805b50508051604051600082901a600183901a60081b1760029290921a60101b91909117639e3779b9810260111c617ffc16909101805160e081811c878603811890911b9091189091528401908183039084841061026857506102a3565b600184019350611fff821161029d578251600081901a600182901a60081b1760029190911a60101b17810361029d57506102a3565b5061020c565b8383106102b1575050610313565b600183039250858311156102cf576102cc87878886036100a9565b96505b6102e3600985016003850160038501610079565b91506102f08782846100f7565b9650506103088461030386848601610186565b610186565b915050809350610200565b5050617fe061032884848589518601036100a9565b03925050506020820180820383525b81811161034e57617fe08101518152602001610337565b5060008152602001604052919050565b634e487b7160e01b600052604160045260246000fd5b60006020828403121561038657600080fd5b813567ffffffffffffffff8082111561039e57600080fd5b818401915084601f8301126103b257600080fd5b8135818111156103c4576103c461035e565b604051601f8201601f19908116603f011681019083821181831017156103ec576103ec61035e565b8160405282815287602084870101111561040557600080fd5b82602086016020830137600092810160200192909252509594505050505056fea264697066735822122000646b2953fc4a6f501bd0456ac52203089443937719e16b3190b7979c39511264736f6c63430008190033"));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @danyalprout as this is an adaptation of your parity fuzz test

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

@clabby mind taking a look at this?

is the fast_lz function maintained anywhere?

Comment on lines 272 to 274
let abi = BaseContract::from(
parse_abi(&["function fastLz(bytes) external view returns (uint256)"]).unwrap(),
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This shoudl be using alloy cc @DaniPopes

alloy_sol_types::sol! {
    interface FaztLz {
       function fastLz(bytes) external view returns (uint256);
   }
}

And then fastLzCall::abi_encode

https://docs.rs/alloy-sol-types/latest/alloy_sol_types/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks I'll apply that refactor to the examples as well if this is the preferred method for interacting with solidity types
Edit: I moved the examples change to a separate PR

crates/revm/Cargo.toml Outdated Show resolved Hide resolved
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

left few nits

@BrianBland BrianBland marked this pull request as ready for review May 24, 2024 18:06
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

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

lgtm

@rakita rakita dismissed gakonst’s stale review May 26, 2024 17:27

addressed in different PR

@@ -64,6 +64,7 @@ pub enum SpecId {
CANCUN = 20,
ECOTONE = 21,
PRAGUE = 22,
FJORD = 23,
Copy link
Member

Choose a reason for hiding this comment

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

Should FJORD be after or before PRAGUE? In secp256r1 precompile PR it was before.

Copy link
Member

@rakita rakita May 28, 2024

Choose a reason for hiding this comment

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

I have put FJORD before PRAGUE as Prague is still not activated

@rakita rakita merged commit d903399 into bluealloy:main May 28, 2024
25 checks passed
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.

None yet

5 participants