diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -4,3 +4,4 @@ This release includes the following features and fixes: - Remove the bip9params configuration. + - Discourage selfish mining by favoring blocks with more accurate timestamps. 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 @@ -3881,10 +3881,17 @@ pindex->GetBlockHash().ToString(), newBlockTimeDiff); } - bool fHasMoreWork = + // New chain either has more work or is more honestly mined + bool chainTipShouldUpdate = + isSameHeightAndMoreHonestlyMined || (chainActive.Tip() ? pindex->nChainWork > chainActive.Tip()->nChainWork : true); + if (chainTipShouldUpdate && isSameHeightAndMoreHonestlyMined) { + 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 // blocks which are too close in height to the tip. Apply this test @@ -3913,7 +3920,7 @@ } // Don't process less-work chains. - if (!fHasMoreWork) { + if (!chainTipShouldUpdate) { return true; } @@ -3941,7 +3948,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,117 @@ +#!/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. 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 run_test(self): + node = NodeConnCB() + connections = [] + connections.append( + NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], node)) + node.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.wait_for_verack() + + self.nodes[0].generate(nblocks=1) + self.sync_all([self.nodes[0:1]]) + + # A selfish miner sending out headers in response to an honest block + # will not get the block relayed, as the honest block was seen first. + common_block = int(self.nodes[0].getbestblockhash(), 16) + block_time = self.nodes[0].getblock( + self.nodes[0].getbestblockhash())['time'] + 1 + height = self.nodes[0].getblockcount() + 1 + + 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 + time.sleep(3) + + # Selfishly mined block gets broadcasted in response to the honest block + node.send_and_ping(msg_block(honest_block1)) + node.send_and_ping(msg_block(selfish_block1)) + + actual_tip = int(self.nodes[0].getbestblockhash(), 16) + + # 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])) + assert_equal(actual_tip, honest_block1.sha256) + + common_block = actual_tip + block_time += 1 + height += 1 + + # A selfish miner withholding a block for too long will allow it to be replaced + # as the active chain tip by an honestly-mined block that was received at a more + # accurate time compared to the block timestamp. + 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 + time.sleep(5) + node.send_and_ping(msg_block(selfish_block2)) + block_time += 5 + + # Honest miner finds a block shortly after the selfish miner + # (without switching chaintips) and broadcasts immediately. + honest_block2 = create_block( + common_block, create_coinbase(height), block_time) + honest_block2.solve() + node.send_and_ping(msg_block(honest_block2)) + + # Selfishly-mined block is no longer the active tip + actual_tip = int(self.nodes[1].getbestblockhash(), 16) + assert_equal(actual_tip, honest_block2.sha256) + + +if __name__ == '__main__': + DiscourageSelfishMiningTests().main()