Skip to content

feat!(chain): implement first_seen tracking#1950

Merged
evanlinjin merged 1 commit into
bitcoindevkit:masterfrom
uvuvuwu:add-first-seen
May 16, 2025
Merged

feat!(chain): implement first_seen tracking#1950
evanlinjin merged 1 commit into
bitcoindevkit:masterfrom
uvuvuwu:add-first-seen

Conversation

@uvuvuwu
Copy link
Copy Markdown
Contributor

@uvuvuwu uvuvuwu commented Apr 30, 2025

Description

This PR solves issue #1947 by implementing first_seen tracking.

  • Added first_seen field to TxGraph and ChangeSet so that first_seen timestamp can be added when inserting a new seen-at using insert_seen_at.

  • first_seen added to TxNode as a way to retrieve the first-seen timestamp for a transaction.

  • first_seen added to ChainPosition::Unconfirmed to order unconfirmed transactions by first_seen.

  • New tests have been added for the above described functionalities.

Changelog notice

  • Add tracking first-seen timestamps of transactions

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@notmandatory notmandatory moved this to Needs Review in BDK Chain Apr 30, 2025
@notmandatory notmandatory added the api A breaking API change label Apr 30, 2025
Copy link
Copy Markdown
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 the bdk_chain crate.

Comment thread crates/chain/src/chain_data.rs Outdated
Comment thread crates/chain/src/chain_data.rs Outdated
Comment thread crates/chain/src/chain_data.rs Outdated
Comment thread crates/chain/src/tx_graph.rs Outdated
Comment thread crates/chain/tests/test_tx_graph.rs Outdated
Comment thread crates/chain/tests/test_tx_graph.rs
Comment thread crates/chain/src/tx_graph.rs Outdated
@uvuvuwu uvuvuwu force-pushed the add-first-seen branch 2 times, most recently from b38d154 to 1d6fb3c Compare May 1, 2025 23:46
@uvuvuwu uvuvuwu changed the title feat: implement first_seen tracking feat(chain): implement first_seen tracking May 1, 2025
@uvuvuwu uvuvuwu force-pushed the add-first-seen branch 2 times, most recently from 180e4ee to 352ccbc Compare May 2, 2025 04:03
@uvuvuwu uvuvuwu requested a review from evanlinjin May 2, 2025 04:18
Copy link
Copy Markdown
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just documentation suggestions.

Comment thread crates/chain/src/tx_graph.rs Outdated
Comment thread crates/chain/src/tx_graph.rs Outdated
@uvuvuwu uvuvuwu changed the title feat(chain): implement first_seen tracking feat!(chain): implement first_seen tracking May 6, 2025
@uvuvuwu uvuvuwu requested a review from evanlinjin May 6, 2025 00:51
Copy link
Copy Markdown
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 8b17bcf

@evanlinjin evanlinjin merged commit b9e4e4c into bitcoindevkit:master May 16, 2025
17 checks passed
@github-project-automation github-project-automation Bot moved this from Needs Review to Done in BDK Chain May 16, 2025
@LagginTimes LagginTimes mentioned this pull request May 23, 2025
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
@notmandatory notmandatory added this to the Wallet 2.0.0 milestone May 26, 2025
@notmandatory notmandatory assigned uvuvuwu and unassigned uvuvuwu May 26, 2025
@ValuedMammal ValuedMammal mentioned this pull request May 26, 2025
39 tasks
@LagginTimes LagginTimes mentioned this pull request Jun 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api A breaking API change

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants