From 3d7134f42cc0f38412245039cbfa9ffb73c27263 Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 1 Sep 2026 19:37:47 +0200 Subject: [PATCH 01/14] Merge ElementsProject/elements#1593: Fix RPC return errors for psbt and invalid rangeproofs a9db3b1c1fce0288f47dac21915f9fceaaeb231a Add startup warning for signed-blocks parent chain (Tom Trevethan) 246c5ab63adb7c3a3f672992109e227ab668d096 Add virtual desctructor to CChainParams (Tom Trevethan) 779e71f545d82d706bd5bec083261dfa03c582ce PAK enforcement on confidential nAsset (Tom Trevethan) ffd91c0512eddcb58b74489b3c07939470f3e79e PartiallySignedTransaction::SetupFromTx indexes vtxinwit checked (Tom Trevethan) 84a05e35aa94bf86b80b9e27ba4e25809068d666 check pubkey validity in tweakfedpegscript to prevent assert failure (Tom Trevethan) 3a8dec1258eceac38e110c20fd7b5d7a04d4c536 Return error for psbt if explicit amounts/assets deleted (Tom Trevethan) 4e5ca94f6b6ce0eecb7dc1fa65780d9a724f67bb Return error for invalid rangproof amounts (Tom Trevethan) Pull request description: Fixes for a number of issues with RPC errors for invalid PSBTs and amounts/rangeproofs. ACKs for top commit: delta1: ACK a9db3b1c1fce0288f47dac21915f9fceaaeb231a; tested locally Tree-SHA512: bf35348a5fad30e0f1f2b3caa2ec35ec521b583155e97f3a5f2504a3d70b41677f215fc01b28ccd30706ff5a7d021afb74c110a2c8f270942f5cba344544a55d --- src/blind.cpp | 12 ++++-- src/blindpsbt.cpp | 18 ++++++++- src/blindpsbt.h | 2 + src/chainparams.h | 3 ++ src/init.cpp | 7 ++++ src/pegins.cpp | 84 ++++++++++++++++++++++++++-------------- src/primitives/pak.cpp | 10 +++++ src/primitives/pak.h | 2 + src/psbt.cpp | 15 +++++-- src/rpc/misc.cpp | 22 +++++++++++ src/test/blind_tests.cpp | 72 ++++++++++++++++++++++++++++++++++ src/util/error.cpp | 2 + src/util/error.h | 1 + src/validation.cpp | 3 ++ src/wallet/wallet.cpp | 22 +++++++---- 15 files changed, 230 insertions(+), 45 deletions(-) diff --git a/src/blind.cpp b/src/blind.cpp index 9cb9ea7a7d8..a6c223d4e86 100644 --- a/src/blind.cpp +++ b/src/blind.cpp @@ -546,7 +546,9 @@ int BlindTransaction(std::vector& input_value_blinding_factors, const // Generate rangeproof, no script committed for issuances bool rangeresult = GenerateRangeproof((nPseudo ? txinwit.vchInflationKeysRangeproof : txinwit.vchIssuanceAmountRangeproof), value_blindptrs, nonce, amount, CScript(), value_commit, asset_gen, asset, asset_blindptrs); - assert(rangeresult); + if (!rangeresult) { + return -1; + } // Successfully blinded this issuance num_blinded++; @@ -621,9 +623,13 @@ int BlindTransaction(std::vector& input_value_blinding_factors, const // Generate rangeproof bool rangeresult = GenerateRangeproof(txoutwit.vchRangeproof, value_blindptrs, nonce, amount, out.scriptPubKey, value_commit, asset_gen, asset, asset_blindptrs); - assert(rangeresult); + if (!rangeresult) { + return -1; + } - // Create surjection proof for this output + // Failed surjection proof is a foreseeable condition + // (no suitable input asset to prove against) and is reported to the + // caller via the returned count. See naive_blinding_test. if (!SurjectOutput(txoutwit, surjection_targets, target_asset_generators, target_asset_blinders, asset_blindptrs, asset_gen, asset)) { continue; } diff --git a/src/blindpsbt.cpp b/src/blindpsbt.cpp index 97404a4b018..4af34fe0ed2 100644 --- a/src/blindpsbt.cpp +++ b/src/blindpsbt.cpp @@ -31,6 +31,10 @@ std::string GetBlindingStatusError(const BlindingStatus& status) return "Unable to create an asset surjection proof"; case BlindingStatus::NO_BLIND_OUTPUTS: return "Transaction has blind inputs belonging to this blinder but does not have outputs to blind"; + case BlindingStatus::RANGEPROOF_UNABLE: + return "Unable to create a value rangeproof for an output"; + case BlindingStatus::INVALID_AMOUNT: + return "Zero-valued output to a spendable script cannot be blinded"; } assert(false); } @@ -498,6 +502,12 @@ BlindingStatus BlindPSBT(PartiallySignedTransaction& psbt, std::mapIsUnspendable()) { + return BlindingStatus::INVALID_AMOUNT; + } + // Check this is our output to blind if (output.m_blinder_index == std::nullopt || our_input_data.count(*output.m_blinder_index) == 0) continue; @@ -560,12 +570,16 @@ BlindingStatus BlindPSBT(PartiallySignedTransaction& psbt, std::map blind_value_proof; rangeresult = CreateBlindValueProof(blind_value_proof, value_blinder, *output.amount, value_commit, asset_generator); - assert(rangeresult); + if (!rangeresult) { + return BlindingStatus::RANGEPROOF_UNABLE; + } // Create surjection proof for this output if (!CreateAssetSurjectionProof(asp, fixed_input_tags, ephemeral_input_tags, input_asset_blinders, asset_blinder, asset_generator, asset)) { diff --git a/src/blindpsbt.h b/src/blindpsbt.h index d79e4e77d43..0eaf1a582e5 100644 --- a/src/blindpsbt.h +++ b/src/blindpsbt.h @@ -28,6 +28,8 @@ enum class BlindingStatus INVALID_BLINDER, ASP_UNABLE, NO_BLIND_OUTPUTS, + RANGEPROOF_UNABLE, + INVALID_AMOUNT, }; enum class BlindProofResult { diff --git a/src/chainparams.h b/src/chainparams.h index 3e318577eca..17148ac8062 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -162,6 +162,9 @@ class CChainParams PeginSubsidy GetPeginSubsidy() const { return pegin_subsidy; } PeginMinimum GetPeginMinimum() const { return pegin_minimum; } + // ELEMENTS: Elements adds classes with their own members so the base pointer needs a virtual destructor. + virtual ~CChainParams() = default; + protected: CChainParams() {} diff --git a/src/init.cpp b/src/init.cpp index fdbf6644f5f..07f30b910c6 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1060,6 +1060,13 @@ bool AppInitParameterInteraction(const ArgsManager& args) LogPrintf("Increasing minrelaytxfee to %s to match incrementalrelayfee\n",::minRelayTxFee.ToString()); } + if (chainparams.GetConsensus().has_parent_chain && !chainparams.GetConsensus().ParentChainHasPow()) { + LogPrintf("This chain is configured with a signed-blocks parent chain. " + "Peg-ins referencing a parent block that has activated dynamic " + "federations will be rejected: such headers cannot be " + "authenticated. See doc/ for details.\n"); + } + // Sanity check argument for min fee for including tx in block // TODO: Harmonize which arguments need sanity checking and where that happens if (args.IsArgSet("-blockmintxfee")) { diff --git a/src/pegins.cpp b/src/pegins.cpp index 3871e526379..b7d22d4cc23 100644 --- a/src/pegins.cpp +++ b/src/pegins.cpp @@ -554,39 +554,63 @@ bool DecomposePeginWitness(const CScriptWitness& witness, CAmount& value, CAsset if (stack.size() != 6) return false; - CDataStream stream(stack[0], SER_NETWORK, PROTOCOL_VERSION); - stream >> value; - - CAsset tmp_asset(stack[1]); - asset = tmp_asset; - - uint256 gh(stack[2]); - genesis_hash = gh; - - CScript s(stack[3].begin(), stack[3].end()); - claim_script = s; + // Fixed-width fields must be size-checked before construction: the + // base_blob vector constructor asserts on a length mismatch + // (uint256.cpp:15), and an assert is not catchable by the try below. + // CAsset delegates to the same constructor. + if (stack[1].size() != 32) return false; // asset + if (stack[2].size() != 32) return false; // parent genesis hash + + // Decompose into locals so a failure part-way through cannot leave the + // caller's out-parameters partially populated. + CAmount tmp_value{0}; + CAsset tmp_asset; + uint256 tmp_genesis_hash; + CScript tmp_claim_script; + std::variant tmp_tx; + std::variant tmp_merkle_block; - CDataStream ss_tx(stack[4], SER_NETWORK, PROTOCOL_VERSION); - if (Params().GetConsensus().ParentChainHasPow()) { - Sidechain::Bitcoin::CTransactionRef btc_tx; - ss_tx >> btc_tx; - tx = btc_tx; - } else { - CTransactionRef elem_tx; - ss_tx >> elem_tx; - tx = elem_tx; - } + try { + CDataStream stream(stack[0], SER_NETWORK, PROTOCOL_VERSION); + stream >> tmp_value; + + tmp_asset = CAsset(stack[1]); + tmp_genesis_hash = uint256(stack[2]); + tmp_claim_script = CScript(stack[3].begin(), stack[3].end()); + + CDataStream ss_tx(stack[4], SER_NETWORK, PROTOCOL_VERSION); + if (Params().GetConsensus().ParentChainHasPow()) { + Sidechain::Bitcoin::CTransactionRef btc_tx; + ss_tx >> btc_tx; + tmp_tx = btc_tx; + } else { + CTransactionRef elem_tx; + ss_tx >> elem_tx; + tmp_tx = elem_tx; + } - CDataStream ss_proof(stack[5], SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS); - if (Params().GetConsensus().ParentChainHasPow()) { - Sidechain::Bitcoin::CMerkleBlock tx_proof; - ss_proof >> tx_proof; - merkle_block = tx_proof; - } else { - CMerkleBlock tx_proof; - ss_proof >> tx_proof; - merkle_block = tx_proof; + CDataStream ss_proof(stack[5], SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS); + if (Params().GetConsensus().ParentChainHasPow()) { + Sidechain::Bitcoin::CMerkleBlock tx_proof; + ss_proof >> tx_proof; + tmp_merkle_block = tx_proof; + } else { + CMerkleBlock tx_proof; + ss_proof >> tx_proof; + tmp_merkle_block = tx_proof; + } + } catch (const std::exception&) { + // Malformed encoding. Report failure rather than propagating, so that + // the bool return means what the signature implies. Callers such as + // PartiallySignedTransaction::SetupFromTx have no exception handling. + return false; } + value = tmp_value; + asset = tmp_asset; + genesis_hash = tmp_genesis_hash; + claim_script = tmp_claim_script; + tx = std::move(tmp_tx); + merkle_block = std::move(tmp_merkle_block); return true; } diff --git a/src/primitives/pak.cpp b/src/primitives/pak.cpp index 308af502640..63b67917341 100644 --- a/src/primitives/pak.cpp +++ b/src/primitives/pak.cpp @@ -208,3 +208,13 @@ bool IsPAKValidTx(const CTransaction& tx, const CPAKList& paklist, const uint256 } return true; } + +bool HasConfidentialPegoutOutput(const CTransaction& tx, const uint256& parent_gen_hash) +{ + for (const auto& txout : tx.vout) { + if (txout.scriptPubKey.IsPegoutScript(parent_gen_hash) && !txout.nAsset.IsExplicit()) { + return true; + } + } + return false; +} \ No newline at end of file diff --git a/src/primitives/pak.h b/src/primitives/pak.h index ba840757682..9bde80036de 100644 --- a/src/primitives/pak.h +++ b/src/primitives/pak.h @@ -68,4 +68,6 @@ bool IsPAKValidOutput(const CTxOut& txout, const CPAKList& paklist, const uint25 bool IsPAKValidTx(const CTransaction& tx, const CPAKList& paklist, const uint256& parent_gen_hash, const CAsset& peg_asset); +bool HasConfidentialPegoutOutput(const CTransaction& tx, const uint256& parent_gen_hash); + #endif // BITCOIN_PRIMITIVES_PAK_H diff --git a/src/psbt.cpp b/src/psbt.cpp index 3733b16448a..b8ebc0323f6 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -868,12 +868,21 @@ void PartiallySignedTransaction::SetupFromTx(const CMutableTransaction& tx) } } // Peg-in things - if (txin.m_is_pegin) { + if (txin.m_is_pegin && i < tx.witness.vtxinwit.size()) { CAmount peg_in_value; CAsset asset; - if (DecomposePeginWitness(tx.witness.vtxinwit[i].m_pegin_witness, peg_in_value, asset, input.m_peg_in_genesis_hash, input.m_peg_in_claim_script, input.m_peg_in_tx, input.m_peg_in_txout_proof)) { + uint256 genesis_hash; + CScript claim_script; + std::variant peg_in_tx; + std::variant txout_proof; + if (DecomposePeginWitness(tx.witness.vtxinwit[i].m_pegin_witness, peg_in_value, asset, + genesis_hash, claim_script, peg_in_tx, txout_proof) + && asset == Params().GetConsensus().pegged_asset) { input.m_peg_in_value = peg_in_value; - assert(asset == Params().GetConsensus().pegged_asset); + input.m_peg_in_genesis_hash = genesis_hash; + input.m_peg_in_claim_script = claim_script; + input.m_peg_in_tx = peg_in_tx; + input.m_peg_in_txout_proof = txout_proof; } } } diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 05a758f948b..2f35c419259 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -839,6 +839,24 @@ static RPCHelpMan getindexinfo() // // ELEMENTS CALLS +static bool FedpegScriptPubkeysAreValid(const CScript& script) +{ + const bool is_liquidv1_watchman = MatchLiquidWatchman(script); + bool liquid_op_else_found = false; + CScript::const_iterator pc = script.begin(); + opcodetype opcode; + std::vector vch; + while (script.GetOp(pc, opcode, vch)) { + if (is_liquidv1_watchman && opcode == OP_ELSE) { + liquid_op_else_found = true; + } + if (vch.size() == 33 && !liquid_op_else_found && !CPubKey(vch).IsFullyValid()) { + return false; + } + } + return true; +} + static RPCHelpMan tweakfedpegscript() { return RPCHelpMan{"tweakfedpegscript", @@ -869,6 +887,10 @@ static RPCHelpMan tweakfedpegscript() if (IsHex(request.params[1].get_str())) { std::vector fedpeg_byte = ParseHex(request.params[1].get_str()); fedpegscript = CScript(fedpeg_byte.begin(), fedpeg_byte.end()); + if (!FedpegScriptPubkeysAreValid(fedpegscript)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + "fedpegscript contains a 33-byte push that is not a valid compressed public key"); + } } else { throw JSONRPCError(RPC_TYPE_ERROR, "fedpegscript must be a hex string"); } diff --git a/src/test/blind_tests.cpp b/src/test/blind_tests.cpp index 02e53a01b15..8cb328f195d 100644 --- a/src/test/blind_tests.cpp +++ b/src/test/blind_tests.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -368,4 +369,75 @@ BOOST_AUTO_TEST_CASE(naive_blinding_test) BOOST_CHECK(!VerifyAmounts(inputs, CTransaction(txtemp), nullptr, false)); } } +BOOST_AUTO_TEST_CASE(rangeproof_zero_value_spendable_script) +{ + // A rangeproof over a spendable script uses min_value = 1 + // (`min_value = scriptPubKey.IsUnspendable() ? 0 : 1`), and + // secp256k1_rangeproof_sign returns 0 when min_value > value. A zero-valued + // output to a spendable script therefore has no valid rangeproof, and the + // creation helpers must report that rather than assert on it. + + const CAsset asset(GetRandHash()); + const uint256 asset_blinder = GetRandHash(); + const uint256 value_blinder = GetRandHash(); + const uint256 nonce = GetRandHash(); + + const CScript spendable = CScript() << OP_TRUE; + const CScript unspendable = CScript() << OP_RETURN; + BOOST_CHECK(!spendable.IsUnspendable()); + BOOST_CHECK(unspendable.IsUnspendable()); + + // Asset generator, shared by every case below + CConfidentialAsset conf_asset; + secp256k1_generator asset_gen; + CreateAssetCommitment(conf_asset, asset_gen, asset, asset_blinder); + + // Commitments to 0 and to 1 under that generator + CConfidentialValue conf_value_zero, conf_value_one; + secp256k1_pedersen_commitment value_commit_zero, value_commit_one; + CreateValueCommitment(conf_value_zero, value_commit_zero, value_blinder, asset_gen, 0); + CreateValueCommitment(conf_value_one, value_commit_one, value_blinder, asset_gen, 1); + + std::vector rangeproof; + + // Zero to a spendable script is unprovable. Before the fix, the caller at + // blindpsbt.cpp:562 turns this false into assert(rangeresult) -> SIGABRT. + BOOST_CHECK(!CreateValueRangeProof(rangeproof, value_blinder, nonce, 0, spendable, + value_commit_zero, asset_gen, asset, asset_blinder)); + + // Zero to an unspendable script gives min_value = 0 and must keep working: + // this is the fee / issuance / OP_RETURN shape. + BOOST_CHECK(CreateValueRangeProof(rangeproof, value_blinder, nonce, 0, unspendable, + value_commit_zero, asset_gen, asset, asset_blinder)); + + // The ordinary case is unaffected. + BOOST_CHECK(CreateValueRangeProof(rangeproof, value_blinder, nonce, 1, spendable, + value_commit_one, asset_gen, asset, asset_blinder)); + + // Confirm the boundary is min_value and not something incidental, mirroring + // the rangeproof_info check in naive_blinding_test. + { + secp256k1_context* ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY); + int exp = 0; + int mantissa = 0; + uint64_t min_value = 0; + uint64_t max_value = 0; + BOOST_CHECK(secp256k1_rangeproof_info(ctx, &exp, &mantissa, &min_value, &max_value, + rangeproof.data(), rangeproof.size()) == 1); + BOOST_CHECK_EQUAL(min_value, 1ULL); + secp256k1_context_destroy(ctx); + } + + std::vector value_blindptrs; + std::vector asset_blindptrs; + value_blindptrs.push_back(const_cast(value_blinder.begin())); + asset_blindptrs.push_back(asset_blinder.begin()); + + BOOST_CHECK(!GenerateRangeproof(rangeproof, value_blindptrs, nonce, 0, spendable, + value_commit_zero, asset_gen, asset, asset_blindptrs)); + BOOST_CHECK(GenerateRangeproof(rangeproof, value_blindptrs, nonce, 0, unspendable, + value_commit_zero, asset_gen, asset, asset_blindptrs)); + BOOST_CHECK(GenerateRangeproof(rangeproof, value_blindptrs, nonce, 1, spendable, + value_commit_one, asset_gen, asset, asset_blindptrs)); +} BOOST_AUTO_TEST_SUITE_END() diff --git a/src/util/error.cpp b/src/util/error.cpp index 3549a29f5c3..f2c27e96aec 100644 --- a/src/util/error.cpp +++ b/src/util/error.cpp @@ -49,6 +49,8 @@ bilingual_str TransactionErrorString(const TransactionError err) return Untranslated("Wallet does not have necessary blinding key"); case TransactionError::MISSING_SIDECHANNEL_DATA: return Untranslated("A rangeproof did not encode necessary blinding data"); + case TransactionError::MISSING_EXPLICIT_OUTPUT_DATA: + return Untranslated("Explicit output data is missing for a blinded output"); // no default case, so the compiler can warn about missing cases } assert(false); diff --git a/src/util/error.h b/src/util/error.h index 4b798b84a6b..29ece0a972d 100644 --- a/src/util/error.h +++ b/src/util/error.h @@ -39,6 +39,7 @@ enum class TransactionError { INVALID_ASSET_PROOF, MISSING_BLINDING_KEY, MISSING_SIDECHANNEL_DATA, + MISSING_EXPLICIT_OUTPUT_DATA, }; bilingual_str TransactionErrorString(const TransactionError error); diff --git a/src/validation.cpp b/src/validation.cpp index 9412753f140..606baa9760c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -846,6 +846,9 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // And now do PAK checks. Filtered by next blocks' enforced list if (chainparams.GetEnforcePak()) { + if (HasConfidentialPegoutOutput(tx, chainparams.ParentGenesisBlockHash())) { + return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "confidential-pegout-asset"); + } if (!IsPAKValidTx(tx, GetActivePAKList(m_active_chainstate.m_chain.Tip(), chainparams.GetConsensus()), chainparams.ParentGenesisBlockHash(), chainparams.GetConsensus().pegged_asset)) { return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "invalid-pegout-proof"); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 58391081ac0..32e9db69db1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2039,6 +2039,13 @@ TransactionError CWallet::SignPSBT(PartiallySignedTransaction& psbtx, bool& comp } if (o.script && IsMine(*o.script)) { + // A counterparty blinding our receive output can + // omit them, disabling both, and we would sign a commitment + // to whatever value they chose. Our own blinder always + // preserves these fields, so requiring them is safe. + if (o.amount == std::nullopt || o.m_asset.IsNull()) { + return TransactionError::MISSING_EXPLICIT_OUTPUT_DATA; + } CKey blinding_key; if ((blinding_key = GetBlindingKey(&*o.script)).IsValid()) { CAmount value; @@ -2049,14 +2056,15 @@ TransactionError CWallet::SignPSBT(PartiallySignedTransaction& psbtx, bool& comp CConfidentialNonce nonce; nonce.vchCommitment.insert(nonce.vchCommitment.end(), o.m_ecdh_pubkey.begin(), o.m_ecdh_pubkey.end()); if (UnblindConfidentialPair(blinding_key, o.m_value_commitment, o.m_asset_commitment, nonce, *o.script, o.m_value_rangeproof, value, value_factor, asset, asset_factor)) { - // These assertions are cryptographically impossible to trigger, as we - // checked the proofs above, and then `UnblindConfidentialPair` checks - // the extracted value/asset against the commitments. - if (o.amount) { - assert(*o.amount == value); + // The explicit fields are required above, so + // VerifyBlindProofs has checked both proofs and + // these should not differ. Return rather than + // assert: the inputs originate off-host. + if (*o.amount != value) { + return TransactionError::INVALID_VALUE_PROOF; } - if (!o.m_asset.IsNull()) { - assert(CAsset(o.m_asset) == asset); + if (CAsset(o.m_asset) != asset) { + return TransactionError::INVALID_ASSET_PROOF; } } else { return TransactionError::MISSING_SIDECHANNEL_DATA; From 6253d7e103655ec015097de1505b5f3785ff6447 Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Mon, 3 Aug 2026 12:53:50 +0200 Subject: [PATCH 02/14] fix: range proof cache bind to asset and scriptpubkey --- src/script/sigcache.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp index 865c7e9e2c3..9f7bb9592b5 100644 --- a/src/script/sigcache.cpp +++ b/src/script/sigcache.cpp @@ -72,9 +72,9 @@ class CSignatureCache } // ELEMENTS: - void ComputeEntryRangeProof(uint256& entry, const std::vector& proof, const std::vector& commitment) { + void ComputeEntryRangeProof(uint256& entry, const std::vector& proof, const std::vector& commitment, const std::vector& asset_commitment, const CScript& scriptPubKey) { CSHA256 hasher = m_salted_hasher_range_proof; - hasher.Write(proof.data(), proof.size()).Write(commitment.data(), commitment.size()).Finalize(entry.begin()); + hasher.Write(proof.data(), proof.size()).Write(commitment.data(), commitment.size()).Write(asset_commitment.data(), asset_commitment.size()).Write(scriptPubKey.data(), scriptPubKey.size()).Finalize(entry.begin()); } void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector& proof, const std::vector& commitment) { CSHA256 hasher = m_salted_hasher_surjection_proof; @@ -176,7 +176,7 @@ void InitSurjectionproofCache() bool CachingRangeProofChecker::VerifyRangeProof(const std::vector& vchRangeProof, const std::vector& vchValueCommitment, const std::vector& vchAssetCommitment, const CScript& scriptPubKey, const secp256k1_context* secp256k1_ctx_verify_amounts) const { uint256 entry; - rangeProofCache.ComputeEntryRangeProof(entry, vchRangeProof, vchValueCommitment); + rangeProofCache.ComputeEntryRangeProof(entry, vchRangeProof, vchValueCommitment, vchAssetCommitment, scriptPubKey); if (rangeProofCache.Get(entry, !store)) { return true; From 2391041c60b0f261edb99b6cfb5a1ab848892bb6 Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Tue, 1 Sep 2026 10:25:04 +0200 Subject: [PATCH 03/14] blindpsbt: return error instead of asserting on surjection proof failure CreateAssetSurjectionProof asserted on secp256k1_surjectionproof_generate and _verify failure. A crafted PSET can supply unrelated tags/generators with no known discrete-log relationship, causing generation to fail and the assert to abort the process. Make these recoverable errors by returning false. --- src/blindpsbt.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/blindpsbt.cpp b/src/blindpsbt.cpp index 4af34fe0ed2..235f5a3109e 100644 --- a/src/blindpsbt.cpp +++ b/src/blindpsbt.cpp @@ -57,10 +57,17 @@ bool CreateAssetSurjectionProof(std::vector& output_proof, const } // Using the input chosen, build proof ret = secp256k1_surjectionproof_generate(secp256k1_blind_context, &proof, &ephemeral_input_tags[0], ephemeral_input_tags.size(), &output_asset_tag, input_index, input_asset_blinders[input_index].begin(), output_asset_blinder.begin()); - assert(ret == 1); + if (ret != 1) { + // Attacker-selected tags/generators without a known discrete-log + // relationship cause generation to fail; this must be a recoverable + // PSET error, not a process abort. + return false; + } // Double-check answer ret = secp256k1_surjectionproof_verify(secp256k1_blind_context, &proof, &ephemeral_input_tags[0], ephemeral_input_tags.size(), &output_asset_tag); - assert(ret == 1); + if (ret != 1) { + return false; + } // Serialize into output witness structure size_t output_len = secp256k1_surjectionproof_serialized_size(secp256k1_blind_context, &proof); From 0ce3c24d7f962a555cbe1f69b2dae06d027842c1 Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Tue, 1 Sep 2026 10:25:04 +0200 Subject: [PATCH 04/14] blindpsbt: reject off-curve blinding pubkey before ECDH BlindPSBT passed the blinding pubkey straight to CKey::ECDH, whose only validation is an assert on the peer key, so a crafted off-curve pubkey (reaching IsBlinded() but failing IsFullyValid()) aborted the process. Mirror the non-PSET path and return BlindingStatus::INVALID_BLINDER when the pubkey is not fully valid. --- src/blindpsbt.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/blindpsbt.cpp b/src/blindpsbt.cpp index 235f5a3109e..d38886f8804 100644 --- a/src/blindpsbt.cpp +++ b/src/blindpsbt.cpp @@ -573,6 +573,13 @@ BlindingStatus BlindPSBT(PartiallySignedTransaction& psbt, std::map Date: Tue, 1 Sep 2026 10:25:04 +0200 Subject: [PATCH 05/14] blindpsbt: refuse to blind a PSET output with no amount BlindPSBT dereferenced output.amount without a nullopt check. A crafted v0 PSET output (m_blinder_index set, amount absent) reached the blinding loop and dereferenced a disengaged std::optional, which is undefined behaviour. Refuse such outputs with BlindingStatus::INVALID_BLINDER. --- src/blindpsbt.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/blindpsbt.cpp b/src/blindpsbt.cpp index d38886f8804..e29df7b1110 100644 --- a/src/blindpsbt.cpp +++ b/src/blindpsbt.cpp @@ -518,6 +518,14 @@ BlindingStatus BlindPSBT(PartiallySignedTransaction& psbt, std::map= 2), so a crafted v0 PSET can reach the blinding + // loop with output.amount == nullopt. Dereferencing it is undefined + // behaviour. Refuse to blind such an output. + if (output.amount == std::nullopt) { + return BlindingStatus::INVALID_BLINDER; + } + // Things we are going to stuff into the PSBTOutput if everything is successful CConfidentialValue value_commitment; CConfidentialAsset asset_commitment; From 43092822794d3daeba096464716a01d09d7c2e74 Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Tue, 1 Sep 2026 10:25:04 +0200 Subject: [PATCH 06/14] dynafed: require at least four-fifths approval for parameter transition NextBlockIsParameterTransition computed the approval threshold as (epoch_length*4)/5, which floor-divides. For epoch lengths not divisible by 5 this is below the intended at-least-four-fifths rule, so a transition could pass with fewer than 80% of the epoch's blocks voting for it. Use the overflow-safe ceiling N - N/5 (== ceil(N*4/5)). This is a no-op for epoch lengths divisible by 5 (the only currently deployed case) and only corrects the under-approximation for non-divisible epoch lengths. --- src/dynafed.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/dynafed.cpp b/src/dynafed.cpp index cb288a83ed7..9918f8de7a4 100644 --- a/src/dynafed.cpp +++ b/src/dynafed.cpp @@ -14,6 +14,10 @@ bool NextBlockIsParameterTransition(const CBlockIndex* pindexPrev, const Consens } std::map vote_tally; assert(next_height >= consensus.dynamic_epoch_length); + // Require at least four-fifths of the epoch's votes. (epoch_length*4)/5 + // floor-divides, under-approximating the 80% threshold for epoch lengths + // not divisible by 5; N - N/5 is the overflow-safe ceiling of N*4/5. + const uint32_t threshold = consensus.dynamic_epoch_length - consensus.dynamic_epoch_length / 5; for (int32_t height = next_height - 1; height >= (int32_t)(next_height - consensus.dynamic_epoch_length); --height) { const CBlockIndex* p_epoch_walk = pindexPrev->GetAncestor(height); assert(p_epoch_walk); @@ -25,8 +29,7 @@ bool NextBlockIsParameterTransition(const CBlockIndex* pindexPrev, const Consens const uint256 proposal_root = proposal.CalculateRoot(); vote_tally[proposal_root]++; // Short-circuit once 4/5 threshold is reached - if (!proposal_root.IsNull() && vote_tally[proposal_root] >= - (consensus.dynamic_epoch_length*4)/5) { + if (!proposal_root.IsNull() && vote_tally[proposal_root] >= threshold) { winning_entry = proposal; return true; } From 8a084537cebf8b10e1e2b0c2c8e672140702455c Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Tue, 1 Sep 2026 10:25:04 +0200 Subject: [PATCH 07/14] validation: always validate and retain dynafed header block_height A dynafed header always serializes block_height as part of its identity (CBlockHeader::Serialize/GetHash), independent of the legacy -con_blockheightinheader option. Previously the height was only validated in ContextualCheckBlockHeader and only reconstructed in CBlockIndex/CDiskBlockIndex::GetBlockHeader when that option was on, so a dynafed header with a mismatched height could be accepted, and a header rebuilt from an accepted index no longer matched the accepted header's hash when the option was off. Validate and reconstruct block_height for all dynafed headers regardless of the option. Non-dynafed headers keep the legacy option behaviour. This does not change any consensus rule: the height was already part of every dynafed header's hash. --- src/chain.h | 10 ++++++++-- src/validation.cpp | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/chain.h b/src/chain.h index 6e023158d12..1a470011dfa 100644 --- a/src/chain.h +++ b/src/chain.h @@ -332,7 +332,10 @@ class CBlockIndex block.hashPrevBlock = pprev->GetBlockHash(); block.hashMerkleRoot = hashMerkleRoot; block.nTime = nTime; - if (g_con_blockheightinheader) { + // Dynafed headers always serialize block_height as part of their + // identity (see CBlockHeader::Serialize), so it must be reconstructed + // regardless of the legacy -con_blockheightinheader option. + if (g_con_blockheightinheader || is_dynafed_block()) { block.block_height = nHeight; } block.nBits = nBits; @@ -542,7 +545,10 @@ class CDiskBlockIndex : public CBlockIndex block.hashPrevBlock = hashPrev; block.hashMerkleRoot = hashMerkleRoot; block.nTime = nTime; - if (g_con_blockheightinheader) { + // Dynafed headers always serialize block_height as part of their + // identity (see CBlockHeader::Serialize), so it must be reconstructed + // regardless of the legacy -con_blockheightinheader option. + if (g_con_blockheightinheader || is_dynafed_block()) { block.block_height = nHeight; } block.nBits = nBits; diff --git a/src/validation.cpp b/src/validation.cpp index 606baa9760c..5eabf266115 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3955,7 +3955,10 @@ static bool ContextualCheckBlockHeader(const CBlockHeader& block, BlockValidatio return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "time-too-old", "block's timestamp is too early"); // Check height in header against prev - if (g_con_blockheightinheader && (uint32_t)nHeight != block.block_height) { + // Dynafed headers always serialize block_height as part of their identity + // (see CBlockHeader::Serialize), so the height must be validated even when + // the legacy -con_blockheightinheader option is disabled. + if ((g_con_blockheightinheader || !block.m_dynafed_params.IsNull()) && (uint32_t)nHeight != block.block_height) { LogPrintf("ERROR: %s: block height in header is incorrect (got %d, expected %d)\n", __func__, block.block_height, nHeight); return state.Invalid(BlockValidationResult::BLOCK_INVALID_HEADER, "bad-header-height"); } From 99e9f250fc9a87b9f59b860545eaffd65f20bdb4 Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Tue, 1 Sep 2026 10:25:04 +0200 Subject: [PATCH 08/14] blindpsbt: require both range bounds to match claim in VerifyBlindValueProof A range-membership proof whose lower bound equalled the displayed PSET amount was accepted even when the committed value was larger, because only min_value was compared. Require both verified bounds to equal the claimed amount so a proof can no longer understate an output's value. --- src/blindpsbt.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/blindpsbt.cpp b/src/blindpsbt.cpp index e29df7b1110..97ad68b3e1f 100644 --- a/src/blindpsbt.cpp +++ b/src/blindpsbt.cpp @@ -220,7 +220,11 @@ bool VerifyBlindValueProof(CAmount value, const CConfidentialValue& conf_value, if (secp256k1_rangeproof_verify(secp256k1_blind_context, &min_value, &max_value, &value_commit, proof.data(), proof.size(), /* extra_commit */ nullptr, /* extra_commit_len */ 0, &gen) == 0) { return false; } - return min_value == (uint64_t)value; + // A range-membership proof is only meaningful as an equality proof if the + // proven interval collapses to the claimed amount. Comparing solely the + // lower bound would accept a proof whose committed value is larger than + // the displayed amount. Require both bounds to equal `value`. + return min_value == (uint64_t)value && max_value == (uint64_t)value; } BlindProofResult VerifyBlindProofs(const PSBTOutput& o) { From 3e11e03125ce92827f2d47c2022ff1d085bcc192 Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Tue, 1 Sep 2026 10:25:04 +0200 Subject: [PATCH 09/14] blindpsbt: require genuine commitments in VerifyBlindValueProof An explicit 9-byte value (or a null field) passed the IsNull() check and its buffer was then parsed as a 33-byte Pedersen commitment, reading past the end. Require IsCommitment() on both the value and asset fields so the parser's length precondition holds and the out-of-bounds read is avoided. --- src/blindpsbt.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/blindpsbt.cpp b/src/blindpsbt.cpp index 97ad68b3e1f..f9dce4a9fdb 100644 --- a/src/blindpsbt.cpp +++ b/src/blindpsbt.cpp @@ -201,7 +201,11 @@ bool CreateBlindAssetProof(std::vector& assetproof, const CAsset& bool VerifyBlindValueProof(CAmount value, const CConfidentialValue& conf_value, const std::vector& proof, const CConfidentialAsset& conf_asset) { - if (conf_value.IsNull() || conf_asset.IsNull()) { + // The value and asset must be genuine commitments (33-byte, PrefixA/B) + // before their buffers are handed to libsecp256k1, which consumes exactly + // 33 serialized bytes. An explicit 9-byte value (or a null field) must not + // reach the parser, which would otherwise read out of bounds. + if (!conf_value.IsCommitment() || !conf_asset.IsCommitment()) { return false; } From 7c23bd1097e48c880f565412cbfc0823532cabf2 Mon Sep 17 00:00:00 2001 From: Byron Hambly Date: Tue, 1 Sep 2026 10:25:04 +0200 Subject: [PATCH 10/14] blind: reject empty surjection-target set in SurjectOutput The raw-blind RPC path can reach SurjectOutput with an empty surjection_targets vector (a zero-input tx with multiple blindable outputs), which indexed element [0] of the empty vector and passed it to secp256k1_surjectionproof_initialize, triggering undefined behaviour. Reject empty target sets up front, matching the existing over-limit guard. --- src/blind.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/blind.cpp b/src/blind.cpp index a6c223d4e86..4a5b5f6f65f 100644 --- a/src/blind.cpp +++ b/src/blind.cpp @@ -206,9 +206,12 @@ bool SurjectOutput(CTxOutWitness& txoutwit, const std::vector SECP256K1_SURJECTIONPROOF_MAX_N_INPUTS) { + if (surjection_targets.empty() || surjection_targets.size() > SECP256K1_SURJECTIONPROOF_MAX_N_INPUTS) { // We must return false here to avoid triggering an assertion within - // secp256k1_surjectionproof_initialize on the next line. + // secp256k1_surjectionproof_initialize on the next line: the + // cryptographic API requires a non-empty set of surjection targets, + // and the raw-blinding path can reach us with an empty vector + // (zero-input tx with multiple blindable outputs). return false; } // Find correlation between asset tag and listed input tags From d47af63760c288681ce90b63c28d5ee2b2042931 Mon Sep 17 00:00:00 2001 From: merge-script Date: Thu, 27 Aug 2026 13:46:33 +0100 Subject: [PATCH 11/14] Merge ElementsProject/elements#1584: script: add and default to SIGHASH_ALL_WITH_RANGEPROOF for pre-taproot signing feb50a3f09fc7a82d0522afde262e48b86dd75d6 doc: release note for default rangeproof-committing sighash (Byron Hambly) a1aacfaacbd67674fb91edaf58984c3b446bc905 test: assert wallet default commits rangeproofs post-dynafed (Byron Hambly) 5ed683cbefb904679782d1db7251aa51daac5550 bitcoin-tx: default to rangeproof-committing sighash via chain params (Byron Hambly) f5e2b1f5b83f3c7cfb6301e005db211f2d9c6745 rpc: default raw signing to rangeproof-committing sighash when dynafed active (Byron Hambly) ce342f58f6bff9aea8076c1d46c7e7748807a0c1 wallet: default to rangeproof-committing sighash when dynafed active (Byron Hambly) 3587d7706f79825bc91370a42638e7ad9c67a517 chainparams: add SighashRangeproofActiveByParams() for offline gating (Byron Hambly) be15b0698bf8c6d35a296698b700d6f502418122 node: expose Chain::isSighashRangeproofActive() (tip-based dynafed check) (Byron Hambly) 6a531c262670f421af23fcfb95a8b090135553df script: add SIGHASH_ALL_WITH_RANGEPROOF and DefaultSighashType; strip rangeproof bit for Taproot signing (Byron Hambly) 529eaa1f3a45f6922acb72309f92e9c85de1b832 test: ruff format for feature_sighash_rangeproof.py (Byron Hambly) Pull request description: Pre-Taproot signatures using the historical SIGHASH_ALL default do not commit to output rangeproofs, leaving a witness malleability gap: an attacker can alter a transaction's rangeproofs without invalidating its signatures. This branch closes that gap by making signing default to SIGHASH_ALL | SIGHASH_RANGEPROOF on chains where dynafed is active, while leaving explicit user-supplied sighash types untouched and preserving the legacy default. Scope: - script: adds SIGHASH_ALL_WITH_RANGEPROOF and a DefaultSighashType() helper; strips the 0x40 bit for Taproot/Schnorr signing so the constant is a valid universal default. - node: exposes Chain::isSighashRangeproofActive() for a live tip-based dynafed check. - chainparams: adds SighashRangeproofActiveByParams() for chainstate-less gating (used by bitcoin-tx). - wallet + raw RPCs (signrawtransactionwithkey, signrawtransactionwithwallet, walletprocesspsbt, descriptorprocesspsbt) + - bitcoin-tx: default to the rangeproof-committing sighash when dynafed is active. - Adds unit and functional test coverage plus a release note. ACKs for top commit: tomt1664: ACK feb50a3f09fc7a82d0522afde262e48b86dd75d6 tested locally Tree-SHA512: acef900cd368cbe9c8e0f5a2082b953ba55fd8dd78bf02c0b99b27c71500e93fdcd3abff39f85681f88b98a86cf6be59fbdbb5fd4b679a8c142500bb17f117f9 --- doc/release-notes-20861.md | 22 +++++ src/bitcoin-tx.cpp | 13 ++- src/chainparams.h | 15 +++ src/interfaces/chain.h | 7 ++ src/node/interfaces.cpp | 8 ++ src/rpc/rawtransaction.cpp | 6 +- src/rpc/rawtransaction_util.cpp | 6 +- src/rpc/rawtransaction_util.h | 2 +- src/script/interpreter.h | 9 ++ src/script/sign.cpp | 21 +++- src/script/sign.h | 14 +++ src/test/sighash_tests.cpp | 17 ++++ src/test/validation_tests.cpp | 24 +++++ src/wallet/rpc/spend.cpp | 12 ++- src/wallet/wallet.cpp | 2 +- src/wallet/wallet.h | 1 + test/functional/feature_sighash_rangeproof.py | 98 ++++++++++++++----- 17 files changed, 240 insertions(+), 37 deletions(-) diff --git a/doc/release-notes-20861.md b/doc/release-notes-20861.md index 5c68e4ab0c9..2a7902e1194 100644 --- a/doc/release-notes-20861.md +++ b/doc/release-notes-20861.md @@ -1,3 +1,25 @@ +Wallet and signing +------------------ + +- ELEMENTS: The default sighash used when signing pre-Taproot (legacy and + segwit v0) inputs now commits to the output rangeproofs on chains where + `SIGHASH_RANGEPROOF` is active (i.e. dynafed is active). Concretely, when no + sighash type is specified, the wallet, the `signrawtransactionwithkey` / + `signrawtransactionwithwallet` / `walletprocesspsbt` / `descriptorprocesspsbt` + RPCs, and `elements-tx` now default to `SIGHASH_ALL|RANGEPROOF` instead of + `SIGHASH_ALL`. This removes the previous default's third-party rangeproof + (witness) malleability and matches the rangeproof coverage that Taproot inputs + already have. + + The new default is gated on activation: node-backed signing checks live + dynafed activation at the current tip, while the offline `elements-tx` tool + gates on the selected chain's parameters (only chains where dynafed is always + active). On chains where `SIGHASH_RANGEPROOF` is not active, the historical + `SIGHASH_ALL` default is used so that signatures remain standard and valid. + Taproot signing is unaffected: the `SIGHASH_RANGEPROOF` bit is ignored for + Taproot (which always commits to rangeproofs). Users can still request any + specific sighash type explicitly to override the default. + Updated RPCs ------------ diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 966e6431f43..82c99a6f872 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -7,6 +7,8 @@ #endif #include +#include +#include #include #include #include @@ -602,7 +604,16 @@ static CAmount AmountFromValue(const UniValue& value) static void MutateTxSign(CMutableTransaction& tx, const std::string& flagStr) { - int nHashType = SIGHASH_ALL; + // ELEMENTS: bitcoin-tx has no chainstate, so we cannot check live dynafed + // activation. Gate the default on chain parameters instead: commit to + // rangeproofs by default on chains where dynafed (which enables + // SCRIPT_SIGHASH_RANGEPROOF) is known to be active. Otherwise use the + // historical SIGHASH_ALL default so offline-built txs stay standard and valid. + // See CChainParams::SighashRangeproofActiveByParams() for the liquidv1 nuance. + int nHashType = DefaultSighashType(Params().SighashRangeproofActiveByParams()); + // DefaultSighashType may return SIGHASH_DEFAULT (0); for the legacy tool path + // treat that as SIGHASH_ALL. + if (nHashType == SIGHASH_DEFAULT) nHashType = SIGHASH_ALL; if (flagStr.size() > 0) if (!findSighashFlags(nHashType, flagStr)) diff --git a/src/chainparams.h b/src/chainparams.h index 17148ac8062..9087c733ce5 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -135,6 +135,21 @@ class CChainParams bool MineBlocksOnDemand() const { return consensus.fPowNoRetargeting; } /** Return the network string */ std::string NetworkIDString() const { return strNetworkID; } + /** + * ELEMENTS: Whether SIGHASH_RANGEPROOF can be assumed active for this chain + * from the chain parameters alone (i.e. without inspecting the current tip). + * Used by offline tooling such as elements-tx that has no chainstate to + * decide the default sighash. This is true when dynafed (which enables + * SCRIPT_SIGHASH_RANGEPROOF) is configured ALWAYS_ACTIVE (e.g. elementsregtest + * with dynafed enabled, or liquidv1test), or on liquidv1 where dynafed is + * height-activated (nStartTime is a block height, not the ALWAYS_ACTIVE + * sentinel) but is long since active on the live chain. + */ + bool SighashRangeproofActiveByParams() const + { + return consensus.vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime == Consensus::BIP9Deployment::ALWAYS_ACTIVE + || NetworkIDString() == CBaseChainParams::LIQUID1; + } /** Return the list of hostnames to look up for DNS seeds */ const std::vector& DNSSeeds() const { return vSeeds; } const std::vector& Base58Prefix(Base58Type type) const { return base58Prefixes[type]; } diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 96aae06e820..3d98072630b 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -163,6 +163,13 @@ class Chain //! Check if transaction is RBF opt in. virtual RBFTransactionState isRBFOptIn(const CTransaction& tx) = 0; + //! ELEMENTS: Check whether SIGHASH_RANGEPROOF is active for signing at the + //! current chain tip (i.e. dynafed, which enables SCRIPT_SIGHASH_RANGEPROOF, + //! is active). Used to decide the default pre-Taproot sighash so that we only + //! commit to output rangeproofs when doing so yields standard, valid + //! signatures. + virtual bool isSighashRangeproofActive() = 0; + //! Check if transaction is in mempool. virtual bool isInMempool(const uint256& txid) = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 2cfa2fc6aef..38ae5dc31c7 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -572,6 +572,14 @@ class ChainImpl : public Chain LOCK(m_node.mempool->cs); return IsRBFOptIn(tx, *m_node.mempool); } + bool isSighashRangeproofActive() override + { + // Mirror the mempool standardness check in MemPoolAccept: dynafed being + // active after the current tip enables SCRIPT_SIGHASH_RANGEPROOF, which + // is what makes SIGHASH_RANGEPROOF signatures standard and valid. + LOCK(::cs_main); + return DeploymentActiveAfter(chainman().ActiveChain().Tip(), Params().GetConsensus(), Consensus::DEPLOYMENT_DYNA_FED); + } bool isInMempool(const uint256& txid) override { if (!m_node.mempool) return false; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 7defd97c88c..37db5020905 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -988,7 +989,10 @@ static RPCHelpMan signrawtransactionwithkey() ParsePrevouts(request.params[2], &keystore, coins); UniValue result(UniValue::VOBJ); - SignTransaction(mtx, &keystore, coins, request.params[3], result, chainman.ActiveChain().Tip()); + const auto [tip, sighash_rangeproof_active] = WITH_LOCK(::cs_main, return std::make_pair( + chainman.ActiveChain().Tip(), + DeploymentActiveAfter(chainman.ActiveChain().Tip(), Params().GetConsensus(), Consensus::DEPLOYMENT_DYNA_FED))); + SignTransaction(mtx, &keystore, coins, request.params[3], result, tip, sighash_rangeproof_active); return result; }, }; diff --git a/src/rpc/rawtransaction_util.cpp b/src/rpc/rawtransaction_util.cpp index e41679401ac..874307376cf 100644 --- a/src/rpc/rawtransaction_util.cpp +++ b/src/rpc/rawtransaction_util.cpp @@ -560,9 +560,11 @@ bool ValidateTransactionPeginInputs(const CMutableTransaction& mtx, const CBlock return immature_pegin; } -void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map& coins, const UniValue& hashType, UniValue& result, const CBlockIndex* active_chain_tip) +void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map& coins, const UniValue& hashType, UniValue& result, const CBlockIndex* active_chain_tip, bool sighash_rangeproof_active) { - int nHashType = ParseSighashString(hashType); + // ELEMENTS: when no sighash is specified, default to committing to + // rangeproofs if SIGHASH_RANGEPROOF is active at the current tip. + int nHashType = hashType.isNull() ? DefaultSighashType(sighash_rangeproof_active) : ParseSighashString(hashType); // Script verification errors std::map input_errors; diff --git a/src/rpc/rawtransaction_util.h b/src/rpc/rawtransaction_util.h index 6776472b538..58d201cc57e 100644 --- a/src/rpc/rawtransaction_util.h +++ b/src/rpc/rawtransaction_util.h @@ -34,7 +34,7 @@ class SigningProvider; * @param hashType The signature hash type * @param result JSON object where signed transaction results accumulate */ -void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map& coins, const UniValue& hashType, UniValue& result, const CBlockIndex* active_chain_tip); +void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map& coins, const UniValue& hashType, UniValue& result, const CBlockIndex* active_chain_tip, bool sighash_rangeproof_active); void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map& coins, const std::map& input_errors, bool immature_pegin, UniValue& result); /** diff --git a/src/script/interpreter.h b/src/script/interpreter.h index 3de1b2272b7..b8ee0b4ac40 100644 --- a/src/script/interpreter.h +++ b/src/script/interpreter.h @@ -37,6 +37,15 @@ enum // ELEMENTS: // A flag that means the rangeproofs should be included in the sighash. SIGHASH_RANGEPROOF = 0x40, + + // ELEMENTS: + // The default sighash used by wallets/tools when signing pre-Taproot + // (BASE/WITNESS_V0) inputs on chains where SIGHASH_RANGEPROOF is active. + // This commits to the output rangeproofs, closing the pre-Taproot + // rangeproof (witness) malleability gap. Note this must only be used once + // dynafed (which enables SCRIPT_SIGHASH_RANGEPROOF) is active for the target + // chain; otherwise the resulting signatures are non-standard and invalid. + SIGHASH_ALL_WITH_RANGEPROOF = SIGHASH_ALL | SIGHASH_RANGEPROOF, }; /** Script verification flags. diff --git a/src/script/sign.cpp b/src/script/sign.cpp index cc8a9f0d83d..ed112ff46f5 100644 --- a/src/script/sign.cpp +++ b/src/script/sign.cpp @@ -36,6 +36,15 @@ MutableTransactionSignatureCreator::MutableTransactionSignatureCreator(const CMu { } +int DefaultSighashType(bool sighash_rangeproof_active) +{ + // When SIGHASH_RANGEPROOF is active for the chain, default to committing to + // rangeproofs for pre-Taproot inputs. The 0x40 bit is stripped for Taproot + // signing (see CreateSchnorrSig), so this is a safe universal default. + // Otherwise fall back to SIGHASH_DEFAULT (== SIGHASH_ALL for pre-Taproot). + return sighash_rangeproof_active ? SIGHASH_ALL_WITH_RANGEPROOF : SIGHASH_DEFAULT; +} + bool MutableTransactionSignatureCreator::CreateSig(const SigningProvider& provider, std::vector& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion, unsigned int flags) const { assert(sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0); @@ -83,12 +92,16 @@ bool MutableTransactionSignatureCreator::CreateSchnorrSig(const SigningProvider& execdata.m_tapleaf_hash_init = true; execdata.m_tapleaf_hash = *leaf_hash; } + // ELEMENTS: SIGHASH_RANGEPROOF is a pre-Taproot-only flag; the BIP341-style + // sighash always commits to rangeproofs and rejects the 0x40 bit. Strip it so + // that a universal default of SIGHASH_ALL_WITH_RANGEPROOF still produces valid + // Taproot signatures. + const int taproot_hashtype = nHashType & ~SIGHASH_RANGEPROOF; uint256 hash; - if (!SignatureHashSchnorr(hash, execdata, *txTo, nIn, nHashType, sigversion, *m_txdata, MissingDataBehavior::FAIL)) return false; + if (!SignatureHashSchnorr(hash, execdata, *txTo, nIn, taproot_hashtype, sigversion, *m_txdata, MissingDataBehavior::FAIL)) return false; sig.resize(64); - // Use uint256{} as aux_rnd for now. if (!key.SignSchnorr(hash, sig, merkle_root, {})) return false; - if (nHashType) sig.push_back(nHashType); + if (taproot_hashtype) sig.push_back(taproot_hashtype); return true; } @@ -702,7 +715,7 @@ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, } ScriptError serror = SCRIPT_ERR_OK; - if (!VerifyScript(txin.scriptSig, prevPubKey, &inWitness.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, txdata, MissingDataBehavior::FAIL), &serror)) { + if (!sigdata.complete && !VerifyScript(txin.scriptSig, prevPubKey, &inWitness.scriptWitness, STANDARD_SCRIPT_VERIFY_FLAGS, TransactionSignatureChecker(&txConst, i, amount, txdata, MissingDataBehavior::FAIL), &serror)) { if (serror == SCRIPT_ERR_INVALID_STACK_OPERATION) { // Unable to sign input and verification failed (possible attempt to partially sign). input_errors[i] = Untranslated("Unable to sign input, invalid stack size (possibly missing key)"); diff --git a/src/script/sign.h b/src/script/sign.h index 33b63c62c61..12dad781510 100644 --- a/src/script/sign.h +++ b/src/script/sign.h @@ -105,4 +105,18 @@ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script); /** Sign the CMutableTransaction */ bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* provider, const std::map& coins, int sighash, const uint256& hash_genesis_block, std::map& input_errors); +/** + * ELEMENTS: Return the default sighash type to use when the caller did not + * specify one. When SIGHASH_RANGEPROOF is active for the target chain, the + * default commits to output rangeproofs (SIGHASH_ALL_WITH_RANGEPROOF for + * pre-Taproot inputs); otherwise the historical default (SIGHASH_DEFAULT, which + * is equivalent to SIGHASH_ALL for pre-Taproot) is used so that signatures stay + * standard and valid on chains where dynafed is not active. + * + * Note: for Taproot inputs the sighash byte's rangeproof bit is ignored (the + * BIP341-style sighash always commits to rangeproofs), so this default is only + * meaningful for BASE/WITNESS_V0 signing. + */ +int DefaultSighashType(bool sighash_rangeproof_active); + #endif // BITCOIN_SCRIPT_SIGN_H diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index 06d0ed758ac..80a8d07dbe8 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -7,6 +7,7 @@ #include #include