search

paritytech / substrate

Substrate: The platform for blockchain innovators
Apache License 2.0
8.39k stars 2.65k forks source link

Get rid of junk in storage proofs. #9170

Open arkpar opened 3 years ago

arkpar commented 3 years ago

Currently block execution storage proofs contain nodes that are not actually necessary to validate the block.

There's a common pattern in the runtime that we use to store intermediate values. Values that need to be passed around between extrinsics. Such values are put in the storage and are removed at the end of block execution with take. Here's one example: https://github.com/paritytech/substrate/blob/df4a58833a650cf37fc97764bf6c9314435e3cb2/frame/system/src/lib.rs#L1361

The problem here is that take attempts to delete the value from the trie, even if it is not there. This attempted deletion query ends up being part of the proof.

A proposed solution would be to use revert rather than take. revert would simply delete the key from the state overlay. Alternatively we could use static memory instead of storage API for intermediate values. This would also be a significant performance win, although some additional work would be needed to implement transactional semantics with this method.

pepyakin commented 3 years ago

Putting those values in the memory is a bigger task than it may sound.

The problems is that we generally do not reuse runtime instances. E.g. in the block builder in order to push an extrinsic we create a new runtime instance. The runtime instance is destroyed (semantically after the call has finished.

If we decide to try to build a block within a single runtime instance then we will bump into another problem. If an extrinsic panics during the inclusion, we should be able to rollback its changes. Rolling back storage overlay changes is easy. However, rolling back changes to the wasm instance itself is not. It would require something like https://github.com/paritytech/polkadot-sdk/issues/370. It is achievable, but is very involved.

kianenigma commented 1 year ago

Putting those values in the memory is a bigger task than it may sound.

Looking at @cheme's suggestion here, as a non-expert in the field, I am actually tempted to say a transient OverlaySetChange with new host functions to manipulate it is a very easy way to solve this.