-
Notifications
You must be signed in to change notification settings - Fork 31
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(blockifier): iterate over aliases on the state diff #2535
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## yoav/aliasing/alias_updater #2535 +/- ##
==============================================================
Coverage ? 71.68%
==============================================================
Files ? 99
Lines ? 13663
Branches ? 13663
==============================================================
Hits ? 9794
Misses ? 3433
Partials ? 436 ☔ View full report in Codecov by Sentry. |
f20a030
to
4a4d28d
Compare
b2d7755
to
dff6b2f
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.
Reviewable status: 0 of 3 files reviewed, 3 unresolved discussions (waiting on @yoavGrs)
crates/blockifier/src/state/stateful_compression.rs
line 23 at r1 (raw file):
// The maximal contract address for which aliases are not used and all keys are serialized as is, // without compression. const MAX_NON_COMPRESSED_CONTRACT_ADDRESS: u8 = 15;
Like in the previous PR (and delete fn get_max_non_compressed_contract_address()
)
Suggestion:
const MAX_NON_COMPRESSED_CONTRACT_ADDRESS: ContractAddress = ContractAddress::from(15)
crates/blockifier/src/state/stateful_compression.rs
line 38 at r1 (raw file):
} pub fn insert_aliases<S: StateReader>(state: &mut CachedState<S>) -> StateResult<()> {
Better IMO, non-blocking
Suggestion:
pub fn allocate_aliases
crates/blockifier/src/state/stateful_compression.rs
line 61 at r1 (raw file):
} alias_updater.set_alias(&StorageKey(address.0))?; }
You also need to filter out all addresses < 16 (not only with storage updates)
Suggestion:
// Collect the addresses and storage keys that need aliases.
let mut addresses = BTreeSet::new();
let mut sorted_storage_keys = HashMap::new();
addresses.extend(writes.class_hashes.keys().chain(writes.nonces.keys()));
for (address, storage_key) in writes.storage.keys() {
addresses.insert(address);
sorted_storage_keys.entry(address).or_insert_with(BTreeSet::new).insert(storage_key);
}
// Iterate over the addresses and the storage keys and update the aliases.
let mut alias_updater = AliasUpdater::new(state)?;
for address in addresses {
if address < 16 {
continue;
}
if let Some(storage_keys) = sorted_storage_keys.get(address) {
for key in storage_keys {
alias_updater.set_alias(key)?;
}
}
alias_updater.set_alias(&StorageKey(address.0))?;
}
4a4d28d
to
dd3c882
Compare
dff6b2f
to
e3f2b88
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.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @nimrod-starkware)
crates/blockifier/src/state/stateful_compression.rs
line 38 at r1 (raw file):
Previously, nimrod-starkware wrote…
Better IMO, non-blocking
Done.
crates/blockifier/src/state/stateful_compression.rs
line 61 at r1 (raw file):
Previously, nimrod-starkware wrote…
You also need to filter out all addresses < 16 (not only with storage updates)
I validate that the addresses and the storage keys are greater than 127 in set_alias
.
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.
Reviewable status: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)
crates/blockifier/src/state/stateful_compression.rs
line 61 at r1 (raw file):
Previously, yoavGrs wrote…
I validate that the addresses and the storage keys are greater than 127 in
set_alias
.
ah ok, my bad
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.
Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @yoavGrs)
crates/blockifier/src/state/stateful_compression.rs
line 23 at r2 (raw file):
// The maximal contract address for which aliases are not used and all keys are serialized as is, // without compression. const MAX_NON_COMPRESSED_CONTRACT_ADDRESS: u8 = 15;
like previous PR: verify consistency with python value.
also, I believe this number is 3 now, no? @nimrod-starkware
Code quote:
const MAX_NON_COMPRESSED_CONTRACT_ADDRESS: u8 = 15;
crates/blockifier/src/state/stateful_compression.rs
line 42 at r2 (raw file):
/// storage keys (in ascending order) and for the address itself. pub fn allocate_aliases<S: StateReader>(state: &mut CachedState<S>) -> StateResult<()> { let writes = state.borrow_updated_state_cache()?.clone().writes;
not sure this clone will be needed if you use a transactional state (see prev PR)
Code quote:
let writes = state.borrow_updated_state_cache()?.clone().writes;
crates/blockifier/src/state/stateful_compression.rs
line 47 at r2 (raw file):
let mut addresses = BTreeSet::new(); let mut sorted_storage_keys = HashMap::new(); addresses.extend(writes.class_hashes.keys().chain(writes.nonces.keys()));
@nimrod-starkware are we still aliasing class hashes?
Code quote:
writes.class_hashes.keys()
crates/blockifier/src/state/stateful_compression.rs
line 48 at r2 (raw file):
let mut sorted_storage_keys = HashMap::new(); addresses.extend(writes.class_hashes.keys().chain(writes.nonces.keys())); for (address, storage_key) in writes.storage.keys() {
@nimrod-starkware the OS also processes reads, right?
is iterating over the writes
enough?
Code quote:
for (address, storage_key) in writes.storage.keys()
crates/blockifier/src/state/stateful_compression.rs
line 50 at r2 (raw file):
for (address, storage_key) in writes.storage.keys() { addresses.insert(address); if address > &get_max_non_compressed_contract_address() {
why add the address to addresses
if it's not over the max-non-compressed address?
@nimrod-starkware which address range does NOT get an alias?
Code quote:
addresses.insert(address);
if address > &get_max_non_compressed_contract_address() {
crates/blockifier/src/state/stateful_compression_test.rs
line 120 at r2 (raw file):
state.get_class_hash_at(ContractAddress::from(0x202_u16)).unwrap(); state.set_class_hash_at(ContractAddress::from(0x202_u16), ClassHash::default()).unwrap(); state.increment_nonce(ContractAddress::from(0x200_u16)).unwrap();
please add a contract address that sets storage at keys 7, 9, 4 in that order (to check alias order).
also, this contract should have address 0x200, so contract address aliasing order is also checked
Code quote:
state
.set_storage_at(ContractAddress::from(0x201_u16), StorageKey::from(0x300_u16), Felt::ONE)
.unwrap();
state
.set_storage_at(
get_max_non_compressed_contract_address(),
StorageKey::from(0x301_u16),
Felt::ONE,
)
.unwrap();
state.get_class_hash_at(ContractAddress::from(0x202_u16)).unwrap();
state.set_class_hash_at(ContractAddress::from(0x202_u16), ClassHash::default()).unwrap();
state.increment_nonce(ContractAddress::from(0x200_u16)).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.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)
crates/blockifier/src/state/stateful_compression.rs
line 23 at r2 (raw file):
Previously, dorimedini-starkware wrote…
like previous PR: verify consistency with python value.
also, I believe this number is 3 now, no? @nimrod-starkware
It's still 15, but we only verify that it's greater than 3 when deploying an account.
crates/blockifier/src/state/stateful_compression.rs
line 47 at r2 (raw file):
Previously, dorimedini-starkware wrote…
@nimrod-starkware are we still aliasing class hashes?
It's for the addresses that their class hash has changed IIUC, so that should be ok
crates/blockifier/src/state/stateful_compression.rs
line 48 at r2 (raw file):
Previously, dorimedini-starkware wrote…
@nimrod-starkware the OS also processes reads, right?
is iterating over thewrites
enough?
We need to verify the written value is different than the read value. Only in that case we should allocate (maybe it's already the case here).
crates/blockifier/src/state/stateful_compression.rs
line 50 at r2 (raw file):
Previously, dorimedini-starkware wrote…
why add the address to
addresses
if it's not over the max-non-compressed address?
@nimrod-starkware which address range does NOT get an alias?
For address < 16, we don't allocate aliases for storage keys (and since address < 16 < 128), we won't allocate for the address as well.
For storage keys < 128 we also don't allocate.
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.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @nimrod-starkware and @yoavGrs)
crates/blockifier/src/state/stateful_compression.rs
line 48 at r2 (raw file):
Previously, nimrod-starkware wrote…
We need to verify the written value is different than the read value. Only in that case we should allocate (maybe it's already the case here).
why should we only allocate in that case? is that what's happening in the OS, or is that what we "should" be doing?
in any case I think Yoav's way is the correct way so I'm unblocking, but @nimrod-starkware please make sure the OS tests (py side) include cases where an address is only read from, and make sure an alias isn't allocated
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.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)
crates/blockifier/src/state/stateful_compression.rs
line 48 at r2 (raw file):
Previously, dorimedini-starkware wrote…
why should we only allocate in that case? is that what's happening in the OS, or is that what we "should" be doing?
in any case I think Yoav's way is the correct way so I'm unblocking, but @nimrod-starkware please make sure the OS tests (py side) include cases where an address is only read from, and make sure an alias isn't allocated
We should do it because we post non-trivial state diff to L1.
That's also what is done in the OS.
There are already cases where only class hash changed and only the nonce has changed, or appeared in the state diff with prev_value == new_value
dd3c882
to
d9c83da
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.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware and @nimrod-starkware)
crates/blockifier/src/state/stateful_compression.rs
line 23 at r1 (raw file):
Previously, nimrod-starkware wrote…
Like in the previous PR (and delete
fn get_max_non_compressed_contract_address()
)
Done.
crates/blockifier/src/state/stateful_compression.rs
line 42 at r2 (raw file):
Previously, dorimedini-starkware wrote…
not sure this clone will be needed if you use a transactional state (see prev PR)
Not needed in the new approach.
crates/blockifier/src/state/stateful_compression.rs
line 47 at r2 (raw file):
Previously, nimrod-starkware wrote…
It's for the addresses that their class hash has changed IIUC, so that should be ok
Right.
crates/blockifier/src/state/stateful_compression.rs
line 48 at r2 (raw file):
Previously, nimrod-starkware wrote…
We should do it because we post non-trivial state diff to L1.
That's also what is done in the OS.
There are already cases where only class hash changed and only the nonce has changed, or appeared in the state diff with prev_value == new_value
For that, we need to take addresses and keys from the state diff.
crates/blockifier/src/state/stateful_compression_test.rs
line 120 at r2 (raw file):
We increment the nonce in address 0x200, for the second check.
state.increment_nonce(ContractAddress::from(0x200_u16)).unwrap();
e3f2b88
to
937947e
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.
Reviewed 3 of 3 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yoavGrs)
crates/blockifier/src/blockifier/transaction_executor.rs
line 172 at r3 (raw file):
let mut block_state = self.block_state.take().expect(BLOCK_STATE_ACCESS_ERR); let state_diff = if self.block_context.versioned_constants.enable_stateful_compression { state_diff_with_allocating_aliases(&mut block_state)?
rename
Suggestion:
state_diff_with_alias_allocation
d9c83da
to
48faf8d
Compare
937947e
to
246b31f
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.
Reviewable status: 0 of 3 files reviewed, 1 unresolved discussion (waiting on @dorimedini-starkware)
crates/blockifier/src/blockifier/transaction_executor.rs
line 172 at r3 (raw file):
Previously, dorimedini-starkware wrote…
rename
Done.
246b31f
to
ebece76
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.
Reviewed 2 of 3 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs)
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.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs)
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.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @yoavGrs)
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.
Reviewed 1 of 4 files at r12, all commit messages.
Reviewable status: 2 of 5 files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
log::debug!("Final block weights: {:?}.", self.bouncer.get_accumulated_weights()); let mut block_state = self.block_state.take().expect(BLOCK_STATE_ACCESS_ERR);
It's going to break the reexecution I think. TAL at this line -
transaction_executor.block_state |
need to solve this @dorimedini-starkware @AvivYossef-starkware
(it would be best if finalize
will consume self
)
Code quote:
let mut block_state = self.block_state.take().
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.
Reviewable status: 2 of 5 files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)
crates/blockifier/src/state/stateful_compression.rs
line 33 at r12 (raw file):
// alias is identical to the key). static MIN_VALUE_FOR_ALIAS_ALLOC: LazyLock<PatriciaKey> = LazyLock::new(|| PatriciaKey::try_from(INITIAL_AVAILABLE_ALIAS).unwrap());
Implement from_hex_unchecked
for PatriciaKey and remove these statics
Code quote:
static MAX_NON_COMPRESSED_CONTRACT_ADDRESS: LazyLock<ContractAddress> = LazyLock::new(|| {
ContractAddress(PatriciaKey::try_from(Felt::from_hex_unchecked("0xf")).unwrap())
});
// The minimal value for a key to be allocated an alias. Smaller keys are serialized as is (their
// alias is identical to the key).
static MIN_VALUE_FOR_ALIAS_ALLOC: LazyLock<PatriciaKey> =
LazyLock::new(|| PatriciaKey::try_from(INITIAL_AVAILABLE_ALIAS).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.
Reviewable status: 2 of 5 files reviewed, 4 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
It's going to break the reexecution I think. TAL at this line -
transaction_executor.block_state need to solve this @dorimedini-starkware @AvivYossef-starkware
(it would be best iffinalize
will consumeself
)
I.e., I guess that the reexecution needs the state with the aliases
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.
Reviewed 3 of 4 files at r12, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @AvivYossef-starkware, @yoavGrs, and @Yoni-Starkware)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
I.e., I guess that the reexecution needs the state with the aliases
WDYM? reexecution tests pass on this PR
e404184
to
46d453e
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.
Reviewable status: 3 of 6 files reviewed, 4 unresolved discussions (waiting on @AvivYossef-starkware, @dorimedini-starkware, @Itay-Tsabary-Starkware, @matan-starkware, @nimrod-starkware, and @Yoni-Starkware)
crates/blockifier/src/state/stateful_compression.rs
line 33 at r12 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Implement
from_hex_unchecked
for PatriciaKey and remove these statics
Done.
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.
Reviewed 3 of 3 files at r13, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @AvivYossef-starkware, @yoavGrs, and @Yoni-Starkware)
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.
Reviewed 1 of 4 files at r12, 1 of 3 files at r13, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @yoavGrs)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
Previously, dorimedini-starkware wrote…
WDYM? reexecution tests pass on this PR
- The broken function is
write_block_reexecution_data_to_file
, maybe it is not tested. - Consider whether this test needs the state before or after the alias allocation. Anyway, I guess that no 0.13.4 block is tested so the tests won't catch it
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.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @yoavGrs)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
- The broken function is
write_block_reexecution_data_to_file
, maybe it is not tested.- Consider whether this test needs the state before or after the alias allocation. Anyway, I guess that no 0.13.4 block is tested so the tests won't catch it
I think there is nothing that needs to be done with this ATM. the correct order IMO is: after 0.13.4 is released on mainnet, we add example blocks to the reexecution tests, and fix issues we find then
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.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @yoavGrs)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
Previously, dorimedini-starkware wrote…
I think there is nothing that needs to be done with this ATM. the correct order IMO is: after 0.13.4 is released on mainnet, we add example blocks to the reexecution tests, and fix issues we find then
This PR breaks the reexecution script even for old blocks, and we need it for next week for debugging the replay. Let's do it properly
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.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
This PR breaks the reexecution script even for old blocks, and we need it for next week for debugging the replay. Let's do it properly
I don't see where it breaks.
For blocks pre-0.13.4, the versioned constant enable_stateful_compression
ensures backward compatibility.
For 0.13.4 blocks, reexecute_and_verify_correctness
calls transaction_executor.finalize()
, the function that updates the aliases.
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.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @AvivYossef-starkware and @yoavGrs)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
This PR breaks the reexecution script even for old blocks, and we need it for next week for debugging the replay. Let's do it properly
@Yoni-Starkware what you're saying is: we will want to test the compressed output of old blocks, even though they were never compressed?
write_block_reexecution_data_to_file
indeed will not write compression data, but the current behavior will be (IIUC) as if no key has an alias yet - each reexecuted block will be treated as if it's the "first 0.13.4 block", which seems fine to me. no?
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.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware and @yoavGrs)
crates/blockifier/src/state/stateful_compression.rs
line 44 at r13 (raw file):
// Collect the contract addresses and the storage keys that need aliases. let contract_addresses: BTreeSet<ContractAddress> = state_diff.get_modified_contracts().into_iter().collect();
Please rename this function to get_contract_addresses
.
(They are the modified ones only when the the struct is created using diff
)
Code quote:
.get_modified_contracts()
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.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @AvivYossef-starkware and @yoavGrs)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
Previously, dorimedini-starkware wrote…
This PR breaks the reexecution script even for old blocks, and we need it for next week for debugging the replay. Let's do it properly
@Yoni-Starkware what you're saying is: we will want to test the compressed output of old blocks, even though they were never compressed?
write_block_reexecution_data_to_file
indeed will not write compression data, but the current behavior will be (IIUC) as if no key has an alias yet - each reexecuted block will be treated as if it's the "first 0.13.4 block", which seems fine to me. no?
Two problems:
- This function consumes the block_state, which is accessed after the call to finalize() -
self.block_state.take()
- the block_state does not contain the compression, so fixing 1 without 2 will postpone the issue. Better to change finalize to consume the entire
self
and let Aviv/Aner fix the reexecution script accordingly
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.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @Yoni-Starkware)
crates/blockifier/src/blockifier/transaction_executor.rs
line 170 at r12 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Two problems:
- This function consumes the block_state, which is accessed after the call to finalize() -
self.block_state.take()
- the block_state does not contain the compression, so fixing 1 without 2 will postpone the issue. Better to change finalize to consume the entire
self
and let Aviv/Aner fix the reexecution script accordingly
TODO :(
crates/blockifier/src/state/stateful_compression.rs
line 44 at r13 (raw file):
Previously, Yoni-Starkware (Yoni) wrote…
Please rename this function to
get_contract_addresses
.
(They are the modified ones only when the the struct is created usingdiff
)
Done.
46d453e
to
de47b5a
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.
Reviewed 5 of 5 files at r14, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @yoavGrs and @Yoni-Starkware)
crates/blockifier_reexecution/src/state_reader/utils.rs
line 242 at r14 (raw file):
// TODO(Yoav): After finalizing, the block state is None. We should return some fields from the // state.
you are doing this in a separate PR...?
Code quote:
// TODO(Yoav): After finalizing, the block state is None. We should return some fields from the
// state.
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.
Reviewed 3 of 5 files at r14, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @yoavGrs)
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.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware)
crates/blockifier_reexecution/src/state_reader/utils.rs
line 242 at r14 (raw file):
Previously, dorimedini-starkware wrote…
you are doing this in a separate PR...?
Me or Aner...
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.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @dorimedini-starkware and @yoavGrs)
de47b5a
to
cc26cc0
Compare
cc26cc0
to
344b579
Compare
Benchmark movements: |
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.
Reviewed 1 of 1 files at r15, 5 of 5 files at r16, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yoavGrs)
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.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @yoavGrs)
No description provided.