From c00040c15202542d1fc6bbfea91e71e1918d4a6d Mon Sep 17 00:00:00 2001 From: johnnylawdgb Date: Sun, 9 Aug 2026 17:03:12 +0000 Subject: [PATCH] wallet: price DD fee inputs by effective value DigiDollar redemptions failed with "Insufficient fee inputs for DD redemption fee" on wallets whose DGB was fragmented into small UTXOs, even when the wallet held plenty of spendable DGB. DigiDollarWallet::SelectFeeCoins() treated a fee UTXO's raw value as its contribution to the fee. It is not. Every input added to a DigiDollar transaction enlarges the transaction the fee is computed from: at the DD fee rate (35,000,000 sat/kvB) and with the per-input accounting in DigiDollar::EstimateTransactionVSize() (41 base bytes plus a flat 110 witness bytes, +35% margin, i.e. ~92 vB), one extra input costs about 3,220,000 sat to spend. Any fee UTXO below ~0.0322 DGB therefore has negative effective value, and SelectFeeCoins() sorted smallest-first and stopped as soon as the raw sum reached the target, so it selected precisely those. The caller compounded it: redeemdigidollar asked for a fixed (400 vB * feeRate) + 50% estimate that was never revisited, while BuildRedemptionTransaction() recomputed the true fee from the transaction it had built and failed when the selection came up short, with no re-selection. SelectFeeCoins() now prices every candidate by effective value (value - EstimateInputSpendCost(fee_rate)), skips UTXOs that cannot pay for their own spending, and requires the target to be met in effective value rather than raw value; selected_total keeps its raw meaning. This applies to all callers, because a selection whose raw sum meets the target but whose effective sum does not is wrong on every path. The fee rate is a parameter so the cost tracks what the transaction will pay. Input ordering stays opt-in. A new minimize_inputs flag, off by default, keeps the smallest-first order DD transfers have always used; only the redemption path sets it, because it must converge on an exact fee with as few inputs as possible, the same reasoning as TxBuilder::SelectCoins. Note that transfers on a fragmented wallet change behaviour: selection that used to come up short now succeeds, using several small inputs and paying a correspondingly larger fee. A new DigiDollarWallet::SelectRedemptionFeeCoins() applies the pattern the DD transfer path already uses in PreflightDDTransferCapacity(): it builds a projected transaction from the actually-selected inputs, derives the fee from EstimateTransactionVSize(), and re-selects against the updated fee, bounded to six rounds. It excludes the collateral outpoint and the DD UTXOs being burned, preserves the MIN_DD_TX_FEE floor and the MAX_STANDARD_TX_WEIGHT guard, and reports its outcome as a category so the RPC can distinguish a funding shortfall from a transaction that cannot be built. Both redemption call sites use it in place of the fixed size guess, and failures now name the numbers and tell the user to consolidate small DGB UTXOs. MIN_DD_FEE_RATE and MIN_DD_TX_FEE move to txbuilder.h and replace the copies scattered across the wallet and RPC. Mint is unaffected: it funds itself through TxBuilder::SelectCoins. No consensus code is touched. Covered by unit tests for fee-input selection and redemption fee convergence, and by a new functional test, digidollar_redeem_fragmented_fees.py, which reproduces the original user-facing failure on regtest: a wallet fragmented into 0.0525 DGB UTXOs could not redeem a matured vault, and now can. --- REPO_MAP_DIGIDOLLAR.md | 6 +- src/digidollar/txbuilder.cpp | 38 +- src/digidollar/txbuilder.h | 32 ++ src/rpc/digidollar.cpp | 53 +-- src/test/digidollar_wallet_tests.cpp | 384 ++++++++++++++++++ src/wallet/digidollarwallet.cpp | 244 ++++++++--- src/wallet/digidollarwallet.h | 62 ++- .../digidollar_redeem_fragmented_fees.py | 115 ++++++ test/functional/test_runner.py | 1 + 9 files changed, 852 insertions(+), 83 deletions(-) create mode 100755 test/functional/digidollar_redeem_fragmented_fees.py diff --git a/REPO_MAP_DIGIDOLLAR.md b/REPO_MAP_DIGIDOLLAR.md index ed9fddd7c4..20f4f6daee 100644 --- a/REPO_MAP_DIGIDOLLAR.md +++ b/REPO_MAP_DIGIDOLLAR.md @@ -99,6 +99,8 @@ This is the granular file index for all DigiDollar and Oracle source code. Read - Global `g_scriptMetadataMap` protected by `RecursiveMutex`, capped at 10,000 entries ### src/digidollar/txbuilder.h +- `DigiDollar::MIN_DD_FEE_RATE` = 35,000,000 sat/kvB (0.35 DGB/kvB) → the DigiDollar fee rate; shared by mint/transfer/redeem RPCs and wallet coin selection +- `DigiDollar::MIN_DD_TX_FEE` = 10,000,000 sat (0.1 DGB) → absolute fee floor for any DigiDollar transaction - `DigiDollar::TxBuilderResult` (struct) → result of tx building: success, CMutableTransaction, error string, totalFees, collateralRequired, ddChange - `DigiDollar::TxBuilderMintParams` (struct) → ddAmount, lockDays, lockTier (0-9), ownerKey, feeRate, utxos, optional dgbChangeDest - `DigiDollar::TxBuilderTransferParams` (struct) → recipients vector (address, amount), feeRate, ddUtxos, ddAmounts, feeUtxos, feeAmounts, spenderKey, optional dgbChangeDest @@ -127,6 +129,7 @@ This is the granular file index for all DigiDollar and Oracle source code. Read - `CreateRedemptionScript(path, owner)` → creates Schnorr-signed redemption script - `DigiDollar::EncodeDigiDollarAddress(dest, chainParams)` → converts CTxDestination to DD address string via CDigiDollarAddress - `DigiDollar::EstimateTransactionVSize(tx)` → estimates vsize with 110-byte witness per input + 35% safety margin +- `DigiDollar::EstimateInputSpendCost(feeRate)` → approximate fee cost of one extra input (92 vB marginal measured against EstimateTransactionVSize at zero inputs → 3,220,000 sat at MIN_DD_FEE_RATE); a fee UTXO worth less has negative effective value. Approximation only: the estimator's double truncation makes the true marginal alternate 92/93 vB, absorbed by the redemption re-projection loop. Returns 0 for a non-positive rate, MAX_MONEY on overflow ### src/digidollar/txbuilder.cpp - Full implementation of all TxBuilder classes (~1,425 lines) @@ -708,7 +711,8 @@ This is the granular file index for all DigiDollar and Oracle source code. Read - `ProcessIncomingTransaction(tx, txid)` → processes any incoming DD tx and adds to history - **Coin Selection:** - `SelectDDCoins(target, selected_utxos, selected_total, amounts)` → selects DD UTXOs for target amount - - `SelectFeeCoins(fee_amount, selected_utxos, selected_total, amounts, exclude)` → selects DGB UTXOs for fees + - `SelectFeeCoins(fee_amount, selected_utxos, selected_total, amounts, exclude, minimize_inputs=false, fee_rate=MIN_DD_FEE_RATE)` → selects DGB UTXOs for fees. Prices candidates by EFFECTIVE value (value − `EstimateInputSpendCost(fee_rate)`): UTXOs that cost at least as much to spend as they are worth are skipped, and `fee_amount` must be met by the sum of effective values, not the raw sum (`selected_total` is still the raw total). Smallest-first by default (spends small UTXOs down, at the cost of a larger fee); `minimize_inputs=true` sorts largest-first for the fewest inputs + - `SelectRedemptionFeeCoins(params, error, projected_fee)` → `DDFeeSelectionResult` {OK, INSUFFICIENT_FUNDS, INVALID_TRANSACTION}. Fee-input selection for redemptions: excludes the collateral outpoint and the DD UTXOs being burned, then converges (≤6 rounds) by projecting the redemption tx, deriving the real fee from `EstimateTransactionVSize`, and re-selecting against it; enforces MAX_STANDARD_TX_WEIGHT and the MIN_DD_TX_FEE floor. Fills `params.feeUtxos`/`feeAmounts`; `projected_fee` is an upper bound - `CalculateTransactionFee(tx)` → estimates fee for transaction - **Utility:** - `IsLockedByDD(outpoint)` → checks if outpoint is locked by DD (protects from UnlockAllCoins) diff --git a/src/digidollar/txbuilder.cpp b/src/digidollar/txbuilder.cpp index 40727d39b3..4fca52ddab 100644 --- a/src/digidollar/txbuilder.cpp +++ b/src/digidollar/txbuilder.cpp @@ -34,7 +34,6 @@ static const size_t ESTIMATED_TX_VSIZE = 500; // Estimated transaction size static const int DEFAULT_SYSTEM_COLLATERAL = 150; // Default system health (150%) static const double MAX_FEE_RATIO = 0.5; // Maximum fee as ratio of total input static const size_t MAX_TX_INPUTS = 400; // Maximum inputs per transaction to stay under MAX_STANDARD_TX_WEIGHT -static const CAmount MIN_DD_TX_FEE = 10000000; // 0.1 DGB minimum DD transaction fee CAmount ApplyCollateralSafetyMargin(CAmount requiredCollateral) { @@ -1295,14 +1294,24 @@ TxBuilderResult RedeemTxBuilder::BuildRedemptionTransaction(const TxBuilderRedee LogPrintf("DigiDollar: Calculated fees: %d sats (fee inputs: %d sats)\n", result.totalFees, totalFeeIn); if (totalFeeIn <= 0) { - result.error = "Insufficient fee inputs for DD redemption fee"; + result.error = strprintf("Insufficient fee inputs for DD redemption fee: no DGB fee input was supplied " + "for a transaction that owes %lld sats. Fund the wallet with spendable DGB and retry.", + static_cast(result.totalFees)); LogPrintf("DigiDollar: BuildRedemptionTransaction FAILED - %s\n", result.error); return result; } CAmount feeChange = totalFeeIn - result.totalFees; if (feeChange < 0) { - result.error = "Insufficient fee inputs for DD redemption fee"; + // Every fee input costs a fee of its own to spend, so a pile of small + // UTXOs can total more than the fee and still not pay it. + result.error = strprintf("Insufficient fee inputs for DD redemption fee: %u fee input(s) totalling %lld sats " + "against a %lld sat fee (each extra fee input costs about %lld sats to spend). " + "Consolidate small DGB UTXOs into fewer, larger ones and retry.", + static_cast(params.feeUtxos.size()), + static_cast(totalFeeIn), + static_cast(result.totalFees), + static_cast(EstimateInputSpendCost(params.feeRate))); LogPrintf("DigiDollar: BuildRedemptionTransaction FAILED - %s\n", result.error); return result; } @@ -1490,4 +1499,27 @@ size_t EstimateTransactionVSize(const CMutableTransaction& tx) { return vsize + (vsize * 35 / 100); } +namespace { +//! Marginal vsize of one extra input, measured against EstimateTransactionVSize() +//! at zero inputs. The estimator truncates twice, so the true marginal alternates +//! between 92 and 93 vB with the size of the rest of the transaction; this is the +//! lower of the two and therefore an approximation, not a bound. See +//! EstimateInputSpendCost() in the header. +size_t EstimateInputVSize() { + CMutableTransaction probe; + const size_t without_input = EstimateTransactionVSize(probe); + probe.vin.emplace_back(); + const size_t with_input = EstimateTransactionVSize(probe); + return with_input - without_input; +} +} // namespace + +CAmount EstimateInputSpendCost(CAmount feeRate) { + if (feeRate <= 0) return 0; + const CAmount vsize = static_cast(EstimateInputVSize()); + // Fee rates reach this from RPC parameters, so guard the multiply. + if (feeRate > std::numeric_limits::max() / vsize) return MAX_MONEY; + return (vsize * feeRate) / 1000; +} + } // namespace DigiDollar diff --git a/src/digidollar/txbuilder.h b/src/digidollar/txbuilder.h index 646b59b22a..dd6899fb4c 100644 --- a/src/digidollar/txbuilder.h +++ b/src/digidollar/txbuilder.h @@ -21,6 +21,16 @@ namespace DigiDollar { +/** + * Minimum fee rate for DigiDollar transactions, in satoshis per kvB + * (0.35 DGB/kvB, i.e. 35,000 sat/vB). DigiByte expresses fee rates per + * kilo-vbyte, not per vbyte. + */ +static constexpr CAmount MIN_DD_FEE_RATE{35000000}; + +/** Absolute fee floor for any DigiDollar transaction (0.1 DGB). */ +static constexpr CAmount MIN_DD_TX_FEE{10000000}; + // Apply the wallet mint collateral safety margin used by MintTxBuilder. // The input and output are DGB satoshis. CAmount ApplyCollateralSafetyMargin(CAmount requiredCollateral); @@ -313,6 +323,28 @@ std::string EncodeDigiDollarAddress(const CTxDestination& dest, const CChainPara */ size_t EstimateTransactionVSize(const CMutableTransaction& tx); +/** + * Approximate fee that spending one additional input costs at the given fee rate. + * + * A fee UTXO worth less than this has negative effective value: adding it to a + * transaction reduces, rather than increases, the amount available to pay the + * fee. Coin selection for DigiDollar fee inputs prices inputs with this. + * + * The marginal input size is measured against EstimateTransactionVSize() at zero + * inputs and comes out at 92 vB (41 base bytes plus the flat 110-byte witness + * allowance, i.e. 68.5 vB, carrying the estimator's 35% margin). It is an + * approximation, not an exact per-input cost: EstimateTransactionVSize() + * truncates twice, so the true marginal alternates between 92 and 93 vB + * depending on the size of the rest of the transaction. Callers that must not + * come up short re-project the transaction and re-select (see + * DigiDollarWallet::SelectRedemptionFeeCoins), which absorbs the difference. + * + * @param feeRate Fee rate in satoshis per kvB + * @return Cost in satoshis of spending one extra input, 0 for a non-positive + * fee rate, MAX_MONEY if the fee rate is large enough to overflow + */ +CAmount EstimateInputSpendCost(CAmount feeRate); + } // namespace DigiDollar #endif // DIGIBYTE_DIGIDOLLAR_TXBUILDER_H diff --git a/src/rpc/digidollar.cpp b/src/rpc/digidollar.cpp index f9e843340f..db1f3b4d00 100644 --- a/src/rpc/digidollar.cpp +++ b/src/rpc/digidollar.cpp @@ -1308,9 +1308,8 @@ RPCHelpMan mintdigidollar() // MIN_DD_TX_FEE = 10,000,000 satoshis = 0.1 DGB // For a typical 300-byte tx, we need feeRate = 10,000,000 / 300 * 1000 = 33,333,333 sat/kB // We use 35,000,000 sat/kB to ensure minimum is always met - static const CAmount MIN_DD_FEE_RATE = 35000000; // 0.35 DGB/kB ensures min 0.1 DGB for typical tx CAmount feeRate = OptionalParamIsSet(request, 2) ? - std::max(request.params[2].getInt(), MIN_DD_FEE_RATE) : MIN_DD_FEE_RATE; + std::max(request.params[2].getInt(), DigiDollar::MIN_DD_FEE_RATE) : DigiDollar::MIN_DD_FEE_RATE; // Validate parameters if (ddAmount <= 0) { @@ -2307,8 +2306,7 @@ RPCHelpMan redeemdigidollar() redeemParams.path = errRedemptionActive ? DigiDollar::RedemptionPath::ERR : DigiDollar::RedemptionPath::NORMAL; redeemParams.ownerKey = ownerKey; // BUG #10 FIX: Use position owner key directly // DigiDollar transactions MUST pay at least 0.1 DGB fee to miners - static const CAmount MIN_DD_FEE_RATE = 35000000; // 0.35 DGB/kB ensures min 0.1 DGB for typical tx - redeemParams.feeRate = MIN_DD_FEE_RATE; + redeemParams.feeRate = DigiDollar::MIN_DD_FEE_RATE; // 0.35 DGB/kB ensures min 0.1 DGB for typical tx // Use the caller's requested DGB return address if supplied. If no // address is supplied, create a wallet destination so the returned @@ -2374,33 +2372,26 @@ RPCHelpMan redeemdigidollar() LogPrintf(" - DD Minted: %d cents\n", foundPosition.dd_minted); LogPrintf(" - Unlock Height: %d\n", foundPosition.unlock_height); - // Select fee UTXOs from wallet - // CRITICAL: Build exclude list to prevent selecting collateral or DD UTXOs as fee inputs - std::vector exclude_utxos; - exclude_utxos.push_back(redeemParams.collateralOutpoint); // Don't select collateral - exclude_utxos.insert(exclude_utxos.end(), redeemParams.ddUtxos.begin(), redeemParams.ddUtxos.end()); // Don't select DD UTXOs - - LogPrintf("DigiDollar: Building exclude list with %d UTXOs (1 collateral + %d DD)\n", - exclude_utxos.size(), redeemParams.ddUtxos.size()); - - // Bug #9 fix: Calculate fee from feeRate and estimated tx size instead of hardcoding. - // Redemption tx: ~3 inputs (collateral + DD + fee), ~2-3 outputs → ~400 vbytes. - // Apply 50% safety margin for script-path spending variance. - CAmount estimatedFee = (400 * redeemParams.feeRate) / 1000; // vsize * feeRate / 1000 - estimatedFee = estimatedFee + (estimatedFee / 2); // 50% safety margin - if (estimatedFee < 10000000) estimatedFee = 10000000; // Floor at 0.1 DGB - LogPrintf("DigiDollar: Estimated redemption fee: %lld sats (%.8f DGB)\n", - static_cast(estimatedFee), estimatedFee / 100000000.0); - CAmount selectedFeeTotal = 0; - std::vector feeAmounts; - - if (!dd_wallet->SelectFeeCoins(estimatedFee, redeemParams.feeUtxos, selectedFeeTotal, &feeAmounts, &exclude_utxos)) { - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient DGB balance for transaction fees"); - } - - redeemParams.feeAmounts = feeAmounts; - LogPrintf("DigiDollar: Selected %d sats in fees from %d UTXOs for redemption\n", - selectedFeeTotal, redeemParams.feeUtxos.size()); + // Select fee UTXOs from the wallet. A fixed size guess cannot work + // here: the fee a redemption owes depends on how many fee inputs it + // ends up carrying, and each input costs a fee of its own to spend. + // SelectRedemptionFeeCoins() re-projects the transaction after every + // selection round, and excludes the collateral outpoint and the DD + // UTXOs being burned from the candidate set. + std::string feeSelectionError; + CAmount projectedFee = 0; + switch (dd_wallet->SelectRedemptionFeeCoins(redeemParams, feeSelectionError, &projectedFee)) { + case DDFeeSelectionResult::OK: + break; + case DDFeeSelectionResult::INSUFFICIENT_FUNDS: + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, feeSelectionError); + case DDFeeSelectionResult::INVALID_TRANSACTION: + // Not a funding problem: the redemption cannot be built as asked. + throw JSONRPCError(RPC_WALLET_ERROR, feeSelectionError); + } + + LogPrintf("DigiDollar: Selected %zu fee UTXOs for redemption (projected fee at most %lld sats)\n", + redeemParams.feeUtxos.size(), static_cast(projectedFee)); DigiDollar::TxBuilderResult redeemResult = redeemBuilder.BuildRedemptionTransaction(redeemParams); diff --git a/src/test/digidollar_wallet_tests.cpp b/src/test/digidollar_wallet_tests.cpp index d4be81ea8a..33b2220d38 100644 --- a/src/test/digidollar_wallet_tests.cpp +++ b/src/test/digidollar_wallet_tests.cpp @@ -10,7 +10,10 @@ #include #include #include +#include #include +#include +#include #include #include