Fix RPC return errors for psbt and invalid rangeproofs

src/blind.cpp

@@ -546,7 +546,9 @@ int BlindTransaction(std::vector<uint256 >& 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<uint256 >& 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;
             }

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::map<uint32_t, st
             continue;
         }
 
+        // A rangeproof over a spendable script uses min_value = 1, so a zero
+        // amount cannot be proven. Reject.
+        if (*output.amount == 0 && !output.script->IsUnspendable()) {
+            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<uint32_t, st
 
         // Generate rangeproof
         bool rangeresult = CreateValueRangeProof(rangeproof, value_blinder, nonce, *output.amount, *output.script, value_commit, asset_generator, asset, asset_blinder);
-        assert(rangeresult);
+        if (!rangeresult) {
+            return BlindingStatus::RANGEPROOF_UNABLE;
+        }
 
         // Create explicit value rangeproof
         std::vector<unsigned char> 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)) {

src/blindpsbt.h

@@ -28,6 +28,8 @@ enum class BlindingStatus
     INVALID_BLINDER,
     ASP_UNABLE,
     NO_BLIND_OUTPUTS,
+    RANGEPROOF_UNABLE,
+    INVALID_AMOUNT,
 };
 
 enum class BlindProofResult {

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);

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
 

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")) {

src/kernel/chainparams.h

@@ -228,6 +228,9 @@ class CChainParams
     static std::unique_ptr<const CChainParams> TestNet();
     static std::unique_ptr<const CChainParams> TestNet4();
 
+    // ELEMENTS: Elements adds classes with their own members so the base pointer needs a virtual destructor.
+    virtual ~CChainParams() = default;
+
 protected:
     CChainParams() = default;
 

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<std::monostate, Sidechain::Bitcoin::CTransactionRef, CTransactionRef> tmp_tx;
+    std::variant<std::monostate, Sidechain::Bitcoin::CMerkleBlock, CMerkleBlock> 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;
 }

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;
+}

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

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<std::monostate, Sidechain::Bitcoin::CTransactionRef, CTransactionRef> peg_in_tx;
+            std::variant<std::monostate, Sidechain::Bitcoin::CMerkleBlock, CMerkleBlock> 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;
             }
         }
     }

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<unsigned char> 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<unsigned char> 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");
         }

src/test/blind_tests.cpp

@@ -4,6 +4,7 @@
 
 #include <arith_uint256.h>
 #include <blind.h>
+#include <blindpsbt.h>
 #include <coins.h>
 #include <random.h>
 #include <uint256.h>
@@ -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<unsigned char> 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<unsigned char*> value_blindptrs;
+    std::vector<const unsigned char*> asset_blindptrs;
+    value_blindptrs.push_back(const_cast<unsigned char*>(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()

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");
         }