Make state witness possible to implement.

[robin-near]

State witness is the complete data needed to execute a chunk and verify the outcome, which includes the state proof (the proof of the current state) as well as the resulting new state root.

The code to produce a state proof already existed as part of the challenge infrastructure, but was disabled due to a minor issue in the trie code (#6316 ). This PR resolves that, and adds a test that indeed, a state witness is feasible to produce and use.

#9292

[Longarithm]

Makes sense, I have several questions:

1. Should we also uncomment all code marked with TODO (#6316)? It includes challenge-related tests which are partially related.
2. Don't we need to check correctness of all other ApplyResult fields, like outcomes? I don't understand why we ignore it.
3. Test request: if partial storage is invalid, transition_for_replay must return some error (RuntimeError::StorageError?)

[robin-near]

@Longarithm Hmm, I tried this out on a live node but I'm almost never getting a consistent gas calculation using the recording storage. The only thing different I guess is the mem/db counters? But I don't see why they would be different. Any ideas you might have, since you're way more knowledgeable about storage costs?

Here's an example mismatch:

Normal execution outgoing receipts:

[Receipt { predecessor_id: AccountId("system"), receiver_id: AccountId("relay.aurora"), receipt_id: DqTfP5nSJvmY4ghFeKjSaLsmCao2uvFtZqeC9xaEJ3zF, receipt: Action(ActionReceipt { signer_id: AccountId("relay.aurora"), signer_public_key: ed25519:3X9pCFMLyFHLKSPuWMH8e5AnA13BXubLFGxk2vZxFyD3, gas_price: 0, output_data_receivers: [], input_data_ids: [], actions: [Transfer(TransferAction { deposit: 188560890027736541439340 })] }) }]

Recording execution outgoing receipts for the same chunk (literally executed right after this, no parameters changed):

[Receipt { predecessor_id: AccountId("system"), receiver_id: AccountId("relay.aurora"), receipt_id: DqTfP5nSJvmY4ghFeKjSaLsmCao2uvFtZqeC9xaEJ3zF, receipt: Action(ActionReceipt { signer_id: AccountId("relay.aurora"), signer_public_key: ed25519:3X9pCFMLyFHLKSPuWMH8e5AnA13BXubLFGxk2vZxFyD3, gas_price: 0, output_data_receivers: [], input_data_ids: [], actions: [Transfer(TransferAction { deposit: 188505601333514141439340 })] }) }]

As you can see, everything's the same except the deposit amount - which means that the gas spent was different.

[robin-near]

I think the problem here is that flat storage is not taken into account for this TrieStorage type. And it's not very straight forward to integrate flat storage into it.

[Longarithm]

I think the problem here is that flat storage is not taken into account for this TrieStorage type. And it's not very straight forward to integrate flat storage into it.

Makes sense. Interesting.
I think flat storage shouldn't be integrated into them:

• TrieRecordingStorage - as it records proof, we need to read all nodes. We can do this in parallel if needed, but let's say we do it in a Trie::lookup as we got used to.
• TrieMemoryPartialStorage - it has all nodes in memory so all reads are fast anyway.

So I'd say the only purpose of KeyLookupMode::FlatStorage is to generate lower trie node counts and don't care how we read nodes. I'll make a quick fix for that.

[Longarithm]

Mmm no, it's not easy.
I wanted to use this snippet https://github.com/near/nearcore/pull/9329/files#diff-9513a9e6a535b2fbf3cc057194372ffe42d7f795e790e87c13551747ad72c79dR825-R830. However, it doesn't work with "recording/recorded" storages because they don't have Store.

At the same time, we want to have some way to simulate flat storage without actually having it. #8741 is waiting for it.

First way: we can create a trait TrieNodeAccess which supports getting a node by hash without impacting costs and require all TrieStorage to implement that.

Second way: implement long-standing refactoring idea discussed here https://near.zulipchat.com/#narrow/stream/313099-pagoda.2Fstorage/topic/handling.20storage.20metrics/near/353884494

In this context, it means:

• Trie::lookup also takes KeyLookupMode and passes it to TrieStorage::retrieve_raw_bytes
• In retrieve_raw_bytes, if lookup mode is FlatStorage, we just return node directly and don't touch any caches. Otherwise we update caches and counters.

If this is done correctly, FlatStorage mode just means that costs are smaller but storage implementation doesn't need to have flat storage view. I'd go with the second way and perhaps show the PR tomorrow - what do you think?

[robin-near]

As this implementation is incorrect, I'm closing this in favor of #9350