aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rwxr-xr-xqa/rpc-tests/p2p-compactblocks.py37
-rw-r--r--qa/rpc-tests/test_framework/authproxy.py11
-rw-r--r--qa/rpc-tests/test_framework/util.py4
-rwxr-xr-xqa/rpc-tests/wallet-dump.py6
-rw-r--r--src/blockencodings.cpp2
-rw-r--r--src/blockencodings.h2
-rw-r--r--src/main.cpp48
-rw-r--r--src/main.h2
-rw-r--r--src/net.cpp9
-rw-r--r--src/net.h5
-rw-r--r--src/qt/clientmodel.cpp3
-rw-r--r--src/qt/paymentserver.cpp2
-rw-r--r--src/rpc/mining.cpp4
-rw-r--r--src/test/miner_tests.cpp2
-rw-r--r--src/test/test_bitcoin.cpp2
-rw-r--r--src/version.h5
16 files changed, 116 insertions, 28 deletions
diff --git a/qa/rpc-tests/p2p-compactblocks.py b/qa/rpc-tests/p2p-compactblocks.py
index 6b5d477131..6d1fb3fd9a 100755
--- a/qa/rpc-tests/p2p-compactblocks.py
+++ b/qa/rpc-tests/p2p-compactblocks.py
@@ -708,6 +708,33 @@ class CompactBlocksTest(BitcoinTestFramework):
l.last_cmpctblock.header_and_shortids.header.calc_sha256()
assert_equal(l.last_cmpctblock.header_and_shortids.header.sha256, block.sha256)
+ # Test that we don't get disconnected if we relay a compact block with valid header,
+ # but invalid transactions.
+ def test_invalid_tx_in_compactblock(self, node, test_node, use_segwit):
+ assert(len(self.utxos))
+ utxo = self.utxos[0]
+
+ block = self.build_block_with_transactions(node, utxo, 5)
+ del block.vtx[3]
+ block.hashMerkleRoot = block.calc_merkle_root()
+ if use_segwit:
+ # If we're testing with segwit, also drop the coinbase witness,
+ # but include the witness commitment.
+ add_witness_commitment(block)
+ block.vtx[0].wit.vtxinwit = []
+ block.solve()
+
+ # Now send the compact block with all transactions prefilled, and
+ # verify that we don't get disconnected.
+ comp_block = HeaderAndShortIDs()
+ comp_block.initialize_from_block(block, prefill_list=[0, 1, 2, 3, 4], use_witness=use_segwit)
+ msg = msg_cmpctblock(comp_block.to_p2p())
+ test_node.send_and_ping(msg)
+
+ # Check that the tip didn't advance
+ assert(int(node.getbestblockhash(), 16) is not block.sha256)
+ test_node.sync_with_ping()
+
# Helper for enabling cb announcements
# Send the sendcmpct request and sync headers
def request_cb_announcements(self, peer, node, version):
@@ -798,6 +825,11 @@ class CompactBlocksTest(BitcoinTestFramework):
self.test_end_to_end_block_relay(self.nodes[0], [self.segwit_node, self.test_node, self.old_node])
self.test_end_to_end_block_relay(self.nodes[1], [self.segwit_node, self.test_node, self.old_node])
+ print("\tTesting handling of invalid compact blocks...")
+ self.test_invalid_tx_in_compactblock(self.nodes[0], self.test_node, False)
+ self.test_invalid_tx_in_compactblock(self.nodes[1], self.segwit_node, False)
+ self.test_invalid_tx_in_compactblock(self.nodes[1], self.old_node, False)
+
# Advance to segwit activation
print ("\nAdvancing to segwit activation\n")
self.activate_segwit(self.nodes[1])
@@ -844,6 +876,11 @@ class CompactBlocksTest(BitcoinTestFramework):
self.request_cb_announcements(self.segwit_node, self.nodes[1], 2)
self.test_end_to_end_block_relay(self.nodes[1], [self.segwit_node, self.test_node, self.old_node])
+ print("\tTesting handling of invalid compact blocks...")
+ self.test_invalid_tx_in_compactblock(self.nodes[0], self.test_node, False)
+ self.test_invalid_tx_in_compactblock(self.nodes[1], self.segwit_node, True)
+ self.test_invalid_tx_in_compactblock(self.nodes[1], self.old_node, True)
+
print("\tTesting invalid index in cmpctblock message...")
self.test_invalid_cmpctblock_message()
diff --git a/qa/rpc-tests/test_framework/authproxy.py b/qa/rpc-tests/test_framework/authproxy.py
index 2858de645d..9bee1962e2 100644
--- a/qa/rpc-tests/test_framework/authproxy.py
+++ b/qa/rpc-tests/test_framework/authproxy.py
@@ -42,6 +42,7 @@ import base64
import decimal
import json
import logging
+import socket
try:
import urllib.parse as urlparse
except ImportError:
@@ -161,7 +162,15 @@ class AuthServiceProxy(object):
return self._request('POST', self.__url.path, postdata.encode('utf-8'))
def _get_response(self):
- http_response = self.__conn.getresponse()
+ try:
+ http_response = self.__conn.getresponse()
+ except socket.timeout as e:
+ raise JSONRPCException({
+ 'code': -344,
+ 'message': '%r RPC took longer than %f seconds. Consider '
+ 'using larger timeout for calls that take '
+ 'longer to return.' % (self._service_name,
+ self.__conn.timeout)})
if http_response is None:
raise JSONRPCException({
'code': -342, 'message': 'missing HTTP response from server'})
diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py
index c818af4bd7..c0c2b3a6ef 100644
--- a/qa/rpc-tests/test_framework/util.py
+++ b/qa/rpc-tests/test_framework/util.py
@@ -341,7 +341,7 @@ def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=
return proxy
-def start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, binary=None):
+def start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, timewait=None, binary=None):
"""
Start multiple bitcoinds, return RPC connections to them
"""
@@ -350,7 +350,7 @@ def start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, binary=None):
rpcs = []
try:
for i in range(num_nodes):
- rpcs.append(start_node(i, dirname, extra_args[i], rpchost, binary=binary[i]))
+ rpcs.append(start_node(i, dirname, extra_args[i], rpchost, timewait=timewait, binary=binary[i]))
except: # If one node failed to start, stop the others
stop_nodes(rpcs)
raise
diff --git a/qa/rpc-tests/wallet-dump.py b/qa/rpc-tests/wallet-dump.py
index a37096a40c..c6dc2e3d10 100755
--- a/qa/rpc-tests/wallet-dump.py
+++ b/qa/rpc-tests/wallet-dump.py
@@ -61,7 +61,11 @@ class WalletDumpTest(BitcoinTestFramework):
self.extra_args = [["-keypool=90"]]
def setup_network(self, split=False):
- self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args)
+ # Use 1 minute timeout because the initial getnewaddress RPC can take
+ # longer than the default 30 seconds due to an expensive
+ # CWallet::TopUpKeyPool call, and the encryptwallet RPC made later in
+ # the test often takes even longer.
+ self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args, timewait=60)
def run_test (self):
tmpdir = self.options.tmpdir
diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp
index 93d3fa372b..737102f168 100644
--- a/src/blockencodings.cpp
+++ b/src/blockencodings.cpp
@@ -167,7 +167,7 @@ ReadStatus PartiallyDownloadedBlock::FillBlock(CBlock& block, const std::vector<
// check its own merkle root and cache that check.
if (state.CorruptionPossible())
return READ_STATUS_FAILED; // Possible Short ID collision
- return READ_STATUS_INVALID;
+ return READ_STATUS_CHECKBLOCK_FAILED;
}
LogPrint("cmpctblock", "Successfully reconstructed block %s with %lu txn prefilled, %lu txn from mempool and %lu txn requested\n", header.GetHash().ToString(), prefilled_count, mempool_count, vtx_missing.size());
diff --git a/src/blockencodings.h b/src/blockencodings.h
index 99b1cb140d..705eaf28aa 100644
--- a/src/blockencodings.h
+++ b/src/blockencodings.h
@@ -124,6 +124,8 @@ typedef enum ReadStatus_t
READ_STATUS_OK,
READ_STATUS_INVALID, // Invalid object, peer is sending bogus crap
READ_STATUS_FAILED, // Failed to process object
+ READ_STATUS_CHECKBLOCK_FAILED, // Used only by FillBlock to indicate a
+ // failure in CheckBlock.
} ReadStatus;
class CBlockHeaderAndShortTxIDs {
diff --git a/src/main.cpp b/src/main.cpp
index e0c614b731..7e5b9528b9 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -180,8 +180,10 @@ namespace {
* Sources of received blocks, saved to be able to send them reject
* messages or ban them when processing happens afterwards. Protected by
* cs_main.
+ * Set mapBlockSource[hash].second to false if the node should not be
+ * punished if the block is invalid.
*/
- map<uint256, NodeId> mapBlockSource;
+ map<uint256, std::pair<NodeId, bool>> mapBlockSource;
/**
* Filter for transactions that were recently rejected by
@@ -3785,7 +3787,7 @@ static bool AcceptBlock(const CBlock& block, CValidationState& state, const CCha
return true;
}
-bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp)
+bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool fMayBanPeerIfInvalid)
{
{
LOCK(cs_main);
@@ -3795,7 +3797,7 @@ bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, C
bool fNewBlock = false;
bool ret = AcceptBlock(*pblock, state, chainparams, &pindex, fForceProcessing, dbp, &fNewBlock);
if (pindex && pfrom) {
- mapBlockSource[pindex->GetBlockHash()] = pfrom->GetId();
+ mapBlockSource[pindex->GetBlockHash()] = std::make_pair(pfrom->GetId(), fMayBanPeerIfInvalid);
if (fNewBlock) pfrom->nLastBlockTime = GetTime();
}
CheckBlockIndex(chainparams.GetConsensus());
@@ -4775,16 +4777,16 @@ void PeerLogicValidation::BlockChecked(const CBlock& block, const CValidationSta
LOCK(cs_main);
const uint256 hash(block.GetHash());
- std::map<uint256, NodeId>::iterator it = mapBlockSource.find(hash);
+ std::map<uint256, std::pair<NodeId, bool>>::iterator it = mapBlockSource.find(hash);
int nDoS = 0;
if (state.IsInvalid(nDoS)) {
- if (it != mapBlockSource.end() && State(it->second)) {
+ if (it != mapBlockSource.end() && State(it->second.first)) {
assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes
CBlockReject reject = {(unsigned char)state.GetRejectCode(), state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), hash};
- State(it->second)->rejects.push_back(reject);
- if (nDoS > 0)
- Misbehaving(it->second, nDoS);
+ State(it->second.first)->rejects.push_back(reject);
+ if (nDoS > 0 && it->second.second)
+ Misbehaving(it->second.first, nDoS);
}
}
if (it != mapBlockSource.end())
@@ -5893,6 +5895,23 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
invs.push_back(CInv(MSG_BLOCK | GetFetchFlags(pfrom, chainActive.Tip(), chainparams.GetConsensus()), resp.blockhash));
connman.PushMessage(pfrom, NetMsgType::GETDATA, invs);
} else {
+ // Block is either okay, or possibly we received
+ // READ_STATUS_CHECKBLOCK_FAILED.
+ // Note that CheckBlock can only fail for one of a few reasons:
+ // 1. bad-proof-of-work (impossible here, because we've already
+ // accepted the header)
+ // 2. merkleroot doesn't match the transactions given (already
+ // caught in FillBlock with READ_STATUS_FAILED, so
+ // impossible here)
+ // 3. the block is otherwise invalid (eg invalid coinbase,
+ // block is too big, too many legacy sigops, etc).
+ // So if CheckBlock failed, #3 is the only possibility.
+ // Under BIP 152, we don't DoS-ban unless proof of work is
+ // invalid (we don't require all the stateless checks to have
+ // been run). This is handled below, so just treat this as
+ // though the block was successfully read, and rely on the
+ // handling in ProcessNewBlock to ensure the block index is
+ // updated, reject messages go out, etc.
MarkBlockAsReceived(resp.blockhash); // it is now an empty pointer
fBlockRead = true;
}
@@ -5901,16 +5920,15 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
CValidationState state;
// Since we requested this block (it was in mapBlocksInFlight), force it to be processed,
// even if it would not be a candidate for new tip (missing previous block, chain not long enough, etc)
- ProcessNewBlock(state, chainparams, pfrom, &block, true, NULL);
+ // BIP 152 permits peers to relay compact blocks after validating
+ // the header only; we should not punish peers if the block turns
+ // out to be invalid.
+ ProcessNewBlock(state, chainparams, pfrom, &block, true, NULL, false);
int nDoS;
if (state.IsInvalid(nDoS)) {
assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes
connman.PushMessage(pfrom, NetMsgType::REJECT, strCommand, (unsigned char)state.GetRejectCode(),
state.GetRejectReason().substr(0, MAX_REJECT_MESSAGE_LENGTH), block.GetHash());
- if (nDoS > 0) {
- LOCK(cs_main);
- Misbehaving(pfrom->GetId(), nDoS);
- }
}
}
}
@@ -6081,7 +6099,7 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
// need it even though it is not a candidate for a new best tip.
forceProcessing |= MarkBlockAsReceived(block.GetHash());
}
- ProcessNewBlock(state, chainparams, pfrom, &block, forceProcessing, NULL);
+ ProcessNewBlock(state, chainparams, pfrom, &block, forceProcessing, NULL, true);
int nDoS;
if (state.IsInvalid(nDoS)) {
assert (state.GetRejectCode() < REJECT_INTERNAL); // Blocks are never rejected with internal reject codes
@@ -6399,7 +6417,7 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman)
// Checksum
CDataStream& vRecv = msg.vRecv;
- uint256 hash = Hash(vRecv.begin(), vRecv.begin() + nMessageSize);
+ const uint256& hash = msg.GetMessageHash();
if (memcmp(hash.begin(), hdr.pchChecksum, CMessageHeader::CHECKSUM_SIZE) != 0)
{
LogPrintf("%s(%s, %u bytes): CHECKSUM ERROR expected %s was %s\n", __func__,
diff --git a/src/main.h b/src/main.h
index e80314a64b..9343330587 100644
--- a/src/main.h
+++ b/src/main.h
@@ -223,7 +223,7 @@ static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 550 * 1024 * 1024;
* @param[out] dbp The already known disk position of pblock, or NULL if not yet stored.
* @return True if state.IsValid()
*/
-bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp);
+bool ProcessNewBlock(CValidationState& state, const CChainParams& chainparams, CNode* pfrom, const CBlock* pblock, bool fForceProcessing, const CDiskBlockPos* dbp, bool fMayBanPeerIfInvalid);
/** Check whether enough disk space is available for an incoming block */
bool CheckDiskSpace(uint64_t nAdditionalBytes = 0);
/** Open a block file (blk?????.dat) */
diff --git a/src/net.cpp b/src/net.cpp
index 4ab8ef98ab..e47a8bb168 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -742,12 +742,21 @@ int CNetMessage::readData(const char *pch, unsigned int nBytes)
vRecv.resize(std::min(hdr.nMessageSize, nDataPos + nCopy + 256 * 1024));
}
+ hasher.Write((const unsigned char*)pch, nCopy);
memcpy(&vRecv[nDataPos], pch, nCopy);
nDataPos += nCopy;
return nCopy;
}
+const uint256& CNetMessage::GetMessageHash() const
+{
+ assert(complete());
+ if (data_hash.IsNull())
+ hasher.Finalize(data_hash.begin());
+ return data_hash;
+}
+
diff --git a/src/net.h b/src/net.h
index 22b80fc504..e712953be3 100644
--- a/src/net.h
+++ b/src/net.h
@@ -543,6 +543,9 @@ public:
class CNetMessage {
+private:
+ mutable CHash256 hasher;
+ mutable uint256 data_hash;
public:
bool in_data; // parsing header (false) or data (true)
@@ -570,6 +573,8 @@ public:
return (hdr.nMessageSize == nDataPos);
}
+ const uint256& GetMessageHash() const;
+
void SetVersion(int nVersionIn)
{
hdrbuf.SetVersion(nVersionIn);
diff --git a/src/qt/clientmodel.cpp b/src/qt/clientmodel.cpp
index 87704c641d..f9caca6878 100644
--- a/src/qt/clientmodel.cpp
+++ b/src/qt/clientmodel.cpp
@@ -6,6 +6,7 @@
#include "bantablemodel.h"
#include "guiconstants.h"
+#include "guiutil.h"
#include "peertablemodel.h"
#include "chainparams.h"
@@ -208,7 +209,7 @@ QString ClientModel::formatClientStartupTime() const
QString ClientModel::dataDir() const
{
- return QString::fromStdString(GetDataDir().string());
+ return GUIUtil::boostPathToQString(GetDataDir());
}
void ClientModel::updateBanlist()
diff --git a/src/qt/paymentserver.cpp b/src/qt/paymentserver.cpp
index 9f23e77a13..478f5ccf12 100644
--- a/src/qt/paymentserver.cpp
+++ b/src/qt/paymentserver.cpp
@@ -80,7 +80,7 @@ static QString ipcServerName()
// Append a simple hash of the datadir
// Note that GetDataDir(true) returns a different path
// for -testnet versus main net
- QString ddir(QString::fromStdString(GetDataDir(true).string()));
+ QString ddir(GUIUtil::boostPathToQString(GetDataDir(true)));
name.append(QString::number(qHash(ddir)));
return name;
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index f418262f02..3e91a79a64 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -132,7 +132,7 @@ UniValue generateBlocks(boost::shared_ptr<CReserveScript> coinbaseScript, int nG
continue;
}
CValidationState state;
- if (!ProcessNewBlock(state, Params(), NULL, pblock, true, NULL))
+ if (!ProcessNewBlock(state, Params(), NULL, pblock, true, NULL, false))
throw JSONRPCError(RPC_INTERNAL_ERROR, "ProcessNewBlock, block not accepted");
++nHeight;
blockHashes.push_back(pblock->GetHash().GetHex());
@@ -757,7 +757,7 @@ UniValue submitblock(const JSONRPCRequest& request)
CValidationState state;
submitblock_StateCatcher sc(block.GetHash());
RegisterValidationInterface(&sc);
- bool fAccepted = ProcessNewBlock(state, Params(), NULL, &block, true, NULL);
+ bool fAccepted = ProcessNewBlock(state, Params(), NULL, &block, true, NULL, false);
UnregisterValidationInterface(&sc);
if (fBlockPresent)
{
diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index a94979fd77..2762cafa38 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
pblock->hashMerkleRoot = BlockMerkleRoot(*pblock);
pblock->nNonce = blockinfo[i].nonce;
CValidationState state;
- BOOST_CHECK(ProcessNewBlock(state, chainparams, NULL, pblock, true, NULL));
+ BOOST_CHECK(ProcessNewBlock(state, chainparams, NULL, pblock, true, NULL, false));
BOOST_CHECK(state.IsValid());
pblock->hashPrevBlock = pblock->GetHash();
}
diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp
index 98f4ed939f..3da0be8ca4 100644
--- a/src/test/test_bitcoin.cpp
+++ b/src/test/test_bitcoin.cpp
@@ -127,7 +127,7 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransaction>&
while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce;
CValidationState state;
- ProcessNewBlock(state, chainparams, NULL, &block, true, NULL);
+ ProcessNewBlock(state, chainparams, NULL, &block, true, NULL, false);
CBlock result = block;
return result;
diff --git a/src/version.h b/src/version.h
index 87bd655066..87fb1a3a75 100644
--- a/src/version.h
+++ b/src/version.h
@@ -9,7 +9,7 @@
* network protocol versioning
*/
-static const int PROTOCOL_VERSION = 70014;
+static const int PROTOCOL_VERSION = 70015;
//! initial proto version, to be increased after version/verack negotiation
static const int INIT_PROTO_VERSION = 209;
@@ -42,4 +42,7 @@ static const int FEEFILTER_VERSION = 70013;
//! short-id-based block download starts with this version
static const int SHORT_IDS_BLOCKS_VERSION = 70014;
+//! not banning for invalid compact blocks starts with this version
+static const int INVALID_CB_NO_BAN_VERSION = 70015;
+
#endif // BITCOIN_VERSION_H
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-13 21:07:54 +0000