feat!(chain): implement first_seen tracking#1950
Merged
Merged
Conversation
evanlinjin
requested changes
May 1, 2025
Member
There was a problem hiding this comment.
Mostly looks good. Some quirks that requires changing in the tests. Also, ChainPosition field order should be changed to make proper use of derive(Ord).
Other things to take note of:
- Documentation comments need full stops!
- Will be nice to include the module changed in commit messages. I.e.
feat(chain):signals that we are changing thebdk_chaincrate.
evanlinjin
reviewed
May 1, 2025
b38d154 to
1d6fb3c
Compare
first_seen trackingfirst_seen tracking
180e4ee to
352ccbc
Compare
evanlinjin
reviewed
May 5, 2025
Member
evanlinjin
left a comment
There was a problem hiding this comment.
LGTM, just documentation suggestions.
first_seen trackingfirst_seen tracking
8 tasks
evanlinjin
added a commit
that referenced
this pull request
May 23, 2025
b27a019 fix(chain): persist `first_seen` (Wei Chen) Pull request description: Fixes #1965. ### Description Adds missing persistence for `first_seen`, which was not included in #1950. ### Changelog notice - Adds `first_seen` column to the `bdk_txs` table via schema v3 migration. - Updates `from_sqlite()` and `persist_to_sqlite()` to handle `first_seen`. - Updates the v0-to-v3 migration test to verify compatibility with older schemas. ### Checklists #### All Submissions: * [x] I've signed all my commits * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md) * [x] I ran `cargo +nightly fmt` and `cargo clippy` before committing #### New Features: * [x] I've added tests for the new feature * [x] I've added docs for the new feature #### Bugfixes: * [ ] This pull request breaks the existing API * [x] I've added tests to reproduce the issue which are now passing * [x] I'm linking the issue being fixed by this PR ACKs for top commit: evanlinjin: ACK b27a019 Tree-SHA512: a8c4cd930e20f7bdf1a02fc3155b5df9f1627676fe10a2d77ea856e71e45f783bba1bb8cf4eceb8dba71c345e7985a9e091002966cec147871e6672c0e2ac5c4
39 tasks
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR solves issue #1947 by implementing
first_seentracking.Added
first_seenfield toTxGraphandChangeSetso thatfirst_seentimestamp can be added when inserting a new seen-at usinginsert_seen_at.first_seenadded toTxNodeas a way to retrieve the first-seen timestamp for a transaction.first_seenadded toChainPosition::Unconfirmedto order unconfirmed transactions byfirst_seen.New tests have been added for the above described functionalities.
Changelog notice
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features: