Skip to content

Conversation

@ValuedMammal
Copy link
Collaborator

Fix #1353 by removing the inner method from keychain_txout module. See commit message for details.

Changelog notice

  • bdk_chain: Removed method KeychainTxOutIndex::inner

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing
  • This pull request breaks the existing API
  • I'm linking the issue being fixed by this PR

@ValuedMammal ValuedMammal added this to the 1.0.0-beta milestone Oct 17, 2024
@ValuedMammal ValuedMammal force-pushed the refactor/ktxindex-inner branch from 445f2f2 to 936bba9 Compare October 17, 2024 22:58
Comment on lines 174 to 175
txout_index.spk_at_index(TestKeychain::External, i),
Some(spk_at_index(&external_descriptor, i))
Copy link
Contributor

Choose a reason for hiding this comment

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

I still might need another pass to fully grasp the tests, but shouldn't this be asserting for the TestKeychain::Internal?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thank you, fixed

It's desirable to have the inner SPK index internal to the
`keychain_txout` module, which aims to provide views into
the set of revealed SPKs for a given keychain.

The inner index also functions as a cache of unrevealed SPKs,
i.e. the lookahead. We test the functionality of the lookahead
by checking the value returned from
`KeychainTxOutIndex::spk_at_index` matches the SPK derived by
the descriptor at that index for indices ranging from the
last revealed to the expected last stored.
Copy link
Contributor

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

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

ACK 8494c12

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

ACK 8494c12

@notmandatory notmandatory merged commit b220d61 into bitcoindevkit:master Oct 30, 2024
@notmandatory
Copy link
Member

BTW, thanks for naming the lookahead and reveal_to values, made the tests easier to follow.

@ValuedMammal ValuedMammal deleted the refactor/ktxindex-inner branch October 31, 2024 17:18
@notmandatory notmandatory mentioned this pull request Dec 11, 2024
32 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api A breaking API change module-blockchain

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Expand the KeychainTxOutIndex::inner docs to provide usage examples

3 participants