-
Notifications
You must be signed in to change notification settings - Fork 47
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
Bump rust bdk to alpha 11 #533
Conversation
d60bf77
to
553755c
Compare
WalkthroughThe updates across various modules primarily involve a transition from using total balance directly to accessing it through the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
bdk-python/tests/test_offline_wallet.py (1)
Line range hint
16-16
: Undefined name 'Wallet' used. This might be due to a missing import or a typo.+ from bdk import Wallet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! My one thought that might need more discussion is around the Amount type being a dictionary instead of an interface. I think if we eventually want to expand it to more (and the from_sat and from_btc method might be the first ones we go after) in order to use the type for what it's pretty good at, we'll need methods on it, and therefore will not be able to have fields.
Another option is to have an actual struct that would hold both as fields, but then no other cool methods could be called on it. Something like
struct Amount {
pub sat: u64,
pub btc: f64,
}
But then you'd still need constructors from_sat and from_btc to create it from the bindigns side (for example when providing it to your txbuilder)? I haven't tried that before but I think that might not be possible.
Oh and the |
my b, fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Out of diff range and nitpick comments (1)
bdk-python/tests/test_offline_wallet.py (1)
Line range hint
16-16
: Undefined nameWallet
used in the test setup.- wallet: Wallet = bdk.Wallet( + wallet: bdk.Wallet = bdk.Wallet(
465a0a5
to
e554088
Compare
@thunderbiscuit ready for review/merge, rebased on the signet PR, test passing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK e554088. Good work! A lot of files changes, but nothing too insane in the end.
Would you mind adding text for the changelog? That way we can just copy/paste it when we make the release.
Sure, added, hopefully its what you were looking for- |
Oh well that's not quite what I meant but it works! |
Description
Bump rust bdk to alpha 11 with things like:
Changelog notice
Checklists
All Submissions:
cargo fmt
andcargo clippy
before committingNew Features:
Bugfixes: