Skip to content

feat(ds): implementing rv-825#549

Draft
danielpapp-trilitech wants to merge 1 commit intomainfrom
dpapp/rv-825
Draft

feat(ds): implementing rv-825#549
danielpapp-trilitech wants to merge 1 commit intomainfrom
dpapp/rv-825

Conversation

@danielpapp-trilitech
Copy link
Contributor

Relates to RV-825

What

I'm adding the CommitOperationCollection
which holds all the changes that need to be
persisted when a commit request comes in.

Why

We need to be able to persist the new nodes that become part of the Merkle Tree between two commits, and also delete the ones that we don't need any more.

How

We store a collection of nodes that either need to be deleted or inserted into RocksDB during a commit. During a commit operation, we iterate through the elements of this collection and act accordingly.

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.

I'm adding the CommitOperationCollection
which holds all the changes that need to be
persisted when a commit request comes in.
@github-actions
Copy link

github-actions bot commented Dec 1, 2025

Benchmark results for revision 1ed526f:

Metric Duration TPS
Mean 1.666790892s 23.999
Worst 1.686760241s 23.714
Best 1.656387709s 24.149
Standard Deviation ±7.907558ms ±0.113
Full results
Run Transfers Duration TPS
1 40 1.680825132s 23.798
2 40 1.659644906s 24.102
3 40 1.666177848s 24.007
4 40 1.664307674s 24.034
5 40 1.683372417s 23.762
6 40 1.666819116s 23.998
7 40 1.671747809s 23.927
8 40 1.686760241s 23.714
9 40 1.666934638s 23.996
10 40 1.656387709s 24.149
11 40 1.65938879s 24.105
12 40 1.665780215s 24.013
13 40 1.664969557s 24.024
14 40 1.660541539s 24.089
15 40 1.658611154s 24.117
16 40 1.665278393s 24.020
17 40 1.6654925s 24.017
18 40 1.664325619s 24.034
19 40 1.664995655s 24.024
20 40 1.663456921s 24.046

Compare the results above with those for the default branch.


impl CommitOperationCollection {
pub(crate) fn add_new_node_to_commit(&mut self, node: &Arc<MavlNode>) {
let node_hash: [u8; 32] = *hash(node).as_bytes();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is a bit too expensive to calculate here. We should store the key rather.

impl CommitOperationCollection {
pub(crate) fn add_new_node_to_commit(&mut self, node: &Arc<MavlNode>) {
let node_hash: [u8; 32] = *hash(node).as_bytes();
let serialized_node = node.encode_to_vec();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably not needed at this stage.

}

pub(crate) fn remove_node_from_commit(&mut self, node: &Arc<MavlNode>) {
let node_hash: [u8; 32] = *hash(node).as_bytes();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also should not hash here, but just store the key. But we should also take ownership of the data of the node, so here we probably want to serialise the data or just own the node by owning its Arc.

@codecov
Copy link

codecov bot commented Dec 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.72%. Comparing base (5985f20) to head (c089c77).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #549   +/-   ##
=======================================
  Coverage   90.72%   90.72%           
=======================================
  Files         110      110           
  Lines       20279    20279           
  Branches    20279    20279           
=======================================
  Hits        18399    18399           
  Misses       1552     1552           
  Partials      328      328           

☔ 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
Copy link
Contributor Author

Scraping this for now and going with a more simple version. The approach needs some refactoring because it uses a lot of copies at the moment. I have an idea for a more efficient approach. I will update the relevant Linear task.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant