refactor: use Bytes instead of Vec<u8> in CarBlock#7023
Conversation
WalkthroughThis PR migrates the ChangesCarBlock Bytes Type Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
369b609 to
31f620b
Compare
31f620b to
95bd88b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tool/subcommands/car_cmd.rs`:
- Line 120: The current validation masks DB/index errors by using
.ok().flatten(), so replace that silent conversion with proper error propagation
and context: call db.get(&block.cid) and propagate errors using ? plus
.context(...) (or .with_context(...)) to attach a helpful message, then flatten
the Option and map(Bytes::from) into a variable (e.g., stored) and use
anyhow::ensure!(stored == Some(block.data), "validation mismatch: ...") to
report mismatches; reference the db.get(&block.cid) call and the ensure!
comparison so you update the exact expression that currently reads
db.get(&block.cid).ok().flatten().map(Bytes::from) == Some(block.data).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 08e2edf0-f5a4-4ad4-a965-2163fcd3a015
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
Cargo.tomlsrc/blocks/tipset.rssrc/chain/mod.rssrc/db/car/forest.rssrc/db/car/mod.rssrc/db/memory.rssrc/db/parity_db/gc.rssrc/ipld/util.rssrc/rpc/methods/state.rssrc/tool/subcommands/archive_cmd.rssrc/tool/subcommands/car_cmd.rssrc/utils/db/car_stream.rssrc/utils/db/car_stream/tests.rssrc/utils/db/car_util.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 13 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary of changes
Changes introduced in this pull request:
CarStream(e.g. Building indices of Forest CAR require iterating all CIDs without accessing data)Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
Refactor
Tests