Skip to content

feat!(chain): implement first_seen tracking #1950

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 16, 2025

Conversation

uvuvuwu
Copy link
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
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.

@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
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.

@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
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
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
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants