diff --git a/src/blind.cpp b/src/blind.cpp index 5f7c1333bd..a871df787a 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 1e55f4883a..93b76c5e3f 100644 --- a/src/blindpsbt.cpp +++ b/src/blindpsbt.cpp @@ -30,6 +30,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); } @@ -497,6 +501,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; @@ -559,12 +569,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 d79e4e77d4..0eaf1a582e 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/common/messages.cpp b/src/common/messages.cpp index 85992d6f1d..cf8fa06dd5 100644 --- a/src/common/messages.cpp +++ b/src/common/messages.cpp @@ -130,6 +130,8 @@ bilingual_str PSBTErrorString(PSBTError err) return Untranslated("Wallet does not have necessary blinding key"); case PSBTError::MISSING_SIDECHANNEL_DATA: return Untranslated("A rangeproof did not encode necessary blinding data"); + case PSBTError::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/common/types.h b/src/common/types.h index fc2fddace3..aeba7ea003 100644 --- a/src/common/types.h +++ b/src/common/types.h @@ -27,6 +27,7 @@ enum class PSBTError { INVALID_ASSET_PROOF, MISSING_BLINDING_KEY, MISSING_SIDECHANNEL_DATA, + MISSING_EXPLICIT_OUTPUT_DATA, }; } // namespace common diff --git a/src/init.cpp b/src/init.cpp index c54ff39b11..37ac88a15d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1104,6 +1104,13 @@ bool AppInitParameterInteraction(const ArgsManager& args) return InitError(Untranslated("peertimeout must be a positive integer.")); } + 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/kernel/chainparams.h b/src/kernel/chainparams.h index daab37858c..1423fa5400 100644 --- a/src/kernel/chainparams.h +++ b/src/kernel/chainparams.h @@ -228,6 +228,9 @@ class CChainParams static std::unique_ptr TestNet(); static std::unique_ptr TestNet4(); + // ELEMENTS: Elements adds classes with their own members so the base pointer needs a virtual destructor. + virtual ~CChainParams() = default; + protected: CChainParams() = default; diff --git a/src/pegins.cpp b/src/pegins.cpp index 6ac209408a..b09677faec 100644 --- a/src/pegins.cpp +++ b/src/pegins.cpp @@ -551,40 +551,55 @@ bool DecomposePeginWitness(const CScriptWitness& witness, CAmount& value, CAsset const auto& stack = witness.stack; if (stack.size() != 6) return false; + if (stack[1].size() != 32) return false; // asset + if (stack[2].size() != 32) return false; // parent genesis hash - DataStream stream{stack[0]}; - 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; + CAmount tmp_value{0}; + CAsset tmp_asset; + uint256 tmp_genesis_hash; + CScript tmp_claim_script; + std::variant tmp_tx; + std::variant tmp_merkle_block; - DataStream ss_tx(stack[4]); - if (Params().GetConsensus().ParentChainHasPow()) { - Sidechain::Bitcoin::CTransactionRef btc_tx; - ss_tx >> TX_WITH_WITNESS(btc_tx); - tx = btc_tx; - } else { - CTransactionRef elem_tx; - ss_tx >> TX_WITH_WITNESS(elem_tx); - tx = elem_tx; - } + try { + DataStream stream{stack[0]}; + stream >> tmp_value; + + tmp_asset = CAsset(stack[1]); + tmp_genesis_hash = uint256(stack[2]); + tmp_claim_script = CScript(stack[3].begin(), stack[3].end()); + + DataStream ss_tx(stack[4]); + if (Params().GetConsensus().ParentChainHasPow()) { + Sidechain::Bitcoin::CTransactionRef btc_tx; + ss_tx >> TX_WITH_WITNESS(btc_tx); + tmp_tx = btc_tx; + } else { + CTransactionRef elem_tx; + ss_tx >> TX_WITH_WITNESS(elem_tx); + tmp_tx = elem_tx; + } - DataStream ss_proof(stack[5]); - if (Params().GetConsensus().ParentChainHasPow()) { - Sidechain::Bitcoin::CMerkleBlock tx_proof; - ss_proof >> TX_WITH_WITNESS(tx_proof); - merkle_block = tx_proof; - } else { - CMerkleBlock tx_proof; - ss_proof >> TX_WITH_WITNESS(tx_proof); - merkle_block = tx_proof; + DataStream ss_proof(stack[5]); + if (Params().GetConsensus().ParentChainHasPow()) { + Sidechain::Bitcoin::CMerkleBlock tx_proof; + ss_proof >> TX_WITH_WITNESS(tx_proof); + tmp_merkle_block = tx_proof; + } else { + CMerkleBlock tx_proof; + ss_proof >> TX_WITH_WITNESS(tx_proof); + tmp_merkle_block = tx_proof; + } + } catch (const std::exception&) { + // Malformed encoding. Report failure rather than propagating + 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 9f553b13ce..1d421d9477 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 3cadba783a..13c064f43c 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 c0241c2faf..79232d2c79 100644 --- a/src/psbt.cpp +++ b/src/psbt.cpp @@ -1022,12 +1022,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/node.cpp b/src/rpc/node.cpp index 5e8b77b856..9ed83ce2db 100644 --- a/src/rpc/node.cpp +++ b/src/rpc/node.cpp @@ -410,6 +410,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", @@ -441,6 +459,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 42dded0ddc..1023338f29 100644 --- a/src/test/blind_tests.cpp +++ b/src/test/blind_tests.cpp @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -372,4 +373,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/validation.cpp b/src/validation.cpp index 79789aae3b..947d16910d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -947,6 +947,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 52b7abd704..86eebe0d64 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2475,6 +2475,13 @@ std::optional CWallet::SignPSBT(PartiallySignedTransaction& psbtx, bo } 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 PSBTError::MISSING_EXPLICIT_OUTPUT_DATA; + } CKey blinding_key; if ((blinding_key = GetBlindingKey(&*o.script)).IsValid()) { CAmount value; @@ -2485,14 +2492,15 @@ std::optional CWallet::SignPSBT(PartiallySignedTransaction& psbtx, bo 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 PSBTError::INVALID_VALUE_PROOF; } - if (!o.m_asset.IsNull()) { - assert(CAsset(o.m_asset) == asset); + if (CAsset(o.m_asset) != asset) { + return PSBTError::INVALID_ASSET_PROOF; } } else { return PSBTError::MISSING_SIDECHANNEL_DATA;