refactor(data): Change the way how we represent and convert Merkle trees

[danielpapp-trilitech]

Relates to RV-829

What

• Replaced MerkleTree::Leaf tuple with a struct
• Removed the impl_modify_map_collect function
• Replaced the uses of the mentioned function with a non-recursive direct implementation.

Why

We need to store data in the internal nodes of Merkle-like trees. This PR is part of that effort.

How

Remove impl_modify_map_collect and replace it with something more flexible. Extract the leaf data from the MerkleTree so that it's easier to operate on it.

Manually Testing

make all

Regressions

Tasks for the Author

• Link all Linear issues related to this MR using magic words (e.g. part of, relates to, closes).
• Eliminate dead code and other spurious artefacts introduced in your changes.
• Document new public functions, methods and types.
• Make sure the documentation for updated functions, methods, and types is correct.
• Add tests for bugs that have been fixed.
• Explain changes to regression test captures when applicable.
• Write commit messages in agreement with our guidelines.
• Self-review your changes to ensure they are high-quality.
• Complete all of the above before assigning this MR to reviewers.

[github-actions]

Benchmark results for revision c49c6d6:

Metric	Duration	TPS
Mean	1.524282325s	26.242
Worst	1.532877724s	26.095
Best	1.517162313s	26.365
Standard Deviation	±4.317972ms	±0.074

Full results

Run	Transfers	Duration	TPS
1	40	1.520223217s	26.312
2	40	1.530782283s	26.130
3	40	1.531324488s	26.121
4	40	1.523912704s	26.248
5	40	1.532877724s	26.095
6	40	1.519768839s	26.320
7	40	1.531002224s	26.127
8	40	1.522088255s	26.280
9	40	1.521846244s	26.284
10	40	1.52468562s	26.235
11	40	1.519036537s	26.332
12	40	1.526433449s	26.205
13	40	1.52476934s	26.233
14	40	1.523579881s	26.254
15	40	1.521673924s	26.287
16	40	1.522643817s	26.270
17	40	1.517162313s	26.365
18	40	1.520431143s	26.308
19	40	1.526750908s	26.199
20	40	1.524653582s	26.235

Compare the results above with those for the default branch.

[codecov]

Codecov Report

❌ Patch coverage is 98.84393% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.26%. Comparing base (792789c) to head (693f480).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
data/src/merkle_proof/proof_tree.rs	98.66%	1 Missing ⚠️
...iscv/lib/src/state_backend/proof_backend/merkle.rs	98.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #535      +/-   ##
==========================================
- Coverage   91.26%   91.26%   -0.01%     
==========================================
  Files         110      108       -2     
  Lines       20197    20233      +36     
  Branches    20197    20233      +36     
==========================================
+ Hits        18433    18465      +32     
- Misses       1388     1392       +4     
  Partials      376      376              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[danielpapp-trilitech]

I went through the diff. I added docs to everything new that I created with public visibility or what I moved and kept the public visibility. Let me know if I've missed anything!

[vapourismo]

The commit message makes it appear like it should be 3 commits.

[vapourismo]

I noticed this PR hasn't received updates in a bit and currently can't land due to the merge conflicts. I'm marking this as draft for now.

[kurtisc]

The commit message title isn't clear, I don't want understanding the most prominent part of a public commit depend on having access to the private Linear tickets. The title should explain what applying this commit does, see https://github.com/tezos/riscv-pvm/blob/main/docs/contributing/commit-messages.md.

I don't think the (data) scope in the title is correct or necessary, which means you'd have more space. Ideally the title is <= 50 characters but this isn't consistently followed in this repo.

Finally, the tense used for the body of the git commit should be present, as it's what the patch is 'doing'. The body lists three things, one of which is a replacement for removing another. That makes me think this should have been two commits, one for changing to use a struct and the other for replacing impl_modify_map_collect.

[danielpapp-trilitech]

Ok. I tried to address your comments regarding the commit message. As for splitting the change into two commits. I prefer to keep it as is.

[kurtisc]

Ok. I tried to address your comments regarding the commit message.

It's almost there but I'd change it to

refactor: Enable the node variant of Merkle tree to contain data
- Removes MerkleTree::Leaf tuple with a struct
- Removes the impl_modify_map_collect function
- Replaces the uses of the mentioned function
  with a non-recursive direct implementation.

While the title should be imperative, the body can still be present tense, active voice.

As for splitting the change into two commits. I prefer to keep it as is.

Sure. I think splitting it in this PR is more effort than it's worth but it's easier to review future PRs that keep them separate.

[danielpapp-trilitech]

Updated the commit message. Thanks for the review!

[emturner]

some initial comments