Skip to content

chore(deps): replace cbor4ii with minicbor-serde in redb Cbor wrapper#728

Merged
praveenperera merged 2 commits into
bitcoinppl:masterfrom
muhahahmad68:fix/consolidate-cbor-deps-replace-cbor4ii-with-minicbor-serde
May 5, 2026
Merged

chore(deps): replace cbor4ii with minicbor-serde in redb Cbor wrapper#728
praveenperera merged 2 commits into
bitcoinppl:masterfrom
muhahahmad68:fix/consolidate-cbor-deps-replace-cbor4ii-with-minicbor-serde

Conversation

@muhahahmad68
Copy link
Copy Markdown
Contributor

@muhahahmad68 muhahahmad68 commented May 5, 2026

Closes #724

Summary

Replaces the direct cbor4ii dependency with minicbor-serde in the redb Cbor<T> wrapper (rust/src/database/cbor.rs), consolidating all Cove-owned CBOR serialization under the minicbor family.

Byte compatibility was verified with 8 cross-encoding tests covering all four stored record types (TransactionRecord, AddressRecord, InputRecord, OutputRecord) including Option::None, Option::Some and variants. cbor4ii and minicbor-serde produce identical bytes for all cases, so existing redb tables remain readable without any migration.

Testing

Platform Coverage

  • cargo tree -i cbor4ii — no output (removed from dependency graph)
  • cargo fmt --all — clean
  • cargo clippy — clean
  • cargo test — 502 passed, 0 failed

Note: just fmt exits non-zero in my environment due to swiftformat
not being installed. No Swift files were modified.

Checklist

Summary by CodeRabbit

  • Chores
    • Updated internal serialization library dependency for improved maintainability.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Warning

Rate limit exceeded

@praveenperera has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 7 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8065aa2d-c561-4dc3-b7c3-0b2fb9bfc753

📥 Commits

Reviewing files that changed from the base of the PR and between 027e865 and 6344b01.

⛔ Files ignored due to path filters (1)
  • rust/Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • rust/Cargo.toml
  • rust/src/database/cbor.rs
📝 Walkthrough

Walkthrough

The Cbor<T> redb value wrapper migrates from cbor4ii::serde to minicbor_serde for serialization and deserialization. The from_bytes and as_bytes methods now call minicbor_serde::{from_slice,to_vec} respectively, with the latter simplifying buffer management.

Changes

CBOR Library Migration

Layer / File(s) Summary
Deserialization
rust/src/database/cbor.rs
Value::from_bytes switches from cbor4ii::serde::from_slice to minicbor_serde::from_slice.
Serialization
rust/src/database/cbor.rs
Value::as_bytes switches from cbor4ii::serde::to_vec(buf, value) to minicbor_serde::to_vec(value), eliminating the manual buffer allocation.
Dependency Resolution
rust/Cargo.toml
cbor4ii dependency can be removed; minicbor-serde becomes the sole Serde CBOR adapter for Cove-owned code.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes


🐰 One CBOR, then two, but now we're through,
Minicbor's the way—a hop, not a crew,
Bye-bye, cbor4ii, your work is done,
One codec to rule them—our mission's won! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: replacing cbor4ii with minicbor-serde in the redb Cbor wrapper, which is the primary objective of this PR.
Description check ✅ Passed The description includes a comprehensive summary of changes, verification of byte compatibility through cross-encoding tests, and detailed testing results matching the provided template structure with summary, testing, and checklist sections.
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements from issue #724: migrated cbor4ii to minicbor-serde in rust/src/database/cbor.rs, verified byte compatibility with 8 cross-encoding tests, confirmed cbor4ii removed from dependency graph, and all linting/testing passed.
Out of Scope Changes check ✅ Passed All changes are directly scoped to consolidating CBOR dependencies as specified in issue #724; only the redb Cbor wrapper file was modified, and no unrelated changes were introduced.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 5, 2026

Greptile Summary

This PR replaces cbor4ii with minicbor-serde in the redb Cbor<T> wrapper, consolidating all CBOR serialization onto the minicbor family. The previously flagged issues (missing crate-level dependency declaration and stale cbor4ii entry) are both resolved in this revision.

  • cbor4ii is fully removed from Cargo.toml and the resolved Cargo.lock dependency graph; minicbor-serde = { workspace = true } is correctly declared in [workspace.dependencies] and in the crate's [dependencies] section.
  • cbor.rs switches to minicbor_serde::from_slice and minicbor_serde::to_vec, which is the correct API for the new crate.
  • The PR description notes that byte-level compatibility was verified with 8 cross-encoding tests across all four stored record types, though these tests are not committed to the repository.

Confidence Score: 5/5

Safe to merge — both previously flagged issues are resolved and the dependency graph is consistent.

The change is a clean library swap: cbor4ii is fully removed from Cargo.toml and Cargo.lock, minicbor-serde is properly declared at both the workspace and crate level, and the two call sites in cbor.rs use the correct minicbor-serde API. Byte compatibility is claimed via manual cross-encoding tests covering all four stored record types.

No files require special attention.

Important Files Changed

Filename Overview
rust/src/database/cbor.rs Replaces cbor4ii::serde calls with minicbor_serde equivalents; API surface is correct and the simpler to_vec signature is used properly.
rust/Cargo.toml cbor4ii removed from [dependencies]; minicbor-serde added to [workspace.dependencies] and to the crate's [dependencies] via workspace = true; Cargo.lock confirms it resolves to 0.6.2.
rust/Cargo.lock cbor4ii 1.2.2 fully removed; minicbor-serde 0.6.2 added with correct dependency on minicbor 2.2.1 and serde.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Cbor<T> wrapper\n(cbor.rs)"] -->|"as_bytes()"| B["minicbor_serde::to_vec(value)"]
    A -->|"from_bytes()"| C["minicbor_serde::from_slice(data)"]
    B --> D["redb table storage\n(Vec<u8>)"]
    D --> C
    E["cbor4ii removed"] -.->|"replaced by"| B
    E -.->|"replaced by"| C
Loading

Reviews (2): Last reviewed commit: "fix: push minicbor-serde in Cargo.toml" | Re-trigger Greptile

Comment thread rust/src/database/cbor.rs
cbor4ii was the only direct CBOR dependency not already in the minicbor family. This migrates the single use site in rust/src/database/cbor.rs to minicbor-serde, consolidating all Cove-owned CBOR serialization under one implementation family.

Byte compatibility was verified with 8 cross-encoding tests covering all four stored record types (TransactionRecord, AddressRecord, InputRecord, OutputRecord) including Option::None, Option::Some and variants. cbor4ii and minicbor-serde produce identical bytes for all cases, so existing redb tables remain readable without any migration.

Closes bitcoinppl#724
@praveenperera praveenperera force-pushed the fix/consolidate-cbor-deps-replace-cbor4ii-with-minicbor-serde branch from 456a2bd to 6344b01 Compare May 5, 2026 15:53
Copy link
Copy Markdown
Contributor

@praveenperera praveenperera left a comment

Choose a reason for hiding this comment

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

thanks @muhahahmad68 approved

@praveenperera praveenperera enabled auto-merge (squash) May 5, 2026 16:10
@praveenperera praveenperera merged commit 76230e4 into bitcoinppl:master May 5, 2026
10 checks passed
@muhahahmad68 muhahahmad68 deleted the fix/consolidate-cbor-deps-replace-cbor4ii-with-minicbor-serde branch May 5, 2026 16:44
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.

Consolidate Rust CBOR dependencies

2 participants