store API proto AccountProof: optimize merkle node compression
#1178
Conversation
drahnr
changed the title
AccountProof: optimize merkle node compression
There was a problem hiding this comment.
Looking at the complexity of the code, I wonder if this should actually live in miden-base. The main reason is that the client would need to deserialize this data but the client doesn't get anything from the node except for protobuf files.
In miden-base, we could attach the logic to the PartialStorageMap struct. Basically, we need two things there:
- Given a
PartialStorageMapwe need to getSmtLeafs andInnerNodes from it. Not sure what the name of the function would be - but getting this data shouldn't be too difficult. - Given a set of
SmtLeafs andInnerNodes, we need a constructor that would build the underlyingPartialSmtfrom this.
Then, here in miden-node we'll just need to convert these to/from protobuf structs - so, the logic will be pretty straight-forward.
| message AccountStateHeader { | ||
| // Represents a single storage slot with the requested keys and their respective values. | ||
| message StorageSlotMapProof { | ||
| message StorageSlotMapPartialSmt { |
There was a problem hiding this comment.
I think this message could be just PartialStorageMap.
| impl From<NodeIndex> for proto::primitives::NodeIndex { | ||
| fn from(value: NodeIndex) -> Self { | ||
| proto::primitives::NodeIndex { | ||
| depth: value.depth() as u32, | ||
| value: value.value(), | ||
| } | ||
| } | ||
| } | ||
| impl TryFrom<proto::primitives::NodeIndex> for NodeIndex { | ||
| type Error = ConversionError; | ||
| fn try_from(index: proto::primitives::NodeIndex) -> Result<Self, Self::Error> { | ||
| let depth = u8::try_from(index.depth)?; | ||
| let value = index.value; | ||
| Ok(NodeIndex::new(depth, value)?) | ||
| } | ||
| } |
There was a problem hiding this comment.
Probably nothing, but we encode the depth as u32 and then cast it to u8. Should a comment be added about why is it always valid?
|
I'm a bit lost in regards to the progress of the endpoint refactors. Will this still happen? There is also a similar PR on the client which we should revisit or close. |
|
The intent was to merge it as the very first piece as part of #1185 - however, since other, larger refecators landed it became somewhat conflict heavy. I'll migrate these changes on constructing the partial tree once we get those merged, I'll downgrade this to draft for the time being. Could you reference the client PR that has overlap? |
I think I mistook the related PR, but I was talking about this one: 0xMiden/miden-client#1167 |
Follow up to #1158
Changes
Previously we used the
Serializable/Deserializableimplementations forPartialSmtwhich uses more leaves than the theoretical minimum required to reconstruct all intermediate nodes.The PR implements a DFS per sibling required, calculating all inner nodes dynamically. Recomputation is avoided by using lookup cache per node index.