From aa75877cd6a6d414f6f7e1f673be4f403a01abea Mon Sep 17 00:00:00 2001 From: Duke Date: Wed, 11 Sep 2024 23:31:44 -0400 Subject: [PATCH 01/19] Print stacktrace when asserting a lock is held --- src/sync.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/sync.cpp b/src/sync.cpp index 4b7dd899b..632c5562e 100644 --- a/src/sync.cpp +++ b/src/sync.cpp @@ -25,6 +25,10 @@ #include #include +#include /* backtrace, backtrace_symbols_fd */ +#include /* STDOUT_FILENO */ + + #ifdef DEBUG_LOCKCONTENTION void PrintLockContention(const char* pszName, const char* pszFile, int nLine) { @@ -177,12 +181,23 @@ std::string LocksHeld() return result; } +void print_stacktrace(void) { + size_t size; + enum Constexpr { MAX_SIZE = 1024 }; + void *array[MAX_SIZE]; + size = backtrace(array, MAX_SIZE); + backtrace_symbols_fd(array, size, STDERR_FILENO); +} + void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) { BOOST_FOREACH (const PAIRTYPE(void*, CLockLocation) & i, *lockstack) if (i.first == cs) return; fprintf(stderr, "Assertion failed: lock %s not held in %s:%i; locks held:\n%s", pszName, pszFile, nLine, LocksHeld().c_str()); + + print_stacktrace(); + abort(); } From b71d95bdd4f8a4de29433c4ea5865c3ee4ff3467 Mon Sep 17 00:00:00 2001 From: Duke Date: Wed, 11 Sep 2024 23:32:46 -0400 Subject: [PATCH 02/19] Delete disabled code --- src/miner.cpp | 38 -------------------------------------- 1 file changed, 38 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index 9a4b17842..356374207 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -756,44 +756,6 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32 } pblock->nSolution.clear(); pblocktemplate->vTxSigOps[0] = GetLegacySigOpCount(pblock->vtx[0]); - /* - if ( SMART_CHAIN_SYMBOL[0] == 0 && IS_HUSH_NOTARY != 0 && My_notaryid >= 0 ) - { - uint32_t r; CScript opret; void **ptr=0; - - CMutableTransaction txNotary = CreateNewContextualCMutableTransaction(Params().GetConsensus(), chainActive.Height() + 1); - if ( pblock->nTime < pindexPrev->nTime+60 ) - pblock->nTime = pindexPrev->nTime + 60; - if ( gpucount < 33 ) - { - uint8_t tmpbuffer[40]; uint32_t r; int32_t n=0; uint256 randvals; - memcpy(&tmpbuffer[n],&My_notaryid,sizeof(My_notaryid)), n += sizeof(My_notaryid); - memcpy(&tmpbuffer[n],&Mining_height,sizeof(Mining_height)), n += sizeof(Mining_height); - memcpy(&tmpbuffer[n],&pblock->hashPrevBlock,sizeof(pblock->hashPrevBlock)), n += sizeof(pblock->hashPrevBlock); - vcalc_sha256(0,(uint8_t *)&randvals,tmpbuffer,n); - memcpy(&r,&randvals,sizeof(r)); - pblock->nTime += (r % (33 - gpucount)*(33 - gpucount)); - } - if ( hush_notaryvin(txNotary,NOTARY_PUBKEY33,ptr) > 0 ) - { - CAmount txfees = 5000; - pblock->vtx.push_back(txNotary); - pblocktemplate->vTxFees.push_back(txfees); - pblocktemplate->vTxSigOps.push_back(GetLegacySigOpCount(txNotary)); - nFees += txfees; - pblocktemplate->vTxFees[0] = -nFees; - //*(uint64_t *)(&pblock->vtx[0].vout[0].nValue) += txfees; - //fprintf(stderr,"added notaryvin\n"); - } else { - fprintf(stderr,"error adding notaryvin, need to create 0.0001 utxos\n"); - if ( SMART_CHAIN_SYMBOL[0] == 0 || (SMART_CHAIN_SYMBOL[0] != 0 && !isStake) ) - { - LEAVE_CRITICAL_SECTION(cs_main); - LEAVE_CRITICAL_SECTION(mempool.cs); - } - return(0); - } - } else */ if ( ASSETCHAINS_CC == 0 && pindexPrev != 0 && ASSETCHAINS_STAKED == 0 && (SMART_CHAIN_SYMBOL[0] != 0 || IS_HUSH_NOTARY == 0 || My_notaryid < 0) ) { From b4e083ae56873a33cce38fdfd87874397b5eead0 Mon Sep 17 00:00:00 2001 From: Duke Date: Wed, 11 Sep 2024 23:59:57 -0400 Subject: [PATCH 03/19] Add what seems to be missing locks in mining code --- src/miner.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/miner.cpp b/src/miner.cpp index 356374207..436a59262 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -893,6 +893,7 @@ static bool ProcessBlockFound(CBlock* pblock) // Found a solution { + LOCK(cs_main); if (pblock->hashPrevBlock != chainActive.LastTip()->GetBlockHash()) { uint256 hash; int32_t i; @@ -1136,7 +1137,11 @@ void static RandomXMiner() // Create new block unsigned int nTransactionsUpdatedLast = mempool.GetTransactionsUpdated(); - CBlockIndex* pindexPrev = chainActive.LastTip(); + CBlockIndex* pindexPrev; + { + LOCK(cs_main); + pindexPrev = chainActive.LastTip(); + } // If we don't have a valid chain tip to work from, wait and try again. if (pindexPrev == nullptr) { From ed86f2dd1da370fe2dbf7db475afc41b218cbc5f Mon Sep 17 00:00:00 2001 From: Duke Date: Thu, 12 Sep 2024 00:21:56 -0400 Subject: [PATCH 04/19] Sleep for 200us before each ActivateBestChainStep call This should lower the main thread's likelihood to immediately reacquire cs_main after dropping it, which should help ThreadNotifyWallets and the RPC methods to acquire cs_main more quickly. Ported from ZEC commit e2cd1b761fe556bc6d61849346902c3611530307 --- src/main.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index ae5e27916..169124334 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -33,6 +33,7 @@ #include "net.h" #include "netmessagemaker.h" #include "pow.h" +#include "time.h" #include "script/interpreter.h" #include "txdb.h" #include "txmempool.h" @@ -4333,7 +4334,15 @@ bool ActivateBestChain(bool fSkipdpow, CValidationState &state, CBlock *pblock) CBlockIndex *pindexMostWork = NULL; const CChainParams& chainParams = Params(); do { - boost::this_thread::interruption_point(); + // Sleep briefly to allow other threads a chance at grabbing cs_main if + // we are connecting a long chain of blocks and would otherwise hold the + // lock almost continuously. This helps + // the internal wallet, if it is enabled, to keep up with the connected + // blocks, reducing the overall time until the node becomes usable. + // + // This is defined to be an interruption point. + // + boost::this_thread::sleep_for(boost::chrono::microseconds(200)); if (ShutdownRequested()) break; From aab240e3b34b934a329d5004ede79419368527fa Mon Sep 17 00:00:00 2001 From: Duke Date: Thu, 12 Sep 2024 11:21:46 -0400 Subject: [PATCH 05/19] Temporary change to add -rdynamic to build flags This allows us to see function names in a backtrace when AssertLockHeld gives us an assert, which is helping to debug our deadlock bug. This code should be improved to only do this for debug builds. --- util/build.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/util/build.sh b/util/build.sh index cbcd9a416..fcb3e2ef7 100755 --- a/util/build.sh +++ b/util/build.sh @@ -118,7 +118,8 @@ HOST="$HOST" BUILD="$BUILD" "$MAKE" "$@" -C ./depends/ V=1 ./autogen.sh -CONFIG_SITE="$PWD/depends/$HOST/share/config.site" ./configure "$HARDENING_ARG" "$LCOV_ARG" "$TEST_ARG" "$MINING_ARG" $CONFIGURE_FLAGS CXXFLAGS='-g' +#TODO: only use rdynamic for a debug build +CONFIG_SITE="$PWD/depends/$HOST/share/config.site" ./configure "$HARDENING_ARG" "$LCOV_ARG" "$TEST_ARG" "$MINING_ARG" $CONFIGURE_FLAGS CXXFLAGS='-g -rdynamic' # Build CryptoConditions stuff WD=$PWD From 7f9878cf0cf8973a87837f4500d1671383573b82 Mon Sep 17 00:00:00 2001 From: Duke Date: Thu, 12 Sep 2024 11:47:58 -0400 Subject: [PATCH 06/19] Take a lock on cs_main in DecrementNoteWitnesses Another possibly better approach would be to take a lock inside GetSaplingSpendDepth since we want the lock to apply to as little code as possible and it is GetSaplingSpendDepth that calls GetDepthInMainChain which requires a cs_main lock . --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 18e38cb0c..6d8c78979 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -977,7 +977,7 @@ void CWallet::ClearNoteWitnessCache() void CWallet::DecrementNoteWitnesses(const CBlockIndex* pindex) { - LOCK(cs_wallet); + LOCK2(cs_main, cs_wallet); extern int32_t HUSH_REWIND; From f8e7df37a10c718a13d257264ba9056c81e8a8c0 Mon Sep 17 00:00:00 2001 From: Duke Date: Thu, 12 Sep 2024 12:12:59 -0400 Subject: [PATCH 07/19] Delete commented out code --- src/main.cpp | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 169124334..7b885f081 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1732,22 +1732,6 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa tiptime = (uint32_t)time(NULL); else tiptime = (uint32_t)chainActive.LastTip()->nTime; - /* - // Node operator can choose to reject tx by number of transparent inputs - static_assert(std::numeric_limits::max() >= std::numeric_limits::max(), "size_t too small"); - const size_t limit = 0; //(size_t) GetArg("-mempooltxinputlimit", 0); - // Limit is ignored if Overwinter is active, which is the case on HUSH3 and all HAC's - if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) { - limit = 0; - } - if (limit > 0) { - size_t n = tx.vin.size(); - if (n > limit) { - LogPrint("mempool", "Dropping txid %s : too many transparent inputs %zu > limit %zu\n", tx.GetHash().ToString(), n, limit ); - return false; - } - } - */ auto verifier = libzcash::ProofVerifier::Strict(); if (!CheckTransaction(tiptime,tx, state, verifier, 0, 0)) From 04165e161ab170936db26ecbe14d1ad1d1791d83 Mon Sep 17 00:00:00 2001 From: Duke Date: Thu, 12 Sep 2024 13:40:30 -0400 Subject: [PATCH 08/19] Explain more details about the burn address --- src/hush_bitcoind.h | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/hush_bitcoind.h b/src/hush_bitcoind.h index de16d72ae..218f09db7 100644 --- a/src/hush_bitcoind.h +++ b/src/hush_bitcoind.h @@ -24,7 +24,17 @@ #include "cc/CCinclude.h" #include "sietch.h" -// this is the address for pubkey = 0x000000000000000000000000000000000 (33 bytes) +// This is the address for pubkey = 0x000000000000000000000000000000000 (33 bytes) +// Funds sent to a burn address can never be spent because the pubkey is invalid +// because it is not a point on the elliptic curve we use (secp256k1, like all other +// Bitcoin-derived codebases). secp256k1 is defined by the equation y^2 = x^3 + 7 . +// Using pubkey=0 also implies privkey=0 and the (pubkey,privkey) pair are the same +// as the (x,y) pair in the equation. If you plug in (0,0) to the equation you get +// 0^2 = 0^3 + 7 or 0=7 which is false. This means that (0,0) is not a valid point +// on the secp256k1 curve. Therefore there is no valid signature to spend the funds +// sent to the burn address, because a valid signature must use a valid point on the +// elliptic curve being used. For these reasons, we can prove that the burn address +// receives funds which can never be spent. #define BURN_ADDRESS "RD6GgnrMpPaTSMn8vai6yiGA7mN4QGPVMY" int32_t hush_notaries(uint8_t pubkeys[64][33],int32_t height,uint32_t timestamp); From 8b0c0493ac175cb12c039407489f3f2e37423d8a Mon Sep 17 00:00:00 2001 From: Duke Date: Thu, 12 Sep 2024 19:47:42 -0400 Subject: [PATCH 09/19] Ensure cs_main lock is held when TestBlockValidity() is called --- src/miner.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/miner.cpp b/src/miner.cpp index 436a59262..6bc205ed4 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -1334,6 +1334,8 @@ void static RandomXMiner() } CValidationState state; + { LOCK(cs_main); + if ( !TestBlockValidity(state,B, chainActive.LastTip(), true, false)) { h = UintToArith256(B.GetHash()); @@ -1344,6 +1346,8 @@ void static RandomXMiner() fprintf(stderr,"\n"); return(false); } + + } SetThreadPriority(THREAD_PRIORITY_NORMAL); LogPrintf("HushRandomXMiner:\n"); LogPrintf("proof-of-work found \n hash: %s \ntarget: %s\n", B.GetHash().GetHex(), HASHTarget.GetHex()); @@ -1702,6 +1706,8 @@ void static BitcoinMiner() fprintf(stderr," mined %s block %d!\n",SMART_CHAIN_SYMBOL,Mining_height); } CValidationState state; + { LOCK(cs_main); + if ( !TestBlockValidity(state,B, chainActive.LastTip(), true, false)) { h = UintToArith256(B.GetHash()); @@ -1711,6 +1717,8 @@ void static BitcoinMiner() gotinvalid = 1; return(false); } + + } HUSH_CHOSEN_ONE = 1; // Found a solution SetThreadPriority(THREAD_PRIORITY_NORMAL); From 215d0b7db4b29efd75806d43453f15b07da992d5 Mon Sep 17 00:00:00 2001 From: Duke Date: Thu, 12 Sep 2024 20:02:10 -0400 Subject: [PATCH 10/19] Remove commented out code --- src/miner.cpp | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index 6bc205ed4..b3d7fd324 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -308,12 +308,7 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32 bool fMissingInputs = false; bool fNotarization = false; std::vector TMP_NotarizationNotaries; - //if (tx.IsCoinImport()) - //{ - // CAmount nValueIn = GetCoinImportValue(tx); // burn amount - // nTotalIn += nValueIn; - // dPriority += (double)nValueIn * 1000; // flat multiplier... max = 1e16. - //} else + { TMP_NotarizationNotaries.clear(); bool fToCryptoAddress = false; @@ -322,15 +317,6 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32 BOOST_FOREACH(const CTxIn& txin, tx.vin) { - /* - if (tx.IsPegsImport() && txin.prevout.n==10e8) - { - CAmount nValueIn = GetCoinImportValue(tx); // burn amount - nTotalIn += nValueIn; - dPriority += (double)nValueIn * 1000; // flat multiplier... max = 1e16. - continue; - } - */ // Read prev transaction if (!view.HaveCoins(txin.prevout.hash)) { From 7e486538c9cb75e91e6df0b7a28ef7c686f8157d Mon Sep 17 00:00:00 2001 From: Duke Date: Thu, 12 Sep 2024 20:12:26 -0400 Subject: [PATCH 11/19] SMART_CHAIN_SYMBOL is always set, so we can remove these checks --- src/miner.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index b3d7fd324..1bc0e984d 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -697,7 +697,7 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32 if ( totalsats == 0 ) { fprintf(stderr, "Could not create notary payment, trying again.\n"); - if ( SMART_CHAIN_SYMBOL[0] == 0 || (SMART_CHAIN_SYMBOL[0] != 0 && !isStake) ) + if ( !isStake ) { LEAVE_CRITICAL_SECTION(cs_main); LEAVE_CRITICAL_SECTION(mempool.cs); @@ -743,13 +743,13 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32 pblock->nSolution.clear(); pblocktemplate->vTxSigOps[0] = GetLegacySigOpCount(pblock->vtx[0]); - if ( ASSETCHAINS_CC == 0 && pindexPrev != 0 && ASSETCHAINS_STAKED == 0 && (SMART_CHAIN_SYMBOL[0] != 0 || IS_HUSH_NOTARY == 0 || My_notaryid < 0) ) + if ( ASSETCHAINS_CC == 0 && pindexPrev != 0 && ASSETCHAINS_STAKED == 0 && (IS_HUSH_NOTARY == 0 || My_notaryid < 0) ) { CValidationState state; //fprintf(stderr,"%s: check validity\n", __func__); if ( !TestBlockValidity(state, *pblock, pindexPrev, false, false)) // invokes CC checks { - if ( (SMART_CHAIN_SYMBOL[0] != 0 && !isStake) ) + if ( !isStake ) { LEAVE_CRITICAL_SECTION(cs_main); LEAVE_CRITICAL_SECTION(mempool.cs); @@ -761,7 +761,7 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32 //fprintf(stderr,"valid\n"); } } - if ( (SMART_CHAIN_SYMBOL[0] != 0 && !isStake) ) + if ( !isStake ) { LEAVE_CRITICAL_SECTION(cs_main); LEAVE_CRITICAL_SECTION(mempool.cs); From 72c530d2811cb5e2cca6d085a1f06178eb3746ca Mon Sep 17 00:00:00 2001 From: Duke Date: Fri, 13 Sep 2024 03:50:07 -0400 Subject: [PATCH 12/19] SMART_CHAIN_SYMBOL is always defined --- src/bitcoind.cpp | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 609fb5555..22bffe71a 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -74,30 +74,17 @@ void WaitForShutdown(boost::thread_group* threadGroup) while (!fShutdown) { - //fprintf(stderr,"call passport iteration\n"); - if ( SMART_CHAIN_SYMBOL[0] == 0 ) - { - if ( HUSH_NSPV_FULLNODE ) - hush_passport_iteration(); - for (i=0; i<10; i++) - { - fShutdown = ShutdownRequested(); - if ( fShutdown != 0 ) - break; - MilliSleep(1000); - } - } else { - //hush_longestchain(); - if ( ASSETCHAINS_CBOPRET != 0 ) - hush_cbopretupdate(0); - for (i=0; i<=ASSETCHAINS_BLOCKTIME/5; i++) - { - fShutdown = ShutdownRequested(); - if ( fShutdown != 0 ) - break; - MilliSleep(1000); - } - } + //hush_longestchain(); + if ( ASSETCHAINS_CBOPRET != 0 ) + hush_cbopretupdate(0); + for (i=0; i<=ASSETCHAINS_BLOCKTIME/5; i++) + { + fShutdown = ShutdownRequested(); + if ( fShutdown != 0 ) + break; + MilliSleep(1000); + } + fShutdown = ShutdownRequested(); } //fprintf(stderr,"%s: fShutdown=%d\n", __FUNCTION__, fShutdown); From be3641158a67cf41d711614f614b14eb96b85fac Mon Sep 17 00:00:00 2001 From: Duke Date: Fri, 13 Sep 2024 04:11:38 -0400 Subject: [PATCH 13/19] Only take cs_main lock in GetSaplingSpendDepth We do not seem to need this lock for the entire DecrementNoteWitnesses function, we need it only when calling GetSaplingSpendDepth. Also protects against the case in the future where some code without cs_main calls GetSaplingSpendDepth. --- src/wallet/wallet.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 6d8c78979..26faed52a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -887,6 +887,8 @@ bool CWallet::IsSaplingSpent(const uint256& nullifier) const { } unsigned int CWallet::GetSaplingSpendDepth(const uint256& nullifier) const { + LOCK(cs_main); + pair range; range = mapTxSaplingNullifiers.equal_range(nullifier); @@ -977,10 +979,11 @@ void CWallet::ClearNoteWitnessCache() void CWallet::DecrementNoteWitnesses(const CBlockIndex* pindex) { - LOCK2(cs_main, cs_wallet); extern int32_t HUSH_REWIND; + LOCK(cs_wallet); + for (std::pair& wtxItem : mapWallet) { //Sapling for (auto& item : wtxItem.second.mapSaplingNoteData) { From 6845fcd93ac237fe9959d1ddfc4baf6dfa77e94c Mon Sep 17 00:00:00 2001 From: Duke Date: Fri, 13 Sep 2024 04:22:28 -0400 Subject: [PATCH 14/19] This rpc can only be used on non z2z chains --- src/wallet/rpcwallet.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ec99d7516..46a059892 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -629,7 +629,7 @@ UniValue kvupdate(const UniValue& params, bool fHelp, const CPubKey& mypk) if (fHelp || params.size() < 3 ) throw runtime_error( "kvupdate key \"value\" days passphrase\n" - "\nStore a key value. This feature is only available for asset chains.\n" + "\nStore a key value. This feature is only available for non-z2z chains.\n" "\nArguments:\n" "1. key (string, required) key\n" "2. \"value\" (string, required) value\n" @@ -654,8 +654,11 @@ UniValue kvupdate(const UniValue& params, bool fHelp, const CPubKey& mypk) ); if (!EnsureWalletIsAvailable(fHelp)) return 0; - if ( SMART_CHAIN_SYMBOL[0] == 0 ) - return(0); + + if (ASSETCHAINS_PRIVATE) { + throw JSONRPCError(RPC_TYPE_ERROR, "This RPC is only available on non z2z chains"); + } + haveprivkey = 0; memset(&sig,0,sizeof(sig)); memset(&privkey,0,sizeof(privkey)); From f5cae3b4035ece57a1a2a69bac0c0aef07ee6eb5 Mon Sep 17 00:00:00 2001 From: Duke Date: Fri, 13 Sep 2024 04:37:48 -0400 Subject: [PATCH 15/19] getinfo rpc needs a cs_wallet lock Without it we get an AssertLockHeld when calling GetKeyPoolSize . We could probably make this lock apply to less code, possibly only the single line that calls GetKeyPoolSize() needs it. --- src/rpc/misc.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index f70f60f79..97ed6348e 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -231,11 +231,12 @@ UniValue getinfo(const UniValue& params, bool fHelp, const CPubKey& mypk) + HelpExampleCli("getinfo", "") + HelpExampleRpc("getinfo", "") ); - //#ifdef ENABLE_WALLET - // LOCK2(cs_main, pwalletMain ? &pwalletMain->cs_wallet : NULL); - //#else + +#ifdef ENABLE_WALLET + LOCK2(cs_main, pwalletMain ? &pwalletMain->cs_wallet : NULL); +#else LOCK(cs_main); - //#endif +#endif proxyType proxy; GetProxy(NET_IPV4, proxy); From 4f4a72da8eb06a3fdea72981f606adee307e5aab Mon Sep 17 00:00:00 2001 From: Duke Date: Fri, 13 Sep 2024 09:28:59 -0400 Subject: [PATCH 16/19] Go back to taking both main+wallet lock in DecrementWitnesses By taking a wallet lock first and then main later we run into a potential deadlock : 2024-09-13 11:14:37 POTENTIAL DEADLOCK DETECTED 2024-09-13 11:14:37 Previous lock order was: 2024-09-13 11:14:37 (1) cs_wallet wallet/wallet.cpp:985 2024-09-13 11:14:37 (2) cs_main wallet/wallet.cpp:890 2024-09-13 11:14:37 Current lock order is: 2024-09-13 11:14:37 (2) cs_main wallet/wallet.cpp:2845 2024-09-13 11:14:37 (1) cs_wallet wallet/wallet.cpp:2845 --- src/wallet/wallet.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 26faed52a..01af7b78e 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -887,7 +887,6 @@ bool CWallet::IsSaplingSpent(const uint256& nullifier) const { } unsigned int CWallet::GetSaplingSpendDepth(const uint256& nullifier) const { - LOCK(cs_main); pair range; range = mapTxSaplingNullifiers.equal_range(nullifier); @@ -979,10 +978,9 @@ void CWallet::ClearNoteWitnessCache() void CWallet::DecrementNoteWitnesses(const CBlockIndex* pindex) { - extern int32_t HUSH_REWIND; - LOCK(cs_wallet); + LOCK2(cs_main, cs_wallet); for (std::pair& wtxItem : mapWallet) { //Sapling From 912942f2a852adfc503fc61c1dce711e32775270 Mon Sep 17 00:00:00 2001 From: Duke Date: Fri, 13 Sep 2024 18:50:28 -0400 Subject: [PATCH 17/19] Delete more CC stuff --- src/cc/CCinclude.h | 5 -- src/cc/CCutils.cpp | 123 ++------------------------------------------- 2 files changed, 3 insertions(+), 125 deletions(-) diff --git a/src/cc/CCinclude.h b/src/cc/CCinclude.h index 29cc60d6c..cacfad593 100644 --- a/src/cc/CCinclude.h +++ b/src/cc/CCinclude.h @@ -498,11 +498,6 @@ bool GetCustomscriptaddress(char *destaddr,const CScript &scriptPubKey,uint8_t t /// @returns public key as byte array std::vector Mypubkey(); -/// Returns my private key, that is private key for the my pubkey -/// @returns private key as byte array -/// @see Mypubkey -bool Myprivkey(uint8_t myprivkey[]); - /// @private uint256 BitcoinGetProofMerkleRoot(const std::vector &proofData, std::vector &txids); diff --git a/src/cc/CCutils.cpp b/src/cc/CCutils.cpp index e46793288..5d8e40c40 100644 --- a/src/cc/CCutils.cpp +++ b/src/cc/CCutils.cpp @@ -102,34 +102,18 @@ uint32_t GetLatestTimestamp(int32_t height) void CCaddr2set(struct CCcontract_info *cp,uint8_t evalcode,CPubKey pk,uint8_t *priv,char *coinaddr) { - cp->unspendableEvalcode2 = evalcode; - cp->unspendablepk2 = pk; - memcpy(cp->unspendablepriv2,priv,32); - strcpy(cp->unspendableaddr2,coinaddr); } void CCaddr3set(struct CCcontract_info *cp,uint8_t evalcode,CPubKey pk,uint8_t *priv,char *coinaddr) { - cp->unspendableEvalcode3 = evalcode; - cp->unspendablepk3 = pk; - memcpy(cp->unspendablepriv3,priv,32); - strcpy(cp->unspendableaddr3,coinaddr); } void CCaddr1of2set(struct CCcontract_info *cp, CPubKey pk1, CPubKey pk2, uint8_t *priv, char *coinaddr) { - cp->coins1of2pk[0] = pk1; - cp->coins1of2pk[1] = pk2; - memcpy(cp->coins1of2priv,priv,32); - strcpy(cp->coins1of2addr,coinaddr); } void CCaddrTokens1of2set(struct CCcontract_info *cp, CPubKey pk1, CPubKey pk2, uint8_t *priv, char *tokenaddr) { - cp->tokens1of2pk[0] = pk1; - cp->tokens1of2pk[1] = pk2; - memcpy(cp->tokens1of2priv,priv,32); - strcpy(cp->tokens1of2addr, tokenaddr); } bool Getscriptaddress(char *destaddr,const CScript &scriptPubKey) @@ -150,13 +134,6 @@ bool Getscriptaddress(char *destaddr,const CScript &scriptPubKey) bool GetCustomscriptaddress(char *destaddr,const CScript &scriptPubKey,uint8_t taddr,uint8_t prefix, uint8_t prefix2) { - CTxDestination address; txnouttype whichType; - if ( ExtractDestination(scriptPubKey,address) != 0 ) - { - strcpy(destaddr,(char *)CCustomBitcoinAddress(address,taddr,prefix,prefix2).ToString().c_str()); - return(true); - } - //fprintf(stderr,"ExtractDestination failed\n"); return(false); } @@ -170,23 +147,7 @@ bool pubkey2addr(char *destaddr,uint8_t *pubkey33) bool ConstrainVout(CTxOut vout, int32_t CCflag, char *cmpaddr, int64_t nValue) { - char destaddr[64]; - if ( vout.scriptPubKey.IsPayToCryptoCondition() != CCflag ) - { - fprintf(stderr,"constrain vout error isCC %d vs %d CCflag\n", vout.scriptPubKey.IsPayToCryptoCondition(), CCflag); - return(false); - } - else if ( cmpaddr != 0 && (Getscriptaddress(destaddr, vout.scriptPubKey) == 0 || strcmp(destaddr, cmpaddr) != 0) ) - { - fprintf(stderr,"constrain vout error: check addr %s vs script addr %s\n", cmpaddr!=0?cmpaddr:"", destaddr!=0?destaddr:""); - return(false); - } - else if ( nValue != 0 && nValue != vout.nValue ) //(nValue == 0 && vout.nValue < 10000) || ( - { - fprintf(stderr,"constrain vout error nValue %.8f vs %.8f\n",(double)nValue/COIN,(double)vout.nValue/COIN); - return(false); - } - else return(true); + return false; } bool priv2addr(char *coinaddr,uint8_t *buf33,uint8_t priv32[32]) @@ -219,60 +180,6 @@ extern char NSPV_wifstr[],NSPV_pubkeystr[]; extern uint32_t NSPV_logintime; #define NSPV_AUTOLOGOUT 777 -bool Myprivkey(uint8_t myprivkey[]) -{ - char coinaddr[64],checkaddr[64]; std::string strAddress; char *dest; int32_t i,n; CBitcoinAddress address; CKeyID keyID; CKey vchSecret; uint8_t buf33[33]; - if ( HUSH_NSPV_SUPERLITE ) - { - if ( NSPV_logintime == 0 || time(NULL) > NSPV_logintime+NSPV_AUTOLOGOUT ) - { - fprintf(stderr,"need to be logged in to get myprivkey\n"); - return false; - } - vchSecret = DecodeSecret(NSPV_wifstr); - memcpy(myprivkey,vchSecret.begin(),32); - //for (i=0; i<32; i++) - // fprintf(stderr,"%02x",myprivkey[i]); - //fprintf(stderr," myprivkey %s\n",NSPV_wifstr); - memset((uint8_t *)vchSecret.begin(),0,32); - return true; - } - if ( Getscriptaddress(coinaddr,CScript() << Mypubkey() << OP_CHECKSIG) != 0 ) - { - n = (int32_t)strlen(coinaddr); - strAddress.resize(n+1); - dest = (char *)strAddress.data(); - for (i=0; iGetKey(keyID,vchSecret) != 0 ) - { - memcpy(myprivkey,vchSecret.begin(),32); - memset((uint8_t *)vchSecret.begin(),0,32); - if ( 0 ) - { - for (i=0; i<32; i++) - fprintf(stderr,"0x%02x, ",myprivkey[i]); - fprintf(stderr," found privkey for %s!\n",dest); - } - if ( priv2addr(checkaddr,buf33,myprivkey) != 0 ) - { - if ( buf2pk(buf33) == Mypubkey() && strcmp(checkaddr,coinaddr) == 0 ) - return(true); - else printf("mismatched privkey -> addr %s vs %s\n",checkaddr,coinaddr); - } - return(false); - } -#endif - } - } - fprintf(stderr,"privkey for the -pubkey= address is not in the wallet, importprivkey!\n"); - return(false); -} - CPubKey GetUnspendable(struct CCcontract_info *cp,uint8_t *unspendablepriv) { if ( unspendablepriv != 0 ) @@ -423,37 +330,13 @@ bool hush_txnotarizedconfirmed(uint256 txid) // returns total of normal inputs signed with this pubkey int64_t TotalPubkeyNormalInputs(const CTransaction &tx, const CPubKey &pubkey) { - int64_t total = 0; - for (auto vin : tx.vin) { - CTransaction vintx; - uint256 hashBlock; - if (!IsCCInput(vin.scriptSig) && myGetTransaction(vin.prevout.hash, vintx, hashBlock)) { - typedef std::vector valtype; - std::vector vSolutions; - txnouttype whichType; - - if (Solver(vintx.vout[vin.prevout.n].scriptPubKey, whichType, vSolutions)) { - switch (whichType) { - case TX_PUBKEY: - if (pubkey == CPubKey(vSolutions[0])) // is my input? - total += vintx.vout[vin.prevout.n].nValue; - break; - case TX_PUBKEYHASH: - if (pubkey.GetID() == CKeyID(uint160(vSolutions[0]))) // is my input? - total += vintx.vout[vin.prevout.n].nValue; - break; - } - } - } - } - return total; + return 0; } // returns total of CC inputs signed with this pubkey int64_t TotalPubkeyCCInputs(const CTransaction &tx, const CPubKey &pubkey) { - int64_t total = 0; - return total; + return 0; } extern std::string MYCCLIBNAME; From c5eeb748078aa6129cf45ddbc4e075ac9bbcf56f Mon Sep 17 00:00:00 2001 From: Duke Date: Mon, 16 Sep 2024 11:06:31 -0400 Subject: [PATCH 18/19] Simplify MAX_BLOCK_SIZE Our code requires sapling activation at height 1 so we can simplify this function which should lead to some performance improvements since it is called from many places, including mining code. --- src/chainparams.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 02ab894a9..0b454e2c0 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -492,13 +492,14 @@ void UpdateNetworkUpgradeParameters(Consensus::UpgradeIndex idx, int nActivation int32_t MAX_BLOCK_SIZE(int32_t height) { - int32_t saplinght = pCurrentParams->consensus.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight; + // this codebase requires sapling activation at height=1 + int32_t saplinght = 1; // pCurrentParams->consensus.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight; //fprintf(stderr,"MAX_BLOCK_SIZE %d vs. %d\n",height,mainParams.consensus.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight); - if ( height <= 0 || (saplinght > 0 && height >= saplinght) ) - { + //if ( height <= 0 || (saplinght > 0 && height >= saplinght) ) + //{ return(_MAX_BLOCK_SIZE); - } - else return(2000000); + //} + //else return(2000000); } // Change the Hush blocktime at run-time(!) From 3f50dd988a812cc72135d339bb573fa91fef0669 Mon Sep 17 00:00:00 2001 From: Duke Date: Mon, 16 Sep 2024 11:26:28 -0400 Subject: [PATCH 19/19] Only use -rdynamic on debug builds --- debug-build.sh | 3 +- util/build.sh | 3 +- util/debug-build.sh | 143 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 3 deletions(-) create mode 100755 util/debug-build.sh diff --git a/debug-build.sh b/debug-build.sh index a18818b20..d3ba81320 100755 --- a/debug-build.sh +++ b/debug-build.sh @@ -9,7 +9,8 @@ echo "Compiling a debug build with --enable-debug..." # run correct build script for detected OS if [[ "$OSTYPE" == "linux-gnu"* ]]; then - CONFIGURE_FLAGS=--enable-debug ./util/build.sh --disable-tests $@ + # TODO: we only have debug builds on linux for now + CONFIGURE_FLAGS=--enable-debug ./util/debug-build.sh --disable-tests $@ elif [[ "$OSTYPE" == "darwin"* ]]; then # code from ./util/build.sh needs to be ported to support --disable-tests #./util/build-mac.sh --disable-tests $@ diff --git a/util/build.sh b/util/build.sh index fcb3e2ef7..cbcd9a416 100755 --- a/util/build.sh +++ b/util/build.sh @@ -118,8 +118,7 @@ HOST="$HOST" BUILD="$BUILD" "$MAKE" "$@" -C ./depends/ V=1 ./autogen.sh -#TODO: only use rdynamic for a debug build -CONFIG_SITE="$PWD/depends/$HOST/share/config.site" ./configure "$HARDENING_ARG" "$LCOV_ARG" "$TEST_ARG" "$MINING_ARG" $CONFIGURE_FLAGS CXXFLAGS='-g -rdynamic' +CONFIG_SITE="$PWD/depends/$HOST/share/config.site" ./configure "$HARDENING_ARG" "$LCOV_ARG" "$TEST_ARG" "$MINING_ARG" $CONFIGURE_FLAGS CXXFLAGS='-g' # Build CryptoConditions stuff WD=$PWD diff --git a/util/debug-build.sh b/util/debug-build.sh new file mode 100755 index 000000000..a47681506 --- /dev/null +++ b/util/debug-build.sh @@ -0,0 +1,143 @@ +#!/usr/bin/env bash +# Copyright (c) 2016-2024 The Hush developers +# Distributed under the GPLv3 software license, see the accompanying +# file COPYING or https://www.gnu.org/licenses/gpl-3.0.en.html + +set -eu -o pipefail + +# Check if cmake, a new dependency for randomx support, is installed on system and exits if it is not +if ! [ -x "$(command -v cmake)" ]; then + echo 'Error: cmake is not installed. Install cmake and try again.' >&2 + exit 1 +fi + +if ! [ -x "$(command -v autoreconf)" ]; then + echo 'Error: autoconf is not installed. Install autoconf and try again.' >&2 + echo 'On Debian-like systems: apt install autoconf' >&2 + exit 1 +fi + +function cmd_pref() { + if type -p "$2" > /dev/null; then + eval "$1=$2" + else + eval "$1=$3" + fi +} +cat <<'EOF' + .~~~~~~~~~~~~~~~~. +{{ Building Hush!! }} + `~~~~~~~~~~~~~~~~` + \ ^__^ + \ (@@)\_______ + (__)\ HUSH )\/\ $ + z zz ||----w | z | +zz zz z || z ||xxx z z|z zz +zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz +zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz +EOF + +# If a g-prefixed version of the command exists, use it preferentially. +function gprefix() { + cmd_pref "$1" "g$2" "$2" +} + +gprefix READLINK readlink +cd "$(dirname "$("$READLINK" -f "$0")")/.." + +# Allow user overrides to $MAKE. Typical usage for users who need it: +# MAKE=gmake ./util/build.sh -j$(nproc) +if [[ -z "${MAKE-}" ]]; then + MAKE=make +fi + +# Allow overrides to $BUILD and $HOST for porters. Most users will not need it. +# BUILD=i686-pc-linux-gnu ./util/build.sh +if [[ -z "${BUILD-}" ]]; then + BUILD="$(./depends/config.guess)" +fi +if [[ -z "${HOST-}" ]]; then + HOST="$BUILD" +fi + +# Allow users to set arbitrary compile flags. Most users will not need this. +if [[ -z "${CONFIGURE_FLAGS-}" ]]; then + CONFIGURE_FLAGS="" +fi + +if [ "x$*" = 'x--help' ] +then + cat ./util/dragon.txt + cat <