Skip to content

Commit

Permalink
Only sign ChainLocks when all included TXs are "safe"
Browse files Browse the repository at this point in the history
Safe means that the TX is either ixlocked or known since at least 10
minutes.

Also change miner code to only include safe TXs in block templates.
  • Loading branch information
codablock committed Mar 7, 2019
1 parent 96291e7 commit 2a7a5c6
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 0 deletions.
80 changes: 80 additions & 0 deletions src/llmq/quorums_chainlocks.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "quorums.h"
#include "quorums_chainlocks.h"
#include "quorums_instantsend.h"
#include "quorums_signing.h"
#include "quorums_utils.h"

Expand Down Expand Up @@ -262,6 +263,61 @@ void CChainLocksHandler::TrySignChainTip()
}
}

LogPrintf("CChainLocksHandler::%s -- trying to sign %s, height=%d\n", __func__, pindex->GetBlockHash().ToString(), pindex->nHeight);

// When the new IX system is activated, we only try to ChainLock blocks which include safe transactions. A TX is
// considered safe when it is ixlocked or at least known since 10 minutes (from mempool or block). These checks are
// performed for the tip (which we try to sign) and the previous 5 blocks. If a ChainLocked block is found on the
// way down, we consider all TXs to be safe.
if (IsNewInstantSendEnabled() && sporkManager.IsSporkActive(SPORK_3_INSTANTSEND_BLOCK_FILTERING)) {
auto pindexWalk = pindex;
while (pindexWalk) {
if (pindex->nHeight - pindexWalk->nHeight > 5) {
// no need to check further down, 6 confs is safe to assume that TXs below this height won't be
// ixlocked anymore if they aren't already
LogPrintf("CChainLocksHandler::%s -- tip and previous 5 blocks all safe\n", __func__);
break;
}
if (HasChainLock(pindexWalk->nHeight, pindexWalk->GetBlockHash())) {
// we don't care about ixlocks for TXs that are ChainLocked already
LogPrintf("CChainLocksHandler::%s -- chainlock at height %d \n", __func__, pindexWalk->nHeight);
break;
}

decltype(blockTxs.begin()->second) txids;
{
LOCK(cs);
auto it = blockTxs.find(pindexWalk->GetBlockHash());
if (it == blockTxs.end()) {
// this should actually not happen as NewPoWValidBlock should have been called before
LogPrintf("CChainLocksHandler::%s -- blockTxs for %s not found\n", __func__,
pindexWalk->GetBlockHash().ToString());
return;
}
txids = it->second;
}

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

if (txAge < WAIT_FOR_IXLOCK_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);
return;
}
}

pindexWalk = pindexWalk->pprev;
}
}

uint256 requestId = ::SerializeHash(std::make_pair(CLSIG_REQUESTID_PREFIX, pindex->nHeight));
uint256 msgHash = pindex->GetBlockHash();

Expand Down Expand Up @@ -324,6 +380,30 @@ void CChainLocksHandler::SyncTransaction(const CTransaction& tx, const CBlockInd
txFirstSeenTime.emplace(tx.GetHash(), curTime);
}

bool CChainLocksHandler::IsTxSafeForMining(const uint256& txid)
{
if (!sporkManager.IsSporkActive(SPORK_19_CHAINLOCKS_ENABLED) || !sporkManager.IsSporkActive(SPORK_3_INSTANTSEND_BLOCK_FILTERING)) {
return true;
}
if (!IsNewInstantSendEnabled()) {
return true;
}

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

if (txAge < WAIT_FOR_IXLOCK_TIMEOUT && !quorumInstantSendManager->IsLocked(txid)) {
return false;
}
return true;
}

// WARNING: cs_main and cs should not be held!
void CChainLocksHandler::EnforceBestChainLock()
{
Expand Down
5 changes: 5 additions & 0 deletions src/llmq/quorums_chainlocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +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_IXLOCK_TIMEOUT = 10 * 60;

private:
CScheduler* scheduler;
CCriticalSection cs;
Expand Down Expand Up @@ -94,6 +97,8 @@ class CChainLocksHandler : public CRecoveredSigsListener
bool HasChainLock(int nHeight, const uint256& blockHash);
bool HasConflictingChainLock(int nHeight, const uint256& blockHash);

bool IsTxSafeForMining(const uint256& txid);

private:
// these require locks to be held already
bool InternalHasChainLock(int nHeight, const uint256& blockHash);
Expand Down
10 changes: 10 additions & 0 deletions src/miner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "evo/deterministicmns.h"

#include "llmq/quorums_blockprocessor.h"
#include "llmq/quorums_chainlocks.h"

#include <algorithm>
#include <boost/thread.hpp>
Expand Down Expand Up @@ -453,6 +454,11 @@ void BlockAssembler::addPackageTxs(int &nPackagesSelected, int &nDescendantsUpda
continue;
}

if (!llmq::chainLocksHandler->IsTxSafeForMining(mi->GetTx().GetHash())) {
++mi;
continue;
}

// Now that mi is not stale, determine which transaction to evaluate:
// the next entry from mapTx, or the best from mapModifiedTx?
bool fUsingModified = false;
Expand Down Expand Up @@ -601,6 +607,10 @@ void BlockAssembler::addPriorityTxs()
continue;
}

if (!llmq::chainLocksHandler->IsTxSafeForMining(iter->GetTx().GetHash())) {
continue;
}

// If this tx fits in the block add it, otherwise keep looping
if (TestForBlock(iter)) {
AddToBlock(iter);
Expand Down

0 comments on commit 2a7a5c6

Please sign in to comment.