fix: update TipsetKey save and load logic to match Lotus

[hanabi1224]

Summary of changes

Changes introduced in this pull request:

• update TipsetKey and EthHash conversion logic to match Lotus (no longer use EthMappings column)
• save TipsetKey to GC snapshot to avoid data loss after GC

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist

• I have performed a self-review of my own code,
• I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
• I have added tests that prove my fix is effective or that my feature works (if possible),
• I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Outside contributions

• I have read and agree to the CONTRIBUTING document.
• I have read and agree to the AI Policy document. I understand that failure to comply with the guidelines will lead to rejection of the pull request.

Summary by CodeRabbit

• New Features

  • Added support for including tipset keys during chain exports
  • Snapshot exports now include tipset keys by default

• Tests

  • Updated test infrastructure to use parameterized testing framework

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 2ace936a-6fa0-4122-b80b-15badd43667a

📥 Commits

Reviewing files that changed from the base of the PR and between 37fa65e and 1b7421c.

⛔ Files ignored due to path filters (4)

• Cargo.lock is excluded by !**/*.lock
• src/rpc/snapshots/forest__rpc__tests__rpc__v0.snap is excluded by !**/*.snap
• src/rpc/snapshots/forest__rpc__tests__rpc__v1.snap is excluded by !**/*.snap
• src/rpc/snapshots/forest__rpc__tests__rpc__v2.snap is excluded by !**/*.snap

📒 Files selected for processing (1)

• Cargo.toml

---

Walkthrough

This PR migrates TipsetKey persistence from a chain-store-backed approach to blockstore-backed storage. It introduces new TipsetKey APIs (car_block, from_bytes, save, load) for blockstore operations, updates ChainStore to remove put_tipset_key and load keys from blockstore, and adds an include_tipset_keys flag to the export pipeline to optionally emit tipset-key blocks in CAR streams.

Changes

Cohort / File(s)	Summary
TipsetKey Persistence APIs src/blocks/tipset.rs	Added new public methods: car_block(), from_bytes(), save(), load() for blockstore-based TipsetKey serialization/deserialization; updated cid() to delegate to car_block(). Adds Arbitrary implementation for testing and LazyLock for lazy initialization.
ChainStore Refactoring src/chain/store/chain_store.rs	Removed put_tipset_key() method; updated get_required_tipset_key() to load directly from blockstore via TipsetKey::load() instead of eth_mappings.
Export Pipeline Options src/chain/mod.rs, src/rpc/methods/chain.rs, src/cli/subcommands/snapshot_cmd.rs, src/tool/subcommands/archive_cmd.rs, src/db/gc/snapshot.rs	Added include_tipset_keys: bool field to ExportOptions and ForestChainExportParams structs; wired through export pipeline initialization with appropriate boolean values.
ChainStream Tipset-Key Emission src/ipld/util.rs	Added tipset_keys: bool field and with_tipset_keys() builder method to ChainStream; updated streaming loop to conditionally emit tipset-key CarBlocks based on flag.
Persistence Layer Updates src/daemon/db_util.rs, src/daemon/mod.rs	Replaced put_tipset_key() calls with tsk.save(blockstore) calls; removed explicit tipset-key persistence from indexer loop.
Utility and Test Updates src/utils/cid/mod.rs, src/rpc/methods/eth.rs, src/rpc/methods/f3/types.rs, Cargo.toml, src/tool/subcommands/api_cmd/test_snapshots.txt	Added from_cbor_encoded_raw_bytes_blake2b256() helper to CID trait; refactored EthHash serde test to use rstest; simplified F3TipSetKey conversion to delegate to from_bytes(); added rstest dev dependency; updated snapshot file identifiers.

Sequence Diagram

sequenceDiagram
participant Client
participant ChainStore
participant Blockstore
participant TipsetKey
rect rgba(100, 150, 200, 0.5)
Note over Client,TipsetKey: New Blockstore-Based Persistence
Client->>ChainStore: set_heaviest_tipset(ts)
ChainStore->>TipsetKey: ts.key().save(blockstore)?
TipsetKey->>TipsetKey: car_block() → CarBlock
TipsetKey->>Blockstore: store CarBlock
Blockstore-->>TipsetKey: Cid
TipsetKey-->>ChainStore: Ok(Cid)
end
rect rgba(150, 100, 200, 0.5)
Note over Client,TipsetKey: Loading from Blockstore
Client->>ChainStore: get_required_tipset_key(hash)
ChainStore->>TipsetKey: TipsetKey::load(blockstore, cid)?
TipsetKey->>Blockstore: fetch by Cid
Blockstore-->>TipsetKey: RawBytes
TipsetKey->>TipsetKey: from_bytes(RawBytes)?
TipsetKey-->>ChainStore: Self
ChainStore-->>Client: TipsetKey
end
rect rgba(200, 150, 100, 0.5)
Note over Client,TipsetKey: Export with Optional Tipset Keys
Client->>ChainStore: export_to_forest_car(options)
ChainStore->>ChainStore: with_tipset_keys(include_tipset_keys)
ChainStore->>Blockstore: emit tipset-key CarBlocks if enabled
Blockstore-->>Client: CAR stream with/without tipset keys
end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix: avoid deep-cloning Tipset and FullTipset #6274: Both PRs modify src/blocks/tipset.rs and Tipset/TipsetKey internals, introducing changes to field wrappers and lazy initialization semantics.
• fix: remove unique events logic from the ChainGetEvents API #6286: Both PRs align on removing chain-store-backed tipset-key storage and migrating to blockstore-based persistence by removing put_tipset_key and related index-based methods.
• feat: add --unordered to forest-cli snapshot export #5867: Both PRs modify the chain export pipeline, ExportOptions, and streaming plumbing to extend export capabilities.

Suggested reviewers

• sudo-shashank
• LesnyRumcajs
• akaladarshi

🚥 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 title accurately describes the main change: updating TipsetKey save and load logic to align with Lotus implementation, which is the core objective reflected throughout the changeset.
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.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch hm/fix-tipset-key-save-load

---

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

[coderabbitai]

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (4)

src/tool/subcommands/archive_cmd.rs (1)

629-633: ⚠️ Potential issue | 🟠 Major

Enable tipset-key blocks in this export path.

With include_tipset_keys: false, snapshots produced here still omit the blocks that ChainStore::get_required_tipset_key() now loads directly from the blockstore. Importing one of these exports will leave historical hash-based tipset lookups broken until the data is rebuilt elsewhere. Default this to true here, or make the user-facing default opt-out instead of opt-in.

Suggested change

 Some(ExportOptions {
skip_checksum: true,
include_receipts: false,
include_events: false,
- include_tipset_keys: false,+ include_tipset_keys: true,
seen,
}),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@src/tool/subcommands/archive_cmd.rs` around lines 629 - 633, The export
currently constructs ExportOptions with include_tipset_keys: false which causes
produced snapshots to omit tipset-key blocks and breaks historical tipset hash
lookups on import; update the ExportOptions construction (the ExportOptions
struct usage that sets skip_checksum, include_receipts, include_events,
include_tipset_keys) to set include_tipset_keys: true (or change the user-facing
default to opt-out) so tipset-key blocks are included in exports; locate the
ExportOptions instantiation in archive_cmd.rs and change the include_tipset_keys
flag accordingly and ensure any related documentation or CLI defaults reflect
the opt-out behavior.

src/cli/subcommands/snapshot_cmd.rs (1)

140-149: ⚠️ Potential issue | 🟠 Major

Don't disable tipset-key emission in CLI snapshot exports.

This CLI now hardcodes include_tipset_keys: false, so a snapshot exported here will not contain the tipset-key blocks that the new load path depends on. That makes the exported snapshot incomplete for historical hash-based tipset lookup after import.

Suggested change

 let params = ForestChainExportParams {
version: format,
epoch: tipset.epoch(),
recent_roots: depth,
output_path: temp_path.to_path_buf(),
tipset_keys: tipset.key().clone().into(),
include_receipts: false,
include_events: false,
- include_tipset_keys: false,+ include_tipset_keys: true,
skip_checksum,
dry_run,
};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@src/cli/subcommands/snapshot_cmd.rs` around lines 140 - 149, The export
currently sets ForestChainExportParams with include_tipset_keys: false which
omits tipset-key blocks needed by the load path; update the snapshot command to
enable tipset-key emission by setting include_tipset_keys: true (or wire through
an explicit CLI flag) when constructing ForestChainExportParams in
snapshot_cmd.rs so exported snapshots include tipset keys for historical
hash-based tipset lookup.

src/chain/store/chain_store.rs (1)

145-150: ⚠️ Potential issue | 🟠 Major

Persist the tipset key before mutating head state.

If save() fails here, the heaviest-tipset key provider and cache have already been updated, but the tipset-key block is still missing and no head-change event is published. That leaves the node in a partially applied state.

Suggested change

 pub fn set_heaviest_tipset(&self, ts: Tipset) -> Result<(), Error> {
+ ts.key().save(self.blockstore())?;
self.heaviest_tipset_key_provider
.set_heaviest_tipset_key(ts.key())?;
*self.heaviest_tipset_cache.write() = Some(ts.clone());
- ts.key().save(self.blockstore())?;
if self.publisher.send(HeadChange::Apply(ts)).is_err() {
debug!("did not publish head change, no active receivers");
}
Ok(())
}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@src/chain/store/chain_store.rs` around lines 145 - 150, The function
set_heaviest_tipset is updating heaviest_tipset_key_provider and
heaviest_tipset_cache before persisting the tipset key, which can leave state
partially applied if Tipset.key().save(self.blockstore()) fails; change the
order so you call ts.key().save(self.blockstore()) first and return the error if
it fails, then update heaviest_tipset_key_provider.set_heaviest_tipset_key(...),
update *self.heaviest_tipset_cache.write(), and finally call
self.publisher.send(HeadChange::Apply(ts)); ensure errors from save() and the
key provider set are propagated and that the publish only happens after
successful persistence and in-memory updates.

src/rpc/methods/chain.rs (1)

685-694: ⚠️ Potential issue | 🟠 Major

Keep Filecoin.ChainExport compatible with the new tipset-key storage model.

This path still forces include_tipset_keys: false, so RPC exports omit the blocks that TipsetKey::load() now expects to find in the blockstore. That leaves Filecoin.ChainExport producing snapshots that regress historical hash-based tipset resolution after import.

Suggested change

 (ForestChainExportParams {
version: FilecoinSnapshotVersion::V1,
epoch,
recent_roots,
output_path,
tipset_keys,
include_receipts: false,
include_events: false,
- include_tipset_keys: false,+ include_tipset_keys: true,
skip_checksum,
dry_run,
},),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@src/rpc/methods/chain.rs` around lines 685 - 694, The export currently sets
ForestChainExportParams.include_tipset_keys = false which causes
Filecoin.ChainExport to omit block records needed by TipsetKey::load; change the
export to include tipset keys by setting include_tipset_keys = true (or make it
configurable) so the exported snapshot contains the blocks used by TipsetKey
resolution; update the ForestChainExportParams construction in the
Filecoin.ChainExport path to set include_tipset_keys to true (or wire through a
flag) to restore historical hash-based tipset resolution after import.

🧹 Nitpick comments (2)

src/blocks/tipset.rs (2)

55-63: Document the new public TipsetKey helpers with rustdoc.

TipsetKey::cid and TipsetKey::car_block are part of the new public API surface, but they still use plain comments instead of /// docs. Please promote the Lotus-compat contract into rustdoc so it shows up in generated API docs.

As per coding guidelines, "Document public functions and structs with doc comments".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@src/blocks/tipset.rs` around lines 55 - 63, The two new public helpers cid
and car_block lack rustdoc; replace the plain comment with /// doc comments
above pub fn cid and pub fn car_block that explain the Lotus-compat encoding
contract (e.g., “Special encoding to match Lotus”), describe the return types
(Cid and CarBlock) and possible errors, and note implementation details used for
car_block (it serializes via fvm_ipld_encoding::to_vec and computes the CID with
Cid::from_cbor_encoded_raw_bytes_blake2b256) so these details appear in
generated API docs for cid, car_block, and the CarBlock helper.

---

60-63: Add operation-specific context to the new TipsetKey persistence helpers.

These helpers sit on the new Lotus-compat persistence path, but encode/store/load failures currently bubble up without telling you which step failed. A small .context(...) on each boundary will make snapshot and RPC regressions much easier to diagnose.

As per coding guidelines, "Use anyhow::Result<T> for most operations and add context with .context() when errors occur".

Also applies to: 146-152

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.
In `@src/blocks/tipset.rs` around lines 60 - 63, The persistence helpers (e.g.,
the car_block method) currently propagate errors without context; update the
error boundaries to add operation-specific context by chaining .context(...) to
fallible calls: for car_block, add .context("serializing TipsetKey to CAR
bytes") to the fvm_ipld_encoding::to_vec(...) call and .context("computing CID
from CAR bytes") to the Cid::from_cbor_encoded_raw_bytes_blake2b256(...) call;
apply the same pattern to the other TipsetKey persistence helpers (the
encode/store/load helpers referenced around lines 146-152) so that each
serialize, compute-cid, write, and read operation records a clear, descriptive
context string.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/blocks/tipset.rs`:
- Around line 125-138: The decoder incorrectly assumes a fixed encoded CID
length by chunking on BLOCK_HEADER_CID_LEN; instead iterate through the raw
bytes and repeatedly parse CIDs until exhausted. In TipsetKey::from_bytes (the
pub fn from_bytes in src/blocks/tipset.rs) remove the static
BLOCK_HEADER_CID_LEN usage, convert the RawBytes into a byte slice/Vec, then
loop with a cursor: call Cid::read_bytes(&buf[cursor..]) (or the crate's
equivalent that returns the parsed Cid and number of bytes consumed), push each
Cid into the Vec<Cid>, advance cursor by the consumed length, and stop when
cursor == buf.len(); return an error if a CID parse fails or leftover bytes
remain. This ensures correct decoding regardless of each CID's encoded length.
In `@src/chain/store/chain_store.rs`:
- Around line 172-174: get_required_tipset_key() can fail for pre-upgrade DBs
because historical tipset keys were never backfilled; call the existing
backfill_db() (from src/daemon/db_util.rs) during normal startup and after
snapshot import so historical tipset keys get saved (tsk.save()) into the
blockstore. Specifically, invoke backfill_db() from start_services() (before
serving EthAPI requests) and from maybe_import_snapshot() immediately after
importing a snapshot, and guard the call so it only runs when the DB is missing
required tipsets or when a migration flag indicates pre-upgrade data, to ensure
get_required_tipset_key() can load older tipset keys.
In `@src/ipld/util.rs`:
- Around line 338-343: The code currently ignores failures from
tipset.borrow().key().car_block() by using if ... let Ok(...) = ..., causing
missing tipset-key data when export_tipset_keys is true; change that to attempt
car_block() explicitly and propagate the error instead of skipping (e.g., match
or use the ? operator) so that when export_tipset_keys is true and car_block()
fails the function returns the error rather than continuing; update the branch
that currently pushes Emit(cid, Some(data)) to only run after a successful
car_block() and ensure any Err from car_block() is returned to the caller.
---
Outside diff comments:
In `@src/chain/store/chain_store.rs`:
- Around line 145-150: The function set_heaviest_tipset is updating
heaviest_tipset_key_provider and heaviest_tipset_cache before persisting the
tipset key, which can leave state partially applied if
Tipset.key().save(self.blockstore()) fails; change the order so you call
ts.key().save(self.blockstore()) first and return the error if it fails, then
update heaviest_tipset_key_provider.set_heaviest_tipset_key(...), update
*self.heaviest_tipset_cache.write(), and finally call
self.publisher.send(HeadChange::Apply(ts)); ensure errors from save() and the
key provider set are propagated and that the publish only happens after
successful persistence and in-memory updates.
In `@src/cli/subcommands/snapshot_cmd.rs`:
- Around line 140-149: The export currently sets ForestChainExportParams with
include_tipset_keys: false which omits tipset-key blocks needed by the load
path; update the snapshot command to enable tipset-key emission by setting
include_tipset_keys: true (or wire through an explicit CLI flag) when
constructing ForestChainExportParams in snapshot_cmd.rs so exported snapshots
include tipset keys for historical hash-based tipset lookup.
In `@src/rpc/methods/chain.rs`:
- Around line 685-694: The export currently sets
ForestChainExportParams.include_tipset_keys = false which causes
Filecoin.ChainExport to omit block records needed by TipsetKey::load; change the
export to include tipset keys by setting include_tipset_keys = true (or make it
configurable) so the exported snapshot contains the blocks used by TipsetKey
resolution; update the ForestChainExportParams construction in the
Filecoin.ChainExport path to set include_tipset_keys to true (or wire through a
flag) to restore historical hash-based tipset resolution after import.
In `@src/tool/subcommands/archive_cmd.rs`:
- Around line 629-633: The export currently constructs ExportOptions with
include_tipset_keys: false which causes produced snapshots to omit tipset-key
blocks and breaks historical tipset hash lookups on import; update the
ExportOptions construction (the ExportOptions struct usage that sets
skip_checksum, include_receipts, include_events, include_tipset_keys) to set
include_tipset_keys: true (or change the user-facing default to opt-out) so
tipset-key blocks are included in exports; locate the ExportOptions
instantiation in archive_cmd.rs and change the include_tipset_keys flag
accordingly and ensure any related documentation or CLI defaults reflect the
opt-out behavior.
---
Nitpick comments:
In `@src/blocks/tipset.rs`:
- Around line 55-63: The two new public helpers cid and car_block lack rustdoc;
replace the plain comment with /// doc comments above pub fn cid and pub fn
car_block that explain the Lotus-compat encoding contract (e.g., “Special
encoding to match Lotus”), describe the return types (Cid and CarBlock) and
possible errors, and note implementation details used for car_block (it
serializes via fvm_ipld_encoding::to_vec and computes the CID with
Cid::from_cbor_encoded_raw_bytes_blake2b256) so these details appear in
generated API docs for cid, car_block, and the CarBlock helper.
- Around line 60-63: The persistence helpers (e.g., the car_block method)
currently propagate errors without context; update the error boundaries to add
operation-specific context by chaining .context(...) to fallible calls: for
car_block, add .context("serializing TipsetKey to CAR bytes") to the
fvm_ipld_encoding::to_vec(...) call and .context("computing CID from CAR bytes")
to the Cid::from_cbor_encoded_raw_bytes_blake2b256(...) call; apply the same
pattern to the other TipsetKey persistence helpers (the encode/store/load
helpers referenced around lines 146-152) so that each serialize, compute-cid,
write, and read operation records a clear, descriptive context string.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: ac00327b-c9ce-4851-a83f-d5e6065620e5

📥 Commits

Reviewing files that changed from the base of the PR and between 3591d1a and d4e4816.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (15)

• Cargo.toml
• mise.toml
• src/blocks/tipset.rs
• src/chain/mod.rs
• src/chain/store/chain_store.rs
• src/cli/subcommands/snapshot_cmd.rs
• src/daemon/db_util.rs
• src/daemon/mod.rs
• src/db/gc/snapshot.rs
• src/ipld/util.rs
• src/rpc/methods/chain.rs
• src/rpc/methods/eth.rs
• src/rpc/methods/f3/types.rs
• src/tool/subcommands/archive_cmd.rs
• src/utils/cid/mod.rs

💤 Files with no reviewable changes (1)

• mise.toml

[LesnyRumcajs]

No ✔️ no review!

[codecov]

Codecov Report

❌ Patch coverage is 75.64103% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.40%. Comparing base (56382dd) to head (1b7421c).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
src/blocks/tipset.rs	89.79%	0 Missing and 5 partials ⚠️
src/ipld/util.rs	63.63%	3 Missing and 1 partial ⚠️
src/rpc/methods/chain.rs	0.00%	3 Missing ⚠️
src/chain/store/chain_store.rs	0.00%	0 Missing and 2 partials ⚠️
src/cli/subcommands/snapshot_cmd.rs	0.00%	1 Missing ⚠️
src/daemon/db_util.rs	0.00%	1 Missing ⚠️
src/daemon/mod.rs	0.00%	1 Missing ⚠️
src/db/gc/snapshot.rs	0.00%	1 Missing ⚠️
src/rpc/methods/f3/types.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
src/chain/mod.rs	75.00% <100.00%> (+0.45%)	⬆️
src/rpc/methods/eth.rs	67.95% <ø> (-0.37%)	⬇️
src/tool/subcommands/archive_cmd.rs	29.56% <100.00%> (+0.09%)	⬆️
src/utils/cid/mod.rs	87.50% <100.00%> (+1.78%)	⬆️
src/cli/subcommands/snapshot_cmd.rs	0.00% <0.00%> (ø)
src/daemon/db_util.rs	54.11% <0.00%> (ø)
src/daemon/mod.rs	29.08% <0.00%> (+0.10%)	⬆️
src/db/gc/snapshot.rs	18.77% <0.00%> (-0.09%)	⬇️
src/rpc/methods/f3/types.rs	62.29% <0.00%> (+1.80%)	⬆️
src/chain/store/chain_store.rs	69.49% <0.00%> (+0.81%)	⬆️
... and 3 more

... and 13 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 56382dd...1b7421c. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.