Skip to content
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

Expand the KeychainTxOutIndex::inner docs to provide usage examples #1353

Open
danielabrozzoni opened this issue Feb 19, 2024 · 2 comments · May be fixed by #1652
Open

Expand the KeychainTxOutIndex::inner docs to provide usage examples #1353

danielabrozzoni opened this issue Feb 19, 2024 · 2 comments · May be fixed by #1652
Labels
documentation Improvements or additions to documentation module-blockchain
Milestone

Comments

@danielabrozzoni
Copy link
Member

See #1203 (comment)

@notmandatory notmandatory added this to the 1.0.0-alpha milestone Feb 22, 2024
@notmandatory notmandatory added the documentation Improvements or additions to documentation label Feb 22, 2024
@notmandatory notmandatory modified the milestones: 1.0.0-alpha, 1.0.0-beta Mar 18, 2024
@ValuedMammal
Copy link
Contributor

@evanlinjin I'm thinking of just removing the KeychainTxOutIndex::inner method from the public interface. I see that it is used in test_keychain_txout_index.rs mainly for the purpose of getting the total count of indexed spks from the inner index. Is there a reason a user might need that information? If not, a simple solution is to move a few tests to unit tests in keychain_txout module. Alternatively there could be another way to express the same idea using the existing APIs, or similarly exposing SpkTxOutIndex::all_spks for KeychainTxOutIndex. Curious if you have an opinion.

@evanlinjin
Copy link
Member

evanlinjin commented Oct 17, 2024

@ValuedMammal my only argument for keeping it is that you can use it to explore lookahead spks. However, you can also use the unbounded.. iterator methods on KeychainTxOutIndex.

Since it's mentioned to be a potential footgun (since it includes lookahead spks), maybe we should remove it? If we need a certain functionality later on, we can add it then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation module-blockchain
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

4 participants