From 7d9c3a51db0d81b65363471ff93bfc2f7b469f91 Mon Sep 17 00:00:00 2001 From: DigiSwarm <13957390+JaredTate@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:02:06 -0600 Subject: [PATCH 01/22] script: DD opcodes are BIP342 OP_SUCCESSx until SCRIPT_VERIFY_DIGIDOLLAR active (RH-54) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this fix, IsOpSuccess() statically excluded 0xbb..0xbf from the OP_SUCCESSx range. This broke BIP342 forward-compatibility: old nodes that don't know SCRIPT_VERIFY_DIGIDOLLAR should see these bytes as unconditional successes, not execute them. Fix: restore the full BIP342 OP_SUCCESSx set in IsOpSuccess(). Add IsOpSuccessForFlags() in the interpreter that short-circuits only when SCRIPT_VERIFY_DIGIDOLLAR is NOT active — so pre-activation nodes keep OP_SUCCESSx semantics and post-activation nodes evaluate the opcodes. Also adds a Tapscript-only guard: DD opcodes return BAD_OPCODE in legacy/witness-v0 script contexts regardless of the activation flag. --- src/script/interpreter.cpp | 50 +++++++++++++++++++++++++++----------- src/script/script.cpp | 15 ++++-------- src/script/script.h | 12 ++++----- 3 files changed, 47 insertions(+), 30 deletions(-) diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp index 8424b5fb24..71d66eca50 100644 --- a/src/script/interpreter.cpp +++ b/src/script/interpreter.cpp @@ -436,6 +436,22 @@ static bool EvalChecksig(const valtype& sig, const valtype& pubkey, CScript::con // OP_CHECKPRICE to fail closed — no hardcoded fallback of any kind. GetOracleConsensusPriceFn g_get_oracle_consensus_price = nullptr; +static bool IsDigiDollarOpcode(opcodetype opcode) +{ + return opcode >= OP_DIGIDOLLAR && opcode <= OP_ORACLE; +} + +static bool IsOpSuccessForFlags(opcodetype opcode, unsigned int flags) +{ + // DigiDollar opcodes are BIP342 OP_SUCCESSx before activation, exactly as + // old Taproot nodes see them. Once SCRIPT_VERIFY_DIGIDOLLAR is active they + // are removed from OP_SUCCESSx and evaluated by the stricter new rules. + if (IsDigiDollarOpcode(opcode) && (flags & SCRIPT_VERIFY_DIGIDOLLAR)) { + return false; + } + return IsOpSuccess(opcode); +} + bool EvalScript(std::vector >& stack, const CScript& script, unsigned int flags, const BaseSignatureChecker& checker, SigVersion sigversion, ScriptExecutionData& execdata, ScriptError* serror) { static const CScriptNum bnZero(0); @@ -635,10 +651,14 @@ bool EvalScript(std::vector >& stack, const CScript& // DigiDollar specific opcodes case OP_DIGIDOLLAR: { - // CRITICAL FIX: Check flag BEFORE stack validation to avoid consensus split + if (sigversion != SigVersion::TAPSCRIPT) { + return set_error(serror, SCRIPT_ERR_BAD_OPCODE); + } + // Before activation, these bytes are still BIP342 OP_SUCCESSx. + // ExecuteWitnessScript normally short-circuits before EvalScript; + // keep direct EvalScript callers compatible too. if (!(flags & SCRIPT_VERIFY_DIGIDOLLAR)) { - // Behave as NOP when flag is not set (soft fork compatibility) - break; + return set_success(serror); } // CRITICAL FIX: Read DD amount from SCRIPT (next element), not from stack @@ -667,9 +687,11 @@ bool EvalScript(std::vector >& stack, const CScript& case OP_DDVERIFY: { + if (sigversion != SigVersion::TAPSCRIPT) { + return set_error(serror, SCRIPT_ERR_BAD_OPCODE); + } if (!(flags & SCRIPT_VERIFY_DIGIDOLLAR)) { - // Behave as NOP when flag is not set (soft fork compatibility) - break; + return set_success(serror); } // Verify DigiDollar conditions @@ -685,10 +707,11 @@ bool EvalScript(std::vector >& stack, const CScript& case OP_CHECKPRICE: { - // CRITICAL FIX: Check flag BEFORE stack validation to avoid consensus split + if (sigversion != SigVersion::TAPSCRIPT) { + return set_error(serror, SCRIPT_ERR_BAD_OPCODE); + } if (!(flags & SCRIPT_VERIFY_DIGIDOLLAR)) { - // Behave as NOP when flag is not set (soft fork compatibility) - break; + return set_success(serror); } // Stack: @@ -725,12 +748,11 @@ bool EvalScript(std::vector >& stack, const CScript& case OP_CHECKCOLLATERAL: { - // CRITICAL FIX: Check flag BEFORE stack validation AND do NOT pop stack in NOP mode - // Popping stack when flag not set causes CONSENSUS SPLIT! + if (sigversion != SigVersion::TAPSCRIPT) { + return set_error(serror, SCRIPT_ERR_BAD_OPCODE); + } if (!(flags & SCRIPT_VERIFY_DIGIDOLLAR)) { - // Behave as TRUE NOP when flag is not set (soft fork compatibility) - // DO NOT touch stack - that would cause consensus split! - break; + return set_success(serror); } // Stack: @@ -1963,7 +1985,7 @@ static bool ExecuteWitnessScript(const Span& stack_span, const CS return set_error(serror, SCRIPT_ERR_BAD_OPCODE); } // New opcodes will be listed here. May use a different sigversion to modify existing opcodes. - if (IsOpSuccess(opcode)) { + if (IsOpSuccessForFlags(opcode, flags)) { if (flags & SCRIPT_VERIFY_DISCOURAGE_OP_SUCCESS) { return set_error(serror, SCRIPT_ERR_DISCOURAGE_OP_SUCCESS); } diff --git a/src/script/script.cpp b/src/script/script.cpp index dca4fd9f10..b3b1cd93bc 100644 --- a/src/script/script.cpp +++ b/src/script/script.cpp @@ -149,6 +149,7 @@ std::string GetOpName(opcodetype opcode) case OP_DDVERIFY : return "OP_DDVERIFY"; case OP_CHECKPRICE : return "OP_CHECKPRICE"; case OP_CHECKCOLLATERAL : return "OP_CHECKCOLLATERAL"; + case OP_ORACLE : return "OP_ORACLE"; case OP_INVALIDOPCODE : return "OP_INVALIDOPCODE"; @@ -341,16 +342,10 @@ bool GetScriptOp(CScriptBase::const_iterator& pc, CScriptBase::const_iterator en bool IsOpSuccess(const opcodetype& opcode) { - // CRITICAL: Exclude DigiDollar opcodes (0xbb-0xbf) from OP_SUCCESSx range - // These opcodes are used for DigiDollar redemption scripts and must be executed. - // OP_ORACLE (0xbf) is a marker opcode used in coinbase OP_RETURN outputs; if it - // were OP_SUCCESS in Tapscript, any leaf containing it would be unconditionally - // spendable. See rh54_op_oracle_opsuccess_tests for PoC. - if (opcode >= OP_DIGIDOLLAR && opcode <= OP_ORACLE) { - return false; // 0xbb OP_DIGIDOLLAR, 0xbc OP_DDVERIFY, 0xbd OP_CHECKPRICE, - // 0xbe OP_CHECKCOLLATERAL, 0xbf OP_ORACLE - } - + // BIP342 raw OP_SUCCESSx set. DigiDollar uses 0xbb..0xbf, which are in + // this range, as future-upgrade Tapscript opcodes. They remain OP_SUCCESSx + // until SCRIPT_VERIFY_DIGIDOLLAR is active; the interpreter applies that + // activation flag when deciding whether to short-circuit or execute them. return opcode == 80 || opcode == 98 || (opcode >= 126 && opcode <= 129) || (opcode >= 131 && opcode <= 134) || (opcode >= 137 && opcode <= 138) || (opcode >= 141 && opcode <= 142) || (opcode >= 149 && opcode <= 153) || diff --git a/src/script/script.h b/src/script/script.h index 48e92a9386..e45dea83c7 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -206,12 +206,12 @@ enum opcodetype // Opcode added by BIP 342 (Tapscript) OP_CHECKSIGADD = 0xba, - // DigiDollar specific opcodes (using OP_NOP slots for soft fork) - OP_DIGIDOLLAR = 0xbb, // OP_NOP11 - Marks DD outputs - OP_DDVERIFY = 0xbc, // OP_NOP12 - Verify DD conditions - OP_CHECKPRICE = 0xbd, // OP_NOP13 - Check oracle price - OP_CHECKCOLLATERAL = 0xbe, // OP_NOP14 - Verify collateral ratio - OP_ORACLE = 0xbf, // OP_NOP15 - Oracle price data marker + // DigiDollar specific opcodes (using Tapscript OP_SUCCESSx slots for soft fork) + OP_DIGIDOLLAR = 0xbb, // Marks DD outputs / payloads + OP_DDVERIFY = 0xbc, // Verify DD conditions + OP_CHECKPRICE = 0xbd, // Check oracle price + OP_CHECKCOLLATERAL = 0xbe, // Verify collateral ratio + OP_ORACLE = 0xbf, // Oracle price data marker OP_INVALIDOPCODE = 0xff, }; From 424645e40cff2471bfde41246922c34b8d3b7fea Mon Sep 17 00:00:00 2001 From: DigiSwarm <13957390+JaredTate@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:02:16 -0600 Subject: [PATCH 02/22] digidollar: canonical P2TR check, remove skipOracleValidation bypass (RH-113, RH-114, RH-115) Three related fixes to validation.cpp: RH-114: IdentifyScriptType() and all P2TR output detection sites used a fragile 34-byte + OP_1 heuristic. A 34-byte script starting with OP_1 but missing the required OP_PUSHBYTES_32 opcode (an impostor) was accepted as a DD output. Replaced with IsCanonicalP2TROutput() which calls IsWitnessProgram() for the proper BIP341 check. RH-115: Oracle price validation and collateral ratio checks were wrapped in `if (!ctx.skipOracleValidation)`. This let IBD/catch-up nodes accept mints that caught-up nodes reject, creating a potential chain split. Collateral and oracle checks are now unconditional. RH-113: FindDDOpReturn() returned the first legacy OP_DIGIDOLLAR marker it found and never looked for the modern "DD" pushdata format. A tx with both formats would use the legacy one. Now the modern format wins; legacy is only returned if no modern marker is present. --- src/digidollar/validation.cpp | 322 +++++++++++++++++++--------------- 1 file changed, 184 insertions(+), 138 deletions(-) diff --git a/src/digidollar/validation.cpp b/src/digidollar/validation.cpp index 3398008fe6..fe2298e1f8 100644 --- a/src/digidollar/validation.cpp +++ b/src/digidollar/validation.cpp @@ -57,13 +57,22 @@ struct ValidationCache { // Global validation cache instance static ValidationCache g_validationCache; +static bool IsCanonicalP2TROutput(const CScript& script) +{ + int witnessVersion = -1; + std::vector witnessProgram; + return script.IsWitnessProgram(witnessVersion, witnessProgram) && + witnessVersion == 1 && + witnessProgram.size() == WITNESS_V1_TAPROOT_SIZE; +} + // ============================================================================ // Script Analysis Functions // ============================================================================ ScriptType IdentifyScriptType(const CScript& script) { // Quick rejection for obviously non-P2TR scripts - if (script.size() != 34 || script[0] != OP_1) { + if (!IsCanonicalP2TROutput(script)) { return ScriptType::NOT_DIGIDOLLAR; } @@ -167,6 +176,8 @@ bool ExtractDDAmount(const CScript& script, CAmount& amount) { } int FindDDOpReturn(const CTransaction& tx) { + int legacyIdx = -1; + for (size_t i = 0; i < tx.vout.size(); i++) { const CScript& script = tx.vout[i].scriptPubKey; if (script.size() < 2) continue; @@ -174,7 +185,10 @@ int FindDDOpReturn(const CTransaction& tx) { // Format 1: OP_RETURN OP_DIGIDOLLAR ... if (script.size() >= 2 && script[1] == OP_DIGIDOLLAR) { - return static_cast(i); + if (legacyIdx < 0) { + legacyIdx = static_cast(i); + } + continue; } // Format 2: OP_RETURN ... @@ -190,7 +204,8 @@ int FindDDOpReturn(const CTransaction& tx) { return static_cast(i); } } - return -1; + + return legacyIdx; } /** @@ -326,8 +341,8 @@ static bool ExtractDDAmountFromTxRef(const CTransactionRef& prev_tx, const COutP if (txout.scriptPubKey.size() > 0 && txout.scriptPubKey[0] == OP_RETURN) continue; if (txout.nValue != 0) continue; - // Check if it's a P2TR output (OP_1 + 32 bytes = DD output) - if (txout.scriptPubKey.size() == 34 && txout.scriptPubKey[0] == OP_1) { + // Check if it's a canonical P2TR output (OP_1 OP_PUSHBYTES_32 ) + if (IsCanonicalP2TROutput(txout.scriptPubKey)) { if (n == prevout.n) { // Found the matching output if (dd_output_idx < dd_amounts.size()) { @@ -410,7 +425,7 @@ bool ExtractMintAccountingAmounts(const CTransaction& tx, continue; } - if (out.scriptPubKey.size() == 34 && out.scriptPubKey[0] == OP_1) { + if (IsCanonicalP2TROutput(out.scriptPubKey)) { collateralOutputs++; collateralAmount = out.nValue; } @@ -750,8 +765,10 @@ bool ValidateMintTransaction(const CTransaction& tx, return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-mint-outputs"); } - // 2. Oracle price validation (skip for historical blocks) - if (!ctx.skipOracleValidation && ctx.oraclePriceMicroUSD <= 0) { + // 2. Oracle price validation. This is consensus-critical for every mint: + // local sync state must not allow IBD/catch-up nodes to accept mints that + // caught-up nodes reject for missing deterministic oracle data. + if (ctx.oraclePriceMicroUSD <= 0) { LogPrintf("DigiDollar: Invalid oracle price: %d\n", ctx.oraclePriceMicroUSD); return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-oracle-price"); } @@ -787,7 +804,7 @@ bool ValidateMintTransaction(const CTransaction& tx, // Phase 1 workaround: Identify outputs by structure since metadata doesn't cross nodes // P2TR outputs: collateral has value > 0, DD token has value = 0 - bool isP2TR = (output.scriptPubKey.size() == 34 && output.scriptPubKey[0] == OP_1); + bool isP2TR = IsCanonicalP2TROutput(output.scriptPubKey); bool isOpReturn = (output.scriptPubKey.size() > 0 && output.scriptPubKey[0] == OP_RETURN); // Check script type using metadata @@ -1144,42 +1161,42 @@ bool ValidateMintTransaction(const CTransaction& tx, return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-dd-mint-amount"); } - // 7. Calculate and verify collateral (skip for historical blocks - oracle price dependent) + // 7. Calculate and verify collateral. This remains mandatory even when + // skipOracleValidation is set during IBD/catch-up; otherwise block validity + // depends on a node-local sync flag. CAmount requiredCollateral = 0; - if (!ctx.skipOracleValidation) { - // SECURITY [T2-01]: Convert absolute lock HEIGHT to relative lock PERIOD for - // collateral ratio calculation. The OP_RETURN stores an absolute lockHeight - // (currentHeight + lockPeriod), but GetCollateralRatioForLockTime expects a - // relative lock period in blocks. Without this conversion, on mainnet (height ~22M) - // the absolute height exceeds ALL tier thresholds (max is 10yr = 21M blocks), - // causing every lock tier to use the 200% (10-year) ratio instead of its correct - // higher ratio. A 1-hour lock would require only 200% instead of 1000% collateral. - int64_t lockPeriod = lockTime - ctx.nHeight; - if (lockPeriod <= 0) { - LogPrintf("DigiDollar: Invalid lock period: lockTime=%lld, height=%d, period=%lld\n", - (long long)lockTime, ctx.nHeight, (long long)lockPeriod); - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-lock-period"); - } - - requiredCollateral = CalculateRequiredCollateral(totalDD, lockPeriod, ctx); - if (requiredCollateral <= 0) { - LogPrintf("DigiDollar: Failed to calculate required collateral\n"); - return state.Invalid(TxValidationResult::TX_CONSENSUS, "collateral-calculation-failed"); - } + // SECURITY [T2-01]: Convert absolute lock HEIGHT to relative lock PERIOD for + // collateral ratio calculation. The OP_RETURN stores an absolute lockHeight + // (currentHeight + lockPeriod), but GetCollateralRatioForLockTime expects a + // relative lock period in blocks. Without this conversion, on mainnet (height ~22M) + // the absolute height exceeds ALL tier thresholds (max is 10yr = 21M blocks), + // causing every lock tier to use the 200% (10-year) ratio instead of its correct + // higher ratio. A 1-hour lock would require only 200% instead of 1000% collateral. + int64_t lockPeriod = lockTime - ctx.nHeight; + if (lockPeriod <= 0) { + LogPrintf("DigiDollar: Invalid lock period: lockTime=%lld, height=%d, period=%lld\n", + (long long)lockTime, ctx.nHeight, (long long)lockPeriod); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-lock-period"); + } + + requiredCollateral = CalculateRequiredCollateral(totalDD, lockPeriod, ctx); + if (requiredCollateral <= 0) { + LogPrintf("DigiDollar: Failed to calculate required collateral\n"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "collateral-calculation-failed"); + } - // Verify sufficient collateral - if (totalCollateral < requiredCollateral) { - LogPrintf("DigiDollar: Insufficient collateral: provided %lld, required %lld (totalDD=%lld, lockPeriod=%lld)\n", - (long long)totalCollateral, (long long)requiredCollateral, - (long long)totalDD, (long long)lockPeriod); - return state.Invalid(TxValidationResult::TX_CONSENSUS, "insufficient-collateral"); - } + // Verify sufficient collateral + if (totalCollateral < requiredCollateral) { + LogPrintf("DigiDollar: Insufficient collateral: provided %lld, required %lld (totalDD=%lld, lockPeriod=%lld)\n", + (long long)totalCollateral, (long long)requiredCollateral, + (long long)totalDD, (long long)lockPeriod); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "insufficient-collateral"); + } - // 8. Additional validation checks - if (!ValidateCollateralRatio(totalCollateral, totalDD, lockPeriod, ctx)) { - LogPrintf("DigiDollar: Collateral ratio validation failed\n"); - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-collateral-ratio"); - } + // 8. Additional validation checks + if (!ValidateCollateralRatio(totalCollateral, totalDD, lockPeriod, ctx)) { + LogPrintf("DigiDollar: Collateral ratio validation failed\n"); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-collateral-ratio"); } // 9. Log successful validation @@ -1244,8 +1261,15 @@ bool ValidateTransferTransaction(const CTransaction& tx, if (!output.scriptPubKey.GetOp(pc, opcode, data)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "transfer-malformed-op-return"); } - CScriptNum txType(data, true); - if (txType.getint() != 2) { + int64_t txType = 0; + try { + CScriptNum txTypeNum(data, true); + txType = txTypeNum.GetInt64(); + } catch (const scriptnum_error&) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "transfer-malformed-op-return", + "Malformed transfer OP_RETURN transaction type"); + } + if (txType != static_cast(DD_TX_TRANSFER)) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "transfer-opreturn-type-mismatch", "Transfer OP_RETURN type must match transaction version"); } @@ -1253,9 +1277,14 @@ bool ValidateTransferTransaction(const CTransaction& tx, // Extract DD amounts while (output.scriptPubKey.GetOp(pc, opcode, data)) { if (data.size() > 0) { - // Allow up to 8 bytes for DD amounts (int64_t range) - CScriptNum amount(data, true, 8); - dd_amounts.push_back(amount.GetInt64()); + try { + // Allow up to 8 bytes for DD amounts (int64_t range) + CScriptNum amount(data, true, 8); + dd_amounts.push_back(amount.GetInt64()); + } catch (const scriptnum_error&) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "transfer-malformed-op-return", + "Malformed transfer OP_RETURN DD amount"); + } } } } @@ -1272,8 +1301,15 @@ bool ValidateTransferTransaction(const CTransaction& tx, if (output.scriptPubKey.size() > 0 && output.scriptPubKey[0] == OP_RETURN) continue; if (output.nValue != 0) continue; - // Check if it's a P2TR output (OP_1 + 32 bytes) - if (output.scriptPubKey.size() == 34 && output.scriptPubKey[0] == OP_1) { + // Reject 34-byte OP_1 impostors instead of treating them as DD outputs. + if (output.scriptPubKey.size() == 34 && output.scriptPubKey[0] == OP_1 && + !IsCanonicalP2TROutput(output.scriptPubKey)) { + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-dd-script", + "DD output must be canonical P2TR"); + } + + // Check if it's a canonical P2TR output (OP_1 OP_PUSHBYTES_32 ) + if (IsCanonicalP2TROutput(output.scriptPubKey)) { if (dd_amount_index >= dd_amounts.size()) { return state.Invalid(TxValidationResult::TX_CONSENSUS, "transfer-dd-output-amount-mismatch"); } @@ -1373,6 +1409,14 @@ bool ValidateTransferTransaction(const CTransaction& tx, if (found) { inputDD += ddAmt; ddInputCount++; + } else if (ctx.coins) { + Coin coin; + if (ctx.coins->GetCoin(txin.prevout, coin) && coin.out.nValue == 0) { + LogPrint(BCLog::DIGIDOLLAR, "DigiDollar: REJECT - Could not determine DD amount for zero-value input %s:%u\n", + txin.prevout.hash.ToString(), txin.prevout.n); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "dd-input-amounts-unknown", + "Cannot verify DD conservation: input DD amount undetermined"); + } } } @@ -1622,9 +1666,9 @@ bool ValidateRedemptionTransaction(const CTransaction& tx, if (output.scriptPubKey.size() > 0 && output.scriptPubKey[0] == OP_RETURN) { continue; } - // Check if it's a P2TR DD output (nValue=0, starts with OP_1) + // Check if it's a canonical P2TR DD output. // For DD outputs, use the amount from OP_RETURN metadata if available - if (output.scriptPubKey.size() > 1 && output.scriptPubKey[0] == OP_1) { + if (IsCanonicalP2TROutput(output.scriptPubKey)) { if (foundOpReturn && ddAmountFromOpReturn > 0) { // Use the authoritative amount from OP_RETURN totalDDOutputs += ddAmountFromOpReturn; @@ -1837,106 +1881,112 @@ bool ValidateCollateralReleaseAmount(const CTransaction& tx, return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-collateral-release-zero-collateral"); } - // Extract original DD amount — first try metadata registry, then creating tx lookup. + // Extract original DD amount from the creating mint transaction before trusting metadata. // // SECURITY [T1-08]: The collateral UTXO is a P2TR script (OP_1 + 32 bytes) which does // NOT contain the DD amount. During cross-node block validation, the ephemeral metadata // registry is empty. Without this fix, the function silently allowed ANY release amount, // enabling an attacker to burn 1 cent of DD and steal all locked collateral. // - // Strategy: 1) metadata registry, 2) txindex, 3) block-db lookup, 4) REJECT + // SECURITY [DD-RH-106]: Metadata is mutable process-local state keyed only by script. + // A rejected mint with the same owner/lock script but a smaller DD amount can overwrite + // that metadata. Prefer the authoritative creating mint transaction whenever available. + // + // Strategy: 1) txindex, 2) block-db lookup, 3) metadata registry fallback, 4) REJECT. CAmount originalDDMinted = 0; - if (!ExtractDDAmount(collateralCoin.out.scriptPubKey, originalDDMinted) || originalDDMinted <= 0) { - LogPrint(BCLog::DIGIDOLLAR, "DigiDollar: Could not extract original DD amount from collateral script, " - "trying creating transaction lookup...\n"); - - // Helper: extract DD minted amount from a mint transaction's OP_RETURN - auto extractDDFromMintTx = [](const CTransactionRef& prev_tx, CAmount& ddOut) -> bool { - // SECURITY [T5-04]: Verify source tx is actually a DD transaction. - // Without this check, a regular (non-DD) tx with a crafted DD OP_RETURN - // could be treated as a legitimate mint, allowing an attacker to set - // originalDDMinted to an arbitrary (e.g. trivially small) value. - // This is consistent with ExtractDDAmountFromTxRef() which also checks - // HasDigiDollarMarker, and isCollateralOutput (T2-06b) which checks nVersion. - if (!DigiDollar::HasDigiDollarMarker(*prev_tx)) { - return false; - } - // Also verify it's a MINT transaction (type byte = 1 in upper nVersion bits) - if (DigiDollar::GetDigiDollarTxType(*prev_tx) != DD_TX_MINT) { - return false; - } - for (const auto& vout : prev_tx->vout) { - if (vout.scriptPubKey.size() == 0 || vout.scriptPubKey[0] != OP_RETURN) continue; + bool found = false; + + // Helper: extract DD minted amount from a mint transaction's OP_RETURN. + auto extractDDFromMintTx = [](const CTransactionRef& prev_tx, CAmount& ddOut) -> bool { + // SECURITY [T5-04]: Verify source tx is actually a DD transaction. + // Without this check, a regular (non-DD) tx with a crafted DD OP_RETURN + // could be treated as a legitimate mint, allowing an attacker to set + // originalDDMinted to an arbitrary (e.g. trivially small) value. + // This is consistent with ExtractDDAmountFromTxRef() which also checks + // HasDigiDollarMarker, and isCollateralOutput (T2-06b) which checks nVersion. + if (!DigiDollar::HasDigiDollarMarker(*prev_tx)) { + return false; + } + // Also verify it's a MINT transaction (type byte = 1 in upper nVersion bits) + if (DigiDollar::GetDigiDollarTxType(*prev_tx) != DD_TX_MINT) { + return false; + } + for (const auto& vout : prev_tx->vout) { + if (vout.scriptPubKey.empty() || vout.scriptPubKey[0] != OP_RETURN) continue; - CScript::const_iterator pc = vout.scriptPubKey.begin(); - opcodetype opcode; - std::vector data; + CScript::const_iterator pc = vout.scriptPubKey.begin(); + opcodetype opcode; + std::vector data; - if (!vout.scriptPubKey.GetOp(pc, opcode)) continue; // Skip OP_RETURN - if (!vout.scriptPubKey.GetOp(pc, opcode, data)) continue; - if (data.size() != 2 || data[0] != 'D' || data[1] != 'D') continue; + if (!vout.scriptPubKey.GetOp(pc, opcode)) continue; // Skip OP_RETURN + if (!vout.scriptPubKey.GetOp(pc, opcode, data)) continue; + if (data.size() != 2 || data[0] != 'D' || data[1] != 'D') continue; - // Read tx type - if (!vout.scriptPubKey.GetOp(pc, opcode, data)) continue; - int64_t txType = 0; - if (data.size() > 0) { - try { - CScriptNum txTypeNum(data, true); - txType = txTypeNum.GetInt64(); - } catch (const scriptnum_error&) { continue; } - } + // Read tx type + if (!vout.scriptPubKey.GetOp(pc, opcode, data)) continue; + int64_t txType = 0; + if (!data.empty()) { + try { + CScriptNum txTypeNum(data, true); + txType = txTypeNum.GetInt64(); + } catch (const scriptnum_error&) { continue; } + } - // Only process MINT (type 1) — that's the transaction that created collateral - if (txType != 1) continue; + // Only process MINT (type 1) — that's the transaction that created collateral + if (txType != 1) continue; - // Read DD amount (first push after type for mint) - if (vout.scriptPubKey.GetOp(pc, opcode, data) && data.size() > 0) { - try { - CScriptNum scriptNum(data, true, 8); - ddOut = scriptNum.GetInt64(); - return ddOut > 0; - } catch (const scriptnum_error&) {} - } + // Read DD amount (first push after type for mint) + if (vout.scriptPubKey.GetOp(pc, opcode, data) && !data.empty()) { + try { + CScriptNum scriptNum(data, true, 8); + ddOut = scriptNum.GetInt64(); + return ddOut > 0; + } catch (const scriptnum_error&) {} } - return false; - }; - - bool found = false; - - // Try txindex (authoritative — reads creating tx from indexed database) - if (!found && g_txindex) { - uint256 block_hash; - CTransactionRef prev_tx; - if (g_txindex->FindTx(tx.vin[0].prevout.hash, block_hash, prev_tx)) { - found = extractDDFromMintTx(prev_tx, originalDDMinted); - if (found) { - LogPrint(BCLog::DIGIDOLLAR, "DigiDollar: Extracted original DD minted (%lld) from txindex\n", - (long long)originalDDMinted); - } + } + return false; + }; + + // Try txindex (authoritative — reads creating tx from indexed database) + if (!found && g_txindex) { + uint256 block_hash; + CTransactionRef prev_tx; + if (g_txindex->FindTx(tx.vin[0].prevout.hash, block_hash, prev_tx)) { + found = extractDDFromMintTx(prev_tx, originalDDMinted); + if (found) { + LogPrint(BCLog::DIGIDOLLAR, "DigiDollar: Extracted original DD minted (%lld) from txindex\n", + (long long)originalDDMinted); } } + } - // Try block-db lookup (universal fallback — every full node has every block) - if (!found && ctx.txLookup) { - CTransactionRef prev_tx; - if (ctx.txLookup(tx.vin[0].prevout.hash, collateralCoin.nHeight, prev_tx)) { - found = extractDDFromMintTx(prev_tx, originalDDMinted); - if (found) { - LogPrint(BCLog::DIGIDOLLAR, "DigiDollar: Extracted original DD minted (%lld) from block db\n", - (long long)originalDDMinted); - } + // Try block-db lookup (universal fallback — every full node has every block) + if (!found && ctx.txLookup) { + CTransactionRef prev_tx; + if (ctx.txLookup(tx.vin[0].prevout.hash, collateralCoin.nHeight, prev_tx)) { + found = extractDDFromMintTx(prev_tx, originalDDMinted); + if (found) { + LogPrint(BCLog::DIGIDOLLAR, "DigiDollar: Extracted original DD minted (%lld) from block db\n", + (long long)originalDDMinted); } } + } - if (!found || originalDDMinted <= 0) { - // SECURITY: REJECT if we cannot determine original DD amount. - // A consensus rule must never be silently bypassed. - LogPrintf("DigiDollar: SECURITY [T1-08] - Cannot determine original DD minted amount " - "for collateral at %s:%d. Rejecting to prevent collateral theft.\n", - tx.vin[0].prevout.hash.ToString(), tx.vin[0].prevout.n); - return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-collateral-release-unknown-dd-amount", - "Cannot verify proportional collateral release: original DD amount undetermined"); - } + // Last resort for legacy/unit-test contexts that lack tx lookup. This is intentionally + // after authoritative sources because script metadata can be overwritten by failed mints + // that reuse the same collateral script. + if (!found && ExtractDDAmount(collateralCoin.out.scriptPubKey, originalDDMinted) && originalDDMinted > 0) { + found = true; + } + + if (!found || originalDDMinted <= 0) { + // SECURITY: REJECT if we cannot determine original DD amount. + // A consensus rule must never be silently bypassed. + LogPrintf("DigiDollar: SECURITY [T1-08] - Cannot determine original DD minted amount " + "for collateral at %s:%d. Rejecting to prevent collateral theft.\n", + tx.vin[0].prevout.hash.ToString(), tx.vin[0].prevout.n); + return state.Invalid(TxValidationResult::TX_CONSENSUS, "bad-collateral-release-unknown-dd-amount", + "Cannot verify proportional collateral release: original DD amount undetermined"); } // SECURITY [T2-03]: Require full DD burn for collateral release. @@ -2003,9 +2053,7 @@ bool ValidateCollateralReleaseAmount(const CTransaction& tx, if (outputIndex >= prev_tx->vout.size()) return false; const CTxOut& candidate = prev_tx->vout[outputIndex]; - if (candidate.nValue <= 0 || - candidate.scriptPubKey.size() != 34 || - candidate.scriptPubKey[0] != OP_1) { + if (candidate.nValue <= 0 || !IsCanonicalP2TROutput(candidate.scriptPubKey)) { return false; } @@ -2014,9 +2062,7 @@ bool ValidateCollateralReleaseAmount(const CTransaction& tx, int collateralCount = 0; for (uint32_t candidateIndex = 0; candidateIndex < prev_tx->vout.size(); ++candidateIndex) { const CTxOut& vout = prev_tx->vout[candidateIndex]; - if (vout.nValue > 0 && - vout.scriptPubKey.size() == 34 && - vout.scriptPubKey[0] == OP_1) { + if (vout.nValue > 0 && IsCanonicalP2TROutput(vout.scriptPubKey)) { collateralIndex = candidateIndex; collateralCount++; } From 1747ed40948fb4859ee6d830f4c74903c13e39c5 Mon Sep 17 00:00:00 2001 From: DigiSwarm <13957390+JaredTate@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:02:24 -0600 Subject: [PATCH 03/22] validation: move DD mempool check after input caching, re-validate on reorg (RH-121) Two fixes to src/validation.cpp: DD contextual validation in PreChecks() was running before CheckTxInputs() cached the input coins. This meant DD validation used CoinsTip() directly, missing unconfirmed parents and allowing stale txindex data to resolve DD amounts across reorgs. Moved DD validation to after CheckTxInputs() so it uses the mempool-aware view with all inputs already fetched. MaybeUpdateMempoolForReorg() now re-validates DD transactions when resurrecting them after a reorg. A DD tx valid on the old chain may have depended on a collateral vault or oracle state that no longer exists on the new chain. Without re-validation, invalid DD txs could re-enter the mempool and cause downstream failures. Also moved the non-final tx check to before DD validation so non-final DD transactions are cheaply rejected without triggering oracle lookups. --- src/validation.cpp | 128 +++++++++++++++++++++++++++++---------------- 1 file changed, 84 insertions(+), 44 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index f62560ef8a..fc2c75c681 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -437,6 +437,44 @@ void Chainstate::MaybeUpdateMempoolForReorg( } } } + + if (DigiDollar::HasDigiDollarMarker(tx)) { + CCoinsViewMemPool view_mempool{&CoinsTip(), *m_mempool}; + CCoinsViewCache dd_view{&view_mempool}; + auto txLookup = [this](const uint256& txid, uint32_t coinHeight, CTransactionRef& tx_out) -> bool { + AssertLockHeld(cs_main); + const CBlockIndex* pblockindex = m_chain[coinHeight]; + if (!pblockindex) return false; + CBlock block; + if (!m_blockman.ReadBlockFromDisk(block, *pblockindex)) return false; + for (const auto& btx : block.vtx) { + if (btx->GetHash() == txid) { + tx_out = btx; + return true; + } + } + return false; + }; + + TxValidationState dd_state; + DigiDollar::ValidationContext ddContext( + m_chain.Height() + 1, + GetOraclePriceForTransaction(tx), + DigiDollar::GetSystemCollateralRatio(), + m_chainman.GetParams(), + &dd_view, + false, + txLookup, + m_mempool + ); + + if (!DigiDollar::ValidateDigiDollarTransaction(tx, ddContext, dd_state)) { + LogPrint(BCLog::DIGIDOLLAR, "DigiDollar: Removing reorg-resurrected tx %s after DD revalidation failed: %s\n", + tx.GetHash().ToString(), dd_state.GetRejectReason()); + return true; + } + } + // Transaction is still valid and cached LockPoints are updated. return false; }; @@ -776,8 +814,15 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return false; // state filled in by CheckTransaction } - // DigiDollar consensus validation with blockchain context - if (DigiDollar::HasDigiDollarMarker(tx)) { + // Only accept nLockTime-using transactions that can be mined in the next + // block; keep this before DigiDollar contextual validation so non-final DD + // transactions cannot force oracle/block-db validation work. + if (!CheckFinalTxAtTip(*Assert(m_active_chainstate.m_chain.Tip()), tx)) { + return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-final"); + } + + const bool is_digidollar_tx = DigiDollar::HasDigiDollarMarker(tx); + if (is_digidollar_tx) { // RH-36c: Early reject invalid DD tx types BEFORE expensive oracle/block-DB lookups. // Transactions with the 0x0770 marker but invalid type (>= DD_TX_MAX) would // otherwise trigger full oracle price lookup + block reads before eventual rejection. @@ -793,41 +838,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return state.Invalid(TxValidationResult::TX_CONSENSUS, "digidollar-not-active", "DigiDollar features not yet activated"); } - - // Create validation context with current blockchain state - // Pass coins tip for UTXO lookup in DD redemption validation - // Include block-db tx lookup for DD amount extraction without txindex - auto txLookup = [this](const uint256& txid, uint32_t coinHeight, CTransactionRef& tx_out) -> bool { - AssertLockHeld(cs_main); - const CBlockIndex* pblockindex = m_active_chainstate.m_chain[coinHeight]; - if (!pblockindex) return false; - CBlock block; - if (!m_active_chainstate.m_blockman.ReadBlockFromDisk(block, *pblockindex)) return false; - for (const auto& btx : block.vtx) { - if (btx->GetHash() == txid) { - tx_out = btx; - return true; - } - } - return false; - }; - - DigiDollar::ValidationContext ddContext( - m_active_chainstate.m_chain.Height() + 1, // Height for next block - GetOraclePriceForTransaction(tx), // Current oracle price - DigiDollar::GetSystemCollateralRatio(), // System health - args.m_chainparams, // Chain parameters - &m_active_chainstate.CoinsTip(), // Coins view for UTXO lookup - false, // Don't skip oracle validation in mempool - txLookup, // Block-db tx lookup for DD amounts - &m_pool // Mempool for unconfirmed DD input lookup - ); - - if (!DigiDollar::ValidateDigiDollarTransaction(tx, ddContext, state)) { - LogPrint(BCLog::DIGIDOLLAR, "DigiDollar: Transaction validation failed (txid: %s): %s\n", - hash.ToString(), state.GetRejectReason()); - return false; // state filled in by DigiDollar validation - } } // Coinbase is only valid in a block, not as a loose transaction @@ -844,13 +854,6 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) if (::GetSerializeSize(tx, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS) < MIN_STANDARD_TX_NONWITNESS_SIZE) return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "tx-size-small"); - // Only accept nLockTime-using transactions that can be mined in the next - // block; we don't want our mempool filled up with transactions that can't - // be mined yet. - if (!CheckFinalTxAtTip(*Assert(m_active_chainstate.m_chain.Tip()), tx)) { - return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-final"); - } - if (m_pool.exists(GenTxid::Wtxid(tx.GetWitnessHash()))) { // Exact transaction already exists in the mempool. return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-already-in-mempool"); @@ -941,6 +944,43 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) return false; // state filled in by CheckTxInputs } + // DigiDollar contextual validation must use the mempool-aware view after + // inputs have been cached. Passing CoinsTip() here would miss unconfirmed + // parents and let stale txindex data resolve DD amounts across reorgs. + if (is_digidollar_tx) { + auto txLookup = [this](const uint256& txid, uint32_t coinHeight, CTransactionRef& tx_out) -> bool { + AssertLockHeld(cs_main); + const CBlockIndex* pblockindex = m_active_chainstate.m_chain[coinHeight]; + if (!pblockindex) return false; + CBlock block; + if (!m_active_chainstate.m_blockman.ReadBlockFromDisk(block, *pblockindex)) return false; + for (const auto& btx : block.vtx) { + if (btx->GetHash() == txid) { + tx_out = btx; + return true; + } + } + return false; + }; + + DigiDollar::ValidationContext ddContext( + m_active_chainstate.m_chain.Height() + 1, // Height for next block + GetOraclePriceForTransaction(tx), // Current oracle price + DigiDollar::GetSystemCollateralRatio(), // System health + args.m_chainparams, // Chain parameters + &m_view, // Mempool-aware coins view + false, // Don't skip oracle validation in mempool + txLookup, // Block-db tx lookup for DD amounts + &m_pool // Mempool for unconfirmed DD input lookup + ); + + if (!DigiDollar::ValidateDigiDollarTransaction(tx, ddContext, state)) { + LogPrint(BCLog::DIGIDOLLAR, "DigiDollar: Transaction validation failed (txid: %s): %s\n", + hash.ToString(), state.GetRejectReason()); + return false; // state filled in by DigiDollar validation + } + } + // Check for non-standard pay-to-script-hash in inputs const bool taproot_active = DeploymentActiveAfter(m_active_chainstate.m_chain.Tip(), m_active_chainstate.m_chainman, Consensus::DEPLOYMENT_TAPROOT); if (m_pool.m_require_standard && !AreInputsStandard(tx, m_view, taproot_active)) { From e229ae6e8bb52efafada3811f2bf4eba5205d0ea Mon Sep 17 00:00:00 2001 From: DigiSwarm <13957390+JaredTate@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:02:32 -0600 Subject: [PATCH 04/22] oracle: reject duplicate oracle outputs in coinbase, harden bundle parse (RH-29) A coinbase with more than one OP_RETURN OP_ORACLE output was accepted; only the first one was read. An attacker could stuff a second malformed oracle output to confuse indexers or cause ambiguity in bundle selection. Now any coinbase with >1 oracle output is rejected outright. Malformed pushdata during bundle parsing used break (silently produced a short/garbage data buffer) instead of return false (hard rejection). All four pushdata parse paths (direct push, OP_PUSHDATA1, OP_PUSHDATA2, unrecognized byte) now return false immediately on truncation or unknown encoding. Phase One bundle size check is now exact (== 18) rather than a lower-bound (< 18) to reject padded bundles. --- src/oracle/bundle_manager.cpp | 48 +++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 16 deletions(-) diff --git a/src/oracle/bundle_manager.cpp b/src/oracle/bundle_manager.cpp index a72c008214..2c5833f327 100644 --- a/src/oracle/bundle_manager.cpp +++ b/src/oracle/bundle_manager.cpp @@ -1173,6 +1173,20 @@ bool OracleBundleManager::ExtractOracleBundle(const CTransaction& coinbase_tx, C } }(); + int oracle_output_count = 0; + for (const auto& output : coinbase_tx.vout) { + if (output.scriptPubKey.size() >= 2 && + output.scriptPubKey[0] == OP_RETURN && + output.scriptPubKey[1] == OP_ORACLE) { + ++oracle_output_count; + } + } + if (oracle_output_count > 1) { + LogPrintf("Oracle: Rejecting transaction with %d oracle outputs (expected at most 1)\n", + oracle_output_count); + return false; + } + // Look for OP_RETURN output with OP_ORACLE marker for (const auto& output : coinbase_tx.vout) { if (output.scriptPubKey.size() > 2 && output.scriptPubKey[0] == OP_RETURN) { @@ -1193,32 +1207,32 @@ bool OracleBundleManager::ExtractOracleBundle(const CTransaction& coinbase_tx, C data.insert(data.end(), script_it, script_it + chunk_size); script_it += chunk_size; } else { - break; + return false; } } else if (*script_it == 0x4c) { // OP_PUSHDATA1: next byte is length ++script_it; - if (script_it >= output.scriptPubKey.end()) break; + if (script_it >= output.scriptPubKey.end()) return false; unsigned int chunk_size = *script_it; ++script_it; if (script_it + chunk_size <= output.scriptPubKey.end()) { data.insert(data.end(), script_it, script_it + chunk_size); script_it += chunk_size; } else { - break; + return false; } } else if (*script_it == 0x4d) { // OP_PUSHDATA2: next 2 bytes are length (LE) ++script_it; - if (script_it + 2 > output.scriptPubKey.end()) break; + if (script_it + 2 > output.scriptPubKey.end()) return false; unsigned int chunk_size = *script_it | (*(script_it + 1) << 8); script_it += 2; if (script_it + chunk_size <= output.scriptPubKey.end()) { data.insert(data.end(), script_it, script_it + chunk_size); script_it += chunk_size; } else { - break; + return false; } } else { - break; + return false; } } @@ -1280,9 +1294,9 @@ bool OracleBundleManager::ExtractOracleBundle(const CTransaction& coinbase_tx, C return true; } else if (data[0] == 0x01) { - // Phase One compact format: oracle_id (1) + price (8) + timestamp (8) = 17 bytes - if (data.size() < 18) { // 1 (version) + 17 (data) - LogPrintf("Oracle: Invalid Phase One bundle size: %d\n", data.size()); + // Phase One compact format: version (1) + oracle_id (1) + price (8) + timestamp (8) + if (data.size() != 18) { + LogPrintf("Oracle: Invalid Phase One bundle size: %zu (expected 18)\n", data.size()); return false; } @@ -1349,10 +1363,11 @@ bool OracleBundleManager::ExtractOracleBundle(const CTransaction& coinbase_tx, C return false; } - // Validate we have enough data for all messages + // Validate exact data size for all messages. Trailing bytes would + // create alternate serializations for the same oracle bundle. size_t expected_size = 1 + 1 + 8 + 8 + num_messages * 65; // version + header + per-msg - if (data.size() < expected_size) { - LogPrintf("Oracle: Phase Two data too short: %zu < %zu (for %d messages)\n", + if (data.size() != expected_size) { + LogPrintf("Oracle: Invalid Phase Two data size: %zu != %zu (for %d messages)\n", data.size(), expected_size, num_messages); return false; } @@ -1468,7 +1483,7 @@ bool OracleBundleManager::ValidateV03BundleFormat(const CScript& script, uint8_t return false; } } else { - break; + return false; } } @@ -1485,14 +1500,15 @@ bool OracleBundleManager::ValidateV03BundleFormat(const CScript& script, uint8_t size_t expected = 1 + 1 + bitmap_len + 4 + 8 + 8 + 64; return data.size() == expected; } else if (version == 0x02) { - // v0x02: version(1) + num_msgs(1) + price(8) + timestamp(8) = 18 minimum + // v0x02: version(1) + num_msgs(1) + price(8) + timestamp(8) + N*(oracle_id(1)+sig(64)) if (data.size() < 18) return false; uint8_t num_msgs = data[1]; + if (num_msgs == 0 || num_msgs > ORACLE_ACTIVE_COUNT) return false; size_t expected = 1 + 1 + 8 + 8 + num_msgs * 65; - return data.size() >= expected; + return data.size() == expected; } else if (version == 0x01) { // v0x01: version(1) + oracle_id(1) + price(8) + timestamp(8) = 18 - return data.size() >= 18; + return data.size() == 18; } return false; From 326a83d1370c627a7e5c1a40bbd24e84c3ed8796 Mon Sep 17 00:00:00 2001 From: DigiSwarm <13957390+JaredTate@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:02:37 -0600 Subject: [PATCH 05/22] miner: recalculate merkle root after oracle bundle is injected into coinbase AddOracleBundleToBlock() mutates the coinbase transaction after GenerateCoinbaseCommitment() has already set the block header merkle root. Any caller that uses the returned block template directly (GBT consumers, mining software that skips IncrementExtraNonce()) would see a stale merkle root that does not match the actual coinbase. Recalculate after injection. --- src/node/miner.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/node/miner.cpp b/src/node/miner.cpp index b0693ac4c8..2232364868 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -372,6 +372,10 @@ std::unique_ptr BlockAssembler::CreateNewBlock(const CScript& sc LogPrintf("CreateNewBlock(): Warning - Failed to add oracle bundle to block %d\n", nHeight); // Continue with block creation even if oracle bundle fails (graceful degradation) } + // AddOracleBundleToBlock() mutates the coinbase after GenerateCoinbaseCommitment(). + // Keep the template header internally consistent for GBT/miner consumers that use + // the returned block directly instead of first calling IncrementExtraNonce(). + pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); } LogPrintf("CreateNewBlock(): block weight: %u txs: %u fees: %ld sigops %d\n", GetBlockWeight(*pblock), nBlockTx, nFees, nBlockSigOpsCost); From 9e144e61a5acaca507d81958f16ac022cd406a59 Mon Sep 17 00:00:00 2001 From: DigiSwarm <13957390+JaredTate@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:02:41 -0600 Subject: [PATCH 06/22] digidollar/rpc: gate getmockoracleprice behind DigiDollar activation check (RH-116) getmockoracleprice was callable on any chain before DigiDollar activated, which could return a price that does not correspond to any live oracle state and mislead callers about the system's actual oracle readiness. Now returns a clear error if DigiDollar is not yet active. --- src/rpc/digidollar.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/rpc/digidollar.cpp b/src/rpc/digidollar.cpp index ef0f8cc54d..10dda03a8a 100644 --- a/src/rpc/digidollar.cpp +++ b/src/rpc/digidollar.cpp @@ -4623,9 +4623,18 @@ static RPCHelpMan getmockoracleprice() RPCExamples{ HelpExampleCli("getmockoracleprice", "") + HelpExampleRpc("getmockoracleprice", "") - }, + }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + // Check DigiDollar activation + { + const node::NodeContext& node = EnsureAnyNodeContext(request.context); + ChainstateManager& chainman = EnsureChainman(node); + const CBlockIndex* tip = WITH_LOCK(cs_main, return chainman.ActiveChain().Tip()); + if (!DigiDollar::IsDigiDollarEnabled(tip, chainman)) { + throw JSONRPCError(RPC_MISC_ERROR, "DigiDollar is not yet active on this blockchain"); + } + } // Only allow in RegTest mode if (Params().GetChainType() != ChainType::REGTEST) { throw JSONRPCError(RPC_METHOD_NOT_FOUND, From dffc1b1b7cf0ba7b904f90419bcd4a0f48c56d89 Mon Sep 17 00:00:00 2001 From: DigiSwarm <13957390+JaredTate@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:02:46 -0600 Subject: [PATCH 07/22] digidollar: return explicit error when transfer fee inputs are underfunded (RH-109) BuildTransferTransaction() calculated the required fee and constructed a change output but never checked whether the selected DGB fee inputs actually covered that fee before continuing. A caller with zero or insufficient fee inputs would get a silently malformed transaction with a negative change output. Now returns a descriptive error immediately. --- src/digidollar/txbuilder.cpp | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/digidollar/txbuilder.cpp b/src/digidollar/txbuilder.cpp index 9dc283bcda..bf139dd938 100644 --- a/src/digidollar/txbuilder.cpp +++ b/src/digidollar/txbuilder.cpp @@ -719,6 +719,17 @@ TxBuilderResult TransferTxBuilder::BuildTransferTransaction(const TxBuilderTrans LogPrintf("DigiDollar: Fee calculation - calculated: %d sats, minimum: %d sats, actual: %d sats\n", calculatedFee, MIN_DD_FEE, actualFee); + if (totalFeeIn <= 0) { + result.error = "Insufficient DGB fee input: no fee inputs selected"; + return result; + } + + if (totalFeeIn < actualFee) { + result.error = strprintf("Insufficient DGB fee input: selected=%d sats, required=%d sats", + totalFeeIn, actualFee); + return result; + } + // Add DGB change output if needed (after we know actual fee) if (totalFeeIn > 0) { CAmount dgbChange = totalFeeIn - actualFee; From 13f6d206914fd7846fd27d254a37169ac8126cd4 Mon Sep 17 00:00:00 2001 From: DigiSwarm <13957390+JaredTate@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:02:52 -0600 Subject: [PATCH 08/22] digidollar/health: remove MAX_DIGIDOLLAR cap from supply overflow protection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OnMintConnected() and OnRedeemDisconnected() capped totalDDSupply at MAX_DIGIDOLLAR, treating it as a hard supply ceiling. DigiDollar has no global supply cap — MAX_DIGIDOLLAR is a per-output serialization bound. Replaced the duplicate cap+overflow guard in both functions with a shared AddClampedAmount() helper that protects against int64_t overflow without imposing an artificial supply limit. --- src/digidollar/health.cpp | 41 ++++++++++++++------------------------- 1 file changed, 15 insertions(+), 26 deletions(-) diff --git a/src/digidollar/health.cpp b/src/digidollar/health.cpp index 917ee31e2f..359bfee281 100644 --- a/src/digidollar/health.cpp +++ b/src/digidollar/health.cpp @@ -446,24 +446,22 @@ void SystemHealthMonitor::ScanUTXOSet(CCoinsView* view, CCoinsView* validation_v // Called from ConnectBlock/DisconnectBlock under cs_main. // ============================================================================ +static void AddClampedAmount(CAmount& total, CAmount amount, const char* label) +{ + if (amount <= 0) return; + if (total <= std::numeric_limits::max() - amount) { + total += amount; + return; + } + LogPrintf("Health: WARNING - %s would overflow, capping\n", label); + total = std::numeric_limits::max(); +} + void SystemHealthMonitor::OnMintConnected(CAmount ddAmount, CAmount dgbCollateral) { std::lock_guard lock(s_metricsMutex); // RH-44: thread safety - // SECURITY [RH-11]: Prevent supply overflow — cap at MAX_DIGIDOLLAR - if (ddAmount > 0 && s_currentMetrics.totalDDSupply <= MAX_DIGIDOLLAR - ddAmount) { - s_currentMetrics.totalDDSupply += ddAmount; - } else if (ddAmount > 0) { - LogPrintf("Health: WARNING - totalDDSupply would exceed MAX_DIGIDOLLAR, capping at %s\n", - FormatMoney(MAX_DIGIDOLLAR)); - s_currentMetrics.totalDDSupply = MAX_DIGIDOLLAR; - } - // Cap collateral at MAX_MONEY to prevent int64_t overflow - if (dgbCollateral > 0 && s_currentMetrics.totalCollateral <= std::numeric_limits::max() - dgbCollateral) { - s_currentMetrics.totalCollateral += dgbCollateral; - } else if (dgbCollateral > 0) { - LogPrintf("Health: WARNING - totalCollateral would overflow, capping\n"); - s_currentMetrics.totalCollateral = std::numeric_limits::max(); - } + AddClampedAmount(s_currentMetrics.totalDDSupply, ddAmount, "totalDDSupply"); + AddClampedAmount(s_currentMetrics.totalCollateral, dgbCollateral, "totalCollateral"); LogPrint(BCLog::DIGIDOLLAR, "Health: Mint connected - DD +%s, Collateral +%s (totals: DD=%s, Collateral=%s)\n", FormatMoney(ddAmount), FormatMoney(dgbCollateral), FormatMoney(s_currentMetrics.totalDDSupply), FormatMoney(s_currentMetrics.totalCollateral)); @@ -492,17 +490,8 @@ void SystemHealthMonitor::OnMintDisconnected(CAmount ddAmount, CAmount dgbCollat void SystemHealthMonitor::OnRedeemDisconnected(CAmount ddAmount, CAmount dgbCollateral) { std::lock_guard lock(s_metricsMutex); // RH-44: thread safety - // SECURITY [RH-11]: Same overflow protection as OnMintConnected - if (ddAmount > 0 && s_currentMetrics.totalDDSupply <= MAX_DIGIDOLLAR - ddAmount) { - s_currentMetrics.totalDDSupply += ddAmount; - } else if (ddAmount > 0) { - s_currentMetrics.totalDDSupply = MAX_DIGIDOLLAR; - } - if (dgbCollateral > 0 && s_currentMetrics.totalCollateral <= std::numeric_limits::max() - dgbCollateral) { - s_currentMetrics.totalCollateral += dgbCollateral; - } else if (dgbCollateral > 0) { - s_currentMetrics.totalCollateral = std::numeric_limits::max(); - } + AddClampedAmount(s_currentMetrics.totalDDSupply, ddAmount, "totalDDSupply"); + AddClampedAmount(s_currentMetrics.totalCollateral, dgbCollateral, "totalCollateral"); LogPrint(BCLog::DIGIDOLLAR, "Health: Redeem disconnected - DD +%s, Collateral +%s (totals: DD=%s, Collateral=%s)\n", FormatMoney(ddAmount), FormatMoney(dgbCollateral), FormatMoney(s_currentMetrics.totalDDSupply), FormatMoney(s_currentMetrics.totalCollateral)); From 569da793326418454a8196909c8fd7cbb32cd2e6 Mon Sep 17 00:00:00 2001 From: DigiSwarm <13957390+JaredTate@users.noreply.github.com> Date: Wed, 29 Apr 2026 16:03:01 -0600 Subject: [PATCH 09/22] test: update unit tests for script activation, canonical P2TR, and oracle fixes - rh54: rewritten to test BIP342 OP_SUCCESSx compatibility instead of the old static exclusion approach; verifies DD opcodes fail as BAD_OPCODE in legacy script contexts - redteam T1-06b: updated to expect BAD_OPCODE in legacy execution and correct Tapscript behavior post-activation - skip_oracle: updated expected results now that skipOracleValidation no longer bypasses collateral checks; insufficient-collateral must reject in all sync states - validation: added test for non-canonical OP_1 impostor rejection and legacy DD OP_RETURN format fallback behavior - opcodes, rh11, rh16, rh29, transfer, txbuilder: minor updates for aligned API and message changes from the production fixes above --- src/test/digidollar_opcodes_tests.cpp | 104 ++++---- src/test/digidollar_redteam_tests.cpp | 117 +++------ src/test/digidollar_rh11_consensus_tests.cpp | 15 +- .../digidollar_rh16_reorg_attacks_tests.cpp | 38 ++- src/test/digidollar_skip_oracle_tests.cpp | 49 ++-- src/test/digidollar_transfer_tests.cpp | 76 ++++-- src/test/digidollar_txbuilder_tests.cpp | 32 ++- src/test/digidollar_validation_tests.cpp | 228 ++++++++++++++++++ src/test/oracle_miner_tests.cpp | 10 +- ...h29_coinbase_oracle_manipulation_tests.cpp | 29 +-- src/test/rh54_op_oracle_opsuccess_tests.cpp | 152 +++++++----- 11 files changed, 558 insertions(+), 292 deletions(-) diff --git a/src/test/digidollar_opcodes_tests.cpp b/src/test/digidollar_opcodes_tests.cpp index 7d835943a1..d5da541abf 100644 --- a/src/test/digidollar_opcodes_tests.cpp +++ b/src/test/digidollar_opcodes_tests.cpp @@ -44,11 +44,11 @@ CAmount ScopedOpcodeOraclePrice::s_current_price = 0; // Test that DigiDollar opcodes have correct values BOOST_AUTO_TEST_CASE(digidollar_opcode_values) { - // These opcodes should use OP_NOP slots for soft fork compatibility - BOOST_CHECK_EQUAL(static_cast(OP_DIGIDOLLAR), 0xbb); // OP_NOP11 - BOOST_CHECK_EQUAL(static_cast(OP_DDVERIFY), 0xbc); // OP_NOP12 - BOOST_CHECK_EQUAL(static_cast(OP_CHECKPRICE), 0xbd); // OP_NOP13 - BOOST_CHECK_EQUAL(static_cast(OP_CHECKCOLLATERAL), 0xbe); // OP_NOP14 + // These opcodes use Tapscript OP_SUCCESSx slots for soft-fork compatibility. + BOOST_CHECK_EQUAL(static_cast(OP_DIGIDOLLAR), 0xbb); + BOOST_CHECK_EQUAL(static_cast(OP_DDVERIFY), 0xbc); + BOOST_CHECK_EQUAL(static_cast(OP_CHECKPRICE), 0xbd); + BOOST_CHECK_EQUAL(static_cast(OP_CHECKCOLLATERAL), 0xbe); // Verify OP_CHECKSIGADD exists (BIP342) BOOST_CHECK_EQUAL(static_cast(OP_CHECKSIGADD), 0xba); @@ -76,7 +76,7 @@ BOOST_AUTO_TEST_CASE(op_digidollar_basic) CScript script; script << OP_DIGIDOLLAR << CScriptNum(100000); // 1.0 DGB in satoshis - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); BOOST_CHECK_EQUAL(stack.size(), 1); BOOST_CHECK(CastToBool(stack.back())); // Should push true for valid amount @@ -94,7 +94,7 @@ BOOST_AUTO_TEST_CASE(op_digidollar_zero_amount) script << OP_DIGIDOLLAR << CScriptNum(0); // SECURITY FIX: Zero amount now correctly fails instead of pushing false - BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_INVALID_DD_AMOUNT); } @@ -109,7 +109,7 @@ BOOST_AUTO_TEST_CASE(op_digidollar_negative_amount) CScript script; script << OP_DIGIDOLLAR << CScriptNum(-100); - BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_INVALID_DD_AMOUNT); } @@ -124,7 +124,7 @@ BOOST_AUTO_TEST_CASE(op_digidollar_overflow_amount) CScript script; script << OP_DIGIDOLLAR << CScriptNum(MAX_MONEY + 1); - BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_INVALID_DD_AMOUNT); } @@ -139,7 +139,7 @@ BOOST_AUTO_TEST_CASE(op_digidollar_insufficient_stack) CScript script; script << OP_DIGIDOLLAR; // No amount in script - BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_INVALID_DD_AMOUNT); // Changed from INVALID_STACK_OPERATION } @@ -155,7 +155,7 @@ BOOST_AUTO_TEST_CASE(op_ddverify_basic) CScript script; script << OP_TRUE << OP_DDVERIFY; - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); BOOST_CHECK_EQUAL(stack.size(), 0); // Should consume the stack item } @@ -171,7 +171,7 @@ BOOST_AUTO_TEST_CASE(op_ddverify_false) CScript script; script << OP_FALSE << OP_DDVERIFY; - BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_DD_VERIFY); } @@ -186,7 +186,7 @@ BOOST_AUTO_TEST_CASE(op_ddverify_insufficient_stack) CScript script; script << OP_DDVERIFY; // No value on stack - BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_INVALID_STACK_OPERATION); } @@ -205,7 +205,7 @@ BOOST_AUTO_TEST_CASE(op_checkprice_basic) CScript script; script << CScriptNum(100000) << OP_CHECKPRICE; - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); BOOST_CHECK_EQUAL(stack.size(), 1); BOOST_CHECK(CastToBool(stack.back())); // Should push true for matching price @@ -222,7 +222,7 @@ BOOST_AUTO_TEST_CASE(op_checkprice_mismatch) CScript script; script << CScriptNum(50000) << OP_CHECKPRICE; // Different from mock oracle price - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); BOOST_CHECK_EQUAL(stack.size(), 1); BOOST_CHECK(!CastToBool(stack.back())); // Should push false for non-matching price @@ -239,7 +239,7 @@ BOOST_AUTO_TEST_CASE(op_checkprice_insufficient_stack) CScript script; script << OP_CHECKPRICE; // No price on stack - BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_INVALID_STACK_OPERATION); } @@ -255,7 +255,7 @@ BOOST_AUTO_TEST_CASE(op_checkcollateral_basic) CScript script; script << CScriptNum(150) << CScriptNum(120) << OP_CHECKCOLLATERAL; // ratio=150, threshold=120 - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); BOOST_CHECK_EQUAL(stack.size(), 1); BOOST_CHECK(CastToBool(stack.back())); // Should push true for sufficient collateral @@ -272,7 +272,7 @@ BOOST_AUTO_TEST_CASE(op_checkcollateral_insufficient) CScript script; script << CScriptNum(100) << CScriptNum(120) << OP_CHECKCOLLATERAL; // ratio=100, threshold=120 - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); BOOST_CHECK_EQUAL(stack.size(), 1); BOOST_CHECK(!CastToBool(stack.back())); // Should push false for insufficient collateral @@ -289,7 +289,7 @@ BOOST_AUTO_TEST_CASE(op_checkcollateral_equal) CScript script; script << CScriptNum(120) << CScriptNum(120) << OP_CHECKCOLLATERAL; // ratio=120, threshold=120 - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); BOOST_CHECK_EQUAL(stack.size(), 1); BOOST_CHECK(CastToBool(stack.back())); // Should push true for equal values @@ -306,29 +306,33 @@ BOOST_AUTO_TEST_CASE(op_checkcollateral_insufficient_stack) CScript script; script << CScriptNum(120) << OP_CHECKCOLLATERAL; // Only one value on stack - BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_INVALID_STACK_OPERATION); } -// Test DigiDollar opcodes behave as NOPs when flag is not set (soft fork compatibility) -BOOST_AUTO_TEST_CASE(opcodes_as_nops_without_flag) +// In legacy and witness-v0 scripts, DD opcode bytes are not old-node NOPs. +// They must remain bad opcodes so activation is not a hard fork for those script versions. +BOOST_AUTO_TEST_CASE(opcodes_are_bad_opcode_outside_tapscript) { - std::vector> stack; MockSignatureChecker checker; - ScriptError error; - ScriptExecutionData execdata; - - // All DD opcodes should behave as NOPs without SCRIPT_VERIFY_DIGIDOLLAR flag - // Push a value on stack first so we can verify stack is unchanged CScript script; - script << CScriptNum(100000) << OP_DIGIDOLLAR << CScriptNum(999); + script << OP_DIGIDOLLAR << CScriptNum(100000); - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_NONE, checker, SigVersion::BASE, execdata, &error)); - BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); - // Without the flag, OP_DIGIDOLLAR behaves as NOP, leaving stack with the two pushed values - BOOST_CHECK_EQUAL(stack.size(), 2); - BOOST_CHECK_EQUAL(CScriptNum(stack[0], false).GetInt64(), 100000); - BOOST_CHECK_EQUAL(CScriptNum(stack[1], false).GetInt64(), 999); + { + std::vector> stack; + ScriptError error; + ScriptExecutionData execdata; + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK_EQUAL(error, SCRIPT_ERR_BAD_OPCODE); + } + + { + std::vector> stack; + ScriptError error; + ScriptExecutionData execdata; + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::WITNESS_V0, execdata, &error)); + BOOST_CHECK_EQUAL(error, SCRIPT_ERR_BAD_OPCODE); + } } // Test complex DigiDollar script combining multiple opcodes @@ -351,7 +355,7 @@ BOOST_AUTO_TEST_CASE(complex_digidollar_script) script << CScriptNum(150) << CScriptNum(120) << OP_CHECKCOLLATERAL; // Check collateral (pushes true) script << OP_BOOLAND; // Combine last two conditions with AND - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); BOOST_CHECK_EQUAL(stack.size(), 1); BOOST_CHECK(CastToBool(stack.back())); // All conditions should pass @@ -366,24 +370,28 @@ BOOST_AUTO_TEST_CASE(digidollar_script_error_strings) BOOST_CHECK_EQUAL(ScriptErrorString(SCRIPT_ERR_INSUFFICIENT_COLLATERAL), "Insufficient collateral ratio"); } -// Test that opcodes work with different signature versions -BOOST_AUTO_TEST_CASE(opcodes_different_sigversions) +// Test that activated DD opcodes execute only under Tapscript. +BOOST_AUTO_TEST_CASE(opcodes_tapscript_only) { - std::vector> stack; MockSignatureChecker checker; - ScriptError error; - ScriptExecutionData execdata; - CScript script; script << OP_DIGIDOLLAR << CScriptNum(100000); - // Test with different signature versions - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); - BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); + { + std::vector> stack; + ScriptError error; + ScriptExecutionData execdata; + BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::TAPSCRIPT, execdata, &error)); + BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); + } - stack.clear(); - BOOST_CHECK(EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::WITNESS_V0, execdata, &error)); - BOOST_CHECK_EQUAL(error, SCRIPT_ERR_OK); + { + std::vector> stack; + ScriptError error; + ScriptExecutionData execdata; + BOOST_CHECK(!EvalScript(stack, script, SCRIPT_VERIFY_DIGIDOLLAR, checker, SigVersion::BASE, execdata, &error)); + BOOST_CHECK_EQUAL(error, SCRIPT_ERR_BAD_OPCODE); + } } BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file diff --git a/src/test/digidollar_redteam_tests.cpp b/src/test/digidollar_redteam_tests.cpp index caae1f211e..75ace78ab6 100644 --- a/src/test/digidollar_redteam_tests.cpp +++ b/src/test/digidollar_redteam_tests.cpp @@ -2511,16 +2511,12 @@ BOOST_AUTO_TEST_CASE(redteam_T1_06a_dd_marker_version_check) } } -// T1-06b: DD opcodes behave as NOPs when SCRIPT_VERIFY_DIGIDOLLAR is NOT set -BOOST_AUTO_TEST_CASE(redteam_T1_06b_dd_opcodes_nop_before_activation) +// T1-06b: DD opcode bytes are only soft-fork-safe in Tapscript +BOOST_AUTO_TEST_CASE(redteam_T1_06b_dd_opcodes_tapscript_only) { - // ATTACK: Before activation, can DD opcodes be used in scripts to create - // unexpected behavior? - - // Create a simple script that uses OP_DIGIDOLLAR with an amount push - // Script: OP_DIGIDOLLAR OP_DROP OP_TRUE - // Pre-activation: OP_DIGIDOLLAR is NOP, <1000> is pushed to stack, OP_DROP removes it, OP_TRUE succeeds - // Post-activation: OP_DIGIDOLLAR consumes <1000>, pushes true, OP_DROP removes it, OP_TRUE succeeds + // 0xbb..0xbf are BIP342 OP_SUCCESSx bytes, not legacy OP_NOP slots. + // Executed legacy/witness-v0 scripts must keep rejecting them as bad opcodes + // so DigiDollar activation does not loosen old-node consensus. CScript scriptPubKey; scriptPubKey << OP_DIGIDOLLAR; @@ -2528,140 +2524,99 @@ BOOST_AUTO_TEST_CASE(redteam_T1_06b_dd_opcodes_nop_before_activation) scriptPubKey << OP_DROP; scriptPubKey << OP_TRUE; - CScript scriptSig; // Empty — not needed for this script structure - - // Without SCRIPT_VERIFY_DIGIDOLLAR (pre-activation behavior): - // OP_DIGIDOLLAR = NOP, <1000> pushed, OP_DROP removes 1000, OP_TRUE → stack has [true] { - unsigned int flags = SCRIPT_VERIFY_P2SH; // No DD flag + unsigned int flags = SCRIPT_VERIFY_P2SH; ScriptError err; - // Use direct EvalScript since this isn't a real spending scenario std::vector> stack; bool result = EvalScript(stack, scriptPubKey, flags, BaseSignatureChecker(), SigVersion::BASE, &err); - BOOST_CHECK_MESSAGE(result, - "Pre-activation: OP_DIGIDOLLAR as NOP, script should succeed"); - BOOST_CHECK_MESSAGE(stack.size() == 1 && !stack.back().empty(), - "Pre-activation: Stack should have [true] at top"); + BOOST_CHECK_MESSAGE(!result, + "Legacy script execution must reject DD opcode bytes, not treat them as NOPs"); + BOOST_CHECK_EQUAL(err, SCRIPT_ERR_BAD_OPCODE); } - // With SCRIPT_VERIFY_DIGIDOLLAR (post-activation behavior): - // OP_DIGIDOLLAR reads <1000>, pushes true (1000 > 0), OP_DROP removes true, OP_TRUE → stack has [true] + // Once active in Tapscript, OP_DIGIDOLLAR reads the following amount push, + // pushes true, OP_DROP removes it, and OP_TRUE leaves a true stack item. { unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DIGIDOLLAR; ScriptError err; std::vector> stack; - bool result = EvalScript(stack, scriptPubKey, flags, BaseSignatureChecker(), SigVersion::BASE, &err); + bool result = EvalScript(stack, scriptPubKey, flags, BaseSignatureChecker(), SigVersion::TAPSCRIPT, &err); BOOST_CHECK_MESSAGE(result, - "Post-activation: OP_DIGIDOLLAR processes amount, script should succeed"); + "Post-activation Tapscript: OP_DIGIDOLLAR processes amount and script succeeds"); BOOST_CHECK_MESSAGE(stack.size() == 1 && !stack.back().empty(), - "Post-activation: Stack should have [true] at top"); + "Post-activation Tapscript: Stack should have [true] at top"); } } -// T1-06c: OP_DDVERIFY as NOP doesn't pop stack (consensus safety) -BOOST_AUTO_TEST_CASE(redteam_T1_06c_ddverify_nop_stack_safety) +// T1-06c: OP_DDVERIFY is Tapscript-only +BOOST_AUTO_TEST_CASE(redteam_T1_06c_ddverify_tapscript_only) { - // ATTACK: OP_DDVERIFY pops and verifies top of stack when active. - // As NOP, it must NOT touch the stack. - // If it incorrectly popped pre-activation, scripts would break at activation. - - // Script: OP_TRUE OP_DDVERIFY - // Pre-activation: OP_TRUE pushes 1, OP_DDVERIFY is NOP → stack has [1] - // Post-activation: OP_TRUE pushes 1, OP_DDVERIFY pops 1 (verifies true) → stack is empty - CScript script; - script << OP_TRUE; - script << OP_DDVERIFY; + script << OP_TRUE << OP_DDVERIFY; - // Pre-activation: stack should still have the true value { - unsigned int flags = SCRIPT_VERIFY_P2SH; // No DD flag + unsigned int flags = SCRIPT_VERIFY_P2SH; ScriptError err; std::vector> stack; bool result = EvalScript(stack, script, flags, BaseSignatureChecker(), SigVersion::BASE, &err); - BOOST_CHECK_MESSAGE(result, "Pre-activation: OP_DDVERIFY as NOP should succeed"); - BOOST_CHECK_MESSAGE(stack.size() == 1, - "CRITICAL: Pre-activation OP_DDVERIFY must NOT pop stack (stack size should be 1, got " + - std::to_string(stack.size()) + ")"); + BOOST_CHECK_MESSAGE(!result, "Legacy OP_DDVERIFY byte must be SCRIPT_ERR_BAD_OPCODE"); + BOOST_CHECK_EQUAL(err, SCRIPT_ERR_BAD_OPCODE); } - // Post-activation: OP_DDVERIFY consumes the true, stack should be empty { unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DIGIDOLLAR; ScriptError err; std::vector> stack; - bool result = EvalScript(stack, script, flags, BaseSignatureChecker(), SigVersion::BASE, &err); - BOOST_CHECK_MESSAGE(result, "Post-activation: OP_DDVERIFY should verify true and succeed"); + bool result = EvalScript(stack, script, flags, BaseSignatureChecker(), SigVersion::TAPSCRIPT, &err); + BOOST_CHECK_MESSAGE(result, "Post-activation Tapscript: OP_DDVERIFY should verify true and succeed"); BOOST_CHECK_MESSAGE(stack.size() == 0, - "Post-activation: OP_DDVERIFY should pop the verified value (stack size should be 0, got " + + "Post-activation Tapscript: OP_DDVERIFY should pop the verified value (stack size should be 0, got " + std::to_string(stack.size()) + ")"); } } -// T1-06d: OP_CHECKCOLLATERAL NOP doesn't touch stack (consensus critical) -BOOST_AUTO_TEST_CASE(redteam_T1_06d_checkcollateral_nop_stack_safety) +// T1-06d: OP_CHECKCOLLATERAL is Tapscript-only +BOOST_AUTO_TEST_CASE(redteam_T1_06d_checkcollateral_tapscript_only) { - // ATTACK: OP_CHECKCOLLATERAL pops 2 items when active. - // As NOP, it MUST NOT touch the stack — the comment in the code says so. - // If it popped pre-activation, it would be a consensus split. - - // Script: OP_CHECKCOLLATERAL - // Pre-activation: both numbers pushed, OP_CHECKCOLLATERAL NOP → stack has [500, 200] - // Post-activation: both popped, 500 >= 200 → true → stack has [true] - CScript script; script << CScriptNum(500); script << CScriptNum(200); script << OP_CHECKCOLLATERAL; - // Pre-activation: stack should have both values { unsigned int flags = SCRIPT_VERIFY_P2SH; ScriptError err; std::vector> stack; bool result = EvalScript(stack, script, flags, BaseSignatureChecker(), SigVersion::BASE, &err); - BOOST_CHECK_MESSAGE(result, "Pre-activation: OP_CHECKCOLLATERAL NOP should succeed"); - BOOST_CHECK_MESSAGE(stack.size() == 2, - "CRITICAL: Pre-activation OP_CHECKCOLLATERAL must NOT touch stack (stack size should be 2, got " + - std::to_string(stack.size()) + ")"); + BOOST_CHECK_MESSAGE(!result, "Legacy OP_CHECKCOLLATERAL byte must be SCRIPT_ERR_BAD_OPCODE"); + BOOST_CHECK_EQUAL(err, SCRIPT_ERR_BAD_OPCODE); } - // Post-activation: stack should have [true] { unsigned int flags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_DIGIDOLLAR; ScriptError err; std::vector> stack; - bool result = EvalScript(stack, script, flags, BaseSignatureChecker(), SigVersion::BASE, &err); - BOOST_CHECK_MESSAGE(result, "Post-activation: OP_CHECKCOLLATERAL(500>=200) should succeed"); + bool result = EvalScript(stack, script, flags, BaseSignatureChecker(), SigVersion::TAPSCRIPT, &err); + BOOST_CHECK_MESSAGE(result, "Post-activation Tapscript: OP_CHECKCOLLATERAL(500>=200) should succeed"); BOOST_CHECK_MESSAGE(stack.size() == 1 && !stack.back().empty(), - "Post-activation: OP_CHECKCOLLATERAL should push true (500 >= 200)"); + "Post-activation Tapscript: OP_CHECKCOLLATERAL should push true (500 >= 200)"); } } -// T1-06e: OP_CHECKPRICE NOP doesn't touch stack -BOOST_AUTO_TEST_CASE(redteam_T1_06e_checkprice_nop_stack_safety) +// T1-06e: OP_CHECKPRICE is Tapscript-only +BOOST_AUTO_TEST_CASE(redteam_T1_06e_checkprice_tapscript_only) { - // ATTACK: OP_CHECKPRICE pops 1 item when active. - // As NOP, it must NOT touch the stack. - - // Script: OP_CHECKPRICE - // Pre-activation: number pushed, OP_CHECKPRICE NOP → stack has [42000] - // Post-activation: number popped, compared to mock oracle → stack has [true/false] - CScript script; script << CScriptNum(42000); script << OP_CHECKPRICE; - // Pre-activation: stack should still have the price value { unsigned int flags = SCRIPT_VERIFY_P2SH; ScriptError err; std::vector> stack; bool result = EvalScript(stack, script, flags, BaseSignatureChecker(), SigVersion::BASE, &err); - BOOST_CHECK_MESSAGE(result, "Pre-activation: OP_CHECKPRICE NOP should succeed"); - BOOST_CHECK_MESSAGE(stack.size() == 1, - "CRITICAL: Pre-activation OP_CHECKPRICE must NOT pop stack (stack size should be 1, got " + - std::to_string(stack.size()) + ")"); + BOOST_CHECK_MESSAGE(!result, "Legacy OP_CHECKPRICE byte must be SCRIPT_ERR_BAD_OPCODE"); + BOOST_CHECK_EQUAL(err, SCRIPT_ERR_BAD_OPCODE); } } @@ -18998,7 +18953,7 @@ BOOST_AUTO_TEST_CASE(redteam_t10_05c_script_flags_at_activation_boundary) // Block 600 (pprev=599): DeploymentActiveAfter(599) = ACTIVE → DD flag set // // This means: - // - Block 599: DD opcodes are NOPs (soft-fork compat) + // - Block 599: DD opcodes remain Tapscript OP_SUCCESSx (soft-fork compat) // - Block 600: DD opcodes enforced // // Mempool PolicyScriptChecks uses STANDARD_SCRIPT_VERIFY_FLAGS which @@ -19018,7 +18973,7 @@ BOOST_AUTO_TEST_CASE(redteam_t10_05c_script_flags_at_activation_boundary) BOOST_TEST_MESSAGE(" SCRIPT_VERIFY_DIGIDOLLAR in STANDARD_SCRIPT_VERIFY_FLAGS ✅"); BOOST_TEST_MESSAGE(" SCRIPT_VERIFY_DIGIDOLLAR NOT in MANDATORY_SCRIPT_VERIFY_FLAGS ✅"); - BOOST_TEST_MESSAGE(" Block 599: DD opcodes are NOPs (standard soft-fork behavior) ✅"); + BOOST_TEST_MESSAGE(" Block 599: DD opcodes remain Tapscript OP_SUCCESSx (old-node behavior) ✅"); BOOST_TEST_MESSAGE(" Block 600: DD opcodes enforced (activation complete) ✅"); BOOST_TEST_MESSAGE(" Mempool flag mismatch at tip=599 is benign (no DD UTXOs to spend) ✅"); } diff --git a/src/test/digidollar_rh11_consensus_tests.cpp b/src/test/digidollar_rh11_consensus_tests.cpp index e0b66e87a8..bd3ca3bfc8 100644 --- a/src/test/digidollar_rh11_consensus_tests.cpp +++ b/src/test/digidollar_rh11_consensus_tests.cpp @@ -52,22 +52,21 @@ BOOST_AUTO_TEST_CASE(rh11_supply_overflow_no_protection) DigiDollar::SystemHealthMonitor::ResetMetrics(); - // After fix [RH-11]: OnMintConnected caps at MAX_DIGIDOLLAR to prevent overflow CAmount bigMint = std::numeric_limits::max() / 2; DigiDollar::SystemHealthMonitor::OnMintConnected(bigMint, 100 * COIN); auto metrics = DigiDollar::SystemHealthMonitor::GetCachedMetrics(); - // Should be capped at MAX_DIGIDOLLAR, not bigMint - BOOST_CHECK_EQUAL(metrics.totalDDSupply, MAX_DIGIDOLLAR); + BOOST_CHECK_EQUAL(metrics.totalDDSupply, bigMint); - // Second mint should still be capped + // Second mint still fits in CAmount and must be accounted exactly. DigiDollar::SystemHealthMonitor::OnMintConnected(bigMint, 100 * COIN); metrics = DigiDollar::SystemHealthMonitor::GetCachedMetrics(); - BOOST_CHECK_EQUAL(metrics.totalDDSupply, MAX_DIGIDOLLAR); + BOOST_CHECK_EQUAL(metrics.totalDDSupply, bigMint * 2); - // Verify no overflow - BOOST_CHECK(metrics.totalDDSupply > 0); - BOOST_CHECK(metrics.totalDDSupply <= MAX_DIGIDOLLAR); + // A true int64 overflow attempt is clamped at CAmount max. + DigiDollar::SystemHealthMonitor::OnMintConnected(2, 100 * COIN); + metrics = DigiDollar::SystemHealthMonitor::GetCachedMetrics(); + BOOST_CHECK_EQUAL(metrics.totalDDSupply, std::numeric_limits::max()); DigiDollar::SystemHealthMonitor::ResetMetrics(); } diff --git a/src/test/digidollar_rh16_reorg_attacks_tests.cpp b/src/test/digidollar_rh16_reorg_attacks_tests.cpp index dd29146a18..52f3763eed 100644 --- a/src/test/digidollar_rh16_reorg_attacks_tests.cpp +++ b/src/test/digidollar_rh16_reorg_attacks_tests.cpp @@ -205,12 +205,14 @@ BOOST_AUTO_TEST_CASE(rh16_clamp_to_zero_hides_double_disconnect) BOOST_AUTO_TEST_CASE(rh16_overflow_after_reorg_reconnect) { - // ATTACK: Get supply near MAX_DIGIDOLLAR, disconnect, then reconnect - // with a different (larger) amount. Does overflow protection still work? + // ATTACK: Get supply near MAX_DIGIDOLLAR, connect a normal valid-size mint, + // then reorg it out. The incremental cache must return to the exact + // pre-connect value even when the connected mint crosses the local cap. DigiDollar::SystemHealthMonitor::ResetMetrics(); const CAmount nearMax = MAX_DIGIDOLLAR - 100; const CAmount smallMint = 50; + const CAmount validMint = 10000000; // Default maxMintAmount ($100k in cents) // Set supply near max DigiDollar::SystemHealthMonitor::OnMintConnected(nearMax, 1000 * COIN); @@ -222,29 +224,19 @@ BOOST_AUTO_TEST_CASE(rh16_overflow_after_reorg_reconnect) metrics = DigiDollar::SystemHealthMonitor::GetCachedMetrics(); BOOST_CHECK_EQUAL(metrics.totalDDSupply, nearMax + smallMint); - // Try to overflow with huge mint - const CAmount hugeMint = MAX_DIGIDOLLAR; - DigiDollar::SystemHealthMonitor::OnMintConnected(hugeMint, 1000 * COIN); - metrics = DigiDollar::SystemHealthMonitor::GetCachedMetrics(); - // Should be capped at MAX_DIGIDOLLAR, not overflow - BOOST_CHECK_EQUAL(metrics.totalDDSupply, MAX_DIGIDOLLAR); - - // Now disconnect the huge mint — since it was capped, how much do we subtract? - // VULNERABILITY: We disconnect hugeMint amount, but only MAX_DIGIDOLLAR was recorded. - // After disconnect: MAX_DIGIDOLLAR - hugeMint could be negative → clamped to 0 - // But the actual state should be nearMax + smallMint. - DigiDollar::SystemHealthMonitor::OnMintDisconnected(hugeMint, 1000 * COIN); - metrics = DigiDollar::SystemHealthMonitor::GetCachedMetrics(); + const CAmount beforeReorg = metrics.totalDDSupply; - // EXPLOIT CONFIRMED: Supply is now 0 (clamped) instead of nearMax + smallMint - // The overflow cap during connect + raw subtraction during disconnect = supply loss - BOOST_CHECK_MESSAGE(metrics.totalDDSupply == 0, - "Expected supply drift to 0 due to cap+disconnect asymmetry. Got: " + - std::to_string(metrics.totalDDSupply)); + // A valid-size mint crossing the cap must still be reversible. Current code + // caps the connected total to MAX_DIGIDOLLAR, then subtracts the full mint + // on disconnect, losing valid pre-existing supply. + DigiDollar::SystemHealthMonitor::OnMintConnected(validMint, 1000 * COIN); + DigiDollar::SystemHealthMonitor::OnMintDisconnected(validMint, 1000 * COIN); + metrics = DigiDollar::SystemHealthMonitor::GetCachedMetrics(); - // Document the expected correct value for when this is fixed: - // BOOST_CHECK_EQUAL(metrics.totalDDSupply, nearMax + smallMint); - // ^^^ This is what it SHOULD be after fix + BOOST_CHECK_MESSAGE(metrics.totalDDSupply == beforeReorg, + "Reorg supply drift after valid mint crossing cap: got " + + std::to_string(metrics.totalDDSupply) + " expected " + + std::to_string(beforeReorg)); } // ============================================================================= diff --git a/src/test/digidollar_skip_oracle_tests.cpp b/src/test/digidollar_skip_oracle_tests.cpp index 6b4cd3e58d..2006f0544b 100644 --- a/src/test/digidollar_skip_oracle_tests.cpp +++ b/src/test/digidollar_skip_oracle_tests.cpp @@ -95,8 +95,8 @@ BOOST_FIXTURE_TEST_CASE(skip_oracle_allows_insufficient_collateral_exploit, Basi // (dust+) collateral for $100 DD. At $0.50/DGB with 500% ratio, the // real requirement is ~10 DGB (1,000,000,000 sats). // - // With skipOracleValidation=true (the current bug in ConnectBlock), - // collateral ratio validation is skipped entirely, so this passes. + // Before DD-RH-115, skipOracleValidation=true skipped collateral ratio + // validation entirely, so this passed only for IBD/catch-up nodes. CKey testKey; testKey.MakeNewKey(true); @@ -114,8 +114,9 @@ BOOST_FIXTURE_TEST_CASE(skip_oracle_allows_insufficient_collateral_exploit, Basi CMutableTransaction mtx = BuildMintTx(ownerKey, trivialCollateral, currentHeight); CTransaction tx(mtx); - // With skipOracleValidation=true (the bug): collateral check is SKIPPED - // This passes because no economic validation occurs + // With skipOracleValidation=true, local sync state must not change the + // consensus result: a post-activation mint still needs oracle-backed + // collateral validation. { DigiDollar::ValidationContext ctx(currentHeight, oraclePrice, 150, Params(), nullptr, true /* skipOracleValidation */); @@ -123,11 +124,11 @@ BOOST_FIXTURE_TEST_CASE(skip_oracle_allows_insufficient_collateral_exploit, Basi bool result = DigiDollar::ValidateDigiDollarTransaction(tx, ctx, state); BOOST_TEST_MESSAGE("skipOracle=true, trivial collateral result: " + std::to_string(result) + " reason: " + state.GetRejectReason()); - // This documents the bug: with skipOracle=true, insufficient collateral passes - // After the fix, this STILL passes in IBD mode (skipOracle=true) because - // historical blocks already passed consensus when they were first validated. - BOOST_CHECK_MESSAGE(result, - "IBD mode should allow through historical blocks (skipOracle=true)"); + BOOST_CHECK_MESSAGE(!result, + "SECURITY BUG [DD-RH-115]: skipOracleValidation=true bypasses " + "collateral ratio, so IBD/catch-up nodes can accept an " + "undercollateralized mint rejected by caught-up nodes"); + BOOST_CHECK_EQUAL(state.GetRejectReason(), "insufficient-collateral"); } // With skipOracleValidation=false (the fix for non-IBD blocks): this MUST FAIL @@ -146,8 +147,9 @@ BOOST_FIXTURE_TEST_CASE(skip_oracle_allows_insufficient_collateral_exploit, Basi BOOST_FIXTURE_TEST_CASE(non_ibd_rejects_zero_oracle_price, BasicTestingSetup) { - // When NOT in IBD, oracle price must be available and positive. - // Zero oracle price should cause rejection (fail-closed). + // Oracle price must be available and positive for mint validation in every + // local sync state. Otherwise IBD/catch-up nodes can accept blocks that + // caught-up nodes reject. CKey testKey; testKey.MakeNewKey(true); @@ -175,7 +177,7 @@ BOOST_FIXTURE_TEST_CASE(non_ibd_rejects_zero_oracle_price, BasicTestingSetup) BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-oracle-price"); } - // IBD with oracle price = 0: allowed (oracle may not be available during sync) + // IBD/catch-up with oracle price = 0: also rejected. { DigiDollar::ValidationContext ctx(currentHeight, 0, 150, Params(), nullptr, true /* IBD mode */); @@ -183,7 +185,9 @@ BOOST_FIXTURE_TEST_CASE(non_ibd_rejects_zero_oracle_price, BasicTestingSetup) bool result = DigiDollar::ValidateDigiDollarTransaction(tx, ctx, state); BOOST_TEST_MESSAGE("IBD, zero oracle result: " + std::to_string(result) + " reason: " + state.GetRejectReason()); - BOOST_CHECK(result); + BOOST_CHECK_MESSAGE(!result, + "SECURITY BUG [DD-RH-115]: IBD/catch-up mint accepted with zero oracle price"); + BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-oracle-price"); } } @@ -248,9 +252,9 @@ BOOST_FIXTURE_TEST_CASE(valid_mint_passes_non_ibd, BasicTestingSetup) BOOST_FIXTURE_TEST_CASE(ibd_err_blocks_minting_with_nonzero_supply, BasicTestingSetup) { - // Reproduce the exact IBD failure: after earlier mints increment totalDDSupply, - // ShouldBlockMintingDuringERR() blocks minting because oracle price is 0 (fail-closed). - // With skipOracleValidation=true (IBD mode), the ERR check should be skipped. + // Regression: after earlier mints increment totalDDSupply, the IBD/catch-up + // path still must fail closed on zero oracle price before any local ERR + // state can make the result node-dependent. CKey testKey; testKey.MakeNewKey(true); @@ -274,9 +278,8 @@ BOOST_FIXTURE_TEST_CASE(ibd_err_blocks_minting_with_nonzero_supply, BasicTesting CMutableTransaction mtx = BuildMintTx(ownerKey, generousCollateral, currentHeight); CTransaction tx(mtx); - // IBD mode: oracle price is 0 (no oracles running during sync), skipOracle=true - // BUG: ShouldBlockMintingDuringERR does NOT check skipOracleValidation, - // so it calls ShouldBlockMinting(0) which fails-closed → "minting-blocked-during-err" + // IBD/catch-up mode: oracle price is 0, so the mint must be rejected + // deterministically with the same reason as a caught-up node. { DigiDollar::ValidationContext ctx(currentHeight, 0 /* no oracle in IBD */, 150, Params(), nullptr, true /* skipOracleValidation = IBD mode */); @@ -284,11 +287,9 @@ BOOST_FIXTURE_TEST_CASE(ibd_err_blocks_minting_with_nonzero_supply, BasicTesting bool result = DigiDollar::ValidateDigiDollarTransaction(tx, ctx, state); BOOST_TEST_MESSAGE("IBD + nonzero supply + zero oracle: result=" + std::to_string(result) + " reason=" + state.GetRejectReason()); - BOOST_CHECK_MESSAGE(result, - "IBD BUG: Mint blocked during IBD with reason: " + state.GetRejectReason() + - " — new nodes cannot sync past this block!"); - // Before fix: fails with "minting-blocked-during-err" - // After fix: passes (ERR check skipped during IBD) + BOOST_CHECK_MESSAGE(!result, + "SECURITY BUG [DD-RH-115]: IBD/catch-up mint accepted with zero oracle price"); + BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-oracle-price"); } // Non-IBD mode with zero oracle: should STILL block (fail-closed is correct for live nodes) diff --git a/src/test/digidollar_transfer_tests.cpp b/src/test/digidollar_transfer_tests.cpp index e0e7d28188..f0835d2b8e 100644 --- a/src/test/digidollar_transfer_tests.cpp +++ b/src/test/digidollar_transfer_tests.cpp @@ -223,7 +223,7 @@ struct DDTransferTestFixture : public TestingSetup { // Add some mock UTXOs params.ddUtxos.push_back(CreateMockDDUTXO(TEST_DD_AMOUNT)); - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); // 0.001 DGB for fees + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); // 1 DGB for fees return params; } @@ -276,8 +276,8 @@ BOOST_FIXTURE_TEST_CASE(test_transfer_with_change, DDTransferTestFixture) BOOST_TEST_MESSAGE("Transfer failed: " << result.error); } BOOST_CHECK(result.success); - // Outputs: recipient P2TR + change P2TR + OP_RETURN = 3 - BOOST_CHECK_EQUAL(result.tx.vout.size(), 3); // recipient + change + OP_RETURN + // Outputs: recipient P2TR + DD change P2TR + DGB fee change + OP_RETURN = 4 + BOOST_CHECK_EQUAL(result.tx.vout.size(), 4); } BOOST_FIXTURE_TEST_CASE(test_multiple_dd_inputs_consolidation, DDTransferTestFixture) @@ -723,7 +723,7 @@ BOOST_FIXTURE_TEST_CASE(test_multiple_dd_inputs_assembly, DDTransferTestFixture) params.recipients = {{CreateDDAddress(recipientKey.GetPubKey()), 15000}}; // $150.00 params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); // Create 5 small UTXOs for (int i = 0; i < 5; ++i) { @@ -760,7 +760,7 @@ BOOST_FIXTURE_TEST_CASE(test_dd_input_count_matches_utxo_count, DDTransferTestFi params.recipients = {{recipientAddr, count * 1000}}; // $10.00 per UTXO params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); for (int i = 0; i < count; ++i) { params.ddUtxos.push_back(CreateMockDDUTXO(1000)); @@ -802,7 +802,7 @@ BOOST_FIXTURE_TEST_CASE(test_build_transfer_outputs, DDTransferTestFixture) params.ddUtxos.push_back(CreateMockDDUTXO(25000)); // $250 // Add fee UTXOs - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); // 0.001 DGB + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); // 1 DGB MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -845,7 +845,7 @@ BOOST_FIXTURE_TEST_CASE(test_single_recipient_output, DDTransferTestFixture) params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; params.ddUtxos.push_back(CreateMockDDUTXO(30000)); // Exact amount - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -882,7 +882,7 @@ BOOST_FIXTURE_TEST_CASE(test_output_p2tr_script_format, DDTransferTestFixture) params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; params.ddUtxos.push_back(CreateMockDDUTXO(10000)); - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -936,7 +936,7 @@ BOOST_FIXTURE_TEST_CASE(test_all_recipients_get_outputs, DDTransferTestFixture) params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; params.ddUtxos.push_back(CreateMockDDUTXO(60000)); // Exact total - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -981,7 +981,7 @@ BOOST_FIXTURE_TEST_CASE(test_output_amounts_match_requested, DDTransferTestFixtu params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; params.ddUtxos.push_back(CreateMockDDUTXO(amount1 + amount2)); - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -1018,7 +1018,7 @@ BOOST_FIXTURE_TEST_CASE(test_fee_inputs_added, DDTransferTestFixture) { // Arrange: Create transfer params with fee inputs COutPoint ddUtxo = CreateMockDDUTXO(50000); // $500.00 - COutPoint feeUtxo = CreateMockDGBUTXO(100000); // 0.001 DGB for fees + COutPoint feeUtxo = CreateMockDGBUTXO(COIN); // 1 DGB for fees std::string recipientAddr = CreateDDAddress(recipientKey.GetPubKey()); @@ -1063,7 +1063,7 @@ BOOST_FIXTURE_TEST_CASE(test_transaction_finalization, DDTransferTestFixture) // Create DD UTXO with sufficient balance params.ddUtxos.push_back(CreateMockDDUTXO(50000)); // Exact amount - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); // Fee UTXO + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); // Fee UTXO MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -1097,7 +1097,7 @@ BOOST_FIXTURE_TEST_CASE(test_transaction_version_and_locktime, DDTransferTestFix params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; params.ddUtxos.push_back(CreateMockDDUTXO(25000)); - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -1121,7 +1121,7 @@ BOOST_FIXTURE_TEST_CASE(test_dd_amount_balance_verification, DDTransferTestFixtu params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; params.ddUtxos.push_back(CreateMockDDUTXO(30000)); // Exact balance - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -1163,7 +1163,7 @@ BOOST_FIXTURE_TEST_CASE(test_dd_amount_mismatch_detection, DDTransferTestFixture params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; params.ddUtxos.push_back(CreateMockDDUTXO(30000)); // Only $300.00 (insufficient!) - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -1176,6 +1176,42 @@ BOOST_FIXTURE_TEST_CASE(test_dd_amount_mismatch_detection, DDTransferTestFixture BOOST_CHECK(result.error.find("Insufficient") != std::string::npos); } +BOOST_FIXTURE_TEST_CASE(transfer_rejects_malformed_opreturn_without_throwing, DDTransferTestFixture) +{ + CMutableTransaction tx; + tx.SetDigiDollarType(::DD_TX_TRANSFER); + tx.vin.push_back(CTxIn(CreateMockDDUTXO(TEST_DD_AMOUNT))); + + XOnlyPubKey recipientXOnly(recipientKey.GetPubKey()); + tx.vout.push_back(CTxOut(0, DigiDollar::CreateDigiDollarP2TR(recipientXOnly, TEST_DD_AMOUNT))); + + CScript malformedType; + malformedType << OP_RETURN + << std::vector{'D', 'D'} + << std::vector(5, 0x01) + << CScriptNum(TEST_DD_AMOUNT); + tx.vout.push_back(CTxOut(0, malformedType)); + + TxValidationState state; + DigiDollar::ValidationContext ctx(currentHeight, oraclePrice, systemCollateral, chainParams); + + bool valid = true; + BOOST_CHECK_NO_THROW(valid = DigiDollar::ValidateTransferTransaction(CTransaction(tx), ctx, state)); + BOOST_CHECK(!valid); + BOOST_CHECK_EQUAL(state.GetRejectReason(), "transfer-malformed-op-return"); + + tx.vout.back().scriptPubKey = CScript() << OP_RETURN + << std::vector{'D', 'D'} + << CScriptNum(2) + << std::vector(9, 0x01); + + state = TxValidationState(); + valid = true; + BOOST_CHECK_NO_THROW(valid = DigiDollar::ValidateTransferTransaction(CTransaction(tx), ctx, state)); + BOOST_CHECK(!valid); + BOOST_CHECK_EQUAL(state.GetRejectReason(), "transfer-malformed-op-return"); +} + BOOST_FIXTURE_TEST_CASE(test_empty_inputs_validation, DDTransferTestFixture) { // Arrange: Create params with no DD inputs @@ -1186,7 +1222,7 @@ BOOST_FIXTURE_TEST_CASE(test_empty_inputs_validation, DDTransferTestFixture) params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; // No ddUtxos provided! - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -1206,7 +1242,7 @@ BOOST_FIXTURE_TEST_CASE(test_empty_outputs_validation, DDTransferTestFixture) params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; params.ddUtxos.push_back(CreateMockDDUTXO(10000)); - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -1228,7 +1264,7 @@ BOOST_FIXTURE_TEST_CASE(test_complete_transaction_structure, DDTransferTestFixtu params.feeRate = 100000; // 100,000 sat/kB params.spenderKey = senderKey; params.ddUtxos.push_back(CreateMockDDUTXO(40000)); - params.feeUtxos.push_back(CreateMockDGBUTXO(100000)); + params.feeUtxos.push_back(CreateMockDGBUTXO(COIN)); MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); @@ -1514,7 +1550,7 @@ BOOST_FIXTURE_TEST_CASE(test_transfer_broadcasts_to_network, DDTransferTestFixtu params.spenderKey = ownerKey; params.ddUtxos = {mint_dd_utxo}; params.ddAmounts = {mint_amount}; // Provide DD amounts for mock UTXO - params.feeUtxos = {CreateMockDGBUTXO(100000)}; + params.feeUtxos = {CreateMockDGBUTXO(COIN)}; MockTransferTxBuilder builder(chainParams, currentHeight, oraclePrice); TxBuilderResult result = builder.BuildTransferTransaction(params); @@ -1687,4 +1723,4 @@ BOOST_FIXTURE_TEST_CASE(test_receive_dd_from_transfer, DDTransferTestFixture) LogPrintf(" - Sender time-lock active: %d\n", sender_positions[0].is_active); } -BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/digidollar_txbuilder_tests.cpp b/src/test/digidollar_txbuilder_tests.cpp index 7dffa32b53..f3354ced7a 100644 --- a/src/test/digidollar_txbuilder_tests.cpp +++ b/src/test/digidollar_txbuilder_tests.cpp @@ -235,6 +235,36 @@ BOOST_AUTO_TEST_CASE(transfer_transaction_basic) BOOST_CHECK(::GetDigiDollarTxType(CTransaction(result.tx)) == ::DD_TX_TRANSFER); } +BOOST_AUTO_TEST_CASE(transfer_rejects_underfunded_fee_inputs) +{ + const CChainParams& params = Params(); + int height = 1000; + CAmount price = 10000; // $0.01 per DGB (10,000 micro-USD) + + TestTransferTxBuilder builder(params, height, price); + + CKey recipient = CreateTestKey(); + CTxDestination dest{WitnessV1Taproot(XOnlyPubKey(recipient.GetPubKey()))}; + std::string addr = DigiDollar::EncodeDigiDollarAddress(dest, params); + + uint256 feeHash; + feeHash.SetHex("abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890"); + + TxBuilderTransferParams transferParams; + transferParams.recipients = {{addr, 5000}}; + transferParams.feeRate = 35000000; + transferParams.ddUtxos = CreateTestUTXOs(1); + transferParams.ddAmounts = {5000}; + transferParams.feeUtxos = {COutPoint(feeHash, 0)}; + transferParams.feeAmounts = {1}; + transferParams.spenderKey = CreateTestKey(); + + TxBuilderResult result = builder.BuildTransferTransaction(transferParams); + + BOOST_CHECK(!result.success); + BOOST_CHECK(result.error.find("Insufficient DGB fee input") != std::string::npos); +} + BOOST_AUTO_TEST_CASE(transfer_transaction_invalid_address) { const CChainParams& params = Params(); @@ -599,4 +629,4 @@ BOOST_AUTO_TEST_CASE(consolidation_input_weight_budget) (size_t)MAX_STANDARD_TX_WEIGHT); } -BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/digidollar_validation_tests.cpp b/src/test/digidollar_validation_tests.cpp index 7f8895179d..9e34df16c4 100644 --- a/src/test/digidollar_validation_tests.cpp +++ b/src/test/digidollar_validation_tests.cpp @@ -64,6 +64,18 @@ static CScript MakeDDOpReturnScript(int tx_type, const std::vector& amo return opReturn; } +static CScript MakeLegacyDDOpReturnScript(CAmount amount) +{ + std::vector amountBytes(8); + for (int i = 0; i < 8; ++i) { + amountBytes[i] = static_cast((amount >> (i * 8)) & 0xff); + } + + CScript opReturn; + opReturn << OP_RETURN << OP_DIGIDOLLAR << amountBytes; + return opReturn; +} + // ============================================================================ // Script Type Detection Tests // ============================================================================ @@ -3130,6 +3142,75 @@ BOOST_FIXTURE_TEST_CASE(bug8_transfer_conservation_utxo_valid, DigiDollarValidat BOOST_CHECK_MESSAGE(result, "Valid transfer should pass, got error: " + state.GetRejectReason()); } +BOOST_FIXTURE_TEST_CASE(transfer_rejects_noncanonical_taproot_like_output, DigiDollarValidationTestSetup) +{ + const CAmount ddAmount = 10000; + const CScript inputScript = DigiDollar::CreateDigiDollarP2TR(testXOnlyKey, ddAmount); + + CScript malformedOutputScript; + malformedOutputScript << OP_1 << OP_DROP << OP_TRUE; + while (malformedOutputScript.size() < 34) { + malformedOutputScript << OP_NOP; + } + BOOST_REQUIRE_EQUAL(malformedOutputScript.size(), 34); + BOOST_REQUIRE_NE(malformedOutputScript[1], 32); + + CCoinsView baseView; + CCoinsViewCache coinsView(&baseView); + + const COutPoint prevOut(uint256S("cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd"), 0); + coinsView.AddCoin(prevOut, Coin(CTxOut(0, inputScript), 500, false), false); + + CMutableTransaction mtx; + mtx.nVersion = 0x02000770; + mtx.vin.push_back(CTxIn(prevOut)); + mtx.vout.push_back(CTxOut(0, malformedOutputScript)); + mtx.vout.push_back(CTxOut(0, MakeDDOpReturnScript(2, {ddAmount}))); + + DigiDollar::ValidationContext ctxWithCoins(1000, 500000, 150, Params(), &coinsView); + TxValidationState state; + + const bool result = DigiDollar::ValidateTransferTransaction(CTransaction(mtx), ctxWithCoins, state); + BOOST_CHECK_MESSAGE(!result, "Transfer must reject non-canonical OP_1 scripts as DD outputs"); + BOOST_CHECK_EQUAL(state.GetRejectReason(), "bad-dd-script"); +} + +BOOST_FIXTURE_TEST_CASE(transfer_rejects_unresolved_zero_value_input_when_other_input_resolves, DigiDollarValidationTestSetup) +{ + const CAmount ddAmount = 10000; + + CScript knownInputScript = DigiDollar::CreateDigiDollarP2TR(testXOnlyKey, ddAmount); + CScript outputScript = DigiDollar::CreateDigiDollarP2TR(testXOnlyKey, ddAmount); + + CScript unresolvedZeroValueP2TR; + unresolvedZeroValueP2TR << OP_1 << std::vector(32, 0x42); + + CCoinsView baseView; + CCoinsViewCache coinsView(&baseView); + + const COutPoint knownPrevOut(uint256S("abababababababababababababababababababababababababababababababab"), 0); + coinsView.AddCoin(knownPrevOut, Coin(CTxOut(0, knownInputScript), 500, false), false); + + const COutPoint unresolvedPrevOut(uint256S("bcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbcbc"), 0); + coinsView.AddCoin(unresolvedPrevOut, Coin(CTxOut(0, unresolvedZeroValueP2TR), 500, false), false); + + CMutableTransaction mtx; + mtx.nVersion = 0x02000770; + mtx.vin.push_back(CTxIn(knownPrevOut)); + mtx.vin.push_back(CTxIn(unresolvedPrevOut)); + mtx.vout.push_back(CTxOut(0, outputScript)); + mtx.vout.push_back(CTxOut(0, MakeDDOpReturnScript(2, {ddAmount}))); + + auto noLookup = [](const uint256&, uint32_t, CTransactionRef&) { return false; }; + DigiDollar::ValidationContext ctxWithCoins(1000, 500000, 150, Params(), &coinsView, false, noLookup); + TxValidationState state; + + const bool result = DigiDollar::ValidateTransferTransaction(CTransaction(mtx), ctxWithCoins, state); + BOOST_CHECK_MESSAGE(!result, + "DD transfer must reject any zero-value input whose DD amount cannot be resolved"); + BOOST_CHECK_EQUAL(state.GetRejectReason(), "dd-input-amounts-unknown"); +} + BOOST_FIXTURE_TEST_CASE(transfer_rejects_first_opreturn_amount_spoof, DigiDollarValidationTestSetup) { const CAmount realAmount = 10000; @@ -3291,6 +3372,153 @@ BOOST_FIXTURE_TEST_CASE(mint_accounting_extraction_allows_change_before_opreturn BOOST_CHECK_EQUAL(extractedCollateral, collateral); } +BOOST_FIXTURE_TEST_CASE(mint_accounting_ignores_legacy_opreturn_spoof, DigiDollarValidationTestSetup) +{ + const CAmount ddAmount = 20000; + const CAmount spoofedDD = 10000; + const int64_t lockBlocks = DigiDollar::LockDaysToBlocks(30); + const int64_t lockHeight = mockHeight + lockBlocks; + const CAmount collateral = DigiDollar::CalculateRequiredCollateral(ddAmount, lockBlocks, validationContext); + BOOST_REQUIRE_GT(collateral, 0); + + DigiDollar::MintParams params; + params.ddAmount = ddAmount; + params.lockHeight = lockHeight; + params.ownerKey = testXOnlyKey; + params.internalKey = DigiDollar::GetCollateralNUMSKey(); + params.oracleKeys = DigiDollar::GetOracleKeys(15); + + const CScript legacySpoof = MakeLegacyDDOpReturnScript(spoofedDD); + const CScript collateralScript = DigiDollar::CreateCollateralP2TR(params); + const CScript ddScript = DigiDollar::CreateDigiDollarP2TR(testXOnlyKey, ddAmount); + BOOST_REQUIRE(!collateralScript.empty()); + BOOST_REQUIRE(!ddScript.empty()); + + CPubKey ownerPubKey = testKey.GetPubKey(); + XOnlyPubKey ownerXOnly(ownerPubKey); + CScript modernOpReturn; + modernOpReturn << OP_RETURN + << std::vector{'D', 'D'} + << CScriptNum(1) + << CScriptNum(ddAmount) + << CScriptNum(lockHeight) + << CScriptNum(1) + << std::vector(ownerXOnly.begin(), ownerXOnly.end()); + + CMutableTransaction mtx; + mtx.nVersion = 0x01000770; + mtx.vin.push_back(CTxIn(COutPoint(uint256S("8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f8f"), 0))); + mtx.vout.push_back(CTxOut(0, legacySpoof)); + mtx.vout.push_back(CTxOut(collateral, collateralScript)); + mtx.vout.push_back(CTxOut(0, ddScript)); + mtx.vout.push_back(CTxOut(0, modernOpReturn)); + const CTransaction tx(mtx); + + TxValidationState state; + BOOST_REQUIRE_MESSAGE(DigiDollar::ValidateMintTransaction(tx, validationContext, state), + "legacy spoof must not make an otherwise valid mint fail validation: " + state.GetRejectReason()); + + CAmount extractedDD = 0; + CAmount extractedCollateral = 0; + BOOST_REQUIRE(DigiDollar::ExtractMintAccountingAmounts(tx, extractedDD, extractedCollateral)); + BOOST_CHECK_EQUAL(extractedDD, ddAmount); + BOOST_CHECK_EQUAL(extractedCollateral, collateral); +} + +BOOST_FIXTURE_TEST_CASE(redemption_uses_authoritative_mint_amount_before_script_metadata, DigiDollarValidationTestSetup) +{ + const CAmount originalDD = 50000; + const CAmount poisonedDD = 10000; + const int64_t lockBlocks = DigiDollar::LockDaysToBlocks(30); + const int64_t lockHeight = mockHeight + lockBlocks; + const CAmount lockedCollateral = DigiDollar::CalculateRequiredCollateral(originalDD, lockBlocks, validationContext); + BOOST_REQUIRE_GT(lockedCollateral, 0); + + DigiDollar::MintParams originalParams; + originalParams.ddAmount = originalDD; + originalParams.lockHeight = lockHeight; + originalParams.ownerKey = testXOnlyKey; + originalParams.internalKey = DigiDollar::GetCollateralNUMSKey(); + originalParams.oracleKeys = DigiDollar::GetOracleKeys(15); + + const CScript collateralScript = DigiDollar::CreateCollateralP2TR(originalParams); + BOOST_REQUIRE(!collateralScript.empty()); + + CScript mintOpReturn; + mintOpReturn << OP_RETURN + << std::vector{'D', 'D'} + << CScriptNum(1) + << CScriptNum(originalDD) + << CScriptNum(lockHeight) + << CScriptNum(1) + << std::vector(testXOnlyKey.begin(), testXOnlyKey.end()); + + CMutableTransaction originalMint; + originalMint.nVersion = 0x01000770; + originalMint.vin.push_back(CTxIn(COutPoint(uint256S("cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd"), 0))); + originalMint.vout.push_back(CTxOut(lockedCollateral, collateralScript)); + originalMint.vout.push_back(CTxOut(0, DigiDollar::CreateDigiDollarP2TR(testXOnlyKey, originalDD))); + originalMint.vout.push_back(CTxOut(0, mintOpReturn)); + const CTransaction originalMintTx(originalMint); + + CScript poisonedMintOpReturn; + poisonedMintOpReturn << OP_RETURN + << std::vector{'D', 'D'} + << CScriptNum(1) + << CScriptNum(poisonedDD) + << CScriptNum(lockHeight) + << CScriptNum(1) + << std::vector(testXOnlyKey.begin(), testXOnlyKey.end()); + + CMutableTransaction poisonedMint; + poisonedMint.nVersion = 0x01000770; + poisonedMint.vin.push_back(CTxIn(COutPoint(uint256S("dededededededededededededededededededededededededededededededede"), 0))); + poisonedMint.vout.push_back(CTxOut(546, collateralScript)); + poisonedMint.vout.push_back(CTxOut(0, DigiDollar::CreateDigiDollarP2TR(testXOnlyKey, poisonedDD))); + poisonedMint.vout.push_back(CTxOut(0, poisonedMintOpReturn)); + + TxValidationState poisonedState; + BOOST_CHECK_MESSAGE(!DigiDollar::ValidateMintTransaction(CTransaction(poisonedMint), validationContext, poisonedState), + "poison mint must be rejected but must not affect later redemption accounting"); + BOOST_CHECK_EQUAL(poisonedState.GetRejectReason(), "insufficient-collateral"); + + DigiDollar::ScriptMetadata poisonedMetadata; + BOOST_REQUIRE(DigiDollar::GetScriptMetadata(collateralScript, poisonedMetadata)); + BOOST_REQUIRE_EQUAL(poisonedMetadata.ddAmount, poisonedDD); + + CCoinsView baseView; + CCoinsViewCache coinsView(&baseView); + const COutPoint collateralOut(originalMintTx.GetHash(), 0); + coinsView.AddCoin(collateralOut, Coin(CTxOut(lockedCollateral, collateralScript), 500, false), false); + + const CScript burnScript = DigiDollar::CreateDigiDollarP2TR(testXOnlyKey, poisonedDD); + const COutPoint ddBurnOut(uint256S("efefefefefefefefefefefefefefefefefefefefefefefefefefefefefefefef"), 0); + coinsView.AddCoin(ddBurnOut, Coin(CTxOut(0, burnScript), 500, false), false); + + CMutableTransaction redeem; + redeem.nVersion = 0x03000770; + redeem.nLockTime = lockHeight; + redeem.vin.push_back(CTxIn(collateralOut)); + redeem.vin.push_back(CTxIn(ddBurnOut)); + redeem.vout.push_back(CTxOut(lockedCollateral, GetScriptForDestination(PKHash(testPubKey)))); + redeem.vout.push_back(CTxOut(0, MakeDDOpReturnScript(3, {poisonedDD}))); + + auto lookupOriginalMint = [mintRef = MakeTransactionRef(originalMintTx)](const uint256& txid, + uint32_t coinHeight, + CTransactionRef& tx_out) { + if (coinHeight != 500 || txid != mintRef->GetHash()) return false; + tx_out = mintRef; + return true; + }; + + DigiDollar::ValidationContext ctxWithLookup(lockHeight + 1, 500000, 150, Params(), &coinsView, false, lookupOriginalMint); + TxValidationState redeemState; + const bool redeemAccepted = DigiDollar::ValidateRedemptionTransaction(CTransaction(redeem), ctxWithLookup, redeemState); + BOOST_CHECK_MESSAGE(!redeemAccepted, + "redemption must require burning the original mint amount from the creating transaction, not poisoned script metadata"); + BOOST_CHECK_EQUAL(redeemState.GetRejectReason(), "bad-collateral-release-partial-burn"); +} + // ============================================================================ // Bug #4: Collateral Release Validation Tests // ============================================================================ diff --git a/src/test/oracle_miner_tests.cpp b/src/test/oracle_miner_tests.cpp index 2391f04a65..a825ce9f7e 100644 --- a/src/test/oracle_miner_tests.cpp +++ b/src/test/oracle_miner_tests.cpp @@ -333,6 +333,12 @@ BOOST_AUTO_TEST_CASE(create_new_block_includes_oracle_bundle) // WILL FAIL: Oracle bundle not added by CreateNewBlock() BOOST_CHECK(found_oracle_opreturn); + + // Regression: CreateNewBlock appends the oracle output after generating the + // witness commitment. The returned template must already have the final + // merkle root; miners/GBT consumers should not need IncrementExtraNonce() + // to repair it. + BOOST_CHECK(block.hashMerkleRoot == BlockMerkleRoot(block)); } /** @@ -371,9 +377,9 @@ BOOST_AUTO_TEST_CASE(create_new_block_no_oracle_if_unavailable) // Coinbase should have at least miner payout BOOST_CHECK_GE(coinbase.vout.size(), 1); - // Compute and verify merkle root (CreateNewBlock doesn't set it) - block.hashMerkleRoot = BlockMerkleRoot(block); + // Verify CreateNewBlock returned a populated merkle root. BOOST_CHECK(!block.hashMerkleRoot.IsNull()); + BOOST_CHECK(block.hashMerkleRoot == BlockMerkleRoot(block)); // Test should pass even without oracle bundle (graceful degradation) BOOST_CHECK(true); diff --git a/src/test/rh29_coinbase_oracle_manipulation_tests.cpp b/src/test/rh29_coinbase_oracle_manipulation_tests.cpp index aee27bf387..42fa25ea5e 100644 --- a/src/test/rh29_coinbase_oracle_manipulation_tests.cpp +++ b/src/test/rh29_coinbase_oracle_manipulation_tests.cpp @@ -199,16 +199,9 @@ BOOST_AUTO_TEST_CASE(attack1_extra_data_appended_to_oracle_script) COracleBundle bundle; bool extracted = manager.ExtractOracleBundle(CTransaction(MakeCoinbaseTx(script, 101)), bundle); - // The extra push data gets concatenated into the data buffer during extraction. - // V01 expects exactly 18 bytes. Extra data means data.size() > 18. - // But ExtractOracleBundle for V01 only checks data.size() < 18, not ==18. BOOST_TEST_MESSAGE(" Extracted: " << extracted); - if (extracted) { - BOOST_TEST_MESSAGE(" >>> FINDING: V01 extraction accepts trailing data (no exact size check)"); - BOOST_TEST_MESSAGE(" >>> Extra bytes after V01 oracle data are silently ignored"); - BOOST_TEST_MESSAGE(" >>> Malleability risk: same oracle data can have different serializations"); - BOOST_CHECK_EQUAL(bundle.median_price_micro_usd, price); - } + BOOST_CHECK_MESSAGE(!extracted, + "V01 extraction must reject trailing data so the same oracle payload has one canonical serialization"); } BOOST_AUTO_TEST_CASE(attack1_extra_data_appended_v02) @@ -241,13 +234,9 @@ BOOST_AUTO_TEST_CASE(attack1_extra_data_appended_v02) COracleBundle bundle; bool extracted = manager.ExtractOracleBundle(CTransaction(MakeCoinbaseTx(script, 101)), bundle); - // V02 calculates expected_size = 1+1+8+8+N*65 and checks data.size() < expected_size - // But does NOT check data.size() > expected_size — trailing bytes allowed BOOST_TEST_MESSAGE(" V02 extraction with trailing data: " << extracted); - if (extracted) { - BOOST_TEST_MESSAGE(" >>> FINDING: V02 extraction accepts trailing data beyond expected_size"); - BOOST_TEST_MESSAGE(" >>> Script malleability: miners can embed hidden data after oracle payload"); - } + BOOST_CHECK_MESSAGE(!extracted, + "V02 extraction must reject trailing data beyond expected_size"); } BOOST_AUTO_TEST_CASE(attack1_extra_data_appended_v03) @@ -325,15 +314,13 @@ BOOST_AUTO_TEST_CASE(attack2_two_oracle_op_returns) BOOST_TEST_MESSAGE(" Validation result: " << valid); BOOST_TEST_MESSAGE(" State: " << state.ToString()); - // ExtractOracleBundle returns the FIRST match — which price wins? + // ExtractOracleBundle should also reject ambiguous oracle outputs rather than + // returning the first match and silently ignoring the second. OracleBundleManager& manager = OracleBundleManager::GetInstance(); COracleBundle bundle; bool extracted = manager.ExtractOracleBundle(*block.vtx[0], bundle); - if (extracted) { - BOOST_TEST_MESSAGE(" Extracted price: " << bundle.median_price_micro_usd); - BOOST_TEST_MESSAGE(" >>> ExtractOracleBundle returns first OP_ORACLE match"); - BOOST_TEST_MESSAGE(" >>> Second oracle output (price=99999) is silently ignored"); - } + BOOST_CHECK_MESSAGE(!extracted, + "ExtractOracleBundle must reject transactions with multiple OP_ORACLE outputs"); // The block validation should reject this if (!valid) { diff --git a/src/test/rh54_op_oracle_opsuccess_tests.cpp b/src/test/rh54_op_oracle_opsuccess_tests.cpp index c9121485c3..4350c0a034 100644 --- a/src/test/rh54_op_oracle_opsuccess_tests.cpp +++ b/src/test/rh54_op_oracle_opsuccess_tests.cpp @@ -3,72 +3,47 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. /** - * RH-54: OP_ORACLE incorrectly classified as OP_SUCCESS in Tapscript - * (Wave-2 adversarial PoC) + * RH-54: DigiDollar Tapscript OP_SUCCESSx activation compatibility * - * Target: src/script/script.cpp::IsOpSuccess - * - * Pre-patch behavior: - * IsOpSuccess() excluded opcodes 0xbb..0xbe (OP_DIGIDOLLAR..OP_CHECKCOLLATERAL) - * from the generic Tapscript OP_SUCCESS range [187..254], but NOT OP_ORACLE - * (0xbf = 191). Because OP_ORACLE falls inside 187..254 without exclusion, - * `IsOpSuccess(OP_ORACLE) == true` — which means any Tapscript leaf containing - * OP_ORACLE becomes **unconditionally spendable** per BIP-342 OP_SUCCESSx - * semantics (ExecuteWitnessScript short-circuits the script as a success). - * - * Why this is novel (not in bug-hunt report priors): - * - C1-C4, H1-H8, M1-M5 and the W1 ledger entries do not cover IsOpSuccess. - * - Existing `digidollar_opcodes_tests.cpp` and `digidollar_script_attacks_tests.cpp` - * assert opcode semantics via EvalScript but never exercise the OP_SUCCESS - * branch of Tapscript execution. - * - * Attack model (fund-theft trap): - * - DD wallets and third-party DD-aware tooling that treat OP_ORACLE as a - * marker opcode (docs describe it as "Oracle price data marker") may - * embed it in a Tapscript leaf as a compact tag — e.g., for on-chain - * indexing of oracle-related transactions. - * - Any P2TR output whose script-path MAST contains OP_ORACLE is spendable - * by anyone because the Tapscript execution path short-circuits as success. - * - The attacker scans the chain for P2TR outputs whose revealed leaf - * includes OP_ORACLE, then takes them with an empty witness / a - * well-formed control block. - * - * Fix (applied in this commit): - * Extend the IsOpSuccess exclusion to cover OP_ORACLE: - * if (opcode >= OP_DIGIDOLLAR && opcode <= OP_ORACLE) return false; - * - * This test asserts the POST-FIX invariant. Pre-fix it fails; post-fix it - * passes. The unit-test binary was re-built twice during audit (pre-patch - * and post-patch) to verify both directions. + * DigiDollar uses opcode bytes 0xbb..0xbf. Under BIP342 those bytes are + * OP_SUCCESSx before the soft fork activates, so old Taproot nodes accept a + * revealed leaf containing them unconditionally. Upgraded nodes must preserve + * that pre-activation behavior and only remove the bytes from OP_SUCCESSx when + * SCRIPT_VERIFY_DIGIDOLLAR is active. */ -#include - +#include