Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 25 additions & 9 deletions rust/crates/cove-types/src/fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,16 +242,10 @@ impl FeeRateOptionsWithTotalFee {
/// Used when we have fee rates but haven't built PSBTs to calculate actual fees yet
#[must_use]
pub fn without_totals(base: FeeRateOptions) -> Self {
let from = |opt: FeeRateOption| FeeRateOptionWithTotalFee {
fee_speed: opt.fee_speed,
fee_rate: opt.fee_rate,
total_fee: None,
};

Self {
fast: from(base.fast),
medium: from(base.medium),
slow: from(base.slow),
fast: FeeRateOptionWithTotalFee::without_total(base.fast),
medium: FeeRateOptionWithTotalFee::without_total(base.medium),
slow: FeeRateOptionWithTotalFee::without_total(base.slow),
custom: None,
}
}
Expand All @@ -265,6 +259,12 @@ pub struct FeeRateOptionWithTotalFee {
}

impl FeeRateOptionWithTotalFee {
#[must_use]
pub fn without_total(option: FeeRateOption) -> Self {
Self { fee_speed: option.fee_speed, fee_rate: option.fee_rate, total_fee: None }
}

#[must_use]
pub fn new(option: FeeRateOption, total_fee: impl Into<Amount>) -> Self {
Self {
fee_speed: option.fee_speed,
Expand Down Expand Up @@ -520,6 +520,22 @@ mod tests {
assert_eq!(option_with_total.sats_per_vbyte_string(), "3.20 sats/vbyte");
}

#[test]
fn fee_rate_options_without_totals_use_no_total_constructor() {
let base = FeeRateOptions {
fast: FeeRateOption::new(FeeSpeed::Fast, 3.2),
medium: FeeRateOption::new(FeeSpeed::Medium, 2.1),
slow: FeeRateOption::new(FeeSpeed::Slow, 1.0),
};

let options = FeeRateOptionsWithTotalFee::without_totals(base);

assert_eq!(options.fast, FeeRateOptionWithTotalFee::without_total(base.fast));
assert_eq!(options.medium, FeeRateOptionWithTotalFee::without_total(base.medium));
assert_eq!(options.slow, FeeRateOptionWithTotalFee::without_total(base.slow));
assert_eq!(options.custom, None);
}

#[test]
fn grouped_integer_formatting_matches_platform_number_format() {
assert_eq!(0_u32.thousands_int(), "0");
Expand Down
252 changes: 211 additions & 41 deletions rust/src/manager/send_flow_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,46 +270,92 @@ impl RustSendFlowManager {
return false;
}

let (spendable_balance, unavailable_balance_alert, is_max_selected) = {
let (
spendable_balance,
unavailable_balance_alert,
is_max_selected,
total_fee_sats,
fee_consumes_coin_control_balance,
) = {
let state = self.state.lock();

let spendable_balance =
validation::spendable_balance_limit(state.unlocked_spendable_sats, &state.mode);

let spendable_balance_for_validation =
validation::spendable_balance_for_validation(spendable_balance);

let total_fee_sats = match state.fee_selection.as_ref() {
Some(selection) => selection.selected.total_fee.map(|fee| fee.as_sats()),
None => Some(0),
};

let fee_consumes_coin_control_balance = state.mode.is_coin_control()
&& spendable_balance_for_validation > 0
&& total_fee_sats.is_some_and(|fee| fee >= spendable_balance_for_validation);

(
validation::spendable_balance_for_validation(state.unlocked_spendable_sats),
spendable_balance_for_validation,
validation::unavailable_spendable_balance_alert(
state.unlocked_spendable_sats,
state.lock_state_load_failed,
&state.mode,
),
state.max_selected.is_some(),
total_fee_sats,
fee_consumes_coin_control_balance,
)
};

if spendable_balance < amount {
if let Some(alert) = unavailable_balance_alert {
let msg = Message::SetAlert(alert);
match validation::spendable_balance_check(amount, total_fee_sats, spendable_balance) {
validation::SpendableBalanceCheck::WithinBalance => true,
validation::SpendableBalanceCheck::ExceedsBalance => {
// fee-only coin-control failures are handled by finalization so users get the fee-specific alert
if fee_consumes_coin_control_balance {
return true;
}

if let Some(alert) = unavailable_balance_alert {
let msg = Message::SetAlert(alert);

if display_alert {
sender.queue(msg);
} else {
debug!("validate_amount_failed: {msg:?}");
}

return false;
}

if is_max_selected {
let me = self.clone();
cove_tokio::task::spawn(async move { me.select_max_send_report_error().await });
return false;
}

let msg = Message::SetAlert(SendFlowError::InsufficientFunds.into());
if display_alert {
sender.queue(msg);
} else {
debug!("validate_amount_failed: {msg:?}");
}

return false;
false
}

if is_max_selected {
let me = self.clone();
cove_tokio::task::spawn(async move { me.select_max_send_report_error().await });
return false;
}
validation::SpendableBalanceCheck::MissingFeeTotal => {
self.schedule_fee_rate_update();

let msg = Message::SetAlert(SendFlowError::InsufficientFunds.into());
if display_alert {
sender.queue(msg);
} else {
debug!("validate_amount_failed: {msg:?}");
let msg = Message::SetAlert(validation::missing_fee_total_alert());
if display_alert {
sender.queue(msg);
} else {
debug!("validate_amount_failed: {msg:?}");
}

false
Comment on lines +346 to +356

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the conflicting paths without executing tests.
rg -n -C4 'fn validate_amount_allows_low_nonzero_amount|SpendableBalanceCheck::MissingFeeTotal|set_selected_fee_total' rust/src/manager/send_flow_manager.rs

Repository: bitcoinppl/cove

Length of output: 7473


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Read the validation logic and the specific test/fixture context.
sed -n '250,370p' rust/src/manager/send_flow_manager.rs
printf '\n--- TEST FIXTURE ---\n'
sed -n '760,920p' rust/src/manager/send_flow_manager.rs
printf '\n--- RELATED TESTS ---\n'
sed -n '920,980p' rust/src/manager/send_flow_manager.rs

Repository: bitcoinppl/cove

Length of output: 12826


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the validation helpers that classify missing fee totals.
rg -n -C4 'spendable_balance_check|missing_fee_total_alert|SpendableBalanceCheck::MissingFeeTotal|spendable_balance_for_validation|total_fee_sats' rust/src/manager -g '*.rs'

Repository: bitcoinppl/cove

Length of output: 21357


Keep the no-fee case out of MissingFeeTotal validate_amount_internal now fails the default manager_for_validation() fixture used by validate_amount_allows_low_nonzero_amount, because it never sets fee_selection. Seed a fee selection there, or split “no fee selected yet” from “selected fee missing total”.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rust/src/manager/send_flow_manager.rs` around lines 346 - 356, The
MissingFeeTotal branch in send_flow_manager::validate_amount_internal is now
conflating “no fee selected” with “fee selected but missing total,” which breaks
the manager_for_validation fixture used by
validate_amount_allows_low_nonzero_amount. Update the validation flow so the
default fixture seeds fee_selection, or introduce a separate state/path for the
no-fee case and keep it out of MissingFeeTotal. Use the validate_amount_internal
match arm and manager_for_validation setup to locate the change.

Source: Path instructions

}
return false;
}

true
}

fn pending_send_warning(&self) -> Option<SendFlowAlertState> {
Expand Down Expand Up @@ -696,12 +742,12 @@ impl RustSendFlowManager {

#[cfg(test)]
mod tests {
use std::{sync::Arc, time::Duration};
use std::sync::Arc;

use cove_types::fees::{
FeeRateOption, FeeRateOptionWithTotalFee, FeeRateOptionsWithTotalFee, FeeSpeed,
use cove_types::{
fees::{FeeRateOption, FeeRateOptionWithTotalFee, FeeRateOptionsWithTotalFee, FeeSpeed},
utxo::{UtxoList, ffi_preview::preview_new_utxo_list},
};
use cove_types::utxo::{UtxoList, ffi_preview::preview_new_utxo_list};

use crate::{
manager::{deferred_sender::SingleOrMany, wallet_manager::RustWalletManager},
Expand Down Expand Up @@ -758,11 +804,8 @@ mod tests {

fn set_selected_fee_without_total(manager: &super::RustSendFlowManager) {
let base_options = super::FeeRateOptions::_ffi_preview_new();
let selected = FeeRateOptionWithTotalFee {
fee_speed: FeeSpeed::Custom { duration_mins: 10 },
fee_rate: FeeRateOption::new(FeeSpeed::Custom { duration_mins: 10 }, 1.0).fee_rate,
total_fee: None,
};
let fee_option = FeeRateOption::new(FeeSpeed::Custom { duration_mins: 10 }, 1.0);
let selected = FeeRateOptionWithTotalFee::without_total(fee_option);
let options = FeeRateOptionsWithTotalFee::without_totals(base_options);

let mut state = manager.state.lock();
Expand Down Expand Up @@ -1228,35 +1271,32 @@ mod tests {
}

#[test]
fn finalize_refreshes_missing_total_fee_before_warning_detection() {
fn finalize_blocks_when_selected_fee_total_is_missing() {
let manager = manager_for_validation();
set_valid_amount_and_address(&manager, 1_000);
set_selected_fee_without_total(&manager);

manager.finalize_and_go_to_next_screen();

assert!(manager.reconciler.receiver().try_recv().is_err());
assert!(matches!(
next_reconcile_message(&manager),
super::Message::SetAlert(super::SendFlowAlertState::General { title, .. })
if title == "Fee Estimate Still Loading"
));
}

#[test]
fn finalize_reports_dust_when_missing_total_fee_refresh_cannot_build_fee_totals() {
fn finalize_reports_missing_fee_before_dust_when_fee_total_is_unavailable() {
let manager = manager_for_validation();
set_valid_amount_and_address(&manager, 1);
set_selected_fee_without_total(&manager);

manager.finalize_and_go_to_next_screen();

let message = manager
.reconciler
.receiver()
.recv_timeout(Duration::from_secs(2))
.expect("dust alert is reconciled");

assert!(matches!(
message,
SingleOrMany::Single(super::Message::SetAlert(super::SendFlowAlertState::Error(
super::SendFlowError::SendBelowDustLimit
)))
next_reconcile_message(&manager),
super::Message::SetAlert(super::SendFlowAlertState::General { title, .. })
if title == "Fee Estimate Still Loading"
));
}

Expand All @@ -1282,4 +1322,134 @@ mod tests {
super::SendFlowAlertState::General { title, .. } if title.contains("Locked")
));
}

#[test]
fn validate_amount_blocks_send_when_amount_plus_fee_exceeds_balance() {
let _guard = crate::test_support::global_state_test_lock().blocking_lock();
let manager = manager_for_validation();
{
let mut state = manager.state.lock();
state.amount_sats = Some(5_000);
state.unlocked_spendable_sats = Some(5_001);
}
set_selected_fee_total(&manager, 156);

assert!(!manager.validate_amount(true));

let super::Message::SetAlert(alert) = next_reconcile_message(&manager) else {
panic!("expected a single insufficient funds alert");
};

assert!(matches!(
alert,
super::SendFlowAlertState::Error(super::SendFlowError::InsufficientFunds)
));
}

#[test]
fn validate_amount_blocks_send_when_selected_fee_total_is_missing() {
let _guard = crate::test_support::global_state_test_lock().blocking_lock();
let manager = manager_for_validation();
{
let mut state = manager.state.lock();
state.amount_sats = Some(5_000);
state.unlocked_spendable_sats = Some(5_001);
state.address = Some(Arc::new(Address::preview_new()));
}
set_selected_fee_without_total(&manager);

assert!(!manager.validate_amount(true));

let super::Message::SetAlert(alert) = next_reconcile_message(&manager) else {
panic!("expected a single missing fee alert");
};

assert!(matches!(
alert,
super::SendFlowAlertState::General { title, .. }
if title == "Fee Estimate Still Loading"
));
}

#[test]
fn validate_amount_blocks_coin_control_send_when_amount_plus_fee_exceeds_selected_total() {
let _guard = crate::test_support::global_state_test_lock().blocking_lock();
let manager = manager_for_validation();
set_coin_control_mode_with_total(&manager, 10_000);
{
let mut state = manager.state.lock();
state.amount_sats = Some(9_900);
state.unlocked_spendable_sats = Some(50_000);
}
set_selected_fee_total(&manager, 156);

assert!(!manager.validate_amount(true));

let super::Message::SetAlert(alert) = next_reconcile_message(&manager) else {
panic!("expected a single insufficient funds alert");
};

assert!(matches!(
alert,
super::SendFlowAlertState::Error(super::SendFlowError::InsufficientFunds)
));
}

#[test]
fn validate_amount_allows_total_equal_to_spendable_balance() {
let _guard = crate::test_support::global_state_test_lock().blocking_lock();
let manager = manager_for_validation();
{
let mut state = manager.state.lock();
state.amount_sats = Some(5_000);
state.unlocked_spendable_sats = Some(5_156);
}
set_selected_fee_total(&manager, 156);

assert!(manager.validate_amount(true));
assert!(manager.reconciler.receiver().try_recv().is_err());
}

#[test]
fn amount_exceeds_balance_includes_selected_fee_total() {
let _guard = crate::test_support::global_state_test_lock().blocking_lock();
let manager = manager_for_validation();
{
let mut state = manager.state.lock();
state.amount_sats = Some(5_000);
state.unlocked_spendable_sats = Some(5_001);
}
set_selected_fee_total(&manager, 156);

assert!(manager.amount_exceeds_balance());
}

#[test]
fn amount_exceeds_balance_uses_coin_control_selected_total() {
let _guard = crate::test_support::global_state_test_lock().blocking_lock();
let manager = manager_for_validation();
set_coin_control_mode_with_total(&manager, 10_000);
{
let mut state = manager.state.lock();
state.amount_sats = Some(9_900);
state.unlocked_spendable_sats = Some(50_000);
}
set_selected_fee_total(&manager, 156);

assert!(manager.amount_exceeds_balance());
}

#[test]
fn amount_exceeds_balance_treats_missing_fee_as_exceeding_at_spendable_limit() {
let _guard = crate::test_support::global_state_test_lock().blocking_lock();
let manager = manager_for_validation();
{
let mut state = manager.state.lock();
state.amount_sats = Some(5_000);
state.unlocked_spendable_sats = Some(5_000);
}
set_selected_fee_without_total(&manager);

assert!(manager.amount_exceeds_balance());
}
}
Loading
Loading