aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rwxr-xr-xcontrib/devtools/test_deterministic_coverage.sh1
-rw-r--r--contrib/gitian-descriptors/gitian-linux.yml6
-rw-r--r--contrib/gitian-descriptors/gitian-osx.yml6
-rw-r--r--contrib/gitian-descriptors/gitian-win.yml4
-rw-r--r--src/net_processing.cpp125
-rw-r--r--src/qt/bitcoingui.cpp5
-rw-r--r--src/rpc/blockchain.cpp7
-rwxr-xr-xtest/functional/feature_pruning.py19
8 files changed, 117 insertions, 56 deletions
diff --git a/contrib/devtools/test_deterministic_coverage.sh b/contrib/devtools/test_deterministic_coverage.sh
index 16d03e1fff..88ac850021 100755
--- a/contrib/devtools/test_deterministic_coverage.sh
+++ b/contrib/devtools/test_deterministic_coverage.sh
@@ -14,6 +14,7 @@ GCOV_EXECUTABLE="gcov"
# Disable tests known to cause non-deterministic behaviour and document the source or point of non-determinism.
NON_DETERMINISTIC_TESTS=(
+ "blockfilter_index_tests/blockfilter_index_initial_sync" # src/checkqueue.h: In CCheckQueue::Loop(): while (queue.empty()) { ... }
"coinselector_tests/knapsack_solver_test" # coinselector_tests.cpp: if (equal_sets(setCoinsRet, setCoinsRet2))
"denialofservice_tests/DoS_mapOrphans" # denialofservice_tests.cpp: it = mapOrphanTransactions.lower_bound(InsecureRand256());
"fs_tests/fsbridge_fstream" # deterministic test failure?
diff --git a/contrib/gitian-descriptors/gitian-linux.yml b/contrib/gitian-descriptors/gitian-linux.yml
index 4f2238abdb..c50750ed44 100644
--- a/contrib/gitian-descriptors/gitian-linux.yml
+++ b/contrib/gitian-descriptors/gitian-linux.yml
@@ -149,7 +149,7 @@ script: |
mkdir -p temp
pushd temp
tar -xf ../$SOURCEDIST
- find bitcoin-* | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST
+ find bitcoin-* | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST
popd
# Workaround for tarball not building with the bare tag version (prep)
@@ -182,8 +182,8 @@ script: |
find ${DISTNAME}/bin -type f -executable -print0 | xargs -0 -n1 -I{} ../contrib/devtools/split-debug.sh {} {} {}.dbg
find ${DISTNAME}/lib -type f -print0 | xargs -0 -n1 -I{} ../contrib/devtools/split-debug.sh {} {} {}.dbg
cp ../doc/README.md ${DISTNAME}/
- find ${DISTNAME} -not -name "*.dbg" | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz
- find ${DISTNAME} -name "*.dbg" | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}-debug.tar.gz
+ find ${DISTNAME} -not -name "*.dbg" | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz
+ find ${DISTNAME} -name "*.dbg" | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}-debug.tar.gz
cd ../../
rm -rf distsrc-${i}
done
diff --git a/contrib/gitian-descriptors/gitian-osx.yml b/contrib/gitian-descriptors/gitian-osx.yml
index 866d233f0f..8c51ce7159 100644
--- a/contrib/gitian-descriptors/gitian-osx.yml
+++ b/contrib/gitian-descriptors/gitian-osx.yml
@@ -113,7 +113,7 @@ script: |
mkdir -p temp
pushd temp
tar -xf ../$SOURCEDIST
- find bitcoin-* | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST
+ find bitcoin-* | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST
popd
# Workaround for tarball not building with the bare tag version (prep)
@@ -150,7 +150,7 @@ script: |
cp ${BASEPREFIX}/${i}/native/bin/${i}-pagestuff unsigned-app-${i}/pagestuff
mv dist unsigned-app-${i}
pushd unsigned-app-${i}
- find . | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-osx-unsigned.tar.gz
+ find . | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-osx-unsigned.tar.gz
popd
make deploy
@@ -160,7 +160,7 @@ script: |
find . -name "lib*.la" -delete
find . -name "lib*.a" -delete
rm -rf ${DISTNAME}/lib/pkgconfig
- find ${DISTNAME} | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz
+ find ${DISTNAME} | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-${i}.tar.gz
cd ../../
done
mkdir -p $OUTDIR/src
diff --git a/contrib/gitian-descriptors/gitian-win.yml b/contrib/gitian-descriptors/gitian-win.yml
index 58ba1ddad5..f227c6ad92 100644
--- a/contrib/gitian-descriptors/gitian-win.yml
+++ b/contrib/gitian-descriptors/gitian-win.yml
@@ -129,7 +129,7 @@ script: |
mkdir -p temp
pushd temp
tar -xf ../$SOURCEDIST
- find bitcoin-* | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST
+ find bitcoin-* | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ../$SOURCEDIST
mkdir -p $OUTDIR/src
cp ../$SOURCEDIST $OUTDIR/src
popd
@@ -175,6 +175,6 @@ script: |
cd $BUILD_DIR/windeploy
mkdir unsigned
cp $OUTDIR/bitcoin-*setup-unsigned.exe unsigned/
- find . | sort | tar --mtime="$REFERENCE_DATE\\\ $REFERENCE_TIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-win-unsigned.tar.gz
+ find . | sort | tar --mtime="$REFERENCE_DATETIME" --no-recursion --mode='u+rw,go+r-w,a+X' --owner=0 --group=0 -c -T - | gzip -9n > ${OUTDIR}/${DISTNAME}-win-unsigned.tar.gz
mv ${OUTDIR}/${DISTNAME}-x86_64-*-debug.zip ${OUTDIR}/${DISTNAME}-win64-debug.zip
mv ${OUTDIR}/${DISTNAME}-x86_64-*.zip ${OUTDIR}/${DISTNAME}-win64.zip
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 0d9e314afd..e57706980a 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -68,11 +68,13 @@ static constexpr int32_t MAX_PEER_TX_IN_FLIGHT = 100;
/** Maximum number of announced transactions from a peer */
static constexpr int32_t MAX_PEER_TX_ANNOUNCEMENTS = 2 * MAX_INV_SZ;
/** How many microseconds to delay requesting transactions from inbound peers */
-static constexpr int64_t INBOUND_PEER_TX_DELAY = 2 * 1000000;
+static constexpr int64_t INBOUND_PEER_TX_DELAY = 2 * 1000000; // 2 seconds
/** How long to wait (in microseconds) before downloading a transaction from an additional peer */
-static constexpr int64_t GETDATA_TX_INTERVAL = 60 * 1000000;
+static constexpr int64_t GETDATA_TX_INTERVAL = 60 * 1000000; // 1 minute
/** Maximum delay (in microseconds) for transaction requests to avoid biasing some peers over others. */
-static constexpr int64_t MAX_GETDATA_RANDOM_DELAY = 2 * 1000000;
+static constexpr int64_t MAX_GETDATA_RANDOM_DELAY = 2 * 1000000; // 2 seconds
+/** How long to wait (in microseconds) before expiring an in-flight getdata request to a peer */
+static constexpr int64_t TX_EXPIRY_INTERVAL = 10 * GETDATA_TX_INTERVAL;
static_assert(INBOUND_PEER_TX_DELAY >= MAX_GETDATA_RANDOM_DELAY,
"To preserve security, MAX_GETDATA_RANDOM_DELAY should not exceed INBOUND_PEER_DELAY");
/** Limit to avoid sending big packets. Not used in processing incoming GETDATA for compatibility */
@@ -343,8 +345,11 @@ struct CNodeState {
//! Store all the transactions a peer has recently announced
std::set<uint256> m_tx_announced;
- //! Store transactions which were requested by us
- std::set<uint256> m_tx_in_flight;
+ //! Store transactions which were requested by us, with timestamp
+ std::map<uint256, int64_t> m_tx_in_flight;
+
+ //! Periodically check for stuck getdata requests
+ int64_t m_check_expiry_timer{0};
};
TxDownloadState m_tx_download;
@@ -702,30 +707,40 @@ void UpdateTxRequestTime(const uint256& txid, int64_t request_time) EXCLUSIVE_LO
}
}
-
-void RequestTx(CNodeState* state, const uint256& txid, int64_t nNow) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
+int64_t CalculateTxGetDataTime(const uint256& txid, int64_t current_time, bool use_inbound_delay) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
- CNodeState::TxDownloadState& peer_download_state = state->m_tx_download;
- if (peer_download_state.m_tx_announced.size() >= MAX_PEER_TX_ANNOUNCEMENTS || peer_download_state.m_tx_announced.count(txid)) {
- // Too many queued announcements from this peer, or we already have
- // this announcement
- return;
- }
- peer_download_state.m_tx_announced.insert(txid);
-
int64_t process_time;
int64_t last_request_time = GetTxRequestTime(txid);
// First time requesting this tx
if (last_request_time == 0) {
- process_time = nNow;
+ process_time = current_time;
} else {
// Randomize the delay to avoid biasing some peers over others (such as due to
// fixed ordering of peer processing in ThreadMessageHandler)
process_time = last_request_time + GETDATA_TX_INTERVAL + GetRand(MAX_GETDATA_RANDOM_DELAY);
}
- // We delay processing announcements from non-preferred (eg inbound) peers
- if (!state->fPreferredDownload) process_time += INBOUND_PEER_TX_DELAY;
+ // We delay processing announcements from inbound peers
+ if (use_inbound_delay) process_time += INBOUND_PEER_TX_DELAY;
+
+ return process_time;
+}
+
+void RequestTx(CNodeState* state, const uint256& txid, int64_t nNow) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
+{
+ CNodeState::TxDownloadState& peer_download_state = state->m_tx_download;
+ if (peer_download_state.m_tx_announced.size() >= MAX_PEER_TX_ANNOUNCEMENTS ||
+ peer_download_state.m_tx_process_time.size() >= MAX_PEER_TX_ANNOUNCEMENTS ||
+ peer_download_state.m_tx_announced.count(txid)) {
+ // Too many queued announcements from this peer, or we already have
+ // this announcement
+ return;
+ }
+ peer_download_state.m_tx_announced.insert(txid);
+
+ // Calculate the time to try requesting this transaction. Use
+ // fPreferredDownload as a proxy for outbound peers.
+ int64_t process_time = CalculateTxGetDataTime(txid, nNow, !state->fPreferredDownload);
peer_download_state.m_tx_process_time.emplace(process_time, txid);
}
@@ -1544,12 +1559,19 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm
if (!vNotFound.empty()) {
// Let the peer know that we didn't find what it asked for, so it doesn't
- // have to wait around forever. Currently only SPV clients actually care
- // about this message: it's needed when they are recursively walking the
- // dependencies of relevant unconfirmed transactions. SPV clients want to
- // do that because they want to know about (and store and rebroadcast and
- // risk analyze) the dependencies of transactions relevant to them, without
- // having to download the entire memory pool.
+ // have to wait around forever.
+ // SPV clients care about this message: it's needed when they are
+ // recursively walking the dependencies of relevant unconfirmed
+ // transactions. SPV clients want to do that because they want to know
+ // about (and store and rebroadcast and risk analyze) the dependencies
+ // of transactions relevant to them, without having to download the
+ // entire memory pool.
+ // Also, other nodes can use these messages to automatically request a
+ // transaction from some other peer that annnounced it, and stop
+ // waiting for us to respond.
+ // In normal operation, we often send NOTFOUND messages for parents of
+ // transactions that we relay; if a peer is missing a parent, they may
+ // assume we have them and request the parents from us.
connman->PushMessage(pfrom, msgMaker.Make(NetMsgType::NOTFOUND, vNotFound));
}
}
@@ -3146,8 +3168,27 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
}
if (strCommand == NetMsgType::NOTFOUND) {
- // We do not care about the NOTFOUND message, but logging an Unknown Command
- // message would be undesirable as we transmit it ourselves.
+ // Remove the NOTFOUND transactions from the peer
+ LOCK(cs_main);
+ CNodeState *state = State(pfrom->GetId());
+ std::vector<CInv> vInv;
+ vRecv >> vInv;
+ if (vInv.size() <= MAX_PEER_TX_IN_FLIGHT + MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
+ for (CInv &inv : vInv) {
+ if (inv.type == MSG_TX || inv.type == MSG_WITNESS_TX) {
+ // If we receive a NOTFOUND message for a txid we requested, erase
+ // it from our data structures for this peer.
+ auto in_flight_it = state->m_tx_download.m_tx_in_flight.find(inv.hash);
+ if (in_flight_it == state->m_tx_download.m_tx_in_flight.end()) {
+ // Skip any further work if this is a spurious NOTFOUND
+ // message.
+ continue;
+ }
+ state->m_tx_download.m_tx_in_flight.erase(in_flight_it);
+ state->m_tx_download.m_tx_announced.erase(inv.hash);
+ }
+ }
+ }
return true;
}
@@ -3945,9 +3986,33 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
//
// Message: getdata (non-blocks)
//
+
+ // For robustness, expire old requests after a long timeout, so that
+ // we can resume downloading transactions from a peer even if they
+ // were unresponsive in the past.
+ // Eventually we should consider disconnecting peers, but this is
+ // conservative.
+ if (state.m_tx_download.m_check_expiry_timer <= nNow) {
+ for (auto it=state.m_tx_download.m_tx_in_flight.begin(); it != state.m_tx_download.m_tx_in_flight.end();) {
+ if (it->second <= nNow - TX_EXPIRY_INTERVAL) {
+ LogPrint(BCLog::NET, "timeout of inflight tx %s from peer=%d\n", it->first.ToString(), pto->GetId());
+ state.m_tx_download.m_tx_announced.erase(it->first);
+ state.m_tx_download.m_tx_in_flight.erase(it++);
+ } else {
+ ++it;
+ }
+ }
+ // On average, we do this check every TX_EXPIRY_INTERVAL. Randomize
+ // so that we're not doing this for all peers at the same time.
+ state.m_tx_download.m_check_expiry_timer = nNow + TX_EXPIRY_INTERVAL/2 + GetRand(TX_EXPIRY_INTERVAL);
+ }
+
auto& tx_process_time = state.m_tx_download.m_tx_process_time;
while (!tx_process_time.empty() && tx_process_time.begin()->first <= nNow && state.m_tx_download.m_tx_in_flight.size() < MAX_PEER_TX_IN_FLIGHT) {
- const uint256& txid = tx_process_time.begin()->second;
+ const uint256 txid = tx_process_time.begin()->second;
+ // Erase this entry from tx_process_time (it may be added back for
+ // processing at a later time, see below)
+ tx_process_time.erase(tx_process_time.begin());
CInv inv(MSG_TX | GetFetchFlags(pto), txid);
if (!AlreadyHave(inv)) {
// If this transaction was last requested more than 1 minute ago,
@@ -3961,20 +4026,20 @@ bool PeerLogicValidation::SendMessages(CNode* pto)
vGetData.clear();
}
UpdateTxRequestTime(inv.hash, nNow);
- state.m_tx_download.m_tx_in_flight.insert(inv.hash);
+ state.m_tx_download.m_tx_in_flight.emplace(inv.hash, nNow);
} else {
// This transaction is in flight from someone else; queue
// up processing to happen after the download times out
// (with a slight delay for inbound peers, to prefer
// requests to outbound peers).
- RequestTx(&state, txid, nNow);
+ int64_t next_process_time = CalculateTxGetDataTime(txid, nNow, !state.fPreferredDownload);
+ tx_process_time.emplace(next_process_time, txid);
}
} else {
// We have already seen this transaction, no need to download.
state.m_tx_download.m_tx_announced.erase(inv.hash);
state.m_tx_download.m_tx_in_flight.erase(inv.hash);
}
- tx_process_time.erase(tx_process_time.begin());
}
diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
index b5c92e10a2..1444dddeb1 100644
--- a/src/qt/bitcoingui.cpp
+++ b/src/qt/bitcoingui.cpp
@@ -335,7 +335,7 @@ void BitcoinGUI::createActions()
openAction->setStatusTip(tr("Open a bitcoin: URI or payment request"));
m_open_wallet_action = new QAction(tr("Open Wallet"), this);
- m_open_wallet_action->setMenu(new QMenu(this));
+ m_open_wallet_action->setEnabled(false);
m_open_wallet_action->setStatusTip(tr("Open a wallet"));
m_close_wallet_action = new QAction(tr("Close Wallet..."), this);
@@ -633,6 +633,9 @@ void BitcoinGUI::setWalletController(WalletController* wallet_controller)
m_wallet_controller = wallet_controller;
+ m_open_wallet_action->setEnabled(true);
+ m_open_wallet_action->setMenu(new QMenu(this));
+
connect(wallet_controller, &WalletController::walletAdded, this, &BitcoinGUI::addWallet);
connect(wallet_controller, &WalletController::walletRemoved, this, &BitcoinGUI::removeWallet);
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
index 49d7ae2016..50c4589d9f 100644
--- a/src/rpc/blockchain.cpp
+++ b/src/rpc/blockchain.cpp
@@ -1061,7 +1061,12 @@ static UniValue pruneblockchain(const JSONRPCRequest& request)
}
PruneBlockFilesManual(height);
- return uint64_t(height);
+ const CBlockIndex* block = ::ChainActive().Tip();
+ assert(block);
+ while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
+ block = block->pprev;
+ }
+ return uint64_t(block->nHeight);
}
static UniValue gettxoutsetinfo(const JSONRPCRequest& request)
diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py
index e2b3b2d544..66c395d7a2 100755
--- a/test/functional/feature_pruning.py
+++ b/test/functional/feature_pruning.py
@@ -23,8 +23,6 @@ from test_framework.util import (
wait_until,
)
-MIN_BLOCKS_TO_KEEP = 288
-
# Rescans start at the earliest block up to 2 hours before a key timestamp, so
# the manual prune RPC avoids pruning blocks in the same window to be
# compatible with pruning based on key creation time.
@@ -273,20 +271,9 @@ class PruneTest(BitcoinTestFramework):
else:
return index
- def prune(index, expected_ret=None):
+ def prune(index):
ret = node.pruneblockchain(height=height(index))
- # Check the return value. When use_timestamp is True, just check
- # that the return value is less than or equal to the expected
- # value, because when more than one block is generated per second,
- # a timestamp will not be granular enough to uniquely identify an
- # individual block.
- if expected_ret is None:
- expected_ret = index
- if use_timestamp:
- assert_greater_than(ret, 0)
- assert_greater_than(expected_ret + 1, ret)
- else:
- assert_equal(ret, expected_ret)
+ assert_equal(ret, node.getblockchaininfo()['pruneheight'])
def has_block(index):
return os.path.isfile(os.path.join(self.nodes[node_number].datadir, "regtest", "blocks", "blk{:05}.dat".format(index)))
@@ -326,7 +313,7 @@ class PruneTest(BitcoinTestFramework):
assert not has_block(1), "blk00001.dat is still there, should be pruned by now"
# height=1000 should not prune anything more, because tip-288 is in blk00002.dat.
- prune(1000, 1001 - MIN_BLOCKS_TO_KEEP)
+ prune(1000)
assert has_block(2), "blk00002.dat is still there, should be pruned by now"
# advance the tip so blk00002.dat and blk00003.dat can be pruned (the last 288 blocks should now be in blk00004.dat)