bitcoindevkit / bitcoindevkit/bdk

TxGraph:: apply_changeset ignores ChangeSet::first_seen

Open Beginner friendly
#2,270 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Rust
Stars
1.1k
Forks
483
Avg merge
20d 3h
Merged PRs (30d)
3

Description

## Describe the bug

`TxGraph::apply_changeset` loops over `txs`, `txouts`, `anchors`, `last_seen` and `last_evicted` but not `first_seen`. Meanwhile `initial_changeset()` does write `first_seen` out, so persist and apply don't match up.

the value doesn't just come back as `None` `apply_changeset` still replays `last_seen` via `insert_seen_at`, which calls `update_first_seen` internally, and on an empty graph that entry is vacant so it just takes the `last_seen` value So after a reload every tx ends up with `first_seen == last_seen`.

SQLite isn't the problem the changeset that comes back off disk has the right values. They get thrown away when it's applied to the graph.

`first_seen` is the primary sort key for unconfirmed txs in `ChainPosition::Ord` Once it equals `last_seen` the ordering can change across a restart, without anything else changing.

## To Reproduce

Two unconfirmed txs where `first_seen` and `last_seen` disagree on order, through a real sqlite file (write, close, reopen, apply):

```
before: A = (100, 500) B = (200, 300) order = [A, B]

changeset off disk: first_seen = {A: 100, B: 200} <- fine
last_seen = {A: 500, B: 300}

after apply_changeset: A = (500, 500) B = (300, 300) order = [B, A]
```

Also reproduces with no database at all:

```rust
let mut reloaded = TxGraph::::default();
reloaded.apply_changeset(graph.initial_changeset());
// first_seen is now last_seen
```

## Expected behavior

`initial_changeset()` to `apply_changeset()` should round-trip, and unconfirmed ordering shouldn't change just because you restarted.

**Build environment**
- BDK tag/commit: `337e9d68`, `bdk_chain` 0.23.2 (`apply_changeset` is the same on `master`)
- OS+version: Windows 11
- Rust/Cargo version: 1.97.1
- Rust/Cargo target: x86_64-pc-windows-msvc

**Which backend(s) are relevant (if any)?**
- [ ] Electrum
- [ ] Esplora
- [ ] Bitcoin Core RPC
- [x] None / not backend-related (e.g. `bdk_chain`, `bdk_core`)
- [ ] Other (please specify): `____`

**Is this blocking production use?**
- [ ] Yes
- [x] No

## Additional context

`first_seen` was added in #1947/#1950 and persisted in #1965/#1966, so it's computed and stored on purpose it just never gets read back. #1966's test checks the reloaded *changeset* has `first_seen` but never applies it to a graph, which is why this slipped through

Contributor guide

Open the contributing guide

Research direction

Start with TxGraph::apply_changeset and initial_changeset, then reproduce the in-memory round trip described in the issue before checking the SQLite reload path. Done means first_seen values survive applying the changeset and the unconfirmed transaction order remains unchanged across reload.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.