From e4dff510b461a7bf3f35332ebac19089efbd5517 Mon Sep 17 00:00:00 2001 From: Jay Graber Date: Wed, 28 Mar 2018 08:28:21 -0700 Subject: [PATCH 1/5] Set ban score for expired txs to 0 --- src/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.cpp b/src/main.cpp index ecf62c222..975aed2f6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -897,7 +897,7 @@ bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, // Check that all transactions are unexpired if (IsExpiredTx(tx, nHeight)) { - return state.DoS(dosLevel, error("ContextualCheckTransaction(): transaction is expired"), REJECT_INVALID, "tx-overwinter-expired"); + return state.DoS(0, error("ContextualCheckTransaction(): transaction is expired"), REJECT_INVALID, "tx-overwinter-expired"); } } From 697140ede506a4c9f9c1143cecc16915b75ba620 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Wed, 4 Apr 2018 00:01:23 +0100 Subject: [PATCH 2/5] Add support for Overwinter v3 transactions to mininode framework. --- qa/rpc-tests/test_framework/mininode.py | 36 ++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index 3c9821259..fcad43cf6 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -44,6 +44,8 @@ BIP0031_VERSION = 60000 MY_VERSION = 170002 # past bip-31 for ping/pong MY_SUBVERSION = "/python-mininode-tester:0.0.1/" +OVERWINTER_VERSION_GROUP_ID = 0x03C48270 + MAX_INV_SZ = 50000 @@ -565,6 +567,7 @@ class CTxOut(object): class CTransaction(object): def __init__(self, tx=None): if tx is None: + self.fOverwintered = False self.nVersion = 1 self.vin = [] self.vout = [] @@ -575,6 +578,7 @@ class CTransaction(object): self.sha256 = None self.hash = None else: + self.fOverwintered = tx.fOverwintered self.nVersion = tx.nVersion self.vin = copy.deepcopy(tx.vin) self.vout = copy.deepcopy(tx.vout) @@ -586,24 +590,46 @@ class CTransaction(object): self.hash = None def deserialize(self, f): - self.nVersion = struct.unpack("> 31) + self.nVersion = header & 0x7FFFFFFF + self.nVersionGroupId = (struct.unpack("= 2: self.vjoinsplit = deser_vector(f, JSDescription) if len(self.vjoinsplit) > 0: self.joinSplitPubKey = deser_uint256(f) self.joinSplitSig = f.read(64) + self.sha256 = None self.hash = None def serialize(self): + header = (int(self.fOverwintered)<<31) | self.nVersion + isOverwinterV3 = (self.fOverwintered and + self.nVersionGroupId == OVERWINTER_VERSION_GROUP_ID and + self.nVersion == 3) + r = "" - r += struct.pack("= 2: r += ser_vector(self.vjoinsplit) if len(self.vjoinsplit) > 0: @@ -628,8 +654,10 @@ class CTransaction(object): return True def __repr__(self): - r = "CTransaction(nVersion=%i vin=%s vout=%s nLockTime=%i" \ - % (self.nVersion, repr(self.vin), repr(self.vout), self.nLockTime) + r = ("CTransaction(fOverwintered=%r nVersion=%i nVersionGroupId=0x%08x " + "vin=%s vout=%s nLockTime=%i nExpiryHeight=%i" + % (self.fOverwintered, self.nVersion, self.nVersionGroupId, + repr(self.vin), repr(self.vout), self.nLockTime, self.nExpiryHeight)) if self.nVersion >= 2: r += " vjoinsplit=%s" % repr(self.vjoinsplit) if len(self.vjoinsplit) > 0: From a0ea82301ad0b4ea797282c2c60895ef3db57c12 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Wed, 4 Apr 2018 00:01:49 +0100 Subject: [PATCH 3/5] Test that receiving an expired transaction does not increase the peer's ban score. --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/p2p_txexpiry_dos.py | 120 +++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100755 qa/rpc-tests/p2p_txexpiry_dos.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index 07789a03a..9900b1c20 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -56,6 +56,7 @@ testScripts=( 'bipdersig-p2p.py' 'overwinter_peer_management.py' 'rewind_index.py' + 'p2p_txexpiry_dos.py' ); testScriptsExt=( 'getblocktemplate_longpoll.py' diff --git a/qa/rpc-tests/p2p_txexpiry_dos.py b/qa/rpc-tests/p2p_txexpiry_dos.py new file mode 100755 index 000000000..477fc5c01 --- /dev/null +++ b/qa/rpc-tests/p2p_txexpiry_dos.py @@ -0,0 +1,120 @@ +#!/usr/bin/env python2 +# Copyright (c) 2018 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.mininode import NodeConn, NodeConnCB, NetworkThread, \ + CTransaction, msg_tx, mininode_lock, OVERWINTER_PROTO_VERSION +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import initialize_chain_clean, start_nodes, \ + p2p_port, assert_equal + +import time, cStringIO +from binascii import hexlify, unhexlify + + +class TestNode(NodeConnCB): + def __init__(self): + NodeConnCB.__init__(self) + self.create_callback_map() + self.connection = None + + def add_connection(self, conn): + self.connection = conn + + # Spin until verack message is received from the node. + # We use this to signal that our test can begin. This + # is called from the testing thread, so it needs to acquire + # the global lock. + def wait_for_verack(self): + while True: + with mininode_lock: + if self.verack_received: + return + time.sleep(0.05) + + # Wrapper for the NodeConn's send_message function + def send_message(self, message): + self.connection.send_message(message) + + def on_close(self, conn): + pass + + def on_reject(self, conn, message): + conn.rejectMessage = message + + +class TxExpiryDoSTest(BitcoinTestFramework): + + def setup_chain(self): + print "Initializing test directory "+self.options.tmpdir + initialize_chain_clean(self.options.tmpdir, 1) + + def setup_network(self): + self.nodes = start_nodes(1, self.options.tmpdir, + extra_args=[['-nuparams=5ba81b19:10']]) + + def create_transaction(self, node, coinbase, to_address, amount, txModifier=None): + from_txid = node.getblock(coinbase)['tx'][0] + inputs = [{ "txid" : from_txid, "vout" : 0}] + outputs = { to_address : amount } + rawtx = node.createrawtransaction(inputs, outputs) + tx = CTransaction() + + if txModifier: + f = cStringIO.StringIO(unhexlify(rawtx)) + tx.deserialize(f) + txModifier(tx) + rawtx = hexlify(tx.serialize()) + + signresult = node.signrawtransaction(rawtx) + f = cStringIO.StringIO(unhexlify(signresult['hex'])) + tx.deserialize(f) + return tx + + def run_test(self): + test_node = TestNode() + + connections = [] + connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], + test_node, "regtest", True)) + test_node.add_connection(connections[0]) + + # Start up network handling in another thread + NetworkThread().start() + + test_node.wait_for_verack() + + # Verify mininodes are connected to zcashd nodes + peerinfo = self.nodes[0].getpeerinfo() + versions = [x["version"] for x in peerinfo] + assert_equal(1, versions.count(OVERWINTER_PROTO_VERSION)) + assert_equal(0, peerinfo[0]["banscore"]) + + self.coinbase_blocks = self.nodes[0].generate(1) + self.nodes[0].generate(100) + self.nodeaddress = self.nodes[0].getnewaddress() + + # Mininodes send transaction to zcashd node. + def setExpiryHeight(tx): + tx.nExpiryHeight = 1 + + spendtx = self.create_transaction(self.nodes[0], + self.coinbase_blocks[0], + self.nodeaddress, 1.0, + txModifier=setExpiryHeight) + test_node.send_message(msg_tx(spendtx)) + + time.sleep(3) + + # Verify test mininode has not been dropped + # and still has a banscore of 0. + peerinfo = self.nodes[0].getpeerinfo() + versions = [x["version"] for x in peerinfo] + assert_equal(1, versions.count(OVERWINTER_PROTO_VERSION)) + assert_equal(0, peerinfo[0]["banscore"]) + + [ c.disconnect_node() for c in connections ] + +if __name__ == '__main__': + TxExpiryDoSTest().main() From 473a1132419d05911933ac0095b207c4e2fc59f3 Mon Sep 17 00:00:00 2001 From: Daira Hopwood Date: Wed, 4 Apr 2018 01:28:57 +0100 Subject: [PATCH 4/5] Don't increase banscore if the transaction only just expired. Author: Jack Grigg --- qa/rpc-tests/p2p_txexpiry_dos.py | 15 ++++++++++++++- src/main.cpp | 4 +++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/qa/rpc-tests/p2p_txexpiry_dos.py b/qa/rpc-tests/p2p_txexpiry_dos.py index 477fc5c01..44838beff 100755 --- a/qa/rpc-tests/p2p_txexpiry_dos.py +++ b/qa/rpc-tests/p2p_txexpiry_dos.py @@ -97,7 +97,7 @@ class TxExpiryDoSTest(BitcoinTestFramework): # Mininodes send transaction to zcashd node. def setExpiryHeight(tx): - tx.nExpiryHeight = 1 + tx.nExpiryHeight = 101 spendtx = self.create_transaction(self.nodes[0], self.coinbase_blocks[0], @@ -114,6 +114,19 @@ class TxExpiryDoSTest(BitcoinTestFramework): assert_equal(1, versions.count(OVERWINTER_PROTO_VERSION)) assert_equal(0, peerinfo[0]["banscore"]) + # Mine a block and resend the transaction + self.nodes[0].generate(1) + test_node.send_message(msg_tx(spendtx)) + + time.sleep(3) + + # Verify test mininode has not been dropped + # but has a banscore of 10. + peerinfo = self.nodes[0].getpeerinfo() + versions = [x["version"] for x in peerinfo] + assert_equal(1, versions.count(OVERWINTER_PROTO_VERSION)) + assert_equal(10, peerinfo[0]["banscore"]) + [ c.disconnect_node() for c in connections ] if __name__ == '__main__': diff --git a/src/main.cpp b/src/main.cpp index 975aed2f6..e1b49a62b 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -897,7 +897,9 @@ bool ContextualCheckTransaction(const CTransaction& tx, CValidationState &state, // Check that all transactions are unexpired if (IsExpiredTx(tx, nHeight)) { - return state.DoS(0, error("ContextualCheckTransaction(): transaction is expired"), REJECT_INVALID, "tx-overwinter-expired"); + // Don't increase banscore if the transaction only just expired + int expiredDosLevel = IsExpiredTx(tx, nHeight - 1) ? dosLevel : 0; + return state.DoS(expiredDosLevel, error("ContextualCheckTransaction(): transaction is expired"), REJECT_INVALID, "tx-overwinter-expired"); } } From 10e97b8f007ef8629f1cdd8e7a426ce31f41c121 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 4 Apr 2018 22:47:38 +0100 Subject: [PATCH 5/5] test: Add missing Overwinter fields to mininode's CTransaction --- qa/rpc-tests/test_framework/mininode.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qa/rpc-tests/test_framework/mininode.py b/qa/rpc-tests/test_framework/mininode.py index fcad43cf6..12b48ae0b 100755 --- a/qa/rpc-tests/test_framework/mininode.py +++ b/qa/rpc-tests/test_framework/mininode.py @@ -569,9 +569,11 @@ class CTransaction(object): if tx is None: self.fOverwintered = False self.nVersion = 1 + self.nVersionGroupId = 0 self.vin = [] self.vout = [] self.nLockTime = 0 + self.nExpiryHeight = 0 self.vjoinsplit = [] self.joinSplitPubKey = None self.joinSplitSig = None @@ -580,9 +582,11 @@ class CTransaction(object): else: self.fOverwintered = tx.fOverwintered self.nVersion = tx.nVersion + self.nVersionGroupId = tx.nVersionGroupId self.vin = copy.deepcopy(tx.vin) self.vout = copy.deepcopy(tx.vout) self.nLockTime = tx.nLockTime + self.nExpiryHeight = tx.nExpiryHeight self.vjoinsplit = copy.deepcopy(tx.vjoinsplit) self.joinSplitPubKey = tx.joinSplitPubKey self.joinSplitSig = tx.joinSplitSig