The reference changeset vectors in bdk_testenv::persist_test_utils (added in #2012) currently only exercise the "insert brand-new key" path. The cases where a persistence backend actually breaks — overwriting a key that is already stored — are not covered, so a backend using plain INSERT (or INSERT OR IGNORE) instead of REPLACE / ON CONFLICT DO UPDATE passes the helper while silently dropping updates in production.
Follow-up to #2012; the drop(store) fix for the reopen phase is being handled separately.
1. local_chain_changesets — no reorg vector
The two changesets use disjoint heights (910425/910426, then 910427), so the LWW overwrite path is never hit. Add a vector that re-inserts an existing height with a different hash.
2. tx_graph_changesets — no upsert vector
Changeset 2 introduces a brand-new tx2, so last_seen / first_seen / last_evicted are never re-persisted with a newer value for a txid that is already stored. Add a second entry for tx1.compute_txid() with a later last_seen to changeset 2.
(keychain_txout_changesets already covers this — last_revealed for descriptor_ids[0] goes 1 → 2.)
3. Block removal (height, None) is currently untestable
Worth deciding before someone tries: the None case cannot just be added to local_chain_changesets. local_chain::ChangeSet::from_sqlite (crates/chain/src/rusqlite_impl.rs) deletes the row and therefore can never return a None entry, so assert_persist_changesets' strict load() == merge(all changesets) equality is unsatisfiable for deletions — a correct backend would fail.
Two options:
- document that
None is out of scope for these vectors, or
- relax the comparison to compare applied state rather than raw merged changesets.
4. Minor: incoherent timestamps in tx_graph_changeset1
last_seen: 1755416650 is earlier than first_seen: 1755416655 for the same txid; changeset 2 is internally consistent, so it looks like a transposition. Harmless for a round-trip check, but these vectors are the published reference for third-party backend authors, and a backend that validates first_seen <= last_seen on write would reject them.
5. Consider splitting the helper
assert_persist_changesets only checks the accumulate-then-reload contract; it never persists the same changeset twice. Splitting into assert_persists and assert_persists_idempotent would let a backend opt into the stronger claim instead of leaving callers to guess which one is covered — relevant for the planned semilattice LocalChain changeset, where idempotency is the point.
The reference changeset vectors in
bdk_testenv::persist_test_utils(added in #2012) currently only exercise the "insert brand-new key" path. The cases where a persistence backend actually breaks — overwriting a key that is already stored — are not covered, so a backend using plainINSERT(orINSERT OR IGNORE) instead ofREPLACE/ON CONFLICT DO UPDATEpasses the helper while silently dropping updates in production.Follow-up to #2012; the
drop(store)fix for the reopen phase is being handled separately.1.
local_chain_changesets— no reorg vectorThe two changesets use disjoint heights (
910425/910426, then910427), so the LWW overwrite path is never hit. Add a vector that re-inserts an existing height with a different hash.2.
tx_graph_changesets— no upsert vectorChangeset 2 introduces a brand-new
tx2, solast_seen/first_seen/last_evictedare never re-persisted with a newer value for a txid that is already stored. Add a second entry fortx1.compute_txid()with a laterlast_seento changeset 2.(
keychain_txout_changesetsalready covers this —last_revealedfordescriptor_ids[0]goes1→2.)3. Block removal
(height, None)is currently untestableWorth deciding before someone tries: the
Nonecase cannot just be added tolocal_chain_changesets.local_chain::ChangeSet::from_sqlite(crates/chain/src/rusqlite_impl.rs) deletes the row and therefore can never return aNoneentry, soassert_persist_changesets' strictload() == merge(all changesets)equality is unsatisfiable for deletions — a correct backend would fail.Two options:
Noneis out of scope for these vectors, or4. Minor: incoherent timestamps in
tx_graph_changeset1last_seen: 1755416650is earlier thanfirst_seen: 1755416655for the same txid; changeset 2 is internally consistent, so it looks like a transposition. Harmless for a round-trip check, but these vectors are the published reference for third-party backend authors, and a backend that validatesfirst_seen <= last_seenon write would reject them.5. Consider splitting the helper
assert_persist_changesetsonly checks the accumulate-then-reload contract; it never persists the same changeset twice. Splitting intoassert_persistsandassert_persists_idempotentwould let a backend opt into the stronger claim instead of leaving callers to guess which one is covered — relevant for the planned semilatticeLocalChainchangeset, where idempotency is the point.