fix: resolve compiler warnings, suppress errors on -Warray-bounds and -Wdangling-reference, re-enable -Werror for GCC

ci/test/00_setup_env_arm.sh

@@ -26,4 +26,3 @@ export GOAL="install"
 # -Wno-psabi is to disable ABI warnings: "note: parameter passing for argument of type ... changed in GCC 7.1"
 # This could be removed once the ABI change warning does not show up by default
 export BITCOIN_CONFIG="--enable-reduce-exports CXXFLAGS=-Wno-psabi --with-boost-process"
-export NO_WERROR=1

ci/test/00_setup_env_native_nowallet.sh

@@ -12,4 +12,3 @@ export PACKAGES="python3-zmq"
 export DEP_OPTS="NO_WALLET=1 CC=gcc-14 CXX=g++-14"
 export GOAL="install"
 export BITCOIN_CONFIG="--enable-reduce-exports --with-boost-process CC=gcc-14 CXX=g++-14"
-export NO_WERROR=1

ci/test/00_setup_env_native_qt5.sh

@@ -16,4 +16,3 @@ export RUN_UNIT_TESTS="false"
 export GOAL="install"
 export PREVIOUS_RELEASES_TO_DOWNLOAD="v0.15.0.0 v0.16.1.1 v0.17.0.3 v18.2.2 v19.3.0 v20.0.1"
 export BITCOIN_CONFIG="--enable-zmq --with-libs=no --enable-reduce-exports --disable-fuzz-binary LDFLAGS=-static-libstdc++ --with-boost-process"
-export NO_WERROR=1

ci/test/00_setup_env_native_sqlite.sh

@@ -11,4 +11,3 @@ export PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libdbus-1-dev libhar
 export DEP_OPTS="NO_BDB=1 NO_UPNP=1 DEBUG=1"
 export GOAL="install"
 export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --with-sqlite --without-bdb CC=gcc-11 CXX=g++-11"
-export NO_WERROR=1

ci/test/00_setup_env_win64.sh

@@ -18,4 +18,3 @@ export GOAL="deploy"
 # https://github.com/mingw-w64/mingw-w64/commit/1690994f515910a31b9fb7c7bd3a52d4ba987abe
 export BITCOIN_CONFIG="--enable-gui --enable-reduce-exports --disable-miner --without-boost-process CXXFLAGS='-Wno-return-type -Wno-error=maybe-uninitialized -Wno-error=array-bounds'"
 export DIRECT_WINE_EXEC_TESTS=true
-export NO_WERROR=1

configure.ac

@@ -452,9 +452,45 @@ if test "$enable_werror" = "yes"; then
   fi
   ERROR_CXXFLAGS=$CXXFLAG_WERROR
 
-  dnl -Wstringop-overread and -Wstringop-overflow are broken in gcc
-  AX_CHECK_COMPILE_FLAG([-Wstringop-overread], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wno-stringop-overread"], [], [$CXXFLAG_WERROR])
-  AX_CHECK_COMPILE_FLAG([-Wstringop-overflow], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wno-stringop-overflow"], [], [$CXXFLAG_WERROR])
+  dnl -Warray-bounds and -Wdangling-reference cause problems with GCC. Do not treat these warnings as errors.
+  dnl Suppress -Warray-bounds entirely because of noisy output, currently unhappy with immer implementation.
+  AX_CHECK_COMPILE_FLAG([-Warray-bounds], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-array-bounds"], [], [$CXXFLAG_WERROR], [AC_LANG_SOURCE([
+    #if defined(__clang__) || defined(__INTEL_COMPILER) || !defined(__GNUC__)
+      #error Non-GCC compiler detected, not setting flag
+    #endif
+    int main(void) { return 0; }
+  ])])
+  dnl TODO: Remove suppression after backporting bitcoin#27605
+  AX_CHECK_COMPILE_FLAG([-Wdangling-reference], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-error=dangling-reference"], [], [$CXXFLAG_WERROR], [AC_LANG_SOURCE([
+    #if defined(__clang__) || defined(__INTEL_COMPILER) || !defined(__GNUC__)
+      #error Non-GCC compiler detected, not setting flag
+    #endif
+    int main(void) { return 0; }
+  ])])
+
+  dnl -Wattributes cause problems with some versions of GCC. Do not treat these warnings as errors.
+  AX_CHECK_COMPILE_FLAG([-Wattributes], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-error=attributes"], [], [$CXXFLAG_WERROR], [AC_LANG_SOURCE([
+    #if defined(__clang__) || defined(__INTEL_COMPILER) || !defined(__GNUC__)
+      #error Non-GCC compiler detected, not setting flag
+    #elif defined(__GNUC__) && __GNUC__ >= 14
+      #error GCC >=14 detected, not setting flag
+    #endif
+    int main(void) { return 0; }
+  ])])
+
+  dnl -Wstringop-overread and -Wstringop-overflow are broken in GCC. Suppress warnings entirely to avoid noisy output.
+  AX_CHECK_COMPILE_FLAG([-Wstringop-overread], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-stringop-overread"], [], [$CXXFLAG_WERROR], [AC_LANG_SOURCE([
+    #if defined(__clang__) || defined(__INTEL_COMPILER) || !defined(__GNUC__)
+      #error Non-GCC compiler detected, not setting flag
+    #endif
+    int main(void) { return 0; }
+  ])])
+  AX_CHECK_COMPILE_FLAG([-Wstringop-overflow], [NOWARN_CXXFLAGS="$NOWARN_CXXFLAGS -Wno-stringop-overflow"], [], [$CXXFLAG_WERROR], [AC_LANG_SOURCE([
+    #if defined(__clang__) || defined(__INTEL_COMPILER) || !defined(__GNUC__)
+      #error Non-GCC compiler detected, not setting flag
+    #endif
+    int main(void) { return 0; }
+  ])])
 fi
 
 AX_CHECK_COMPILE_FLAG([-Wall], [WARN_CXXFLAGS="$WARN_CXXFLAGS -Wall"], [], [$CXXFLAG_WERROR])

src/Makefile.am

@@ -773,6 +773,7 @@ libbitcoin_common_a_SOURCES = \
   core_read.cpp \
   core_write.cpp \
   deploymentinfo.cpp \
+  evo/core_write.cpp \
   governance/common.cpp \
   init/common.cpp \
   key.cpp \

src/coinjoin/client.cpp

@@ -4,6 +4,7 @@
 
 #include <coinjoin/client.h>
 
+#include <chain.h>
 #include <chainparams.h>
 #include <coinjoin/options.h>
 #include <consensus/validation.h>
@@ -21,7 +22,6 @@
 #include <util/ranges.h>
 #include <util/system.h>
 #include <util/translation.h>
-#include <validation.h>
 #include <version.h>
 #include <wallet/coincontrol.h>
 #include <wallet/fees.h>

src/coinjoin/util.cpp

@@ -7,7 +7,6 @@
 #include <policy/fees.h>
 #include <policy/policy.h>
 #include <script/sign.h>
-#include <validation.h>
 #include <wallet/fees.h>
 #include <wallet/wallet.h>
 #include <util/translation.h>

src/evo/assetlocktx.cpp

@@ -14,9 +14,9 @@
 #include <consensus/validation.h>
 #include <deploymentstatus.h>
 #include <logging.h>
+#include <node/blockstorage.h>
 #include <tinyformat.h>
 #include <util/ranges_set.h>
-#include <validation.h>
 
 #include <algorithm>
 

src/evo/assetlocktx.h

@@ -22,10 +22,6 @@ namespace llmq {
 class CQuorumManager;
 } // namespace llmq
 
-// Forward declaration from core_io to get rid of circular dependency
-UniValue ValueFromAmount(const CAmount amount);
-void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool fIncludeHex, bool include_addresses);
-
 class CAssetLockPayload
 {
 public:
@@ -53,23 +49,7 @@ class CAssetLockPayload
 
     std::string ToString() const;
 
-    [[nodiscard]] UniValue ToJson() const
-    {
-        UniValue outputs(UniValue::VARR);
-        for (const CTxOut& credit_output : creditOutputs) {
-            UniValue out(UniValue::VOBJ);
-            out.pushKV("value", ValueFromAmount(credit_output.nValue));
-            out.pushKV("valueSat", credit_output.nValue);
-            UniValue spk(UniValue::VOBJ);
-            ScriptPubKeyToUniv(credit_output.scriptPubKey, spk, /* fIncludeHex = */ true, /* include_addresses = */ false);
-            out.pushKV("scriptPubKey", spk);
-            outputs.push_back(out);
-        }
-        UniValue obj(UniValue::VOBJ);
-        obj.pushKV("version", int(nVersion));
-        obj.pushKV("creditOutputs", outputs);
-        return obj;
-    }
+    [[nodiscard]] UniValue ToJson() const;
 
     // getters
     uint8_t getVersion() const
@@ -126,18 +106,7 @@ class CAssetUnlockPayload
 
     std::string ToString() const;
 
-    [[nodiscard]] UniValue ToJson() const
-    {
-        UniValue obj;
-        obj.setObject();
-        obj.pushKV("version", int(nVersion));
-        obj.pushKV("index", int(index));
-        obj.pushKV("fee", int(fee));
-        obj.pushKV("requestedHeight", int(requestedHeight));
-        obj.pushKV("quorumHash", quorumHash.ToString());
-        obj.pushKV("quorumSig", quorumSig.ToString());
-        return obj;
-    }
+    [[nodiscard]] UniValue ToJson() const;
 
     bool VerifySig(const llmq::CQuorumManager& qman, const uint256& msgHash, gsl::not_null<const CBlockIndex*> pindexTip, TxValidationState& state) const;
 

src/evo/cbtx.cpp

@@ -18,7 +18,6 @@
 #include <chainparams.h>
 #include <consensus/merkle.h>
 #include <deploymentstatus.h>
-#include <validation.h>
 
 
 bool CheckCbTx(const CTransaction& tx, const CBlockIndex* pindexPrev, TxValidationState& state)

src/evo/cbtx.h

@@ -24,9 +24,6 @@ class CQuorumBlockProcessor;
 class CQuorumSnapshotManager;
 }// namespace llmq
 
-// Forward declaration from core_io to get rid of circular dependency
-UniValue ValueFromAmount(const CAmount amount);
-
 // coinbase transaction
 class CCbTx
 {
@@ -65,23 +62,7 @@ class CCbTx
 
     std::string ToString() const;
 
-    [[nodiscard]] UniValue ToJson() const
-    {
-        UniValue obj;
-        obj.setObject();
-        obj.pushKV("version", (int)nVersion);
-        obj.pushKV("height", nHeight);
-        obj.pushKV("merkleRootMNList", merkleRootMNList.ToString());
-        if (nVersion >= Version::MERKLE_ROOT_QUORUMS) {
-            obj.pushKV("merkleRootQuorums", merkleRootQuorums.ToString());
-            if (nVersion >= Version::CLSIG_AND_BALANCE) {
-                obj.pushKV("bestCLHeightDiff", static_cast<int>(bestCLHeightDiff));
-                obj.pushKV("bestCLSignature", bestCLSignature.ToString());
-                obj.pushKV("creditPoolBalance", ValueFromAmount(creditPoolBalance));
-            }
-        }
-        return obj;
-    }
+    [[nodiscard]] UniValue ToJson() const;
 };
 template<> struct is_serializable_enum<CCbTx::Version> : std::true_type {};
 

src/evo/chainhelper.cpp

@@ -6,9 +6,12 @@
 
 #include <consensus/params.h>
 #include <evo/specialtxman.h>
+#include <index/txindex.h>
 #include <llmq/chainlocks.h>
 #include <llmq/instantsend.h>
 #include <masternode/payments.h>
+#include <node/transaction.h>
+#include <txmempool.h>
 
 CChainstateHelper::CChainstateHelper(CCreditPoolManager& cpoolman, CDeterministicMNManager& dmnman,
                                      CMNHFManager& mnhfman, CGovernanceManager& govman, llmq::CInstantSendManager& isman,
@@ -58,3 +61,10 @@ bool CChainstateHelper::RemoveConflictingISLockByTx(const CTransaction& tx)
 }
 
 bool CChainstateHelper::ShouldInstantSendRejectConflicts() const { return isman.RejectConflictingBlocks(); }
+
+std::pair<CTransactionRef, uint256> GetTransactionBlock(const uint256& hash, const CTxMemPool* const mempool)
+{
+    uint256 hashBlock{};
+    if (!g_txindex && !mempool) return {nullptr, hashBlock}; // Fast-fail as we don't have any other way to search
+    return {GetTransaction(/*block_index=*/nullptr, mempool, hash, Params().GetConsensus(), hashBlock), hashBlock};
+}

src/evo/chainhelper.h

@@ -18,6 +18,7 @@ class CGovernanceManager;
 class CSpecialTxProcessor;
 class CSporkManager;
 class CTransaction;
+class CTxMemPool;
 class uint256;
 
 namespace Consensus { struct Params; }
@@ -29,6 +30,8 @@ class CQuorumManager;
 class CQuorumSnapshotManager;
 }
 
+using CTransactionRef = std::shared_ptr<const CTransaction>;
+
 class CChainstateHelper
 {
 private:
@@ -63,4 +66,8 @@ class CChainstateHelper
     const std::unique_ptr<CSpecialTxProcessor> special_tx;
 };
 
+/* Retrieve transaction and block from txindex (or mempool) */
+std::pair</*tx=*/CTransactionRef, /*hash_block=*/uint256> GetTransactionBlock(const uint256& hash,
+                                                                              const CTxMemPool* const mempool = nullptr);
+
 #endif // BITCOIN_EVO_CHAINHELPER_H