Skip to content

units: add consolidate_feerate as cli arg #77

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.

Sign up for GitHub

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

Merged
rustaceanrob merged 1 commit into from
Jul 18, 2026
Merged

units: add consolidate_feerate as cli arg #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion capnp/wallet.capnp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ interface Wallet {
getHistory @2 () -> (entries :Text);
receive @3 () -> (address :Text);
broadcastRawTx @4 (tx :Data) -> (txid :Text);
sendToAddress @5 (address :Text, amountSat :UInt64, feeRateSatPerVb :Float64) -> (ok :Bool, message :Text);
sendToAddress @5 (address :Text, amountSat :UInt64, feeRateSatPerVb :Float64, consolidateFeeRateSatPerVb :Float64 = 10) -> (ok :Bool, message :Text);
}
60 changes: 49 additions & 11 deletions crates/wallet/src/silentpayments/sending.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ use silentpayments::{Network, SilentPaymentAddress};

use crate::silentpayments::wallet::{Coin, Wallet};

const LONG_TERM_FEERATE_SAT_PER_VB: f32 = 1.0;

#[derive(Debug)]
pub enum SendError {
WatchOnly,
Expand Down Expand Up @@ -143,6 +141,7 @@ impl Wallet {
recipient: Recipient,
amount: Amount,
fee_rate: FeeRate,
long_term_fee_rate: FeeRate,
) -> Result<Transaction, SendError> {
let spend_secret = self.spend_secret.ok_or(SendError::WatchOnly)?;
let keys = self.keys.as_ref().ok_or(SendError::WatchOnly)?;
Expand All @@ -163,18 +162,21 @@ impl Wallet {
recipient,
amount,
fee_rate,
long_term_fee_rate,
self.scan_height,
change_address,
&coins,
)
}
}

#[allow(clippy::too_many_arguments)]
fn build_transaction(
spend_secret: &SecretKey,
recipient: Recipient,
amount: Amount,
fee_rate: FeeRate,
long_term_fee_rate: FeeRate,
tip_height: u32,
change_address: SilentPaymentAddress,
coins: &[SpendableCoin],
Expand Down Expand Up @@ -213,10 +215,11 @@ fn build_transaction(
DrainWeights::TR_KEYSPEND,
change_dust.to_sat(),
feerate,
bdk_coin_select::FeeRate::from_sat_per_vb(LONG_TERM_FEERATE_SAT_PER_VB),
cs_feerate(long_term_fee_rate),
);

let (mut selected, drain) = select_coins(coins, target, change_policy, 100_000)?;
let (mut selected, drain) =
select_coins(coins, target, change_policy, long_term_fee_rate, 100_000)?;
let mut rng = bitcoin::secp256k1::rand::thread_rng();
selected.shuffle(&mut rng);

Expand Down Expand Up @@ -319,6 +322,7 @@ fn select_coins<'a, 'c>(
coins: &'c [SpendableCoin<'a>],
target: Target,
change_policy: ChangePolicy,
long_term_fee_rate: FeeRate,
max_bnb_rounds: usize,
) -> Result<(Vec<&'c SpendableCoin<'a>>, Drain), SendError> {
let spendable: Vec<&SpendableCoin> = coins.iter().collect();
Expand All @@ -328,9 +332,10 @@ fn select_coins<'a, 'c>(
.collect();

let mut selector = CoinSelector::new(&candidates);
let long_term_feerate = cs_feerate(long_term_fee_rate);
let metric = LowestFee {
target,
long_term_feerate: bdk_coin_select::FeeRate::from_sat_per_vb(LONG_TERM_FEERATE_SAT_PER_VB),
long_term_feerate,
change_policy,
};
if selector.run_bnb(metric, max_bnb_rounds).is_err() {
Expand Down Expand Up @@ -450,12 +455,14 @@ mod tests {
}];

let fee_rate = FeeRate::from_sat_per_vb(2).unwrap();
let long_term_fee_rate = FeeRate::from_sat_per_vb(10).unwrap();
let amount = Amount::from_sat(50_000);
let tx = build_transaction(
&spend_secret,
Recipient::SilentPayment(recipient),
amount,
fee_rate,
long_term_fee_rate,
100,
change_address,
&coins,
Expand Down Expand Up @@ -510,6 +517,7 @@ mod tests {
Recipient::SilentPayment(recipient),
Amount::from_sat(20_000),
FeeRate::from_sat_per_vb(2).unwrap(),
FeeRate::from_sat_per_vb(10).unwrap(),
100,
change_address,
&coins,
Expand Down Expand Up @@ -540,12 +548,22 @@ mod tests {
let amount = Amount::from_sat(50_000);

let tx = wallet
.build_transaction(Recipient::SilentPayment(recipient), amount, fee_rate)
.build_transaction(
Recipient::SilentPayment(recipient),
amount,
fee_rate,
fee_rate,
)
.unwrap();
wallet.reserve_coins(tx.input.iter().map(|i| i.previous_output));

let err = wallet
.build_transaction(Recipient::SilentPayment(recipient), amount, fee_rate)
.build_transaction(
Recipient::SilentPayment(recipient),
amount,
fee_rate,
fee_rate,
)
.unwrap_err();
assert!(matches!(err, SendError::NoSpendableCoins));
}
Expand All @@ -572,19 +590,34 @@ mod tests {
let amount = Amount::from_sat(50_000);

let tx = wallet
.build_transaction(Recipient::SilentPayment(recipient), amount, fee_rate)
.build_transaction(
Recipient::SilentPayment(recipient),
amount,
fee_rate,
fee_rate,
)
.unwrap();
let outpoints: Vec<_> = tx.input.iter().map(|i| i.previous_output).collect();

wallet.reserve_coins(outpoints.iter().copied());
assert!(matches!(
wallet.build_transaction(Recipient::SilentPayment(recipient), amount, fee_rate),
wallet.build_transaction(
Recipient::SilentPayment(recipient),
amount,
fee_rate,
fee_rate
),
Err(SendError::NoSpendableCoins)
));

wallet.release_coins(outpoints);
assert!(wallet
.build_transaction(Recipient::SilentPayment(recipient), amount, fee_rate)
.build_transaction(
Recipient::SilentPayment(recipient),
amount,
fee_rate,
fee_rate
)
.is_ok());
}

Expand Down Expand Up @@ -625,7 +658,9 @@ mod tests {
let change_policy = ChangePolicy::min_value(DrainWeights::TR_KEYSPEND, 330);

// max_bnb_rounds = 0 skips branch and bound, forcing the largest-first fallback.
let (selected, _) = select_coins(&coins, target, change_policy, 0).unwrap();
let long_term_fee_rate = FeeRate::from_sat_per_vb(2).unwrap();
let (selected, _) =
select_coins(&coins, target, change_policy, long_term_fee_rate, 0).unwrap();

let mut values: Vec<u64> = selected.iter().map(|c| c.coin.value.to_sat()).collect();
values.sort_unstable();
Expand Down Expand Up @@ -671,6 +706,7 @@ mod tests {
Recipient::SilentPayment(recipient),
Amount::from_sat(50_000),
FeeRate::from_sat_per_vb(30).unwrap(),
FeeRate::from_sat_per_vb(30).unwrap(),
0,
change_address,
&coins,
Expand Down Expand Up @@ -726,6 +762,7 @@ mod tests {
Recipient::Address(addr),
amount,
FeeRate::from_sat_per_vb(2).unwrap(),
FeeRate::from_sat_per_vb(10).unwrap(),
)
.unwrap_or_else(|e| panic!("{kind}: {e}"));

Expand Down Expand Up @@ -753,6 +790,7 @@ mod tests {
Recipient::SilentPayment(recipient),
Amount::from_sat(1_000),
FeeRate::from_sat_per_vb(2).unwrap(),
FeeRate::from_sat_per_vb(10).unwrap(),
100,
change_address,
&[],
Expand Down
19 changes: 19 additions & 0 deletions src/bin/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,18 @@ enum WalletCmd {
amount_sat: u64,
/// Fee rate, in satoshis per virtual byte.
fee_rate_sat_per_vb: f64,
/// The maximum feerate in sats/vB at which transaction building may
/// use more inputs than strictly necessary so that the wallet's UTXO
/// pool can be reduced (default: 10 sats/vB). Long term fee rate,
/// in satoshis per virtual byte.
///
/// Setting the consolidate fee rate helps the coin-selection
/// algorithm know if to use more UTXOs or less when building a
/// transaction. That is, if the current fee rate is high
/// (fee rate > consolidate fee rate), then consume less inputs making
/// the transaction cheaper. Likewise, if the current fee rate is low,
/// use more inputs and consolidate the UTXO set to be fewer.
consolidate_fee_rate_sat_per_vb: Option<f64>,

@rustaceanrob rustaceanrob Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm still lost as to what this value is for. Can we explain how this interacts with the fee_rate_sat_per_vb here? I think that is what most users would care about

@yancyribbens yancyribbens Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

To add a little more to the docs comment, the consolidation fee (also known as long_term_fee_rate) helps to determine if the current fee_rate is high or low. So, if fee_rate > consolidation_fee_rate, then the current fee_rate is high based on historical trends, and if consolidation_fee_rate > fee_rate then the current fee_rate level is considered cheap. The reason this matters is that BnB will select more inputs to build a transaction when fees are cheap to consolidate the utxo set. Likewise, if the current environment is a low fee_rate environment, BnB will try to find a solution with the least number of UTXOs possible.

@yancyribbens yancyribbens Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is why hard_coding the long_term_fee_rate to 1.0 is bad, because it will direct the selection algorithms to think that we are always in a high fee environment, since it's fairly likely that fee_rate is greater than 1 sat/vB.

@yancyribbens yancyribbens Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Also, by adding this as a parameter, its possible you could do a curl or wget request from some external API to find this value.

@rustaceanrob rustaceanrob Jul 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ok much clearer now. Would you be able to condense that a bit and add that logic in the doc comment?

@yancyribbens yancyribbens Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, done, with the caveat that this is how the bitcoin core selection works, as well as rust bitcoin coin-selection. However, I'm reasonably sure that's how bdk coin-select works as well. Although, as stated before, I'm not really as well versed on that project.

},
}

Expand Down Expand Up @@ -250,11 +262,18 @@ fn main() {
address,
amount_sat,
fee_rate_sat_per_vb,
consolidate_fee_rate_sat_per_vb,
} => {
let mut req = client.send_to_address_request();
req.get().set_address(&address);
req.get().set_amount_sat(amount_sat);
req.get().set_fee_rate_sat_per_vb(fee_rate_sat_per_vb);

if let Some(consolidate_fee_rate) = consolidate_fee_rate_sat_per_vb {
req.get()
.set_consolidate_fee_rate_sat_per_vb(consolidate_fee_rate);
}

let result = req.send().promise.await.unwrap();
let r = result.get().unwrap();
let message = r.get_message().unwrap().to_string().unwrap();
Expand Down
17 changes: 15 additions & 2 deletions src/ipc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,22 @@ impl wallet_capnp::wallet::Server for WalletIpcInterface {
}
// 250 sat/kwu equals 1 sat/vB, rounded up so the rate is never below what was asked
let fee_rate = FeeRate::from_sat_per_kwu((fee_rate_sat_per_vb * 250.0).ceil() as u64);

let long_term_fee_rate_sat_per_vb = p.get_consolidate_fee_rate_sat_per_vb();
if !long_term_fee_rate_sat_per_vb.is_finite() || long_term_fee_rate_sat_per_vb < 0.0 {
return Err(capnp::Error::failed(
"consolidate fee rate must be a non-negative number".to_string(),
));
}

// 250 sat/kwu equals 1 sat/vB, rounded up so the rate is never below what was asked
let long_term_fee_rate =
FeeRate::from_sat_per_kwu((long_term_fee_rate_sat_per_vb * 250.0).ceil() as u64);

let mut wallet = self.state.lock().unwrap();
let build = Recipient::parse(&address, wallet.network)
.and_then(|recipient| wallet.build_transaction(recipient, amount, fee_rate));
let build = Recipient::parse(&address, wallet.network).and_then(|recipient| {
wallet.build_transaction(recipient, amount, fee_rate, long_term_fee_rate)
});
let tx = match build {
Ok(tx) => tx,
Err(e) if e.is_user_error() => {
Expand Down
Loading