-
Notifications
You must be signed in to change notification settings - Fork 311
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
refactor(wallet): use Amount
everywhere
#1595
refactor(wallet): use Amount
everywhere
#1595
Conversation
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 292ec3c
impl IsDust for u64 { | ||
fn is_dust(&self, script: &Script) -> bool { | ||
*self < script.minimal_non_dust().to_sat() | ||
Amount::from_sat(*self).is_dust(script) | ||
} | ||
} |
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.
nit: I guess this could be moved to coin_selection
, it's the only place still using is_dust
with a u64
, right ?
let mut tx_builder = wallet.build_tx(); | ||
tx_builder | ||
.add_recipient(faucet_address.script_pubkey(), SEND_AMOUNT) | ||
.add_recipient(address.script_pubkey(), SEND_AMOUNT) |
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.
👍 thanks for cleaning this up!
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 292ec3c
This is a followup to #1426 that refactors wallet internals to use
bitcoin::Amount
(nearly) everywhere. I chose not to change any public types incoin_selection.rs
that still useu64
as that might require more discussion.partially addresses #1432
fixes #1434
Checklists
All Submissions:
cargo fmt
andcargo clippy
before committing