Merge pull request #1405 from psgreco/elem-23.2.7rc2

Prepare 23.2.7 rc2

Author: delta1

.cirrus.yml

@@ -223,6 +223,7 @@ task:
   env:
     << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV
     FILE_ENV: "./ci/test/00_setup_env_native_tsan.sh"
+    MAKEJOBS: "-j2"  # Avoid excessive memory use due to MSan
 
 task:
   name: '[MSan, depends] [focal]'
@@ -232,7 +233,6 @@ task:
   env:
     << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV
     FILE_ENV: "./ci/test/00_setup_env_native_msan.sh"
-    MAKEJOBS: "-j4"  # Avoid excessive memory use due to MSan
 
 task:
   name: '[ASan + LSan + UBSan + integer, no depends] [jammy]'
@@ -244,7 +244,6 @@ task:
   env:
     << : *CIRRUS_EPHEMERAL_WORKER_TEMPLATE_ENV
     FILE_ENV: "./ci/test/00_setup_env_native_asan.sh"
-    MAKEJOBS: "-j4"  # Avoid excessive memory use
 
 task:
   name: '[fuzzer,address,undefined,integer, no depends] [jammy]'

configure.ac

@@ -2,7 +2,7 @@ AC_PREREQ([2.69])
 define(_CLIENT_VERSION_MAJOR, 23)
 define(_CLIENT_VERSION_MINOR, 2)
 define(_CLIENT_VERSION_BUILD, 7)
-define(_CLIENT_VERSION_RC, 1)
+define(_CLIENT_VERSION_RC, 2)
 define(_CLIENT_VERSION_IS_RELEASE, true)
 define(_COPYRIGHT_YEAR, 2025)
 define(_COPYRIGHT_HOLDERS,[The %s developers])

src/chain.h

@@ -469,7 +469,7 @@ class CDiskBlockIndex : public CBlockIndex
     bool RemoveDynaFedMaskOnSerialize(bool for_read) {
         if (for_read) {
             bool is_dyna = nVersion < 0;
-            nVersion = ~CBlockHeader::DYNAFED_HF_MASK & nVersion;
+            nVersion = (int32_t) (~CBlockHeader::DYNAFED_HF_MASK & (uint32_t)nVersion);
             return is_dyna;
         } else {
             return is_dynafed_block();

src/chainparamsbase.h

@@ -5,6 +5,7 @@
 #ifndef BITCOIN_CHAINPARAMSBASE_H
 #define BITCOIN_CHAINPARAMSBASE_H
 
+#include <cstdint>
 #include <memory>
 #include <string>
 

src/node/miner.h

@@ -46,7 +46,7 @@ struct CTxMemPoolModifiedEntry {
         nSigOpCostWithAncestors = entry->GetSigOpCostWithAncestors();
     }
 
-    int64_t GetModifiedFee() const { return iter->GetModifiedFee(); }
+    CAmount GetModifiedFee() const { return iter->GetModifiedFee(); }
     uint64_t GetSizeWithAncestors() const { return nSizeWithAncestors; }
     uint64_t GetDiscountSizeWithAncestors() const { return discountSizeWithAncestors; }
     CAmount GetModFeesWithAncestors() const { return nModFeesWithAncestors; }

src/node/ui_interface.h

@@ -6,9 +6,10 @@
 #ifndef BITCOIN_NODE_UI_INTERFACE_H
 #define BITCOIN_NODE_UI_INTERFACE_H
 
+#include <cstdint>
 #include <functional>
-#include <memory>
 #include <string>
+#include <vector>
 
 class CBlockIndex;
 enum class SynchronizationState;

src/primitives/confidential.h

@@ -135,6 +135,11 @@ class CConfidentialValue : public CConfidentialCommitment<9, 8, 9>
     CConfidentialValue() { SetNull(); }
     CConfidentialValue(CAmount nAmount) { SetToAmount(nAmount); }
 
+    template <typename Stream>
+    inline void Unserialize(Stream& s) {
+        CConfidentialCommitment::Unserialize(s);
+    }
+
     /* An explicit value is called an amount. The first byte indicates it is
      * an explicit value, and the remaining 8 bytes is the value serialized as
      * a 64-bit big-endian integer. */

src/primitives/transaction.h

@@ -295,6 +295,9 @@ class CTxOut
             s >> nAsset;
             s >> nValue;
             s >> nNonce;
+            if (nAsset.IsNull() || nValue.IsNull()) {
+                throw std::ios_base::failure("Confidential values may not be null");
+            }
         } else {
             CAmount value;
             s >> value;

src/rpc/mining.cpp

@@ -1527,6 +1527,10 @@ static RPCHelpMan getcompactsketch()
     CDataStream ssBlock(block_bytes, SER_NETWORK, PROTOCOL_VERSION);
     ssBlock >> block;
 
+    if (block.vtx.empty()) {
+        throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Cannot obtain sketch of empty block.");
+    }
+
     CBlockHeaderAndShortTxIDs cmpctblock(block, true);
 
     CDataStream ssCompactBlock(SER_NETWORK, PROTOCOL_VERSION);

src/rpc/rawtransaction.cpp

@@ -1476,6 +1476,11 @@ static RPCHelpMan decodepsbt()
             } else {
                 out.pushKV("amountcommitment", txout.nValue.GetHex());
             }
+            if (txout.nAsset.IsExplicit()) {
+                out.pushKV("asset", txout.nAsset.GetAsset().GetHex());
+            } else {
+                out.pushKV("assetcommitment", txout.nAsset.GetHex());
+            }
             out.pushKV("scriptPubKey", o);
 
             in.pushKV("witness_utxo", out);

src/test/fuzz/rbf.cpp

@@ -15,7 +15,13 @@
 #include <string>
 #include <vector>
 
-FUZZ_TARGET(rbf)
+void initialize_rbf(void) {
+    // ELEMENTS: our mempool needs Params() to be set for multiple reasons -- to check
+    //  the discount CT rate, to figure out pegin policy, etc
+    SelectParams(CBaseChainParams::LIQUID1);
+}
+
+FUZZ_TARGET_INIT(rbf, initialize_rbf)
 {
     FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
     SetMockTime(ConsumeTime(fuzzed_data_provider));

src/test/fuzz/witness_program.cpp

@@ -45,7 +45,7 @@ FUZZ_TARGET_INIT(witness_program, initialize_witness_program)
 
         CScriptWitness witness;
         int fuzz_control;
-        int flags;
+        unsigned flags;
         ds >> fuzz_control;
         ds >> witness.stack;
         ds >> flags;
@@ -64,7 +64,7 @@ FUZZ_TARGET_INIT(witness_program, initialize_witness_program)
 
         if (fuzz_control & 1) {
             unsigned char hash_program[32];
-            CSHA256().Write(&program[0], program.size()).Finalize(hash_program);
+            CSHA256().Write(program.data(), program.size()).Finalize(hash_program);
             CScript scriptPubKey = CScript{} << OP_0 << std::vector<unsigned char>(hash_program, hash_program + sizeof(hash_program));
             witness.stack.push_back(program);
 

src/txmempool.cpp

@@ -18,7 +18,9 @@
 #include <policy/policy.h>
 #include <policy/settings.h>
 #include <reverse_iterator.h>
+#include <util/check.h>
 #include <util/moneystr.h>
+#include <util/overflow.h>
 #include <util/system.h>
 #include <util/time.h>
 #include <validationinterface.h>
@@ -60,16 +62,6 @@ struct update_ancestor_state
         int64_t discountSize;
 };
 
-struct update_fee_delta
-{
-    explicit update_fee_delta(int64_t _feeDelta) : feeDelta(_feeDelta) { }
-
-    void operator() (CTxMemPoolEntry &e) { e.UpdateFeeDelta(feeDelta); }
-
-private:
-    int64_t feeDelta;
-};
-
 bool TestLockPointValidity(CChain& active_chain, const LockPoints& lp)
 {
     AssertLockHeld(cs_main);
@@ -99,6 +91,7 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee,
       entryHeight{entry_height},
       spendsCoinbase{spends_coinbase},
       sigOpCost{sigops_cost},
+      m_modified_fee{nFee},
       lockPoints{lp},
       nSizeWithDescendants{GetTxSize()},
       nModFeesWithDescendants{nFee},
@@ -108,11 +101,11 @@ CTxMemPoolEntry::CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee,
       discountSizeWithAncestors{GetDiscountTxSize()},
       setPeginsSpent(_setPeginsSpent) {}
 
-void CTxMemPoolEntry::UpdateFeeDelta(int64_t newFeeDelta)
+void CTxMemPoolEntry::UpdateModifiedFee(CAmount fee_diff)
 {
-    nModFeesWithDescendants += newFeeDelta - feeDelta;
-    nModFeesWithAncestors += newFeeDelta - feeDelta;
-    feeDelta = newFeeDelta;
+    nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, fee_diff);
+    nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, fee_diff);
+    m_modified_fee = SaturatingAdd(m_modified_fee, fee_diff);
 }
 
 void CTxMemPoolEntry::UpdateLockPoints(const LockPoints& lp)
@@ -467,7 +460,7 @@ void CTxMemPoolEntry::UpdateDescendantState(int64_t modifySize, CAmount modifyFe
 {
     nSizeWithDescendants += modifySize;
     assert(int64_t(nSizeWithDescendants) > 0);
-    nModFeesWithDescendants += modifyFee;
+    nModFeesWithDescendants = SaturatingAdd(nModFeesWithDescendants, modifyFee);
     nCountWithDescendants += modifyCount;
     assert(int64_t(nCountWithDescendants) > 0);
 }
@@ -476,7 +469,7 @@ void CTxMemPoolEntry::UpdateAncestorState(int64_t modifySize, CAmount modifyFee,
 {
     nSizeWithAncestors += modifySize;
     assert(int64_t(nSizeWithAncestors) > 0);
-    nModFeesWithAncestors += modifyFee;
+    nModFeesWithAncestors = SaturatingAdd(nModFeesWithAncestors, modifyFee);
     nCountWithAncestors += modifyCount;
     assert(int64_t(nCountWithAncestors) > 0);
     nSigOpCostWithAncestors += modifySigOps;
@@ -519,8 +512,10 @@ void CTxMemPool::addUnchecked(const CTxMemPoolEntry &entry, setEntries &setAnces
     // into mapTx.
     CAmount delta{0};
     ApplyDelta(entry.GetTx().GetHash(), delta);
+    // The following call to UpdateModifiedFee assumes no previous fee modifications
+    Assume(entry.GetFee() == entry.GetModifiedFee());
     if (delta) {
-            mapTx.modify(newit, update_fee_delta(delta));
+        mapTx.modify(newit, [&delta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(delta); });
     }
 
     // Update cachedInnerUsage to include contained transaction's usage.
@@ -1029,10 +1024,10 @@ void CTxMemPool::PrioritiseTransaction(const uint256& hash, const CAmount& nFeeD
     {
         LOCK(cs);
         CAmount &delta = mapDeltas[hash];
-        delta += nFeeDelta;
+        delta = SaturatingAdd(delta, nFeeDelta);
         txiter it = mapTx.find(hash);
         if (it != mapTx.end()) {
-            mapTx.modify(it, update_fee_delta(delta));
+            mapTx.modify(it, [&nFeeDelta](CTxMemPoolEntry& e) { e.UpdateModifiedFee(nFeeDelta); });
             // Now update all ancestors' modified fees with descendants
             setEntries setAncestors;
             uint64_t nNoLimit = std::numeric_limits<uint64_t>::max();

src/txmempool.h

@@ -102,7 +102,7 @@ class CTxMemPoolEntry
     const unsigned int entryHeight; //!< Chain height when entering the mempool
     const bool spendsCoinbase;      //!< keep track of transactions that spend a coinbase
     const int64_t sigOpCost;        //!< Total sigop cost
-    int64_t feeDelta{0};            //!< Used for determining the priority of the transaction for mining in a block
+    CAmount m_modified_fee;         //!< Used for determining the priority of the transaction for mining in a block
     LockPoints lockPoints;     //!< Track the height and time at which tx was final
 
     // Information about descendants of this transaction that are in the
@@ -135,17 +135,16 @@ class CTxMemPoolEntry
     std::chrono::seconds GetTime() const { return std::chrono::seconds{nTime}; }
     unsigned int GetHeight() const { return entryHeight; }
     int64_t GetSigOpCost() const { return sigOpCost; }
-    int64_t GetModifiedFee() const { return nFee + feeDelta; }
+    CAmount GetModifiedFee() const { return m_modified_fee; }
     size_t DynamicMemoryUsage() const { return nUsageSize; }
     const LockPoints& GetLockPoints() const { return lockPoints; }
 
     // Adjusts the descendant state.
     void UpdateDescendantState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount);
     // Adjusts the ancestor state
     void UpdateAncestorState(int64_t modifySize, CAmount modifyFee, int64_t modifyCount, int64_t modifySigOps, int64_t discountSize);
-    // Updates the fee delta used for mining priority score, and the
-    // modified fees with descendants.
-    void UpdateFeeDelta(int64_t feeDelta);
+    // Updates the modified fees with descendants/ancestors.
+    void UpdateModifiedFee(CAmount fee_diff);
     // Update the LockPoints after a reorg
     void UpdateLockPoints(const LockPoints& lp);
 

src/zmq/zmqabstractnotifier.h

@@ -6,6 +6,7 @@
 #define BITCOIN_ZMQ_ZMQABSTRACTNOTIFIER_H
 
 
+#include <cstdint>
 #include <memory>
 #include <string>
 

src/zmq/zmqpublishnotifier.h

@@ -7,6 +7,8 @@
 
 #include <zmq/zmqabstractnotifier.h>
 
+#include <cstdint>
+
 class CBlockIndex;
 
 class CZMQAbstractPublishNotifier : public CZMQAbstractNotifier

test/functional/rpc_psbt.py

@@ -261,6 +261,10 @@ def run_basic_tests(self, confidential):
         decoded = self.nodes[1].decodepsbt(walletsignpsbt_out['psbt'])
         assert 'non_witness_utxo' in decoded['inputs'][0]
         assert 'witness_utxo' in decoded['inputs'][0]
+        if 'asset' in decoded['inputs'][0]['witness_utxo']:
+            assert_equal(decoded['inputs'][0]['witness_utxo']['asset'], 'b2e15d0d7a0c94e4e2ce0fe6e8691b9e451377f6e46e8045a86f7c4b5d4f0f23')
+        else:
+            assert 'assetcommitment' in decoded['inputs'][0]['witness_utxo']
         # Check decodepsbt fee calculation (input values shall only be counted once per UTXO)
         #assert_equal(decoded['fee'], created_psbt['fee']) # ELEMENTS: we do not have this field. Should be fixed by #900
         assert_equal(walletsignpsbt_out['complete'], True)

test/sanitizer_suppressions/ubsan

@@ -1,10 +1,10 @@
 # -fsanitize=undefined suppressions
 # =================================
-# This would be `signed-integer-overflow:CTxMemPool::PrioritiseTransaction`,
+# The suppressions would be `sanitize-type:ClassName::MethodName`,
 # however due to a bug in clang the symbolizer is disabled and thus no symbol
 # names can be used.
 # See https://github.com/google/sanitizers/issues/1364
-signed-integer-overflow:txmempool.cpp
+
 # https://github.com/bitcoin/bitcoin/pull/21798#issuecomment-829180719
 signed-integer-overflow:policy/feerate.cpp