diff --git a/qa/rpc-tests/p2p-instantsend.py b/qa/rpc-tests/p2p-instantsend.py index 5e1ed9cfb..39a75cc40 100755 --- a/qa/rpc-tests/p2p-instantsend.py +++ b/qa/rpc-tests/p2p-instantsend.py @@ -55,7 +55,9 @@ class InstantSendTest(DashTestFramework): # instantsend to receiver receiver_addr = receiver.getnewaddress() is_id = sender.instantsendtoaddress(receiver_addr, 0.9) - self.wait_for_instantlock(is_id, sender) + for node in self.nodes: + if node is not isolated: + self.wait_for_instantlock(is_id, node) # send doublespend transaction to isolated node isolated.sendrawtransaction(dblspnd_tx['hex']) # generate block on isolated node with doublespend transaction diff --git a/qa/rpc-tests/test_framework/test_framework.py b/qa/rpc-tests/test_framework/test_framework.py index c48a3571b..c355f3658 100755 --- a/qa/rpc-tests/test_framework/test_framework.py +++ b/qa/rpc-tests/test_framework/test_framework.py @@ -535,10 +535,14 @@ class DashTestFramework(BitcoinTestFramework): start = time() locked = False while True: - is_tx = node.getrawtransaction(txid, True) - if is_tx['instantlock']: - locked = True - break + try: + is_tx = node.getrawtransaction(txid, True) + if is_tx['instantlock']: + locked = True + break + except: + # TX not received yet? + pass if time() > start + 10: break sleep(0.5) diff --git a/src/llmq/quorums_chainlocks.cpp b/src/llmq/quorums_chainlocks.cpp index 2ab637204..f9a73c539 100644 --- a/src/llmq/quorums_chainlocks.cpp +++ b/src/llmq/quorums_chainlocks.cpp @@ -333,11 +333,8 @@ void CChainLocksHandler::NewPoWValidBlock(const CBlockIndex* pindex, const std:: auto txs = std::make_shared>(); for (const auto& tx : block->vtx) { - if (tx->nVersion == 3) { - if (tx->nType == TRANSACTION_COINBASE || - tx->nType == TRANSACTION_QUORUM_COMMITMENT) { - continue; - } + if (tx->IsCoinBase() || tx->vin.empty()) { + continue; } txs->emplace(tx->GetHash()); txFirstSeenTime.emplace(tx->GetHash(), curTime); @@ -347,11 +344,8 @@ void CChainLocksHandler::NewPoWValidBlock(const CBlockIndex* pindex, const std:: void CChainLocksHandler::SyncTransaction(const CTransaction& tx, const CBlockIndex* pindex, int posInBlock) { - if (tx.nVersion == 3) { - if (tx.nType == TRANSACTION_COINBASE || - tx.nType == TRANSACTION_QUORUM_COMMITMENT) { - return; - } + if (tx.IsCoinBase() || tx.vin.empty()) { + return; } LOCK(cs); @@ -492,7 +486,7 @@ void CChainLocksHandler::DoInvalidateBlock(const CBlockIndex* pindex, bool activ CValidationState state; if (!InvalidateBlock(state, params, pindex2)) { - LogPrintf("CChainLocksHandler::UpdatedBlockTip -- InvalidateBlock failed: %s\n", FormatStateMessage(state)); + LogPrintf("CChainLocksHandler::%s -- InvalidateBlock failed: %s\n", __func__, FormatStateMessage(state)); // This should not have happened and we are in a state were it's not safe to continue anymore assert(false); } @@ -500,7 +494,7 @@ void CChainLocksHandler::DoInvalidateBlock(const CBlockIndex* pindex, bool activ CValidationState state; if (activateBestChain && !ActivateBestChain(state, params)) { - LogPrintf("CChainLocksHandler::UpdatedBlockTip -- ActivateBestChain failed: %s\n", FormatStateMessage(state)); + LogPrintf("CChainLocksHandler::%s -- ActivateBestChain failed: %s\n", __func__, FormatStateMessage(state)); // This should not have happened and we are in a state were it's not safe to continue anymore assert(false); } diff --git a/src/llmq/quorums_instantsend.cpp b/src/llmq/quorums_instantsend.cpp index 55d693770..7db72015c 100644 --- a/src/llmq/quorums_instantsend.cpp +++ b/src/llmq/quorums_instantsend.cpp @@ -174,7 +174,7 @@ void CInstantSendManager::UnregisterAsRecoveredSigsListener() quorumSigningManager->UnregisterRecoveredSigsListener(this); } -bool CInstantSendManager::ProcessTx(CNode* pfrom, const CTransaction& tx, CConnman& connman, const Consensus::Params& params) +bool CInstantSendManager::ProcessTx(const CTransaction& tx, const Consensus::Params& params) { if (!IsNewInstantSendEnabled()) { return true; @@ -246,13 +246,15 @@ bool CInstantSendManager::CheckCanLock(const CTransaction& tx, bool printDebug, return false; } - int nInstantSendConfirmationsRequired = params.nInstantSendConfirmationsRequired; + if (tx.vin.empty()) { + // can't lock TXs without inputs (e.g. quorum commitments) + return false; + } - uint256 txHash = tx.GetHash(); CAmount nValueIn = 0; for (const auto& in : tx.vin) { CAmount v = 0; - if (!CheckCanLock(in.prevout, printDebug, &txHash, &v, params)) { + if (!CheckCanLock(in.prevout, printDebug, tx.GetHash(), &v, params)) { return false; } @@ -274,7 +276,7 @@ bool CInstantSendManager::CheckCanLock(const CTransaction& tx, bool printDebug, return true; } -bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256* _txHash, CAmount* retValue, const Consensus::Params& params) +bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash, CAmount* retValue, const Consensus::Params& params) { int nInstantSendConfirmationsRequired = params.nInstantSendConfirmationsRequired; @@ -283,44 +285,36 @@ bool CInstantSendManager::CheckCanLock(const COutPoint& outpoint, bool printDebu return true; } - static uint256 txHashNull; - const uint256* txHash = &txHashNull; - if (_txHash) { - txHash = _txHash; - } - auto mempoolTx = mempool.get(outpoint.hash); if (mempoolTx) { if (printDebug) { LogPrint("instantsend", "CInstantSendManager::%s -- txid=%s: parent mempool TX %s is not locked\n", __func__, - txHash->ToString(), outpoint.hash.ToString()); + txHash.ToString(), outpoint.hash.ToString()); } return false; } Coin coin; const CBlockIndex* pindexMined = nullptr; + int nTxAge; { LOCK(cs_main); if (!pcoinsTip->GetCoin(outpoint, coin) || coin.IsSpent()) { if (printDebug) { LogPrint("instantsend", "CInstantSendManager::%s -- txid=%s: failed to find UTXO %s\n", __func__, - txHash->ToString(), outpoint.ToStringShort()); + txHash.ToString(), outpoint.ToStringShort()); } return false; } pindexMined = chainActive[coin.nHeight]; + nTxAge = chainActive.Height() - coin.nHeight + 1; } - int nTxAge = chainActive.Height() - coin.nHeight + 1; - // 1 less than the "send IX" gui requires, in case of a block propagating the network at the time - int nConfirmationsRequired = nInstantSendConfirmationsRequired - 1; - - if (nTxAge < nConfirmationsRequired) { + if (nTxAge < nInstantSendConfirmationsRequired) { if (!llmq::chainLocksHandler->HasChainLock(pindexMined->nHeight, pindexMined->GetBlockHash())) { if (printDebug) { - LogPrint("instantsend", "CInstantSendManager::%s -- txid=%s: outpoint %s too new and not ChainLocked. nTxAge=%d, nConfirmationsRequired=%d\n", __func__, - txHash->ToString(), outpoint.ToStringShort(), nTxAge, nConfirmationsRequired); + LogPrint("instantsend", "CInstantSendManager::%s -- txid=%s: outpoint %s too new and not ChainLocked. nTxAge=%d, nInstantSendConfirmationsRequired=%d\n", __func__, + txHash.ToString(), outpoint.ToStringShort(), nTxAge, nInstantSendConfirmationsRequired); } return false; } @@ -719,18 +713,19 @@ void CInstantSendManager::SyncTransaction(const CTransaction& tx, const CBlockIn } } -void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindex) +void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindexChainLock) { uint256 lastChainLockBlock; { LOCK(cs); - db.GetLastChainLockBlock(); + lastChainLockBlock = db.GetLastChainLockBlock(); } // Let's find all islocks that correspond to TXs which are part of the freshly ChainLocked chain and then delete // the islocks. We do this because the ChainLocks imply locking and thus it's not needed to further track // or propagate the islocks std::unordered_set toDelete; + auto pindex = pindexChainLock; while (pindex && pindex->GetBlockHash() != lastChainLockBlock) { CBlock block; { @@ -758,7 +753,7 @@ void CInstantSendManager::NotifyChainLock(const CBlockIndex* pindex) { LOCK(cs); - db.WriteLastChainLockBlock(pindex ? pindex->GetBlockHash() : uint256()); + db.WriteLastChainLockBlock(pindexChainLock->GetBlockHash()); } RetryLockMempoolTxs(uint256()); @@ -849,7 +844,7 @@ void CInstantSendManager::RetryLockMempoolTxs(const uint256& lockedParentTx) tx->GetHash().ToString()); } - ProcessTx(nullptr, *tx, *g_connman, Params().GetConsensus()); + ProcessTx(*tx, Params().GetConsensus()); } } diff --git a/src/llmq/quorums_instantsend.h b/src/llmq/quorums_instantsend.h index 8eb763b50..bd689d641 100644 --- a/src/llmq/quorums_instantsend.h +++ b/src/llmq/quorums_instantsend.h @@ -99,9 +99,9 @@ public: void UnregisterAsRecoveredSigsListener(); public: - bool ProcessTx(CNode* pfrom, const CTransaction& tx, CConnman& connman, const Consensus::Params& params); + bool ProcessTx(const CTransaction& tx, const Consensus::Params& params); bool CheckCanLock(const CTransaction& tx, bool printDebug, const Consensus::Params& params); - bool CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256* _txHash, CAmount* retValue, const Consensus::Params& params); + bool CheckCanLock(const COutPoint& outpoint, bool printDebug, const uint256& txHash, CAmount* retValue, const Consensus::Params& params); bool IsLocked(const uint256& txHash); bool IsConflicted(const CTransaction& tx); bool GetConflictingTx(const CTransaction& tx, uint256& retConflictTxHash); @@ -120,7 +120,7 @@ public: void UpdateWalletTransaction(const uint256& txid, const CTransactionRef& tx); void SyncTransaction(const CTransaction &tx, const CBlockIndex *pindex, int posInBlock); - void NotifyChainLock(const CBlockIndex* pindex); + void NotifyChainLock(const CBlockIndex* pindexChainLock); void RemoveFinalISLock(const uint256& hash, const CInstantSendLockPtr& islock); void RemoveMempoolConflictsForLock(const uint256& hash, const CInstantSendLock& islock); diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 6d6b3b8ff..957f51e9c 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -2165,7 +2165,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } if (nInvType != MSG_TXLOCK_REQUEST) { - llmq::quorumInstantSendManager->ProcessTx(pfrom, tx, connman, chainparams.GetConsensus()); + llmq::quorumInstantSendManager->ProcessTx(tx, chainparams.GetConsensus()); } mempool.check(pcoinsTip); @@ -2213,7 +2213,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr } vEraseQueue.push_back(orphanHash); - llmq::quorumInstantSendManager->ProcessTx(pfrom, orphanTx, connman, chainparams.GetConsensus()); + llmq::quorumInstantSendManager->ProcessTx(orphanTx, chainparams.GetConsensus()); } else if (!fMissingInputs2) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index afdece3aa..8e5985537 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -1020,7 +1020,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) throw JSONRPCError(RPC_TRANSACTION_ERROR, state.GetRejectReason()); } } - llmq::quorumInstantSendManager->ProcessTx(nullptr, *tx, *g_connman, Params().GetConsensus()); + llmq::quorumInstantSendManager->ProcessTx(*tx, Params().GetConsensus()); } else if (fHaveChain) { throw JSONRPCError(RPC_TRANSACTION_ALREADY_IN_CHAIN, "transaction already in block chain"); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3fb3529c5..c2b63bb16 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1921,7 +1921,7 @@ bool CWalletTx::RelayWalletTransaction(CConnman* connman, const std::string& str } } - llmq::quorumInstantSendManager->ProcessTx(nullptr, *this->tx, *connman, Params().GetConsensus()); + llmq::quorumInstantSendManager->ProcessTx(*this->tx, Params().GetConsensus()); if (connman) { connman->RelayTransaction((CTransaction)*this);