The code is much more readable 🎉 Great job!

Now, I cannot understand what is written in the fn's comment.
So we are given proofNodes and constructing storageNodes following along the path to the single leaf node specified by given leafKey.
These storageNodes will have hashes of the children and keys? - not get this "but only the key of the children along the path".

Also: "The final node must contain the hash of the leaf ..." - which node is the final node, root or the nodes at the bottom of the trie 🤔

And: "It will not contain the leaf node even if it is set."
Hmm, the leaf is set because we have the leafKey right? Also the leaf node refers to the storageNodes we are constructing here.

Can you please try to improve this comment?

These storageNodes will have hashes of the children and keys? - not get this "but only the key of the children along the path".

The storageNodes will have the hashes of the children, but only the key of the child along the path outlined by the proof.

Also: "The final node must contain the hash of the leaf ..." - which node is the final node, root or the nodes at the bottom of the trie 🤔

Ah sorry, this is a confusing sentence. I was just trying to comment that we don't include the leaf node in the generated storage nodes. But if the path outlined by the proof leads the whole way to the leaf, and the leaf is set, then we should include it's hash. (This is kind of redundant given the above sentence)

Hmm, the leaf is set because we have the leafKey right?

Nope. The Key that the proof node points to may not be set, but we can still have the key.

Also the leaf node refers to the storageNodes we are constructing here.

Yup

I've updated the comment

Don't we need ro check that proofNodes[parentInd+1].Binary != nil as we did with the previous node?
also parentInd+1 < len(proofNodes) 🤔

Maybe we should assign the parent and have a single return:

parent := &proofNodes[parentInd]
if parent.Binary == nil {
    parent := &proofNodes[parentInd+1]
}
return parent.Binary.LeftHash, parent.Binary.RightHash

Don't we need ro check that proofNodes[parentInd+1].Binary != nil as we did with the previous node?

Nope, if proofNode[i] is an edge then we know proofNode[i+1] is Binary (it's not possible for an edge node to follow an edge node).

Maybe we should assign the parent and have a single return

Yup, this is much nicer!

parentInd+1 < len(proofNodes)

Good point. Let me think about that

parentInd+1 < len(proofNodes)

I added a check for this

What is proofKeys and proofValues? What is the difference from proofs?

Proofs are the set of nodes from the root to the boundary / proof keys and values

I explicitly labelled the proof keys and values for clarity (I find this is a bit easier to understand than placing the proof key as the first element in keys iff the proof key has actually been set - like seems to be done in geth)

Still not quite sure... In geth code, the firstKey parameter I'm guessing is there because the left proof should be generated based on the start key query (not actual first leaf that exist). Thats a little nuances.. No idea where proofKeys and proofValues come from. Remember, only one proof array, which in geth case is added like here https://github.com/ethereum/go-ethereum/blob/master/eth/protocols/snap/sync.go#L2816. The proof is like a bundle of nodes data, no idea which node is located where (the key). You only know the state root, and iterate from there to recalculate the path, well, at least nethermind do it that way..

the left proof should be generated based on the start key query (not actual first leaf that exist)

Currently the left proof key would just correspond to the start key query. And the last proofKey would correspond to the end key query. Similarly with the proof Values (which can be nil if they don't exist)

Remember, only one proof array, ..

Yup, but that should be a simple matter of merging the proof nodes into a single array, and then splitting them again, right? I've opened an issue for this

Also, regarding there only being a single proof, geth does store the proof nodes in a key-value database, but I believe there are two boundary proofs, which you can see here. I've kept them separate for clarity.

Afaiu, when there are key-value pairs to the left and right of the given range we need two boundary proofs. This is because we need to compute the correct shape of the sub-trie, and store the relevant hashes so that we can recompute the root hash for that range / sub trie. Having only a single boundary proof would no allow us to do this.

This seems like an encoding change. Is this intended?

Yes, I figured it made sense to store the LeftHash and RightHash fields in the node type