blind/blindpsbt fixes

src/blind.cpp

@@ -206,9 +206,12 @@ bool SurjectOutput(CTxOutWitness& txoutwit, const std::vector<secp256k1_fixed_as
     //  with more than 256 inputs. The Elements verification code will always try to give
     //  secp-zkp the complete list of inputs, and if this exceeds 256 then surjectionproof_verify
     //  will always return false, so there is no way to work around this situation at signing time
-    if (surjection_targets.size() > 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

src/blindpsbt.cpp

@@ -52,10 +52,17 @@ bool CreateAssetSurjectionProof(std::vector<unsigned char>& 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);
@@ -189,7 +196,11 @@ bool CreateBlindAssetProof(std::vector<unsigned char>& assetproof, const CAsset&
 
 bool VerifyBlindValueProof(CAmount value, const CConfidentialValue& conf_value, const std::vector<unsigned char>& 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;
     }
 
@@ -208,7 +219,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) {
@@ -500,6 +515,14 @@ BlindingStatus BlindPSBT(PartiallySignedTransaction& psbt, std::map<uint32_t, st
         // Check this is our output to blind
         if (output.m_blinder_index == std::nullopt || our_input_data.count(*output.m_blinder_index) == 0) continue;
 
+        // PSET v0 does not require an output amount (it is only enforced for
+        // m_psbt_version >= 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;
@@ -555,6 +578,13 @@ BlindingStatus BlindPSBT(PartiallySignedTransaction& psbt, std::map<uint32_t, st
         CreateValueCommitment(value_commitment, value_commit, value_blinder, asset_generator, *output.amount);
 
         // Generate rangproof nonce
+        if (!output.m_blinding_pubkey.IsFullyValid()) {
+            // An attacker-controlled (off-curve) blinding pubkey would otherwise
+            // reach CKey::ECDH, whose only validation is an assert on the peer
+            // key, aborting the process. The non-PSET path (blind.cpp) requires
+            // IsFullyValid() before ECDH; mirror it here.
+            return BlindingStatus::INVALID_BLINDER;
+        }
         uint256 nonce = GenerateRangeproofECDHKey(ecdh_key, output.m_blinding_pubkey);
 
         // Generate rangeproof

src/chain.h

@@ -315,7 +315,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;
@@ -540,7 +543,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;

src/dynafed.cpp

@@ -14,6 +14,10 @@ bool NextBlockIsParameterTransition(const CBlockIndex* pindexPrev, const Consens
     }
     std::map<uint256, uint32_t> 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;
         }

src/headerssync.cpp

@@ -19,9 +19,15 @@ constexpr size_t HEADER_COMMITMENT_PERIOD{624};
 //! received and validated against commitments.
 constexpr size_t REDOWNLOAD_BUFFER_SIZE{14827}; // 14827/624 = ~23.8 commitments
 
-// Our memory analysis assumes 48 bytes for a CompressedHeader (so we should
-// re-calculate parameters if we compress further)
-static_assert(sizeof(CompressedHeader) == 48);
+// NOTE (ELEMENTS): The upstream Bitcoin memory analysis assumed 48 bytes for
+// a CompressedHeader, which holds only the PoW fields. Elements must retain
+// the identity/proof fields (block_height, proof, dynafed params, signblock
+// witness) so that signed/dynafed headers can be reconstructed faithfully, so
+// CompressedHeader is now larger than 48 bytes. The redownload buffer is
+// bounded by REDOWNLOAD_BUFFER_SIZE headers per peer, so the per-peer memory
+// cost is REDOWNLOAD_BUFFER_SIZE * sizeof(CompressedHeader); this remains
+// small but should be reconsidered if REDOWNLOAD_BUFFER_SIZE is ever raised.
+static_assert(sizeof(CompressedHeader) <= 512);
 
 HeadersSyncState::HeadersSyncState(NodeId id, const Consensus::Params& consensus_params,
         const CBlockIndex* chain_start, const arith_uint256& minimum_required_work) :

src/headerssync.h

@@ -25,6 +25,15 @@ struct CompressedHeader {
     uint32_t nTime{0};
     uint32_t nBits{0};
     uint32_t nNonce{0};
+    // ELEMENTS: fields needed to faithfully reconstruct signed/dynafed
+    // headers. These participate in the header's identity (block_height,
+    // dynafed params, proof challenge) or are required for downstream
+    // validation (proof solution, signblock witness), so they must be retained
+    // across the headers-sync presync/redownload path.
+    uint32_t block_height{0};
+    CProof proof;
+    DynaFedParams m_dynafed_params;
+    CScriptWitness m_signblock_witness;
 
     CompressedHeader()
     {
@@ -38,6 +47,10 @@ struct CompressedHeader {
         nTime = header.nTime;
         nBits = header.nBits;
         nNonce = header.nNonce;
+        block_height = header.block_height;
+        proof = header.proof;
+        m_dynafed_params = header.m_dynafed_params;
+        m_signblock_witness = header.m_signblock_witness;
     }
 
     CBlockHeader GetFullHeader(const uint256& hash_prev_block) {
@@ -46,8 +59,12 @@ struct CompressedHeader {
         ret.hashPrevBlock = hash_prev_block;
         ret.hashMerkleRoot = hashMerkleRoot;
         ret.nTime = nTime;
+        ret.block_height = block_height;
         ret.nBits = nBits;
         ret.nNonce = nNonce;
+        ret.proof = proof;
+        ret.m_dynafed_params = m_dynafed_params;
+        ret.m_signblock_witness = m_signblock_witness;
         return ret;
     };
 };

src/script/sigcache.cpp

@@ -55,9 +55,9 @@ void SignatureCache::ComputeEntrySchnorr(uint256& entry, const uint256& hash, Sp
 }
 
 // ELEMENTS:
-void SignatureCache::ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const {
+void SignatureCache::ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<unsigned char>& asset_commitment, const CScript& scriptPubKey) const {
     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 SignatureCache::ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const {
     CSHA256 hasher = m_salted_hasher_surjection_proof;
@@ -131,7 +131,7 @@ bool InitSurjectionproofCache(size_t max_size_bytes)
 bool CachingRangeProofChecker::VerifyRangeProof(const std::vector<unsigned char>& vchRangeProof, const std::vector<unsigned char>& vchValueCommitment, const std::vector<unsigned char>& 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;

src/script/sigcache.h

@@ -84,7 +84,7 @@ class SignatureCache
     void ComputeEntrySchnorr(uint256& entry, const uint256 &hash, Span<const unsigned char> sig, const XOnlyPubKey& pubkey) const;
 
     // ELEMENTS:
-    void ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const;
+    void ComputeEntryRangeProof(uint256& entry, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment, const std::vector<unsigned char>& asset_commitment, const CScript& scriptPubKey) const;
 
     void ComputeEntrySurjectionProof(uint256& entry, const uint256 &hash, const std::vector<unsigned char>& proof, const std::vector<unsigned char>& commitment) const;
 

src/validation.cpp

@@ -4845,7 +4845,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");
     }