trie-db: Fetch the closest merkle value

[lexnv]

This PR adds support for fetching the merkle value directly from the trie-db.

• Adds get_closest_merkle_value on the Trie trait
• TrieDb iterates over the nibbles of the provided key
  • At each iteration DB is queried for the containing node
  • last value encountered on the path from the root to the key is returned
• During trie traversal, the internal recorder is updated with trie actions
  • This should make other trie queries to the key more efficient
• Test with HashedValueNoExtThreshold<1> as trie layout and value update at given key

We could build on this PR to have support for recorded Merkel Value in the cache.

Would love to get your thoughts on this @cheme @arkpar 🙏
(For reference @tomaka to ensure we don't break anything wrt to compatibilities between smoldot and substrate)

Part of paritytech/substrate#14550.

// @paritytech/subxt-team

[arkpar]

The test should also check that the functions works correctly for keys that aren't acuatlly in the tree.
E.g. for a tree with nodes ["AAA"] it should return a closest descendant for the key "AA" but not for "AB". For the tree with nodes ["AAAA", "AABA] closes descendant for the key "A" should be the branch node, etc.

Also there's no need to make changes to the test trie after it is created really.

[jsdw]

Also, you don't need to

Argh, the suspense :D

[arkpar]

After reading the spec more carefully, I think the logic here requires some changes. We need to return the descendant of the provided key, not just the lower bound. Basically this means traversing the tree as usual, but for Leaf, Extension and NibbledBranch nodes if you run out of key, just check if the current node key slice extends the remainder.

if slice.starts_with(&partial) {
  return Ok(Some(hash)))
else
  return Ok(None)
} 

[cheme]

@lexnv , thanks for the work, I got a few question on corner cases, notably inline node (I am not 100% sure, depends of how the spec is understood).

[cheme]

Looks good.
Worth remembering that if at some point this get used in runtime (especially by a host function), not simply rpc, it would be worth fuzzing a bit.

[bkchr]

Left mainly some nitpicks. However, the return value is wrong. The merkle value is either the node data or the hash, depending on if this is an inline node or not.

[bkchr]

Now we always need to create an owned node, which is a little bit shitty. But fine for now I think.

[bkchr]

The return type is wrong.

enum MerkleValue {
     /// Is returned if `v.len() < MAX_INLINE_VALUE`.
     Node(Vec<u8>),
     Hash(H),
}

[cheme]

I did propose to use hash for inline node (generally I would prefer if inline node is something users don't need to be aware off). But I did not double check the rpc proposal.

[cheme]

Maybe trie crate api could return this enum, but the rpc return hash of the data value for inline node (this way we can avoid some hashing if this api get use by runtime in the future).

[bkchr]

But the spec is speaking about the merkle value and not the hash. The point being that if the data is smaller, we save bandwidth etc.

(this way we can avoid some hashing if this api get use by runtime in the future)

This doesn't add any extra hashing. If you need hashing for the runtime, we can just do it where we use it. There would be no extra hashing involved as we currently also hash the inline node manually.

[lexnv]

That makes sense! I've added the MerkleValue enum to be explicit about what the trie-db returns.

Then in substrate's RPC layer we could further hash(MerkleValue::Node(..)) such that we return a consistent view of nodes and not care about inline ones. And if this function gets ever called from the runtime we save a Layout:hash() call 🙏

[bkchr]

Then in substrate's RPC layer we could further hash(MerkleValue::Node(..)) such that we return a consistent view of nodes and not care about inline ones.

This still goes against the size argument I laid out above. I opened paritytech/json-rpc-interface-spec#88 to be sure that we use the correct value in the RPC.