-
Notifications
You must be signed in to change notification settings - Fork 602
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
Conversation
55b4648
to
e9fbb8a
Compare
79987f4
to
c10718e
Compare
let abi = BaseContract::from( | ||
parse_abi(&["function fastLz(bytes) external view returns (uint256)"]).unwrap(), | ||
); | ||
let contract_bytecode = Bytecode::new_raw(bytes!("608060405234801561001057600080fd5b506004361061002b5760003560e01c8063920a769114610030575b600080fd5b61004361003e366004610374565b610055565b60405190815260200160405180910390f35b600061006082610067565b5192915050565b60606101e0565b818153600101919050565b600082840393505b838110156100a25782810151828201511860001a1590930292600101610081565b9392505050565b825b602082106100d75782516100c0601f8361006e565b5260209290920191601f19909101906021016100ab565b81156100a25782516100ec600184038361006e565b520160010192915050565b60006001830392505b61010782106101385761012a8360ff1661012560fd6101258760081c60e0018961006e565b61006e565b935061010682039150610100565b600782106101655761015e8360ff16610125600785036101258760081c60e0018961006e565b90506100a2565b61017e8360ff166101258560081c8560051b018761006e565b949350505050565b80516101d890838303906101bc90600081901a600182901a60081b1760029190911a60101b17639e3779b90260131c611fff1690565b8060021b6040510182815160e01c1860e01b8151188152505050565b600101919050565b5060405161800038823961800081016020830180600d8551820103826002015b81811015610313576000805b50508051604051600082901a600183901a60081b1760029290921a60101b91909117639e3779b9810260111c617ffc16909101805160e081811c878603811890911b9091189091528401908183039084841061026857506102a3565b600184019350611fff821161029d578251600081901a600182901a60081b1760029190911a60101b17810361029d57506102a3565b5061020c565b8383106102b1575050610313565b600183039250858311156102cf576102cc87878886036100a9565b96505b6102e3600985016003850160038501610079565b91506102f08782846100f7565b9650506103088461030386848601610186565b610186565b915050809350610200565b5050617fe061032884848589518601036100a9565b03925050506020820180820383525b81811161034e57617fe08101518152602001610337565b5060008152602001604052919050565b634e487b7160e01b600052604160045260246000fd5b60006020828403121561038657600080fd5b813567ffffffffffffffff8082111561039e57600080fd5b818401915084601f8301126103b257600080fd5b8135818111156103c4576103c461035e565b604051601f8201601f19908116603f011681019083821181831017156103ec576103ec61035e565b8160405282815287602084870101111561040557600080fd5b82602086016020830137600092810160200192909252509594505050505056fea264697066735822122000646b2953fc4a6f501bd0456ac52203089443937719e16b3190b7979c39511264736f6c63430008190033")); |
There was a problem hiding this comment.
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
c10718e
to
08954f3
Compare
There was a problem hiding this 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?
crates/revm/src/optimism/fast_lz.rs
Outdated
let abi = BaseContract::from( | ||
parse_abi(&["function fastLz(bytes) external view returns (uint256)"]).unwrap(), | ||
); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
08954f3
to
a9f0988
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left few nits
8317b6f
to
631b12d
Compare
cf1e673
to
a2ea6eb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -64,6 +64,7 @@ pub enum SpecId { | |||
CANCUN = 20, | |||
ECOTONE = 21, | |||
PRAGUE = 22, | |||
FJORD = 23, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
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 withinL1BlockInfo.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.