-
Notifications
You must be signed in to change notification settings - Fork 17
feat: Interop roots processing #419
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
base: dev
Are you sure you want to change the base?
Conversation
## What ❔ This PR removes support for EIP-712 transactions, as it's no longer needed. <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- The `Why` has to be clear to non-Matter Labs entities running their own ZK Chain --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Is this a breaking change? - [ ] Yes - [ ] No ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted.
## What ❔ Fixes issues reported by Audittens, mostly related to rlp parsing (not rejecting non-canonical encodings) <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- The `Why` has to be clear to non-Matter Labs entities running their own ZK Chain --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Is this a breaking change? - [ ] Yes - [ ] No ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. - [ ] Code has been formatted.
| [workspace.dependencies] | ||
| zksync_os_evm_errors = { version = "0.0.10", default-features = false } | ||
| zksync_os_interface = { version = "0.0.10"} | ||
| zksync_os_evm_errors = { git = "https://github.com/matter-labs/zksync-os-interface", branch = "vv-interop-roots", package = "zksync_os_evm_errors", default-features = false } |
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.
To not forget to update
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.
| @@ -0,0 +1 @@ | |||
| 608060405234801561000f575f5ffd5b506004361061004a575f3560e01c80633b43dbde1461004e57806377cfd1711461006a578063cca2f7bc1461009a578063fb6200c6146100b6575b5f5ffd5b61006860048036038101906100639190610369565b6100d2565b005b610084600480360381019061007f91906103e3565b6100f6565b6040516100919190610439565b60405180910390f35b6100b460048036038101906100af91906104b3565b610115565b005b6100d060048036038101906100cb9190610553565b6101c4565b005b6100f3815f013582602001358380604001906100ee91906105d0565b6101d6565b50565b5f602052815f5260405f20602052805f5260405f205f91509150505481565b5f8282905090505f5b818110156101be576101b384848381811061013c5761013b610632565b5b905060200281019061014e919061065f565b5f013585858481811061016457610163610632565b5b9050602002810190610176919061065f565b6020013586868581811061018d5761018c610632565b5b905060200281019061019f919061065f565b80604001906101ae91906105d0565b6101d6565b80600101905061011e565b50505050565b6101d0848484846101d6565b50505050565b60018282905014610213576040517f2f59bd0d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5f5f1b82825f81811061022957610228610632565b5b9050602002013503610267576040517f9b5f85eb00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b5f5f1b5f5f8681526020019081526020015f205f8581526020019081526020015f2054146102c1576040517f2d48e8cf00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b81815f8181106102d4576102d3610632565b5b905060200201355f5f8681526020019081526020015f205f8581526020019081526020015f208190555082847f6b451b8422636e45b93bf7f594fa2c1769d039766c4254a6e7f9c0ee1715cdb084846040516103319291906106fe565b60405180910390a350505050565b5f5ffd5b5f5ffd5b5f5ffd5b5f606082840312156103605761035f610347565b5b81905092915050565b5f6020828403121561037e5761037d61033f565b5b5f82013567ffffffffffffffff81111561039b5761039a610343565b5b6103a78482850161034b565b91505092915050565b5f819050919050565b6103c2816103b0565b81146103cc575f5ffd5b50565b5f813590506103dd816103b9565b92915050565b5f5f604083850312156103f9576103f861033f565b5b5f610406858286016103cf565b9250506020610417858286016103cf565b9150509250929050565b5f819050919050565b61043381610421565b82525050565b5f60208201905061044c5f83018461042a565b92915050565b5f5ffd5b5f5ffd5b5f5ffd5b5f5f83601f84011261047357610472610452565b5b8235905067ffffffffffffffff8111156104905761048f610456565b5b6020830191508360208202830111156104ac576104ab61045a565b5b9250929050565b5f5f602083850312156104c9576104c861033f565b5b5f83013567ffffffffffffffff8111156104e6576104e5610343565b5b6104f28582860161045e565b92509250509250929050565b5f5f83601f84011261051357610512610452565b5b8235905067ffffffffffffffff8111156105305761052f610456565b5b60208301915083602082028301111561054c5761054b61045a565b5b9250929050565b5f5f5f5f6060858703121561056b5761056a61033f565b5b5f610578878288016103cf565b9450506020610589878288016103cf565b935050604085013567ffffffffffffffff8111156105aa576105a9610343565b5b6105b6878288016104fe565b925092505092959194509250565b5f5ffd5b5f5ffd5b5f5ffd5b5f5f833560016020038436030381126105ec576105eb6105c4565b5b80840192508235915067ffffffffffffffff82111561060e5761060d6105c8565b5b60208301925060208202360383131561062a576106296105cc565b5b509250929050565b7f4e487b71000000000000000000000000000000000000000000000000000000005f52603260045260245ffd5b5f8235600160600383360303811261067a576106796105c4565b5b80830191505092915050565b5f82825260208201905092915050565b5f5ffd5b82818337505050565b5f6106ae8385610686565b93507f07ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff8311156106e1576106e0610696565b5b6020830292506106f283858461069a565b82840190509392505050565b5f6020820190508181035f8301526107178184866106a3565b9050939250505056fea26469706673582212209947a4f0615e892247c992ba4bdf07b26d3f770db9fe5638a346c2685973b8c864736f6c634300081e0033 No newline at end of file | |||
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 be used in tests if matter-labs/era-contracts#1644 will be merged and deployed
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.
|
|
||
| /// Calls the L2 interop root storage contract to register multiple interop roots in a single batch. | ||
| /// Returns updated resource limits after the contract call. | ||
| /// TODO: use after L2InteropRootStorage.sol supports corresponding functionality |
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.
We should use this function if matter-labs/era-contracts#1644 will be merged and deployed
It allows us to do just one call instead of 1 call per 1 root. Ideally we also want to wrap it as a formal system transaction. Otherwise it will emit events that will pollute the first tx in the block.
| let mut block_context = block.get_block_context(); | ||
|
|
||
| // To check benchmarks | ||
| // TODO remove |
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.
To be discussed. Maybe it makes sense to keep it to always have ~worst case in benchmarks
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 think we can keep it, we basically don't use single run for anything other than our benches. Will be useful to catch any regression related to interop root processing
| let (interop_roots, computational_native_used_for_interop_roots) = | ||
| Self::process_interop_roots(&mut system, &mut system_functions, &mut memories, tracer)?; | ||
| block_computational_native_used += computational_native_used_for_interop_roots; | ||
| // TODO pubdata used by interop roots |
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.
We should also count the pubdata used here. It would be easier with a formal tx, since we update storage slots, potentially emit events etc.
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.
Also we should check for any side effects of this. E.g. state of the caches
| tx_data_responder, | ||
| preimage_responder, | ||
| } = dump; | ||
| // TODO also include interop roots |
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.
We should also include roots in the dump and then read them here
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 think we can just remove those functions, we're not using them
| with: | ||
| repository: matter-labs/era-evm-tester | ||
| ref: zk-ee-dev | ||
| ref: vv-interop-roots |
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.
To update
Benchmark report
|
| where | ||
| S::IO: IOSubsystemExt, | ||
| { | ||
| let interop_roots = system.get_interop_roots().map_err(|x| wrap_error!(x))?; |
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.
Do we want to use Vec instead of ArrayVec (as done for 4844 hashes in #381 )?
| tx_data_responder, | ||
| preimage_responder, | ||
| } = dump; | ||
| // TODO also include interop roots |
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 think we can just remove those functions, we're not using them
What ❔
Initial support for the interop roots processing.
Why ❔
Is this a breaking change?
Checklist