re: #25269 (review)
Update: think I finally figured this out. In the SFFO case the fee exceeds balance error can still happen but the checks here will not detect it. So I think the change I would suggest would just be dropping the m_subtract_fee_outputs code and writing a more descriptive comment:
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -1164,12 +1164,19 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal(
// Check if we have enough balance but cannot cover the fees
CAmount available_balance = preset_inputs.total_amount + available_coins.GetTotalAmount();
- // Note: Since SFFO reduces existing outputs to cover fees, recipients_sum should already be
- // strictly less than available_balance when enabled. Still, as a sanity-check, skip when SFFO is enabled.
- if (available_balance >= recipients_sum && !coin_selection_params.m_subtract_fee_outputs) {
+ if (available_balance >= recipients_sum) {
CAmount available_effective_balance = preset_inputs.total_amount + *available_coins.GetEffectiveTotalAmount();
if (available_effective_balance < selection_target) {
- return util::Error{_("The total transaction amount exceeds your balance when fees are included")};
+ // Note: this fees exceeds balance error will not currently
+ // trigger when SFFO is enabled because the effective balance
+ // will equal the original balance in this case. This is not
+ // ideal because it doesn't provide the most descriptive error
+ // message, but it's not worth making this code more complex by
+ // checking the sizes of outputs fees can be subtracted from
+ // here. Also, fees exceeding the balance should happen more
+ // rarely with SFFO given the ability to take fees from outputs.
+ Assume(!coin_selection_params.m_subtract_fee_outputs);
+ return util::Error{strprintf(_("The total exceeds your balance when the %s transaction fee is included."), FormatMoney(selection_target - recipients_sum))};
}
}
