Skip to content

Add lookahead as optional argument#770

Merged
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
rustaceanrob:lookahead-5-28
May 28, 2025
Merged

Add lookahead as optional argument#770
thunderbiscuit merged 1 commit intobitcoindevkit:masterfrom
rustaceanrob:lookahead-5-28

Conversation

@rustaceanrob
Copy link
Copy Markdown
Collaborator

Description

Users may want to configure the initial lookahead value of a wallet, but we can leave this optional.

Using the default argument here, so this should actually be non-breaking. Compared to a builder this is ergonomic in the target languages IMO

Changelog notice

  • Add lookahead as optional wallet configuration

Checklists

All Submissions:

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

Copy link
Copy Markdown
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

Thanks for adding this! Just one question/comment.

Comment thread bdk-ffi/src/wallet.rs Outdated
///
/// If you have previously created a wallet, use load instead.
#[uniffi::constructor]
#[uniffi::constructor(default(lookahead = None))]
Copy link
Copy Markdown
Member

@thunderbiscuit thunderbiscuit May 28, 2025

Choose a reason for hiding this comment

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

I wonder if using the default lookahead in bdk_wallet might make more sense for users looking at the default values. As is, you might think that unless one is set, your lookahead will be null (even though that doesn't really make sense) but that's not true (the default is 25).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Gotcha. Is there a way to use constants or do we hard-code 25?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Answered that for myself. Macro requires a literal, so must be hard coded at 25.

Using the default argument here, so this should actually be
non-breaking. Compared to a builder this is ergonomic in the target
languages IMO
Copy link
Copy Markdown
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

ACK 04756c0.

@thunderbiscuit thunderbiscuit merged commit 04756c0 into bitcoindevkit:master May 28, 2025
26 checks passed
@rustaceanrob rustaceanrob deleted the lookahead-5-28 branch May 28, 2025 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants