diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -3,6 +3,7 @@ This release includes the following features and fixes: + - Discourage selfish mining by favoring blocks with more accurate timestamps. - Remove support for Qt4 - Upgrade reproducible build to us Qt 5.9.6 - Improve SHA256 performance using SSE4.1, AVX2 and/or SHA if available. diff --git a/src/blockindexworkcomparator.h b/src/blockindexworkcomparator.h --- a/src/blockindexworkcomparator.h +++ b/src/blockindexworkcomparator.h @@ -18,6 +18,16 @@ return true; } + // ... then sort by most honestly mined, ... + int64_t aTimeDiff = std::llabs(pa->GetReceivedTimeDiff()); + int64_t bTimeDiff = std::llabs(pb->GetReceivedTimeDiff()); + if (aTimeDiff < bTimeDiff) { + return false; + } + if (aTimeDiff > bTimeDiff) { + return true; + } + // ... then by earliest time received, ... if (pa->nSequenceId < pb->nSequenceId) { return false; diff --git a/src/test/work_comparator_tests.cpp b/src/test/work_comparator_tests.cpp --- a/src/test/work_comparator_tests.cpp +++ b/src/test/work_comparator_tests.cpp @@ -16,14 +16,57 @@ indexB.nChainWork = 1; for (int sequenceIdA = 1; sequenceIdA < 1024; sequenceIdA *= 2) { for (int sequenceIdB = 1; sequenceIdB < 1024; sequenceIdB *= 2) { - // Differing sequenceId doesn't affect chain work - indexA.nSequenceId = sequenceIdA; - indexB.nSequenceId = sequenceIdB; - BOOST_CHECK(CBlockIndexWorkComparator()(&indexA, &indexB)); + for (int timeReceivedA = 1; timeReceivedA < 1024; + timeReceivedA *= 2) { + for (int timeReceivedB = 1; timeReceivedB < 1024; + timeReceivedB *= 2) { + // Differing sequenceId doesn't affect chain work + indexA.nSequenceId = sequenceIdA; + indexB.nSequenceId = sequenceIdB; + + // Differing received time doesn't affect chain work + indexA.nTimeReceived = timeReceivedA; + indexB.nTimeReceived = timeReceivedB; + + BOOST_CHECK(CBlockIndexWorkComparator()(&indexA, &indexB)); + } + } + } + } + + // Same chain work, but differing received time + indexA = CBlockIndex(); + indexB = CBlockIndex(); + for (int sequenceIdA = 1; sequenceIdA < 1024; sequenceIdA *= 2) { + for (int sequenceIdB = 1; sequenceIdB < 1024; sequenceIdB *= 2) { + for (int timeReceivedA = 1; timeReceivedA < 1024; + timeReceivedA *= 2) { + for (int timeReceivedB = 1; timeReceivedB < 1024; + timeReceivedB *= 2) { + if (timeReceivedA == timeReceivedB) { + continue; + } + + indexA.nTimeReceived = timeReceivedA; + indexB.nTimeReceived = timeReceivedB; + + // Differing sequenceId doesn't affect chain work + indexA.nSequenceId = sequenceIdA; + indexB.nSequenceId = sequenceIdB; + + if (timeReceivedA > timeReceivedB) { + BOOST_CHECK( + CBlockIndexWorkComparator()(&indexA, &indexB)); + } else { + BOOST_CHECK( + CBlockIndexWorkComparator()(&indexB, &indexA)); + } + } + } } } - // Same chain work, but differing sequenceId + // Same chain work and received time, but differing sequenceId indexA = CBlockIndex(); indexB = CBlockIndex(); for (int sequenceIdA = 1; sequenceIdA < 1024; sequenceIdA *= 2) { diff --git a/src/validation.cpp b/src/validation.cpp --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3486,9 +3486,18 @@ pindex->GetBlockHash().ToString(), newBlockTimeDiff); } - bool fHasMoreWork = + // New chain either has more work or is more honestly mined + bool hasMoreWork = (chainActive.Tip() ? pindex->nChainWork > chainActive.Tip()->nChainWork : true); + bool isMoreHonestlyMined = newBlockTimeDiff < chainTipTimeDiff; + bool chainTipShouldUpdate = + (isSameHeight && isMoreHonestlyMined) || hasMoreWork; + + if (chainTipShouldUpdate && isSameHeight && isMoreHonestlyMined) { + LogPrintf("More honestly broadcasted block will replace chaintip: %s\n", + pindex->GetBlockHash().ToString()); + } // Blocks that are too out-of-order needlessly limit the effectiveness of // pruning, because pruning will not delete block files that contain any @@ -3518,7 +3527,7 @@ } // Don't process less-work chains. - if (!fHasMoreWork) { + if (!chainTipShouldUpdate) { return true; } @@ -3546,7 +3555,7 @@ // Header is valid/has work and the merkle tree is good. // Relay now, but if it does not build on our best tip, let the // SendMessages loop relay it. - if (!IsInitialBlockDownload() && chainActive.Tip() == pindex->pprev) { + if (!IsInitialBlockDownload() && chainTipShouldUpdate) { GetMainSignals().NewPoWValidBlock(pindex, pblock); } diff --git a/test/functional/sendheaders-selfish-mining.py b/test/functional/sendheaders-selfish-mining.py new file mode 100755 --- /dev/null +++ b/test/functional/sendheaders-selfish-mining.py @@ -0,0 +1,145 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018 The Bitcoin ABC developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +"""Discourage selfish mining tests + +This test ensures that ABC gives priority to "honest" nodes when the selfish +mining behavior is obvious. The "obviousness" is determined by most accurate +timestamp at the time of receiving the block header. This selfish mining +discouragement should work regardless of if the selfish miner is broadcasting +before or after the honest miner. +""" + +import time + +from test_framework.blocktools import (create_block, create_coinbase) +from test_framework.mininode import ( + NetworkThread, + NodeConn, + NodeConnCB, + msg_block, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + p2p_port, +) + + +class DiscourageSelfishMiningTests(BitcoinTestFramework): + + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 2 + + def get_chaintip_hashes(self, node): + hashes = [] + for tip in node.getchaintips(): + hashes.append(tip['hash']) + return hashes + + def setmocktime(self, time): + for node in self.nodes: + node.setmocktime(time) + + def hash_to_int(self, hash_str): + return int(hash_str, 16) + + def get_tip(self, node): + return self.hash_to_int(node.getbestblockhash()) + + def run_test(self): + node_helpers = NodeConnCB() + connections = [] + connections.append( + NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], node_helpers)) + node_helpers.add_connection(connections[0]) + + # Start up network handling in another thread. This needs to be called + # after the P2P connections have been created. + NetworkThread().start() + # wait_for_verack ensures that the P2P connection is fully up. + node_helpers.wait_for_verack() + + self.nodes[0].generate(nblocks=1) + self.sync_all([self.nodes[0:1]]) + + block_time = self.nodes[0].getblock( + self.nodes[0].getbestblockhash())['time'] + 1 + self.setmocktime(block_time) + + common_block = self.get_tip(self.nodes[0]) + height = self.nodes[0].getblockcount() + 1 + + # Test 1.a: More accurate timestamped block is retained when less + # accurate block is withheld for some time, but both blocks are + # received at the same time. + selfish_block1 = create_block( + common_block, create_coinbase(height), block_time) + selfish_block1.solve() + + block_time += 3 + honest_block1 = create_block( + common_block, create_coinbase(height), block_time) + honest_block1.solve() + + # Wait for enough time to pass so that the selfish block is identified + self.setmocktime(block_time) + + # Selfishly mined block gets broadcasted in response to the honest block + node_helpers.send_and_ping(msg_block(honest_block1)) + node_helpers.send_and_ping(msg_block(selfish_block1)) + + # node[0] should have both blocks as chaintips, with the honest block as the active tip + assert(honest_block1.hash in self.get_chaintip_hashes(self.nodes[0])) + assert(selfish_block1.hash in self.get_chaintip_hashes(self.nodes[0])) + actual_tip = self.get_tip(self.nodes[0]) + assert_equal(actual_tip, honest_block1.sha256) + + block_time += 1 + height += 1 + + # Test 1.b: Less accurate block's chain is extended. The chaintip + # must switch now that that chain is longest PoW. + longest_pow_block1 = create_block( + self.hash_to_int(selfish_block1.hash), create_coinbase(height), block_time) + longest_pow_block1.solve() + node_helpers.send_and_ping(msg_block(longest_pow_block1)) + + print(self.hash_to_int(honest_block1.hash)) + print(self.hash_to_int(selfish_block1.hash)) + print(self.hash_to_int(self.nodes[0].getbestblockhash())) + print(self.hash_to_int(longest_pow_block1.hash)) + actual_tip = self.get_tip(self.nodes[0]) + assert_equal(actual_tip, longest_pow_block1.sha256) + + # Test 2: More accurately timestamped block replaces less accurate + # block despite receiving the less accurate block first. + common_block = actual_tip + selfish_block2 = create_block( + common_block, create_coinbase(height), block_time) + selfish_block2.solve() + + # Selfish miner holds onto the block for a while before broadcasting + block_time += 5 + self.setmocktime(block_time) + node_helpers.send_and_ping(msg_block(selfish_block2)) + + # Honest miner finds a block shortly after the selfish miner + # (without switching chaintips) and broadcasts immediately. + block_time += 1 + self.setmocktime(block_time) + honest_block2 = create_block( + common_block, create_coinbase(height), block_time) + honest_block2.solve() + node_helpers.send_and_ping(msg_block(honest_block2)) + + # Selfishly-mined block is no longer the active tip and synced to other + # node(s) as expected. + assert_equal(self.get_tip(self.nodes[1]), honest_block2.sha256) + + +if __name__ == '__main__': + DiscourageSelfishMiningTests().main()