Improvements in MockStoreSuccessBuilder
#253
Conversation
| @@ -224,7 +224,7 @@ impl BlockProver { | |||
| let proof_hash = Digest::default(); | |||
| let timestamp: Felt = SystemTime::now() | |||
| .duration_since(UNIX_EPOCH) | |||
| .expect("today is expected to be before 1970") | |||
| .expect("today is expected to be after 1970") | |||
| let produced_nullifiers = batches | ||
| .iter() | ||
| .cloned() | ||
| .flat_map(TransactionBatch::produced_nullifiers) | ||
| .map(|nullifier| nullifier.inner()) | ||
| .collect(); |
There was a problem hiding this comment.
This should be removed, and fixes the test (as you mentioned offline).
The problem is that you're initializing the store with the "new" nullifiers that the transaction wants to update. That is, when a batch produces nullifiers (i.e. sets their value to [block_num, 0, 0, 0] in the store), they should have a value of [0, 0, 0, 0] in the store. Thus, in this function, we should not take the produced nullifiers and add them to the store - the block kernel will be doing that.
The error BlockProverFailed(ProgramExecutionFailed(MerkleStoreLookupFailed(RootNotInStore(...)))) is doesn't give much insight into why this fails (maybe we can fix that). When we initialize the Merkle store from the block witness, we basically assume that the value is 0 in the store (here). But since you set the value to something else here, the Merkle store root becomes inconsistent with the actual store's root (due to the fact that 2 nullifiers are set in the Merkle store, and not set in the actual store). So when we try to smt::set the new nullifier here, we provide the incorrect Merkle store root, which the VM complains about.
There was a problem hiding this comment.
That was my main concern! Thank you! I've fixed this and CI is expected to pass now. Please, make a review if you have a chance!
MockStoreSuccessBuilderandbuild_expected_block_header()now compute produced notes rootMockStoreSuccessBuilder::new()was replaced with two constructors:from_batchesandfrom_accountswhich allow it to fill initial account IDs and state and also nullifiers and produced notes.This will resolve #79