paritytech / paritytech/trie

Non optimal TrieDBMut key values queries on batch processing.

Open
#109 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
274
Forks
72
PR merge metrics
No merged PRs in 30d

Description

https://github.com/paritytech/substrate/pull/6780 did reveal that different input order of changes when processing a root with triedbmut can result in a slightly different query plan and thus different proof of execution.

The root cause is that triedbmut algorithm do 'fuse a branch without value and a single child with this single child' too eagerly.

The operation is applied in function fix and do access the single child.

If later we add a children to the deleted (when fused) branch, the deleted branch can be restored and there will be no use to query the single child.

So we end up querying an unneeded node in respect to a batch update.
This is not really bad as the hash of the fuse node is not calculated (hash are calculated lazily), but it ends up being an issue with the way we register proof in substrate (we store all node queried on the kv backend).

A first way to solve this should be to apply fix lazily (on root calculation) as we do with hash calculation and db writing, but it seems to me that this will require ordering the fix calls and is not as simple as insert node into memorydb.

A second way should be to rewrite the batch update.
I would propose/illustrate with (see #110 draft pr), to use a batch update that works on a sorted change and thus allow applying the node fusing only when we are sure no other change will be done on the branch (when exiting it). This kind of batch update is also good when looking at memory footprint (only a stack of max 16 nodes needs to be kept in memory), but it is not really relevant in substrate use case.

A third way would be to add the additional fix related query to substrate spec.
This is not a sound idea, specifying that the proof should be the strictly the required set of trie nodes to run the operation seems more correct to me and would make conflict with other implementation easier to rules out.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the TrieDBMut batch-processing path and its fix function, then read paritytech/substrate#6780 and the proposed approach in #110. The work is done when batch updates avoid querying nodes that are later restored or unused, while preserving the required execution proof behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.