chore: Fixes to the boojum integration PR #415
Conversation
StanislavBreadless
changed the title
| let pubdata = pubdata_information.build_pubdata(); | ||
|
|
||
| assert!( | ||
| pubdata.len() / 32 <= OPERATOR_PROVIDED_L1_MESSENGER_PUBDATA_SLOTS - 2, |
There was a problem hiding this comment.
nit: this assert is a little bit incorrect - if pubdata.len is not divisible by 32.
There was a problem hiding this comment.
it is enforced by build_pubdata that it is divisible by 32
core/lib/multivm/src/interface/types/outputs/execution_state.rs
Outdated
Show resolved
Hide resolved
| @@ -49,6 +47,7 @@ impl<S: WriteStorage, H: HistoryMode> Vm<S, H> { | |||
| tracers, | |||
| self.storage.clone(), | |||
| refund_tracers, | |||
| Some(PubdataTracer::new(self.batch_env.clone(), execution_mode)), | |||
There was a problem hiding this comment.
why do we need some if it's always enabled?
There was a problem hiding this comment.
I am thinking that maybe custom implementations might not need it
There was a problem hiding this comment.
what is the custom implementation?
There was a problem hiding this comment.
For instance, the bootloader tester written by @mm-zk: https://github.com/matter-labs/era-system-contracts/tree/main/bootloader/test_infra
There was a problem hiding this comment.
I don't think that @mm-zk 's tooling would be broken if the pubdata tracer is always put there, though still I thought that our general approach is to preserve choices for the users of the VM
There was a problem hiding this comment.
ok... just less checks is a bit better performance wise :)
There was a problem hiding this comment.
I don't have strong opinions here (as long as this tracer doesn't prevent test bootloader from running)
Detected VM performance changes
|
What ❔
Fixes comments by @Deniallugo and @mm-zk
Why ❔
Checklist
zk fmtandzk lint.