Issue
Builder::data_dir accepts a path and documents it as the directory "where data should be stored", but nothing reads it. Node::new destructures the config and discards the field:
let Config {
required_peers,
white_list,
whitelist_only,
data_path: _, // <-- src/node.rs
chain_state,
...Tracing it back, this looks vestigial rather than broken. 0.2.0 removed rusqlite "along with the underlying header and peer stores" and introduced ChainState on the builder in the same release, so consumer-supplied state replaced the internal stores by design. data_dir just outlived that change with its original doc comment. I confirmed the field is discarded identically in 0.5.0, 0.6.0 and 0.6.3.
The reason it is worth fixing rather than ignoring: the method reads as an opt-in to persistence, so it is easy to call it and assume chain data survives a Node rebuild. We did exactly that. Our supervisor rebuilds the node when it loses its peers, and because ChainState::Checkpoint rebuilds the block tree from the checkpoint alone, each rebuild re-syncs the entire range from that anchor. We had a data_dir configured the whole time and it did nothing.
Possible resolutions:
- Remove
Builder::data_dir (and Config::data_path) as part of a breaking release.
- Keep it and mark it
#[deprecated] with a note pointing at ChainState.
Happy to send a PR for any of these if useful.
Context on what I was doing:
we use ChainState::Checkpoint with an anchor resolved from a trusted Bitcoin Core node, because the header chain is what independently validates block hashes that node reports to us. ChainState::Snapshot looked like the way to avoid the re-sync, but SyncUpdate::recent_history only carries the last ten headers, so restoring from it would move the anchor up to tip-10 and drop the range we still need to verify against. Not a request for anything, just noting why the data_dir path looked attractive to us.
Issue
Builder::data_diraccepts a path and documents it as the directory "where data should be stored", but nothing reads it.Node::newdestructures the config and discards the field:Tracing it back, this looks vestigial rather than broken. 0.2.0 removed
rusqlite"along with the underlying header and peer stores" and introducedChainStateon the builder in the same release, so consumer-supplied state replaced the internal stores by design.data_dirjust outlived that change with its original doc comment. I confirmed the field is discarded identically in 0.5.0, 0.6.0 and 0.6.3.The reason it is worth fixing rather than ignoring: the method reads as an opt-in to persistence, so it is easy to call it and assume chain data survives a
Noderebuild. We did exactly that. Our supervisor rebuilds the node when it loses its peers, and becauseChainState::Checkpointrebuilds the block tree from the checkpoint alone, each rebuild re-syncs the entire range from that anchor. We had adata_dirconfigured the whole time and it did nothing.Possible resolutions:
Builder::data_dir(andConfig::data_path) as part of a breaking release.#[deprecated]with a note pointing atChainState.Happy to send a PR for any of these if useful.
Context on what I was doing:
we use
ChainState::Checkpointwith an anchor resolved from a trusted Bitcoin Core node, because the header chain is what independently validates block hashes that node reports to us.ChainState::Snapshotlooked like the way to avoid the re-sync, butSyncUpdate::recent_historyonly carries the last ten headers, so restoring from it would move the anchor up to tip-10 and drop the range we still need to verify against. Not a request for anything, just noting why thedata_dirpath looked attractive to us.