Skip to content

Conversation

@lklimek
Copy link

@lklimek lklimek commented Jan 14, 2026

Issue being fixed or feature implemented

Bincode is not maintained anymore.

We use bincode v2.0.0-rc.3. We want to switch to stable v2.0.1, in order to maybe switch to one of bincode's forks in future.

What was done?

Updated bincode to 2.0.1, including code adjustments.

How Has This Been Tested?

cargo check

Breaking Changes

All dependencies should use the same fork of bincode.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Summary by CodeRabbit

  • Chores

    • Updated serialization dependencies (bincode/bincode_derive) to 2.0.1.
  • Refactor

    • Reorganized benchmarking flows and batch application logic for clarity and maintainability.
    • Generalized decoding implementations to accept a contextual parameter without changing runtime behavior.
  • Style

    • Standardized and reformatted project manifest metadata and feature lists for consistency.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 14, 2026

📝 Walkthrough

Walkthrough

Multiple Cargo manifest formatting and bincode version bumps; benches updated to initialize and pass GroveVersion; merk benches refactored to use new apply/restore and chunk APIs; merk proof decoding impls generalized with a Context type parameter.

Changes

Cohort / File(s) Summary
Cargo manifests — version bump & formatting
grovedb-element/Cargo.toml, grovedb/Cargo.toml, merk/Cargo.toml
bincode and bincode_derive version updated from =2.0.0-rc.3=2.0.1. Authors, features, and dependency blocks reformatted (multi-line authors, feature list reflow, trailing-comma adjustments). No API exports added.
grovedb benches
grovedb/benches/insertion_benchmark.rs
Adds local GroveVersion::latest() usage in several benchmark functions and introduces conditional imports under the minimal feature.
merk benches — small updates
merk/benches/branch_queries.rs, merk/benches/ops.rs
Minor changes: removed CostsExt import, extended get_key_from_node to handle Node::KVDigestCount, and added local grove_version initialization in several benches.
merk benches — major bench flow refactor
merk/benches/merk.rs
Large refactor: replaced direct apply_unchecked with apply_batch_default, changed open_standalone call to include TreeType, switched restoration flow to Restorer::new(...), and reworked chunk production/iteration logic across many benches.
Proof decoding generics
merk/src/proofs/query/mod.rs, merk/src/proofs/query/query_item/mod.rs
Decode and BorrowDecode impls for Query and QueryItem generalized to impl<Context> Decode<Context> / impl<'de, Context> BorrowDecode<'de, Context>, updating decoder trait bounds to Decoder<Context = Context> / BorrowDecoder<'de, Context = Context>. Signatures changed; decoding logic unchanged.

Sequence Diagram(s)

sequenceDiagram
  participant Bench as Benchmark
  participant Merk as Merk
  participant Storage as Storage
  participant Restorer as Restorer/ChunkProducer

  Bench->>Merk: open_standalone(ctx, TreeType, None, grove_version)
  Bench->>Merk: prepare batches
  Bench->>Merk: apply_batch_default(batches)
  Merk->>Storage: persist batch changes (incl. removals)
  Merk->>Restorer: request chunks / chunk_with_index(i, grove_version)
  loop restore loop
    Bench->>Restorer: Restorer::new(m, root_hash, None)
    Restorer->>Merk: apply chunk(s)
    Restorer->>Storage: read/write chunk state
    Restorer-->>Bench: restored root
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I hop through toml and benches with glee,
Bumped bincode, tidied arrays of three,
Chunks and restorers now hop in a line,
Decoders got Context — neat and fine,
A rabbit's small merge, nimble and spry.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'build!: update bincode to v2.0.1' clearly and concisely summarizes the primary change: upgrading the bincode dependency across multiple Cargo.toml files from v2.0.0-rc.3 to v2.0.1.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@grovedb/Cargo.toml`:
- Around line 5-9: Fix the malformed author entry in the authors array by adding
the missing closing angle bracket to Wisdom Ogwu's email string so it reads
"Wisdom Ogwu <wisdom@dash.org>"; update the authors array entry in the
Cargo.toml authors list where the malformed string appears to match the other
entries.

In `@merk/Cargo.toml`:
- Around line 5-10: The authors array contains a malformed author entry: the
"Wisdom Ogwu <wisdom@dash.org" string is missing the closing '>' angle bracket;
fix the authors list by adding the missing '>' to that entry so it reads "Wisdom
Ogwu <wisdom@dash.org>" ensuring the authors array syntax is valid.
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a7bc60a and a06d8df.

📒 Files selected for processing (3)
  • grovedb-element/Cargo.toml
  • grovedb/Cargo.toml
  • merk/Cargo.toml
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to **/*.rs : Format Rust code with cargo fmt
📚 Learning: 2025-06-06T06:13:27.562Z
Learnt from: ktechmidas
Repo: dashpay/grovedb PR: 378
File: .github/workflows/release-crates.yml:34-34
Timestamp: 2025-06-06T06:13:27.562Z
Learning: The crate `grovedbg-types` is a valid crate name in the GroveDB project, not a typo of `grovedb-types`.

Applied to files:

  • grovedb-element/Cargo.toml
  • grovedb/Cargo.toml
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb,merk,storage,costs}/src/**/*.rs : Wrap errors with context (e.g., .map_err(|e| Error::CorruptedData(format!("context: {}", e))))

Applied to files:

  • grovedb-element/Cargo.toml
  • grovedb/Cargo.toml
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {storage/src/**/*.rs,grovedb/src/batch/**/*.rs} : Use transactions for multi-step operations to ensure safety

Applied to files:

  • grovedb-element/Cargo.toml
📚 Learning: 2025-04-18T06:12:25.554Z
Learnt from: pauldelucia
Repo: dashpay/grovedb PR: 366
File: grovedb/src/error.rs:7-7
Timestamp: 2025-04-18T06:12:25.554Z
Learning: The `Error::InvalidProofError` in grovedb/src/operations/proof/util.rs is from the merk error handling system, distinct from grovedb's own `Error::InvalidProof` variant. These are separate error types from different crates.

Applied to files:

  • grovedb-element/Cargo.toml
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb,merk,storage,costs}/src/**/*.rs : Use cost_return_on_error! for early returns while accumulating cost

Applied to files:

  • grovedb/Cargo.toml
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb/src/operations/proof,merk/src/proofs}/**/*.rs : Never trust unverified proofs; always verify before use

Applied to files:

  • merk/Cargo.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: Code Coverage
  • GitHub Check: Tests
  • GitHub Check: Linting
  • GitHub Check: Compilation errors
🔇 Additional comments (3)
grovedb-element/Cargo.toml (1)

14-15: LGTM!

The bincode dependencies are correctly switched to the dashpay fork using git source with a pinned tag. The combination of git, tag, and version ensures Cargo will fetch from the fork while maintaining version compatibility checks.

merk/Cargo.toml (1)

26-27: LGTM!

The bincode dependencies are correctly switched to the dashpay fork, consistent with the other crate manifests.

grovedb/Cargo.toml (1)

28-29: LGTM!

The bincode dependencies are correctly switched to the dashpay fork, completing the consistent migration across all crate manifests.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

lklimek and others added 2 commits January 14, 2026 13:23
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
merk/benches/merk.rs (1)

230-256: Delete benchmark likely not doing what the comment says (batch selection + tree reset).
Right now delete_batch / insert_batch are bound once (with i = 0) and never updated, and the merk isn’t restored to ~1m keys per iteration—so the workload drifts and you probably measure repeated deletes of the same keys.

Suggested fix (pick batches per-iter and keep size stable)
 c.bench_function("delete_1m_2k_rand", |b| {
     let mut i = 0;
-
-    let delete_batch = &delete_batches[i % n_batches];
-    let insert_batch = &batches[i % n_batches];
-
-    // Merk tree is kept with 1m elements before each bench iteration for more or
-    // less same inputs.
-    apply_batch_default(&mut merk, insert_batch, grove_version);
-
     b.iter_with_large_drop(|| {
-        apply_batch_default(&mut merk, delete_batch, grove_version);
-        i += 1;
+        let idx = i % n_batches;
+        let delete_batch = &delete_batches[idx];
+        let insert_batch = &batches[idx];
+
+        // Keep merk ~constant-size: delete then re-insert (or swap order if desired).
+        apply_batch_default(&mut merk, delete_batch, grove_version);
+        apply_batch_default(&mut merk, insert_batch, grove_version);
+
+        i += 1;
     });
 });
merk/src/proofs/query/query_item/mod.rs (1)

340-395: Fix BorrowDecode implementation to match bincode v2.0.1 API.

The signature impl<'de, Context> BorrowDecode<'de, Context> does not match the standard bincode v2.0.1 trait, which is BorrowDecode<'de> with only a lifetime parameter. The Context should not be a generic parameter on the trait itself; instead, access the context via the associated type on BorrowDecoder. Update the implementation to impl<'de> BorrowDecode<'de> for QueryItem and use D::Context to reference context within the implementation.

♻️ Duplicate comments (1)
grovedb/Cargo.toml (1)

5-9: Missing closing > in author email address.

Line 7 is still missing the closing angle bracket for Wisdom Ogwu's email address. The past review indicated this was addressed in commit 562c19f, but the current code still shows the malformed entry.

📝 Proposed fix
 authors = [
     "Samuel Westrich <sam@dash.org>",
-    "Wisdom Ogwu <wisdom@dash.org",
+    "Wisdom Ogwu <wisdom@dash.org>",
     "Evgeny Fomin <evgeny.fomin@dash.org>",
 ]
🧹 Nitpick comments (2)
merk/benches/merk.rs (2)

32-70: apply_batch_default: consider returning a Result instead of panicking (even in benches).
unwrap().expect(...) is fine for local benches, but turning this into -> Result<(), Error> would make failures easier to diagnose/propagate in future bench reuse.

Possible tweak
-fn apply_batch_default<KB: AsRef<[u8]>>(
+fn apply_batch_default<KB: AsRef<[u8]>>(
     merk: &mut TempMerk,
     batch: &MerkBatch<KB>,
     grove_version: &GroveVersion,
-) {
-    merk.apply_unchecked::<KB, Vec<u8>, _, _, _, _, _>(
+) -> Result<(), grovedb_merk::Error> {
+    merk.apply_unchecked::<KB, Vec<u8>, _, _, _, _, _>(
         batch,
         &[],
         None,
         &|_k, _v| Ok(0),
         None::<&fn(&[u8], &GroveVersion) -> Option<ValueDefinedCostType>>,
         &|_old_value, _new_value| Ok(None),
         &mut |_costs: &StorageCost, _old_value: &Vec<u8>, _new_value: &mut Vec<u8>| {
             Ok((false, None))
         },
         &mut |_key: &Vec<u8>, key_bytes_to_remove: u32, value_bytes_to_remove: u32| {
             Ok((
                 BasicStorageRemoval(key_bytes_to_remove),
                 BasicStorageRemoval(value_bytes_to_remove),
             ))
         },
         grove_version,
     )
-    .unwrap()
-    .expect("apply failed");
+    .map_err(|e| e)?
+    .map_err(|e| e)
 }

316-323: Chunk benches: watch for repeated chunks().unwrap() + handle empty chunk sets defensively.
Minor, but a debug_assert!(chunk_count > 0) (or early return) would avoid panics if these benches ever run on an unexpectedly empty merk.

Also applies to: 342-353, 371-383

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 562c19f and b8abdc5.

📒 Files selected for processing (9)
  • grovedb-element/Cargo.toml
  • grovedb/Cargo.toml
  • grovedb/benches/insertion_benchmark.rs
  • merk/Cargo.toml
  • merk/benches/branch_queries.rs
  • merk/benches/merk.rs
  • merk/benches/ops.rs
  • merk/src/proofs/query/mod.rs
  • merk/src/proofs/query/query_item/mod.rs
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Format Rust code with cargo fmt
Run cargo clippy with -D warnings and fix all lints

Files:

  • grovedb/benches/insertion_benchmark.rs
  • merk/src/proofs/query/query_item/mod.rs
  • merk/benches/ops.rs
  • merk/benches/branch_queries.rs
  • merk/benches/merk.rs
  • merk/src/proofs/query/mod.rs
{grovedb,merk,storage,costs}/src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

{grovedb,merk,storage,costs}/src/**/*.rs: Use cost_return_on_error! for early returns while accumulating cost
Wrap errors with context (e.g., .map_err(|e| Error::CorruptedData(format!("context: {}", e))))

Files:

  • merk/src/proofs/query/query_item/mod.rs
  • merk/src/proofs/query/mod.rs
{grovedb/src/operations/proof,merk/src/proofs}/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Never trust unverified proofs; always verify before use

Files:

  • merk/src/proofs/query/query_item/mod.rs
  • merk/src/proofs/query/mod.rs
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb,merk,storage,costs}/src/**/*.rs : Wrap errors with context (e.g., .map_err(|e| Error::CorruptedData(format!("context: {}", e))))
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to grovedb/src/batch/**/*.rs : Prefer batch operations for multiple related changes

Applied to files:

  • grovedb/benches/insertion_benchmark.rs
  • merk/benches/ops.rs
  • merk/benches/branch_queries.rs
  • merk/benches/merk.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {storage/src/**/*.rs,grovedb/src/batch/**/*.rs} : Use transactions for multi-step operations to ensure safety

Applied to files:

  • grovedb/benches/insertion_benchmark.rs
  • merk/benches/ops.rs
  • merk/benches/merk.rs
📚 Learning: 2025-02-18T08:17:30.690Z
Learnt from: fominok
Repo: dashpay/grovedb PR: 345
File: grovedb/src/operations/delete/v1.rs:19-35
Timestamp: 2025-02-18T08:17:30.690Z
Learning: Refactoring suggestions for legacy code should be avoided as they are considered out of scope for feature PRs. This includes the function `delete_internal_on_transaction` in `grovedb/src/operations/delete/v1.rs` which is legacy code.

Applied to files:

  • grovedb/benches/insertion_benchmark.rs
  • merk/benches/branch_queries.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb,merk,storage,costs}/src/**/*.rs : Wrap errors with context (e.g., .map_err(|e| Error::CorruptedData(format!("context: {}", e))))

Applied to files:

  • grovedb/benches/insertion_benchmark.rs
  • grovedb-element/Cargo.toml
  • merk/benches/branch_queries.rs
  • merk/benches/merk.rs
  • grovedb/Cargo.toml
📚 Learning: 2025-06-06T06:13:27.562Z
Learnt from: ktechmidas
Repo: dashpay/grovedb PR: 378
File: .github/workflows/release-crates.yml:34-34
Timestamp: 2025-06-06T06:13:27.562Z
Learning: The crate `grovedbg-types` is a valid crate name in the GroveDB project, not a typo of `grovedb-types`.

Applied to files:

  • grovedb/benches/insertion_benchmark.rs
  • grovedb-element/Cargo.toml
  • grovedb/Cargo.toml
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to merk/src/tree/walk/**/*.rs : Use lazy loading to avoid loading unnecessary data

Applied to files:

  • grovedb/benches/insertion_benchmark.rs
📚 Learning: 2025-04-18T06:12:25.554Z
Learnt from: pauldelucia
Repo: dashpay/grovedb PR: 366
File: grovedb/src/error.rs:7-7
Timestamp: 2025-04-18T06:12:25.554Z
Learning: The `Error::InvalidProofError` in grovedb/src/operations/proof/util.rs is from the merk error handling system, distinct from grovedb's own `Error::InvalidProof` variant. These are separate error types from different crates.

Applied to files:

  • grovedb/benches/insertion_benchmark.rs
  • grovedb-element/Cargo.toml
  • merk/benches/branch_queries.rs
  • merk/benches/merk.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to **/tests/**/*.rs : Verify that cost calculations in tests match actual operations

Applied to files:

  • merk/benches/branch_queries.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb,merk,storage,costs}/src/**/*.rs : Use cost_return_on_error! for early returns while accumulating cost

Applied to files:

  • merk/benches/branch_queries.rs
  • merk/benches/merk.rs
  • grovedb/Cargo.toml
📚 Learning: 2025-12-17T18:39:20.910Z
Learnt from: QuantumExplorer
Repo: dashpay/grovedb PR: 397
File: grovedb/src/tests/provable_count_sum_tree_tests.rs:82-90
Timestamp: 2025-12-17T18:39:20.910Z
Learning: In grovedb_merk test code, the `execute()` function returns a cost-wrapped result (e.g., tuple or CostResult). The pattern `.unwrap().map_err(|e| e)` is correct when `.unwrap()` extracts the inner Result from the cost wrapper, and `.map_err()` then converts the error type. This is commonly used in test helpers to discard cost tracking.

Applied to files:

  • merk/benches/branch_queries.rs
  • merk/benches/merk.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to merk/src/tree/**/*.rs : Maintain AVL balance factor in {-1, 0, 1} using single/double rotations

Applied to files:

  • merk/benches/merk.rs
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb/src/operations/proof,merk/src/proofs}/**/*.rs : Never trust unverified proofs; always verify before use

Applied to files:

  • merk/benches/merk.rs
  • merk/Cargo.toml
🧬 Code graph analysis (5)
grovedb/benches/insertion_benchmark.rs (1)
grovedb-version/src/version/mod.rs (1)
  • latest (26-30)
merk/benches/ops.rs (1)
grovedb-version/src/version/mod.rs (1)
  • latest (26-30)
merk/benches/branch_queries.rs (1)
merk/src/proofs/tree.rs (1)
  • key (361-374)
merk/benches/merk.rs (6)
merk/src/merk/mod.rs (26)
  • merk (1064-1064)
  • merk (1083-1083)
  • merk (1091-1091)
  • merk (1100-1100)
  • merk (1108-1108)
  • merk (1121-1121)
  • merk (1127-1127)
  • merk (1142-1142)
  • merk (1158-1158)
  • merk (1170-1170)
  • merk (1175-1175)
  • merk (1187-1187)
  • merk (1219-1219)
  • merk (1239-1239)
  • merk (1283-1283)
  • merk (1287-1287)
  • None (741-741)
  • None (955-955)
  • None (1277-1277)
  • None (1305-1305)
  • None (1329-1329)
  • None (1340-1340)
  • None (1367-1367)
  • root_hash (323-330)
  • new (96-108)
  • new (133-144)
merk/src/merk/chunks.rs (4)
  • chunk (109-125)
  • new (36-42)
  • new (54-64)
  • new (83-94)
merk/src/tree/encoding.rs (1)
  • encode_into (81-84)
merk/src/tree/kv.rs (2)
  • encode_into (463-469)
  • new (64-82)
merk/src/merk/restore.rs (1)
  • new (71-84)
merk/src/merk/open.rs (1)
  • open_standalone (30-48)
merk/src/proofs/query/mod.rs (1)
merk/src/proofs/query/query_item/mod.rs (2)
  • decode (283-336)
  • borrow_decode (341-394)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Tests
  • GitHub Check: Code Coverage
🔇 Additional comments (18)
grovedb/benches/insertion_benchmark.rs (1)

37-38: LGTM!

The addition of GroveVersion::latest() initialization across all six benchmark functions is consistent and aligns with the broader pattern of explicit grove version usage in benchmark code paths throughout the repository.

Also applies to: 52-52, 90-90, 129-129, 155-155, 184-184, 226-226

grovedb-element/Cargo.toml (2)

29-29: LGTM!

Minor formatting normalization for the visualize feature declaration.


14-15: The bincode dependency is sourced from the official crates.io registry, not from a fork. No [patch.crates-io] section exists in the workspace root Cargo.toml, and no references to dashpay/bincode are present anywhere in the codebase. The version 2.0.1 is the stable release from crates.io, which is the expected behavior.

Likely an incorrect or invalid review comment.

merk/Cargo.toml (3)

5-10: LGTM!

Authors array properly formatted with the closing angle bracket fix applied.


26-27: LGTM!

Bincode dependencies updated to 2.0.1, consistent with the workspace-wide upgrade.


44-55: LGTM!

Feature declarations reformatted for consistency. No functional changes.

grovedb/Cargo.toml (2)

28-29: LGTM!

Bincode dependencies updated to 2.0.1, consistent with the workspace-wide upgrade to the dashpay fork.


39-42: LGTM!

Feature and dependency formatting changes are cosmetic cleanup with no functional impact.

Also applies to: 70-70, 82-82, 99-99

merk/benches/branch_queries.rs (2)

231-231: LGTM!

Adding Node::KVDigestCount to the key extraction logic is correct and aligns with the pattern in merk/src/proofs/tree.rs:360-373 where KVDigestCount is similarly handled in the key() method.


73-73: Remove this review comment—the import is correct and no issue exists.

unwrap_add_cost is a method on CostContext<T>, not on the CostsExt trait. The methods trunk_query() and branch_query() return CostResult<...> (a type alias for CostContext<Result<...>>), and calling .unwrap_add_cost(&mut query_cost) on a CostContext is valid without importing CostsExt. The import of only OperationCost on line 73 is correct and sufficient.

Likely an incorrect or invalid review comment.

merk/benches/ops.rs (4)

38-63: GroveVersion plumbed consistently through ops benches.
Nice mechanical update; each bench now uses GroveVersion::latest() and threads grove_version into tree/batch ops as expected.


67-94: GroveVersion plumbed consistently through ops benches (rand inserts).
No behavior change beyond adding the version input.


98-121: GroveVersion plumbed consistently through ops benches (seq updates).
No issues spotted.


125-154: GroveVersion plumbed consistently through ops benches (rand updates).
No issues spotted.

merk/benches/merk.rs (1)

398-431: Add finalize() call to complete the restoration workflow.

The Restorer requires an explicit finalize(grove_version) call after all chunks are processed. This validates that all chunks have been received, verifies and corrects tree heights if needed, and performs final tree validation. The current benchmark skips this step entirely.

merk/src/proofs/query/mod.rs (2)

181-220: No issue found. bincode 2.0's #[derive(Decode)] automatically generates context-generic trait implementations. SubqueryBranch's derived Decode impl will be impl<Context> Decode<Context> by default, matching the requirements of Query's context-generic decoder calls.


223-262: The concern is unfounded. SubqueryBranch derives Decode, and bincode 2.0.1 provides a blanket implementation of BorrowDecode<'de, Context> for any type implementing Decode. This means SubqueryBranch::borrow_decode(decoder)? works correctly with any Context, not just (). The code compiles and functions as intended.

Likely an incorrect or invalid review comment.

merk/src/proofs/query/query_item/mod.rs (1)

282-337: The Decode<Context> signature matches bincode 2.0.1 API exactly. The trait bound Decoder<Context = Context> is correct per bincode's trait definition, and the implementation is consistent with other similar implementations in the codebase (Query, BorrowDecode variants). No changes required.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@lklimek lklimek changed the title build!: switch to our fork of bincode build!: update bincode to v2.0.1 Jan 14, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@grovedb/Cargo.toml`:
- Around line 28-29: Cargo.toml now pins bincode = "2.0.1" but the codebase
likely still uses the pre-2.0 API; search for usages of the Options trait,
serialize/deserialize, serialize_into/deserialize_from, and
#[derive(serde::Serialize/Deserialize)] and update them: replace Options-based
configuration with bincode::config::legacy() or the appropriate bincode::config
constructor, replace serialize()/deserialize()/deserialize_from() calls with
bincode::serde::encode_to_vec()/decode_from_std_read() (or the matching serde
helpers), and swap serde derives to bincode::Encode / bincode::Decode or enable
serde compatibility mode where needed; update any imports and fully-qualify
bincode::serde or bincode::config references (e.g., functions/methods in modules
that call serialize/deserialize, structs with serde derives, and any config
creation sites) so the code compiles against 2.0.1.
♻️ Duplicate comments (1)
grovedb/Cargo.toml (1)

5-9: Missing closing > in author email address.

Line 7 is still missing the closing angle bracket for Wisdom Ogwu's email address. The previous review indicated this was addressed in commit 562c19f, but the issue persists in the current code.

📝 Proposed fix
 authors = [
     "Samuel Westrich <sam@dash.org>",
-    "Wisdom Ogwu <wisdom@dash.org",
+    "Wisdom Ogwu <wisdom@dash.org>",
     "Evgeny Fomin <evgeny.fomin@dash.org>",
 ]
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b8abdc5 and 776331a.

📒 Files selected for processing (3)
  • grovedb-element/Cargo.toml
  • grovedb/Cargo.toml
  • merk/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (2)
  • grovedb-element/Cargo.toml
  • merk/Cargo.toml
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb,merk,storage,costs}/src/**/*.rs : Wrap errors with context (e.g., .map_err(|e| Error::CorruptedData(format!("context: {}", e))))

Applied to files:

  • grovedb/Cargo.toml
📚 Learning: 2025-06-06T06:13:27.562Z
Learnt from: ktechmidas
Repo: dashpay/grovedb PR: 378
File: .github/workflows/release-crates.yml:34-34
Timestamp: 2025-06-06T06:13:27.562Z
Learning: The crate `grovedbg-types` is a valid crate name in the GroveDB project, not a typo of `grovedb-types`.

Applied to files:

  • grovedb/Cargo.toml
📚 Learning: 2025-09-06T12:46:28.692Z
Learnt from: CR
Repo: dashpay/grovedb PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-06T12:46:28.692Z
Learning: Applies to {grovedb,merk,storage,costs}/src/**/*.rs : Use cost_return_on_error! for early returns while accumulating cost

Applied to files:

  • grovedb/Cargo.toml

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

Comment on lines +28 to +29
bincode = { version = "=2.0.1" }
bincode_derive = { version = "=2.0.1" }
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

bincode 2.0.1 crates.io release

💡 Result:

bincode 2.0.1 was released on March 10, 2025. [1][2]

Sources:
[1] Lib.rs release listing for bincode (2.0.1 — Mar 10, 2025).
[2] docs.rs build page showing bincode 2.0.1 (build date Mar 10, 2025).


🌐 Web query:

bincode 2.0.1 changelog migration from rc3

💡 Result:

Short summary of what changed / how to migrate from 2.0.0-rc.3 → 2.0.1

  • Release info: bincode 2.0.1 is the published stable release following the 2.0 release candidates (rc.3). (lib.rs)
  • Key migration points (rc → 2.x stable are covered by the bincode 2 migration guide):
    • Options → Configuration: the old Options trait is replaced by bincode::config::Configuration; use config::legacy() (and .with_variable_int_encoding() if you need bincode‑1.3+ compatibility). (docs.rs)
    • API renames: deserialize/serialize functions moved into bincode::serde::* (e.g. serialize(T) → serde::encode_to_vec(T, Configuration), deserialize_from(Read) → serde::decode_from_std_read(Read, Configuration), etc.). See the migration table in the docs. (docs.rs)
    • Derives: bincode now has its own derive crate/macros (bincode_derive). If you disabled default features for serde, enable the derive feature (or keep using serde_derive). Replace #[derive(serde::Serialize/Deserialize)] with #[derive(bincode::Encode)] / #[derive(bincode::Decode)] or keep serde when using the serde feature. (docs.rs)
  • Practical steps to migrate:
    1. Change Cargo.toml to bincode = "2.0.1" (or "2.0") and enable features you need (serde, derive, std/alloc). (lib.rs)
    2. Replace Options usage with config::legacy() (and add .with_variable_int_encoding() if you relied on 1.3+ behavior). (docs.rs)
    3. Replace serialize/deserialize calls with the bincode::serde functions (or use the new Encode/Decode traits and encode/decode functions). (docs.rs)
    4. Update derives to bincode::Encode / bincode::Decode or enable serde compatibility as needed. (docs.rs)

If you want the exact release changelog/commit list for 2.0.1 (diff vs rc.3) I can fetch the crate repo / release tags and show the commits or CHANGELOG entries.

Citations:


Bincode 2.0.1 release confirmed, but codebase migration to new API must be verified.

bincode 2.0.1 exists (released March 10, 2025) and is the stable release following rc.3. However, the migration from rc.3 to 2.0.1 includes breaking API changes that require codebase updates:

  • Options trait → Configuration: Replace Options usage with config::legacy() and related configuration methods
  • API renames: serialize/deserialize functions moved to bincode::serde::* namespace (e.g., serialize()serde::encode_to_vec(), deserialize_from()serde::decode_from_std_read())
  • Derives: Update #[derive(serde::Serialize/Deserialize)] to #[derive(bincode::Encode/bincode::Decode)] or enable serde compatibility mode

Verify that grovedb's codebase has been updated to use the new API signatures and derive macros. The exact version pinning is appropriate, but the code migration must be completed to successfully use bincode 2.0.1.

🤖 Prompt for AI Agents
In `@grovedb/Cargo.toml` around lines 28 - 29, Cargo.toml now pins bincode =
"2.0.1" but the codebase likely still uses the pre-2.0 API; search for usages of
the Options trait, serialize/deserialize, serialize_into/deserialize_from, and
#[derive(serde::Serialize/Deserialize)] and update them: replace Options-based
configuration with bincode::config::legacy() or the appropriate bincode::config
constructor, replace serialize()/deserialize()/deserialize_from() calls with
bincode::serde::encode_to_vec()/decode_from_std_read() (or the matching serde
helpers), and swap serde derives to bincode::Encode / bincode::Decode or enable
serde compatibility mode where needed; update any imports and fully-qualify
bincode::serde or bincode::config references (e.g., functions/methods in modules
that call serialize/deserialize, structs with serde derives, and any config
creation sites) so the code compiles against 2.0.1.

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.

2 participants