Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions doc/release-notes-20861.md
Original file line number Diff line number Diff line change
@@ -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
------------

Expand Down
13 changes: 12 additions & 1 deletion src/bitcoin-tx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <bitcoin-build-config.h> // IWYU pragma: keep

#include <asset.h>
#include <chainparams.h>
#include <chainparamsbase.h>
#include <clientversion.h>
#include <coins.h>
Expand All @@ -21,6 +22,7 @@
#include <script/sign.h>
#include <script/signingprovider.h>
#include <univalue.h>
#include <util/chaintype.h>
#include <util/exception.h>
#include <util/fs.h>
#include <util/moneystr.h>
Expand Down Expand Up @@ -615,7 +617,16 @@ static std::vector<unsigned char> ParseHexUV(const UniValue& v, const std::strin

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))
Expand Down
7 changes: 7 additions & 0 deletions src/interfaces/chain.h
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,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;

Expand Down
15 changes: 15 additions & 0 deletions src/kernel/chainparams.h
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,21 @@ class CChainParams
std::string GetChainTypeString() const { return m_chain_type.chain_name; }
/** Return the chain type */
ChainTypeMeta GetChainTypeMeta() const { return m_chain_type; }
/**
* 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
|| m_chain_type.chain_type == ChainType::LIQUID1;
}
/** Return the list of hostnames to look up for DNS seeds */
const std::vector<std::string>& DNSSeeds() const { return vSeeds; }
const std::vector<unsigned char>& Base58Prefix(Base58Type type) const { return base58Prefixes[type]; }
Expand Down
8 changes: 8 additions & 0 deletions src/node/interfaces.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,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(), chainman(), Consensus::DEPLOYMENT_DYNA_FED);
}
bool isInMempool(const uint256& txid) override
{
if (!m_node.mempool) return false;
Expand Down
18 changes: 15 additions & 3 deletions src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <consensus/amount.h>
#include <consensus/validation.h>
#include <core_io.h>
#include <deploymentstatus.h>
#include <index/txindex.h>
#include <key_io.h>
#include <logging.h>
Expand Down Expand Up @@ -891,8 +892,10 @@ static RPCHelpMan signrawtransactionwithkey()
ParsePrevouts(request.params[2], &keystore, coins);

UniValue result(UniValue::VOBJ);
auto tip = WITH_LOCK(::cs_main, return chainman.ActiveChain().Tip());
SignTransaction(mtx, &keystore, coins, request.params[3], result, tip);
const auto [tip, sighash_rangeproof_active] = WITH_LOCK(::cs_main, return std::make_pair(
chainman.ActiveChain().Tip(),
DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_DYNA_FED)));
SignTransaction(mtx, &keystore, coins, request.params[3], result, tip, sighash_rangeproof_active);
return result;
},
};
Expand Down Expand Up @@ -3300,7 +3303,16 @@ RPCHelpMan descriptorprocesspsbt()
EvalDescriptorStringOrObject(descs[i], provider, /*expand_priv=*/true);
}

int sighash_type = ParseSighashString(request.params[2]);
// ELEMENTS: when no sighash is specified, default to committing to
// rangeproofs if SIGHASH_RANGEPROOF is active at the current tip.
int sighash_type;
if (request.params[2].isNull()) {
ChainstateManager& chainman = EnsureAnyChainman(request.context);
const bool sighash_rangeproof_active = WITH_LOCK(::cs_main, return DeploymentActiveAfter(chainman.ActiveChain().Tip(), chainman, Consensus::DEPLOYMENT_DYNA_FED));
sighash_type = DefaultSighashType(sighash_rangeproof_active);
} else {
sighash_type = ParseSighashString(request.params[2]);
}
bool bip32derivs = request.params[3].isNull() ? true : request.params[3].get_bool();
bool finalize = request.params[4].isNull() ? true : request.params[4].get_bool();

Expand Down
6 changes: 4 additions & 2 deletions src/rpc/rawtransaction_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -608,9 +608,11 @@ bool ValidateTransactionPeginInputs(const CMutableTransaction& mtx, const CBlock
return immature_pegin;
}

void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType, UniValue& result, const CBlockIndex* active_chain_tip)
void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& 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<int, bilingual_str> input_errors;
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/rawtransaction_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,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<COutPoint, Coin>& coins, const UniValue& hashType, UniValue& result, const CBlockIndex* active_chain_tip);
void SignTransaction(CMutableTransaction& mtx, const SigningProvider* keystore, const std::map<COutPoint, Coin>& coins, const UniValue& hashType, UniValue& result, const CBlockIndex* active_chain_tip, bool sighash_rangeproof_active);
void SignTransactionResultToJSON(CMutableTransaction& mtx, bool complete, const std::map<COutPoint, Coin>& coins, const std::map<int, bilingual_str>& input_errors, bool immature_pegin, UniValue& result);

/**
Expand Down
9 changes: 9 additions & 0 deletions src/script/interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,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.
Expand Down
18 changes: 16 additions & 2 deletions src/script/sign.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,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<unsigned char>& vchSig, const CKeyID& address, const CScript& scriptCode, SigVersion sigversion, unsigned int flags) const
{
assert(sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0);
Expand Down Expand Up @@ -85,12 +94,17 @@ 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, m_txto, nIn, nHashType, sigversion, *m_txdata, MissingDataBehavior::FAIL)) return false;
if (!SignatureHashSchnorr(hash, execdata, m_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;
}

Expand Down
14 changes: 14 additions & 0 deletions src/script/sign.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,4 +107,18 @@ bool IsSegWitOutput(const SigningProvider& provider, const CScript& script);
/** Sign the CMutableTransaction */
bool SignTransaction(CMutableTransaction& mtx, const SigningProvider* provider, const std::map<COutPoint, Coin>& coins, int sighash, const uint256& hash_genesis_block, std::map<int, bilingual_str>& 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
18 changes: 18 additions & 0 deletions src/test/sighash_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <hash.h>
#include <script/interpreter.h>
#include <script/script.h>
#include <script/sign.h>
#include <serialize.h>
#include <streams.h>
#include <test/data/sighash.json.h>
Expand Down Expand Up @@ -208,4 +209,21 @@ BOOST_AUTO_TEST_CASE(sighash_from_data)
BOOST_CHECK_MESSAGE(sh.GetHex() == sigHashHex, strTest);
}
}

// ELEMENTS: verify the default sighash selection helper.
BOOST_AUTO_TEST_CASE(sighash_default_type)
{
// The named constant sets both the ALL and RANGEPROOF bits.
BOOST_CHECK_EQUAL(SIGHASH_ALL_WITH_RANGEPROOF, SIGHASH_ALL | SIGHASH_RANGEPROOF);
BOOST_CHECK(SIGHASH_ALL_WITH_RANGEPROOF & SIGHASH_RANGEPROOF);

// When rangeproof signing is not active, fall back to SIGHASH_DEFAULT
// (== SIGHASH_ALL for pre-Taproot); do not set the rangeproof bit.
BOOST_CHECK_EQUAL(DefaultSighashType(/*sighash_rangeproof_active=*/false), SIGHASH_DEFAULT);
BOOST_CHECK((DefaultSighashType(false) & SIGHASH_RANGEPROOF) == 0);

// When active, default to committing to rangeproofs.
BOOST_CHECK_EQUAL(DefaultSighashType(/*sighash_rangeproof_active=*/true), SIGHASH_ALL_WITH_RANGEPROOF);
BOOST_CHECK((DefaultSighashType(true) & SIGHASH_RANGEPROOF) == SIGHASH_RANGEPROOF);
}
BOOST_AUTO_TEST_SUITE_END()
24 changes: 24 additions & 0 deletions src/test/validation_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -364,4 +364,28 @@ BOOST_AUTO_TEST_CASE(block_malleation)
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

#include <bitcoin-build-config.h> // IWYU pragma: keep at top for linter

// ELEMENTS: the offline (chainstate-less) SIGHASH_RANGEPROOF gating used by
// elements-tx must treat liquidv1 as known-active even though dynafed there is
// height-activated rather than ALWAYS_ACTIVE.
BOOST_AUTO_TEST_CASE(sighash_rangeproof_by_params_test)
{
// liquidv1: dynafed is height-activated (nStartTime = 1000000), NOT the
// ALWAYS_ACTIVE sentinel, but must be treated as active by params.
const auto liquidv1 = CreateChainParams(*m_node.args, ChainType::LIQUID1);
BOOST_CHECK(liquidv1->GetConsensus().vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime
!= Consensus::BIP9Deployment::ALWAYS_ACTIVE);
BOOST_CHECK(liquidv1->SighashRangeproofActiveByParams());

// liquidv1test: overrides dynafed to ALWAYS_ACTIVE, so it is active by params
// via the ALWAYS_ACTIVE branch (independent of the liquidv1 chain-type check).
const auto liquidv1test = CreateChainParams(*m_node.args, ChainType::LIQUID1TEST);
BOOST_CHECK_EQUAL(liquidv1test->GetConsensus().vDeployments[Consensus::DEPLOYMENT_DYNA_FED].nStartTime,
Consensus::BIP9Deployment::ALWAYS_ACTIVE);
BOOST_CHECK(liquidv1test->SighashRangeproofActiveByParams());

// regtest: dynafed never active by default; must be inactive by params.
const auto regtest = CreateChainParams(*m_node.args, ChainType::REGTEST);
BOOST_CHECK(!regtest->SighashRangeproofActiveByParams());
}

BOOST_AUTO_TEST_SUITE_END()
12 changes: 10 additions & 2 deletions src/wallet/rpc/spend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1050,7 +1050,11 @@ RPCHelpMan signrawtransactionwithwallet()
LOCK(pwallet->cs_wallet);
EnsureWalletIsUnlocked(*pwallet);

int nHashType = ParseSighashString(request.params[2]);
// ELEMENTS: when no sighash is specified, default to committing to
// rangeproofs if SIGHASH_RANGEPROOF is active at the current tip.
int nHashType = request.params[2].isNull()
? DefaultSighashType(pwallet->chain().isSighashRangeproofActive())
: ParseSighashString(request.params[2]);

CMutableTransaction mtx;
if (!DecodeHexTx(mtx, request.params[0].get_str())) {
Expand Down Expand Up @@ -1755,7 +1759,11 @@ RPCHelpMan walletprocesspsbt()
}

// Get the sighash type
int nHashType = ParseSighashString(request.params[2]);
// ELEMENTS: when no sighash is specified, default to committing to
// rangeproofs if SIGHASH_RANGEPROOF is active at the current tip.
int nHashType = request.params[2].isNull()
? DefaultSighashType(pwallet->chain().isSighashRangeproofActive())
: ParseSighashString(request.params[2]);

// Don't sign, just fill data.
bool bip32derivs = request.params[3].isNull() ? true : request.params[3].get_bool();
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2306,7 +2306,7 @@ bool CWallet::SignTransaction(CMutableTransaction& tx) const
coins[input.prevout] = Coin(wtx.tx->vout[input.prevout.n], prev_height, wtx.IsCoinBase());
}
std::map<int, bilingual_str> input_errors;
return SignTransaction(tx, coins, SIGHASH_DEFAULT, input_errors);
return SignTransaction(tx, coins, DefaultSighashType(chain().isSighashRangeproofActive()), input_errors); // ELEMENTS
}

bool CWallet::SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const
Expand Down
1 change: 1 addition & 0 deletions src/wallet/wallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
OutputType TransactionChangeType(const std::optional<OutputType>& change_type, const std::vector<CRecipient>& vecSend) const;

/** Fetch the inputs and sign with SIGHASH_ALL. */
// ELEMENTS: sign with SIGHASH_ALL_WITH_RANGEPROOF for DynaFed chains

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor nit: this comment seems to contradict the one above

bool SignTransaction(CMutableTransaction& tx) const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
/** Sign the tx given the input coins and sighash. */
bool SignTransaction(CMutableTransaction& tx, const std::map<COutPoint, Coin>& coins, int sighash, std::map<int, bilingual_str>& input_errors) const;
Expand Down
Loading
Loading