Skip to content

Use confirmations instead of time to decide if tx is "safe" for mining #2759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
UdjinM6 wants to merge 3 commits into from
Closed

Use confirmations instead of time to decide if tx is "safe" for mining #2759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7a1c165
Use confirmations instead of time to decide if tx is "safe" for mining
UdjinM6 Mar 10, 2019
e06db02
s/confirmations/blocks/
UdjinM6 Mar 11, 2019
3a2783e
Split WAIT_FOR_ISLOCK_BLOCKS(6) into WAIT_FOR_ISLOCK_BLOCKS(4) and CL…
UdjinM6 Mar 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 36 additions & 25 deletions src/llmq/quorums_chainlocks.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,18 +298,18 @@ void CChainLocksHandler::TrySignChainTip()
}

for (auto& txid : *txids) {
int64_t txAge = 0;
int nMissedBlocks = 0;
{
LOCK(cs);
auto it = txFirstSeenTime.find(txid);
if (it != txFirstSeenTime.end()) {
txAge = GetAdjustedTime() - it->second;
auto it = txFirstSeenBlockHeight.find(txid);
if (it != txFirstSeenBlockHeight.end()) {
nMissedBlocks = pindex->nHeight - it->second;
}
}

if (txAge < WAIT_FOR_ISLOCK_TIMEOUT && !quorumInstantSendManager->IsLocked(txid)) {
LogPrintf("CChainLocksHandler::%s -- not signing block %s due to TX %s not being ixlocked and not old enough. age=%d\n", __func__,
pindexWalk->GetBlockHash().ToString(), txid.ToString(), txAge);
if (nMissedBlocks < WAIT_FOR_ISLOCK_BLOCKS && !quorumInstantSendManager->IsLocked(txid)) {
LogPrintf("CChainLocksHandler::%s -- not signing block %s due to TX %s not being ixlocked and not old enough, nMissedBlocks=%d\n", __func__,
pindexWalk->GetBlockHash().ToString(), txid.ToString(), nMissedBlocks);
return;
}
}
Expand Down Expand Up @@ -344,8 +344,6 @@ void CChainLocksHandler::NewPoWValidBlock(const CBlockIndex* pindex, const std::
return;
}

int64_t curTime = GetAdjustedTime();

// We listen for NewPoWValidBlock so that we can collect all TX ids of all included TXs of newly received blocks
// We need this information later when we try to sign a new tip, so that we can determine if all included TXs are
// safe.
Expand All @@ -359,7 +357,7 @@ void CChainLocksHandler::NewPoWValidBlock(const CBlockIndex* pindex, const std::
}
}
txs->emplace(tx->GetHash());
txFirstSeenTime.emplace(tx->GetHash(), curTime);
txFirstSeenBlockHeight.emplace(tx->GetHash(), pindex->nHeight - 1);
}
blockTxs[pindex->GetBlockHash()] = txs;
}
Expand All @@ -373,12 +371,25 @@ void CChainLocksHandler::SyncTransaction(const CTransaction& tx, const CBlockInd
}
}

LOCK(cs);
int64_t curTime = GetAdjustedTime();
txFirstSeenTime.emplace(tx.GetHash(), curTime);
uint256 txHash = tx.GetHash();

{
LOCK(cs);
if (txFirstSeenBlockHeight.count(txHash)) {
return;
}
}

if (pindex) {
LOCK(cs);
txFirstSeenBlockHeight.emplace(txHash, pindex->nHeight);
} else {
LOCK2(cs_main, cs);
txFirstSeenBlockHeight.emplace(txHash, chainActive.Height());
}
}

bool CChainLocksHandler::IsTxSafeForMining(const uint256& txid)
bool CChainLocksHandler::IsTxSafeForMining(const uint256& txid, int nHeight)
{
if (!sporkManager.IsSporkActive(SPORK_19_CHAINLOCKS_ENABLED) || !sporkManager.IsSporkActive(SPORK_3_INSTANTSEND_BLOCK_FILTERING)) {
return true;
Expand All @@ -387,16 +398,16 @@ bool CChainLocksHandler::IsTxSafeForMining(const uint256& txid)
return true;
}

int64_t txAge = 0;
int nMissedBlocks = 0;
{
LOCK(cs);
auto it = txFirstSeenTime.find(txid);
if (it != txFirstSeenTime.end()) {
txAge = GetAdjustedTime() - it->second;
auto it = txFirstSeenBlockHeight.find(txid);
if (it != txFirstSeenBlockHeight.end()) {
nMissedBlocks = nHeight - it->second;
}
}

if (txAge < WAIT_FOR_ISLOCK_TIMEOUT && !quorumInstantSendManager->IsLocked(txid)) {
if (nMissedBlocks < WAIT_FOR_ISLOCK_BLOCKS && !quorumInstantSendManager->IsLocked(txid)) {
return false;
}
return true;
Expand Down Expand Up @@ -594,7 +605,7 @@ void CChainLocksHandler::Cleanup()
auto pindex = mapBlockIndex.at(it->first);
if (InternalHasChainLock(pindex->nHeight, pindex->GetBlockHash())) {
for (auto& txid : *it->second) {
txFirstSeenTime.erase(txid);
txFirstSeenBlockHeight.erase(txid);
}
it = blockTxs.erase(it);
} else if (InternalHasConflictingChainLock(pindex->nHeight, pindex->GetBlockHash())) {
Expand All @@ -603,17 +614,17 @@ void CChainLocksHandler::Cleanup()
++it;
}
}
for (auto it = txFirstSeenTime.begin(); it != txFirstSeenTime.end(); ) {
for (auto it = txFirstSeenBlockHeight.begin(); it != txFirstSeenBlockHeight.end(); ) {
CTransactionRef tx;
uint256 hashBlock;
if (!GetTransaction(it->first, tx, Params().GetConsensus(), hashBlock)) {
// tx has vanished, probably due to conflicts
it = txFirstSeenTime.erase(it);
it = txFirstSeenBlockHeight.erase(it);
} else if (!hashBlock.IsNull()) {
auto pindex = mapBlockIndex.at(hashBlock);
if (chainActive.Tip()->GetAncestor(pindex->nHeight) == pindex && chainActive.Height() - pindex->nHeight >= 6) {
// tx got confirmed >= 6 times, so we can stop keeping track of it
it = txFirstSeenTime.erase(it);
if (chainActive.Tip()->GetAncestor(pindex->nHeight) == pindex && chainActive.Height() - pindex->nHeight >= CLEANUP_ISLOCK_BLOCKS) {
// tx got confirmed >= CLEANUP_ISLOCK_BLOCKS times, so we can stop keeping track of it
it = txFirstSeenBlockHeight.erase(it);
} else {
++it;
}
Expand Down
9 changes: 5 additions & 4 deletions src/llmq/quorums_chainlocks.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ class CChainLocksHandler : public CRecoveredSigsListener
static const int64_t CLEANUP_INTERVAL = 1000 * 30;
static const int64_t CLEANUP_SEEN_TIMEOUT = 24 * 60 * 60 * 1000;

// how long to wait for ixlocks until we consider a block with non-ixlocked TXs to be safe to sign
static const int64_t WAIT_FOR_ISLOCK_TIMEOUT = 10 * 60;
// how many blocks to wait for ixlocks until we consider a block with non-ixlocked TXs to be safe to sign
static const int WAIT_FOR_ISLOCK_BLOCKS = 4;
static const int CLEANUP_ISLOCK_BLOCKS = 6;

private:
CScheduler* scheduler;
Expand All @@ -68,7 +69,7 @@ class CChainLocksHandler : public CRecoveredSigsListener

// We keep track of txids from recently received blocks so that we can check if all TXs got ixlocked
std::unordered_map<uint256, std::shared_ptr<std::unordered_set<uint256, StaticSaltedHasher>>> blockTxs;
std::unordered_map<uint256, int64_t> txFirstSeenTime;
std::unordered_map<uint256, int> txFirstSeenBlockHeight;

std::map<uint256, int64_t> seenChainLocks;

Expand Down Expand Up @@ -97,7 +98,7 @@ class CChainLocksHandler : public CRecoveredSigsListener
bool HasChainLock(int nHeight, const uint256& blockHash);
bool HasConflictingChainLock(int nHeight, const uint256& blockHash);

bool IsTxSafeForMining(const uint256& txid);
bool IsTxSafeForMining(const uint256& txid, int nHeight);

private:
// these require locks to be held already
Expand Down
4 changes: 2 additions & 2 deletions src/miner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ bool BlockAssembler::TestPackageTransactions(const CTxMemPool::setEntries& packa
BOOST_FOREACH (const CTxMemPool::txiter it, package) {
if (!IsFinalTx(it->GetTx(), nHeight, nLockTimeCutoff))
return false;
if (!llmq::chainLocksHandler->IsTxSafeForMining(it->GetTx().GetHash())) {
if (!llmq::chainLocksHandler->IsTxSafeForMining(it->GetTx().GetHash(), nHeight)) {
return false;
}
}
Expand Down Expand Up @@ -337,7 +337,7 @@ bool BlockAssembler::TestForBlock(CTxMemPool::txiter iter)
if (!IsFinalTx(iter->GetTx(), nHeight, nLockTimeCutoff))
return false;

if (!llmq::chainLocksHandler->IsTxSafeForMining(iter->GetTx().GetHash())) {
if (!llmq::chainLocksHandler->IsTxSafeForMining(iter->GetTx().GetHash(), nHeight)) {
return false;
}

Expand Down