From 7f073594c9f5b518dc1fb66dfb0189e8803e3545 Mon Sep 17 00:00:00 2001
From: Jon Atack <jon@atack.com>
Date: Sun, 11 Jul 2021 12:18:43 +0200
Subject: [PATCH 1/9] Test src/node/transaction::GetTransaction() without
 -txindex

---
 test/functional/rpc_rawtransaction.py | 59 ++++++++++++++++-----------
 1 file changed, 36 insertions(+), 23 deletions(-)

diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index 9d4a5525d14..4c3dcac0465 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -50,11 +50,12 @@ class multidict(dict):
 class RawTransactionsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
-        self.num_nodes = 3
+        self.num_nodes = 4
         self.extra_args = [
             ["-txindex"],
             ["-txindex"],
             ["-txindex"],
+            [],
         ]
         # whitelist all peers to speed up tx relay / mempool sync
         for args in self.extra_args:
@@ -226,28 +227,40 @@ class RawTransactionsTest(BitcoinTestFramework):
         tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1)
         block1, block2 = self.nodes[2].generate(2)
         self.sync_all()
-        # We should be able to get the raw transaction by providing the correct block
-        gottx = self.nodes[0].getrawtransaction(tx, True, block1)
-        assert_equal(gottx['txid'], tx)
-        assert_equal(gottx['in_active_chain'], True)
-        # We should not have the 'in_active_chain' flag when we don't provide a block
-        gottx = self.nodes[0].getrawtransaction(tx, True)
-        assert_equal(gottx['txid'], tx)
-        assert 'in_active_chain' not in gottx
-        # We should not get the tx if we provide an unrelated block
-        assert_raises_rpc_error(-5, "No such transaction found", self.nodes[0].getrawtransaction, tx, True, block2)
-        # An invalid block hash should raise the correct errors
-        assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getrawtransaction, tx, True, True)
-        assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[0].getrawtransaction, tx, True, "foobar")
-        assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[0].getrawtransaction, tx, True, "abcd1234")
-        assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getrawtransaction, tx, True, "ZZZ0000000000000000000000000000000000000000000000000000000000000")
-        assert_raises_rpc_error(-5, "Block hash not found", self.nodes[0].getrawtransaction, tx, True, "0000000000000000000000000000000000000000000000000000000000000000")
-        # Undo the blocks and check in_active_chain
-        self.nodes[0].invalidateblock(block1)
-        gottx = self.nodes[0].getrawtransaction(txid=tx, verbose=True, blockhash=block1)
-        assert_equal(gottx['in_active_chain'], False)
-        self.nodes[0].reconsiderblock(block1)
-        assert_equal(self.nodes[0].getbestblockhash(), block2)
+        for n in [0, 3]:
+            self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex, with blockhash")
+            # We should be able to get the raw transaction by providing the correct block
+            gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1)
+            assert_equal(gottx['txid'], tx)
+            assert_equal(gottx['in_active_chain'], True)
+            if n == 0:
+                self.log.info("Test getrawtransaction with -txindex, without blockhash: 'in_active_chain' should be absent")
+                gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True)
+                assert_equal(gottx['txid'], tx)
+                assert 'in_active_chain' not in gottx
+            else:
+                self.log.info("Test getrawtransaction without -txindex, without blockhash: expect the call to raise")
+                err_msg = (
+                    "No such mempool transaction. Use -txindex or provide a block hash to enable"
+                    " blockchain transaction queries. Use gettransaction for wallet transactions."
+                )
+                assert_raises_rpc_error(-5, err_msg, self.nodes[n].getrawtransaction, txid=tx, verbose=True)
+            # We should not get the tx if we provide an unrelated block
+            assert_raises_rpc_error(-5, "No such transaction found", self.nodes[n].getrawtransaction, txid=tx, blockhash=block2)
+            # An invalid block hash should raise the correct errors
+            assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[n].getrawtransaction, txid=tx, blockhash=True)
+            assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[n].getrawtransaction, txid=tx, blockhash="foobar")
+            assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[n].getrawtransaction, txid=tx, blockhash="abcd1234")
+            foo = "ZZZ0000000000000000000000000000000000000000000000000000000000000"
+            assert_raises_rpc_error(-8, f"parameter 3 must be hexadecimal string (not '{foo}')", self.nodes[n].getrawtransaction, txid=tx, blockhash=foo)
+            bar = "0000000000000000000000000000000000000000000000000000000000000000"
+            assert_raises_rpc_error(-5, "Block hash not found", self.nodes[n].getrawtransaction, txid=tx, blockhash=bar)
+            # Undo the blocks and verify that "in_active_chain" is false.
+            self.nodes[n].invalidateblock(block1)
+            gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1)
+            assert_equal(gottx['in_active_chain'], False)
+            self.nodes[n].reconsiderblock(block1)
+            assert_equal(self.nodes[n].getbestblockhash(), block2)
 
         if not self.options.descriptors:
             # The traditional multisig workflow does not work with descriptor wallets so these are legacy only.

From 8c19d1329f1f28000ca32d826cecf04680c6be69 Mon Sep 17 00:00:00 2001
From: Jon Atack <jon@atack.com>
Date: Mon, 12 Jul 2021 14:48:54 +0200
Subject: [PATCH 2/9] refactor: dedup/reorg createrawtransaction sequence
 number tests

---
 test/functional/rpc_rawtransaction.py | 36 ++++++++++-----------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index 4c3dcac0465..deb2a668af0 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -105,7 +105,19 @@ class RawTransactionsTest(BitcoinTestFramework):
         assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid}], {})
         assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': 'foo'}], {})
         assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': -1}], {})
-        assert_raises_rpc_error(-8, "Invalid parameter, sequence number is out of range", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': 0, 'sequence': -1}], {})
+        # sequence number out of range
+        for invalid_seq in [-1, 4294967296]:
+            inputs = [{'txid': txid, 'vout': 1, 'sequence': invalid_seq}]
+            outputs = {self.nodes[0].getnewaddress(): 1}
+            assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range',
+                                    self.nodes[0].createrawtransaction, inputs, outputs)
+        # with valid sequence number
+        for valid_seq in [1000, 4294967294]:
+            inputs = [{'txid': txid, 'vout': 1, 'sequence': valid_seq}]
+            outputs = {self.nodes[0].getnewaddress(): 1}
+            rawtx = self.nodes[0].createrawtransaction(inputs, outputs)
+            decrawtx = self.nodes[0].decoderawtransaction(rawtx)
+            assert_equal(decrawtx['vin'][0]['sequence'], valid_seq)
 
         # Test `createrawtransaction` invalid `outputs`
         address = self.nodes[0].getnewaddress()
@@ -437,28 +449,6 @@ class RawTransactionsTest(BitcoinTestFramework):
         # 8. invalid parameters - supply txid and empty dict
         assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, {})
 
-        inputs  = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 1000}]
-        outputs = { self.nodes[0].getnewaddress() : 1 }
-        rawtx   = self.nodes[0].createrawtransaction(inputs, outputs)
-        decrawtx= self.nodes[0].decoderawtransaction(rawtx)
-        assert_equal(decrawtx['vin'][0]['sequence'], 1000)
-
-        # 9. invalid parameters - sequence number out of range
-        inputs  = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : -1}]
-        outputs = { self.nodes[0].getnewaddress() : 1 }
-        assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range', self.nodes[0].createrawtransaction, inputs, outputs)
-
-        # 10. invalid parameters - sequence number out of range
-        inputs  = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 4294967296}]
-        outputs = { self.nodes[0].getnewaddress() : 1 }
-        assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range', self.nodes[0].createrawtransaction, inputs, outputs)
-
-        inputs  = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 4294967294}]
-        outputs = { self.nodes[0].getnewaddress() : 1 }
-        rawtx   = self.nodes[0].createrawtransaction(inputs, outputs)
-        decrawtx= self.nodes[0].decoderawtransaction(rawtx)
-        assert_equal(decrawtx['vin'][0]['sequence'], 4294967294)
-
         ####################################
         # TRANSACTION VERSION NUMBER TESTS #
         ####################################

From 00977407732969593800d15de39abbb7e0250abc Mon Sep 17 00:00:00 2001
From: Jon Atack <jon@atack.com>
Date: Mon, 12 Jul 2021 16:12:21 +0200
Subject: [PATCH 3/9] refactor: txid to constant in rpc_rawtransaction to
 isolate tests

---
 test/functional/rpc_rawtransaction.py | 38 ++++++++++++++-------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index deb2a668af0..619ad1bcd78 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -28,6 +28,9 @@ from test_framework.util import (
 )
 
 
+TXID = "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000"
+
+
 class multidict(dict):
     """Dictionary that allows duplicate keys.
 
@@ -96,24 +99,23 @@ class RawTransactionsTest(BitcoinTestFramework):
         assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, [], {}, 0, False, 'foo')
 
         # Test `createrawtransaction` invalid `inputs`
-        txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000'
         assert_raises_rpc_error(-3, "Expected type array", self.nodes[0].createrawtransaction, 'foo', {})
         assert_raises_rpc_error(-1, "JSON value is not an object as expected", self.nodes[0].createrawtransaction, ['foo'], {})
         assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].createrawtransaction, [{}], {})
         assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {})
         assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", self.nodes[0].createrawtransaction, [{'txid': 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844'}], {})
-        assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid}], {})
-        assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': 'foo'}], {})
-        assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': -1}], {})
+        assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': TXID}], {})
+        assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': TXID, 'vout': 'foo'}], {})
+        assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", self.nodes[0].createrawtransaction, [{'txid': TXID, 'vout': -1}], {})
         # sequence number out of range
         for invalid_seq in [-1, 4294967296]:
-            inputs = [{'txid': txid, 'vout': 1, 'sequence': invalid_seq}]
+            inputs = [{'txid': TXID, 'vout': 1, 'sequence': invalid_seq}]
             outputs = {self.nodes[0].getnewaddress(): 1}
             assert_raises_rpc_error(-8, 'Invalid parameter, sequence number is out of range',
                                     self.nodes[0].createrawtransaction, inputs, outputs)
         # with valid sequence number
         for valid_seq in [1000, 4294967294]:
-            inputs = [{'txid': txid, 'vout': 1, 'sequence': valid_seq}]
+            inputs = [{'txid': TXID, 'vout': 1, 'sequence': valid_seq}]
             outputs = {self.nodes[0].getnewaddress(): 1}
             rawtx = self.nodes[0].createrawtransaction(inputs, outputs)
             decrawtx = self.nodes[0].decoderawtransaction(rawtx)
@@ -146,25 +148,25 @@ class RawTransactionsTest(BitcoinTestFramework):
 
         self.log.info('Check that createrawtransaction accepts an array and object as outputs')
         # One output
-        tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs={address: 99}))
+        tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs={address: 99}))
         assert_equal(len(tx.vout), 1)
         assert_equal(
             tx.serialize().hex(),
-            self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}]),
+            self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}]),
         )
         # Two outputs
-        tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=OrderedDict([(address, 99), (address2, 99)])))
+        tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=OrderedDict([(address, 99), (address2, 99)])))
         assert_equal(len(tx.vout), 2)
         assert_equal(
             tx.serialize().hex(),
-            self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}, {address2: 99}]),
+            self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}, {address2: 99}]),
         )
         # Multiple mixed outputs
-        tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=multidict([(address, 99), (address2, 99), ('data', '99')])))
+        tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=multidict([(address, 99), (address2, 99), ('data', '99')])))
         assert_equal(len(tx.vout), 3)
         assert_equal(
             tx.serialize().hex(),
-            self.nodes[2].createrawtransaction(inputs=[{'txid': txid, 'vout': 9}], outputs=[{address: 99}, {address2: 99}, {'data': '99'}]),
+            self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}, {address2: 99}, {'data': '99'}]),
         )
 
         for type in ["bech32", "p2sh-segwit", "legacy"]:
@@ -175,11 +177,11 @@ class RawTransactionsTest(BitcoinTestFramework):
             self.log.info('sendrawtransaction with missing prevtx info (%s)' %(type))
 
             # Test `signrawtransactionwithwallet` invalid `prevtxs`
-            inputs  = [ {'txid' : txid, 'vout' : 3, 'sequence' : 1000}]
+            inputs  = [ {'txid' : TXID, 'vout' : 3, 'sequence' : 1000}]
             outputs = { self.nodes[0].getnewaddress() : 1 }
             rawtx   = self.nodes[0].createrawtransaction(inputs, outputs)
 
-            prevtx = dict(txid=txid, scriptPubKey=pubkey, vout=3, amount=1)
+            prevtx = dict(txid=TXID, scriptPubKey=pubkey, vout=3, amount=1)
             succ = self.nodes[0].signrawtransactionwithwallet(rawtx, [prevtx])
             assert succ["complete"]
             if type == "legacy":
@@ -190,7 +192,7 @@ class RawTransactionsTest(BitcoinTestFramework):
             if type != "legacy":
                 assert_raises_rpc_error(-3, "Missing amount", self.nodes[0].signrawtransactionwithwallet, rawtx, [
                     {
-                        "txid": txid,
+                        "txid": TXID,
                         "scriptPubKey": pubkey,
                         "vout": 3,
                     }
@@ -198,7 +200,7 @@ class RawTransactionsTest(BitcoinTestFramework):
 
             assert_raises_rpc_error(-3, "Missing vout", self.nodes[0].signrawtransactionwithwallet, rawtx, [
                 {
-                    "txid": txid,
+                    "txid": TXID,
                     "scriptPubKey": pubkey,
                     "amount": 1,
                 }
@@ -212,7 +214,7 @@ class RawTransactionsTest(BitcoinTestFramework):
             ])
             assert_raises_rpc_error(-3, "Missing scriptPubKey", self.nodes[0].signrawtransactionwithwallet, rawtx, [
                 {
-                    "txid": txid,
+                    "txid": TXID,
                     "vout": 3,
                     "amount": 1
                 }
@@ -223,7 +225,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         #########################################
 
         self.log.info('sendrawtransaction with missing input')
-        inputs  = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1}] #won't exists
+        inputs  = [{'txid' : TXID, 'vout' : 1}]  # won't exist
         outputs = { self.nodes[0].getnewaddress() : 4.998 }
         rawtx   = self.nodes[2].createrawtransaction(inputs, outputs)
         rawtx   = self.nodes[2].signrawtransactionwithwallet(rawtx)

From 85d8869cf89fedf243748e3e15b3ed39de1b0385 Mon Sep 17 00:00:00 2001
From: Jon Atack <jon@atack.com>
Date: Sun, 11 Jul 2021 15:42:42 +0200
Subject: [PATCH 4/9] test: run 2nd getrawtransaction section with/without
 -txindex

(and make the 'string "Flase"' test clearer as requested by reviewers)
---
 test/functional/rpc_rawtransaction.py | 37 +++++++++++++++------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index 619ad1bcd78..89088e9fc6b 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -426,30 +426,33 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.sync_all()
 
         # getrawtransaction tests
-        # 1. valid parameters - only supply txid
-        assert_equal(self.nodes[0].getrawtransaction(txId), rawTxSigned['hex'])
+        for n in [0, 3]:
+            self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex")
+            # 1. valid parameters - only supply txid
+            assert_equal(self.nodes[n].getrawtransaction(txId), rawTxSigned['hex'])
 
-        # 2. valid parameters - supply txid and 0 for non-verbose
-        assert_equal(self.nodes[0].getrawtransaction(txId, 0), rawTxSigned['hex'])
+            # 2. valid parameters - supply txid and 0 for non-verbose
+            assert_equal(self.nodes[n].getrawtransaction(txId, 0), rawTxSigned['hex'])
 
-        # 3. valid parameters - supply txid and False for non-verbose
-        assert_equal(self.nodes[0].getrawtransaction(txId, False), rawTxSigned['hex'])
+            # 3. valid parameters - supply txid and False for non-verbose
+            assert_equal(self.nodes[n].getrawtransaction(txId, False), rawTxSigned['hex'])
 
-        # 4. valid parameters - supply txid and 1 for verbose.
-        # We only check the "hex" field of the output so we don't need to update this test every time the output format changes.
-        assert_equal(self.nodes[0].getrawtransaction(txId, 1)["hex"], rawTxSigned['hex'])
+            # 4. valid parameters - supply txid and 1 for verbose.
+            # We only check the "hex" field of the output so we don't need to update this test every time the output format changes.
+            assert_equal(self.nodes[n].getrawtransaction(txId, 1)["hex"], rawTxSigned['hex'])
 
-        # 5. valid parameters - supply txid and True for non-verbose
-        assert_equal(self.nodes[0].getrawtransaction(txId, True)["hex"], rawTxSigned['hex'])
+            # 5. valid parameters - supply txid and True for non-verbose
+            assert_equal(self.nodes[n].getrawtransaction(txId, True)["hex"], rawTxSigned['hex'])
 
-        # 6. invalid parameters - supply txid and string "Flase"
-        assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, "Flase")
+            # 6. invalid parameters - supply txid and invalid boolean values (strings) for verbose
+            for value in ["True", "False"]:
+                assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txid=txId, verbose=value)
 
-        # 7. invalid parameters - supply txid and empty array
-        assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, [])
+            # 7. invalid parameters - supply txid and empty array
+            assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, [])
 
-        # 8. invalid parameters - supply txid and empty dict
-        assert_raises_rpc_error(-1, "not a boolean", self.nodes[0].getrawtransaction, txId, {})
+            # 8. invalid parameters - supply txid and empty dict
+            assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, {})
 
         ####################################
         # TRANSACTION VERSION NUMBER TESTS #

From 14398b30d6242db14670b3286f988b2badda83fb Mon Sep 17 00:00:00 2001
From: Jon Atack <jon@atack.com>
Date: Mon, 12 Jul 2021 15:13:29 +0200
Subject: [PATCH 5/9] test: add and harmonize getrawtransaction logging

---
 test/functional/rpc_rawtransaction.py | 28 +++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index 89088e9fc6b..2aa80de90da 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -74,7 +74,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.connect_nodes(0, 2)
 
     def run_test(self):
-        self.log.info('prepare some coins for multiple *rawtransaction commands')
+        self.log.info("Prepare some coins for multiple *rawtransaction commands")
         self.nodes[2].generate(1)
         self.sync_all()
         self.nodes[0].generate(COINBASE_MATURITY + 1)
@@ -86,11 +86,11 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.nodes[0].generate(5)
         self.sync_all()
 
-        self.log.info('Test getrawtransaction on genesis block coinbase returns an error')
+        self.log.info("Test getrawtransaction on genesis block coinbase returns an error")
         block = self.nodes[0].getblock(self.nodes[0].getblockhash(0))
         assert_raises_rpc_error(-5, "The genesis block coinbase is not considered an ordinary transaction", self.nodes[0].getrawtransaction, block['merkleroot'])
 
-        self.log.info('Check parameter types and required parameters of createrawtransaction')
+        self.log.info("Test createrawtransaction")
         # Test `createrawtransaction` required parameters
         assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction)
         assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, [])
@@ -146,7 +146,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         # Test `createrawtransaction` invalid `replaceable`
         assert_raises_rpc_error(-3, "Expected type bool", self.nodes[0].createrawtransaction, [], {}, 0, 'foo')
 
-        self.log.info('Check that createrawtransaction accepts an array and object as outputs')
+        # Test that createrawtransaction accepts an array and object as outputs
         # One output
         tx = tx_from_hex(self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs={address: 99}))
         assert_equal(len(tx.vout), 1)
@@ -170,13 +170,10 @@ class RawTransactionsTest(BitcoinTestFramework):
         )
 
         for type in ["bech32", "p2sh-segwit", "legacy"]:
+            self.log.info(f"Test signrawtransactionwithwallet with missing prevtx info ({type})")
             addr = self.nodes[0].getnewaddress("", type)
             addrinfo = self.nodes[0].getaddressinfo(addr)
             pubkey = addrinfo["scriptPubKey"]
-
-            self.log.info('sendrawtransaction with missing prevtx info (%s)' %(type))
-
-            # Test `signrawtransactionwithwallet` invalid `prevtxs`
             inputs  = [ {'txid' : TXID, 'vout' : 3, 'sequence' : 1000}]
             outputs = { self.nodes[0].getnewaddress() : 1 }
             rawtx   = self.nodes[0].createrawtransaction(inputs, outputs)
@@ -224,7 +221,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         # sendrawtransaction with missing input #
         #########################################
 
-        self.log.info('sendrawtransaction with missing input')
+        self.log.info("Test sendrawtransaction with missing input")
         inputs  = [{'txid' : TXID, 'vout' : 1}]  # won't exist
         outputs = { self.nodes[0].getnewaddress() : 4.998 }
         rawtx   = self.nodes[2].createrawtransaction(inputs, outputs)
@@ -237,7 +234,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         # getrawtransaction with block hash #
         #####################################
 
-        # make a tx by sending then generate 2 blocks; block1 has the tx in it
+        # Make a tx by sending, then generate 2 blocks; block1 has the tx in it
         tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1)
         block1, block2 = self.nodes[2].generate(2)
         self.sync_all()
@@ -277,6 +274,7 @@ class RawTransactionsTest(BitcoinTestFramework):
             assert_equal(self.nodes[n].getbestblockhash(), block2)
 
         if not self.options.descriptors:
+            self.log.info("Test raw multisig transactions (legacy)")
             # The traditional multisig workflow does not work with descriptor wallets so these are legacy only.
             # The multisig workflow with descriptor wallets uses PSBTs and is tested elsewhere, no need to do them here.
             #########################
@@ -395,7 +393,7 @@ class RawTransactionsTest(BitcoinTestFramework):
             self.sync_all()
             assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx
 
-        # decoderawtransaction tests
+        self.log.info("Test decoderawtransaction")
         # witness transaction
         encrawtx = "010000000001010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f50500000000000102616100000000"
         decrawtx = self.nodes[0].decoderawtransaction(encrawtx, True) # decode as witness transaction
@@ -413,7 +411,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         assert_equal(decrawtx, decrawtx_wit) # the witness interpretation should be chosen
         assert_equal(decrawtx['vin'][0]['coinbase'], "03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000")
 
-        # Basic signrawtransaction test
+        self.log.info("Test signrawtransactionwithwallet")
         addr = self.nodes[1].getnewaddress()
         txid = self.nodes[0].sendtoaddress(addr, 10)
         self.nodes[0].generate(1)
@@ -458,6 +456,8 @@ class RawTransactionsTest(BitcoinTestFramework):
         # TRANSACTION VERSION NUMBER TESTS #
         ####################################
 
+        self.log.info("Test transaction version numbers")
+
         # Test the minimum transaction version number that fits in a signed 32-bit integer.
         # As transaction version is unsigned, this should convert to its unsigned equivalent.
         tx = CTransaction()
@@ -473,7 +473,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         decrawtx = self.nodes[0].decoderawtransaction(rawtx)
         assert_equal(decrawtx['version'], 0x7fffffff)
 
-        self.log.info('sendrawtransaction/testmempoolaccept with maxfeerate')
+        self.log.info("Test sendrawtransaction/testmempoolaccept with maxfeerate")
 
         # Test a transaction with a small fee.
         txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0)
@@ -523,7 +523,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         assert_equal(testres['allowed'], True)
         self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.20000000')
 
-        self.log.info('sendrawtransaction/testmempoolaccept with tx that is already in the chain')
+        self.log.info("Test sendrawtransaction/testmempoolaccept with tx already in the chain")
         self.nodes[2].generate(1)
         self.sync_blocks()
         for node in self.nodes:

From d861040dd24a321e1ceec1f07c7bb80d59779081 Mon Sep 17 00:00:00 2001
From: Jon Atack <jon@atack.com>
Date: Mon, 12 Jul 2021 15:16:43 +0200
Subject: [PATCH 6/9] test: remove no longer needed (ASCII art) comments

---
 test/functional/rpc_rawtransaction.py | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index 2aa80de90da..f354d4faaac 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -49,7 +49,6 @@ class multidict(dict):
         return self.x
 
 
-# Create one-input, one-output, no-fee transaction:
 class RawTransactionsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
@@ -217,10 +216,6 @@ class RawTransactionsTest(BitcoinTestFramework):
                 }
             ])
 
-        #########################################
-        # sendrawtransaction with missing input #
-        #########################################
-
         self.log.info("Test sendrawtransaction with missing input")
         inputs  = [{'txid' : TXID, 'vout' : 1}]  # won't exist
         outputs = { self.nodes[0].getnewaddress() : 4.998 }
@@ -230,10 +225,6 @@ class RawTransactionsTest(BitcoinTestFramework):
         # This will raise an exception since there are missing inputs
         assert_raises_rpc_error(-25, "bad-txns-inputs-missingorspent", self.nodes[2].sendrawtransaction, rawtx['hex'])
 
-        #####################################
-        # getrawtransaction with block hash #
-        #####################################
-
         # Make a tx by sending, then generate 2 blocks; block1 has the tx in it
         tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1)
         block1, block2 = self.nodes[2].generate(2)
@@ -277,9 +268,6 @@ class RawTransactionsTest(BitcoinTestFramework):
             self.log.info("Test raw multisig transactions (legacy)")
             # The traditional multisig workflow does not work with descriptor wallets so these are legacy only.
             # The multisig workflow with descriptor wallets uses PSBTs and is tested elsewhere, no need to do them here.
-            #########################
-            # RAW TX MULTISIG TESTS #
-            #########################
             # 2of2 test
             addr1 = self.nodes[2].getnewaddress()
             addr2 = self.nodes[2].getnewaddress()
@@ -452,10 +440,6 @@ class RawTransactionsTest(BitcoinTestFramework):
             # 8. invalid parameters - supply txid and empty dict
             assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, {})
 
-        ####################################
-        # TRANSACTION VERSION NUMBER TESTS #
-        ####################################
-
         self.log.info("Test transaction version numbers")
 
         # Test the minimum transaction version number that fits in a signed 32-bit integer.

From 409779df95f886b08dbf6d44219e2fbeb3405a43 Mon Sep 17 00:00:00 2001
From: Jon Atack <jon@atack.com>
Date: Mon, 12 Jul 2021 15:42:21 +0200
Subject: [PATCH 7/9] move-only: regroup similar rpc_rawtransaction tests
 together

---
 test/functional/rpc_rawtransaction.py | 344 +++++++++++++-------------
 1 file changed, 170 insertions(+), 174 deletions(-)

diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index f354d4faaac..a7d86ae4fdf 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -85,6 +85,85 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.nodes[0].generate(5)
         self.sync_all()
 
+        # getrawtransaction tests
+        addr = self.nodes[1].getnewaddress()
+        txid = self.nodes[0].sendtoaddress(addr, 10)
+        self.nodes[0].generate(1)
+        self.sync_all()
+        vout = find_vout_for_address(self.nodes[1], txid, addr)
+        rawTx = self.nodes[1].createrawtransaction([{'txid': txid, 'vout': vout}], {self.nodes[1].getnewaddress(): 9.999})
+        rawTxSigned = self.nodes[1].signrawtransactionwithwallet(rawTx)
+        txId = self.nodes[1].sendrawtransaction(rawTxSigned['hex'])
+        self.nodes[0].generate(1)
+        self.sync_all()
+
+        for n in [0, 3]:
+            self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex")
+            # 1. valid parameters - only supply txid
+            assert_equal(self.nodes[n].getrawtransaction(txId), rawTxSigned['hex'])
+
+            # 2. valid parameters - supply txid and 0 for non-verbose
+            assert_equal(self.nodes[n].getrawtransaction(txId, 0), rawTxSigned['hex'])
+
+            # 3. valid parameters - supply txid and False for non-verbose
+            assert_equal(self.nodes[n].getrawtransaction(txId, False), rawTxSigned['hex'])
+
+            # 4. valid parameters - supply txid and 1 for verbose.
+            # We only check the "hex" field of the output so we don't need to update this test every time the output format changes.
+            assert_equal(self.nodes[n].getrawtransaction(txId, 1)["hex"], rawTxSigned['hex'])
+
+            # 5. valid parameters - supply txid and True for non-verbose
+            assert_equal(self.nodes[n].getrawtransaction(txId, True)["hex"], rawTxSigned['hex'])
+
+            # 6. invalid parameters - supply txid and invalid boolean values (strings) for verbose
+            for value in ["True", "False"]:
+                assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txid=txId, verbose=value)
+
+            # 7. invalid parameters - supply txid and empty array
+            assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, [])
+
+            # 8. invalid parameters - supply txid and empty dict
+            assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, {})
+
+        # Make a tx by sending, then generate 2 blocks; block1 has the tx in it
+        tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1)
+        block1, block2 = self.nodes[2].generate(2)
+        self.sync_all()
+        for n in [0, 3]:
+            self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex, with blockhash")
+            # We should be able to get the raw transaction by providing the correct block
+            gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1)
+            assert_equal(gottx['txid'], tx)
+            assert_equal(gottx['in_active_chain'], True)
+            if n == 0:
+                self.log.info("Test getrawtransaction with -txindex, without blockhash: 'in_active_chain' should be absent")
+                gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True)
+                assert_equal(gottx['txid'], tx)
+                assert 'in_active_chain' not in gottx
+            else:
+                self.log.info("Test getrawtransaction without -txindex, without blockhash: expect the call to raise")
+                err_msg = (
+                    "No such mempool transaction. Use -txindex or provide a block hash to enable"
+                    " blockchain transaction queries. Use gettransaction for wallet transactions."
+                )
+                assert_raises_rpc_error(-5, err_msg, self.nodes[n].getrawtransaction, txid=tx, verbose=True)
+            # We should not get the tx if we provide an unrelated block
+            assert_raises_rpc_error(-5, "No such transaction found", self.nodes[n].getrawtransaction, txid=tx, blockhash=block2)
+            # An invalid block hash should raise the correct errors
+            assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[n].getrawtransaction, txid=tx, blockhash=True)
+            assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[n].getrawtransaction, txid=tx, blockhash="foobar")
+            assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[n].getrawtransaction, txid=tx, blockhash="abcd1234")
+            foo = "ZZZ0000000000000000000000000000000000000000000000000000000000000"
+            assert_raises_rpc_error(-8, f"parameter 3 must be hexadecimal string (not '{foo}')", self.nodes[n].getrawtransaction, txid=tx, blockhash=foo)
+            bar = "0000000000000000000000000000000000000000000000000000000000000000"
+            assert_raises_rpc_error(-5, "Block hash not found", self.nodes[n].getrawtransaction, txid=tx, blockhash=bar)
+            # Undo the blocks and verify that "in_active_chain" is false.
+            self.nodes[n].invalidateblock(block1)
+            gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1)
+            assert_equal(gottx['in_active_chain'], False)
+            self.nodes[n].reconsiderblock(block1)
+            assert_equal(self.nodes[n].getbestblockhash(), block2)
+
         self.log.info("Test getrawtransaction on genesis block coinbase returns an error")
         block = self.nodes[0].getblock(self.nodes[0].getblockhash(0))
         assert_raises_rpc_error(-5, "The genesis block coinbase is not considered an ordinary transaction", self.nodes[0].getrawtransaction, block['merkleroot'])
@@ -221,48 +300,100 @@ class RawTransactionsTest(BitcoinTestFramework):
         outputs = { self.nodes[0].getnewaddress() : 4.998 }
         rawtx   = self.nodes[2].createrawtransaction(inputs, outputs)
         rawtx   = self.nodes[2].signrawtransactionwithwallet(rawtx)
-
-        # This will raise an exception since there are missing inputs
         assert_raises_rpc_error(-25, "bad-txns-inputs-missingorspent", self.nodes[2].sendrawtransaction, rawtx['hex'])
 
-        # Make a tx by sending, then generate 2 blocks; block1 has the tx in it
-        tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1)
-        block1, block2 = self.nodes[2].generate(2)
+        self.log.info("Test sendrawtransaction/testmempoolaccept with maxfeerate")
+        # Test a transaction with a small fee.
+        txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0)
+        rawTx = self.nodes[0].getrawtransaction(txId, True)
+        vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000'))
+
         self.sync_all()
-        for n in [0, 3]:
-            self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex, with blockhash")
-            # We should be able to get the raw transaction by providing the correct block
-            gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1)
-            assert_equal(gottx['txid'], tx)
-            assert_equal(gottx['in_active_chain'], True)
-            if n == 0:
-                self.log.info("Test getrawtransaction with -txindex, without blockhash: 'in_active_chain' should be absent")
-                gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True)
-                assert_equal(gottx['txid'], tx)
-                assert 'in_active_chain' not in gottx
-            else:
-                self.log.info("Test getrawtransaction without -txindex, without blockhash: expect the call to raise")
-                err_msg = (
-                    "No such mempool transaction. Use -txindex or provide a block hash to enable"
-                    " blockchain transaction queries. Use gettransaction for wallet transactions."
-                )
-                assert_raises_rpc_error(-5, err_msg, self.nodes[n].getrawtransaction, txid=tx, verbose=True)
-            # We should not get the tx if we provide an unrelated block
-            assert_raises_rpc_error(-5, "No such transaction found", self.nodes[n].getrawtransaction, txid=tx, blockhash=block2)
-            # An invalid block hash should raise the correct errors
-            assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[n].getrawtransaction, txid=tx, blockhash=True)
-            assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[n].getrawtransaction, txid=tx, blockhash="foobar")
-            assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[n].getrawtransaction, txid=tx, blockhash="abcd1234")
-            foo = "ZZZ0000000000000000000000000000000000000000000000000000000000000"
-            assert_raises_rpc_error(-8, f"parameter 3 must be hexadecimal string (not '{foo}')", self.nodes[n].getrawtransaction, txid=tx, blockhash=foo)
-            bar = "0000000000000000000000000000000000000000000000000000000000000000"
-            assert_raises_rpc_error(-5, "Block hash not found", self.nodes[n].getrawtransaction, txid=tx, blockhash=bar)
-            # Undo the blocks and verify that "in_active_chain" is false.
-            self.nodes[n].invalidateblock(block1)
-            gottx = self.nodes[n].getrawtransaction(txid=tx, verbose=True, blockhash=block1)
-            assert_equal(gottx['in_active_chain'], False)
-            self.nodes[n].reconsiderblock(block1)
-            assert_equal(self.nodes[n].getbestblockhash(), block2)
+        inputs = [{ "txid" : txId, "vout" : vout['n'] }]
+        # Fee 10,000 satoshis, (1 - (10000 sat * 0.00000001 BTC/sat)) = 0.9999
+        outputs = { self.nodes[0].getnewaddress() : Decimal("0.99990000") }
+        rawTx = self.nodes[2].createrawtransaction(inputs, outputs)
+        rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx)
+        assert_equal(rawTxSigned['complete'], True)
+        # Fee 10,000 satoshis, ~100 b transaction, fee rate should land around 100 sat/byte = 0.00100000 BTC/kB
+        # Thus, testmempoolaccept should reject
+        testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']], 0.00001000)[0]
+        assert_equal(testres['allowed'], False)
+        assert_equal(testres['reject-reason'], 'max-fee-exceeded')
+        # and sendrawtransaction should throw
+        assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000)
+        # and the following calls should both succeed
+        testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0]
+        assert_equal(testres['allowed'], True)
+        self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'])
+
+        # Test a transaction with a large fee.
+        txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0)
+        rawTx = self.nodes[0].getrawtransaction(txId, True)
+        vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000'))
+
+        self.sync_all()
+        inputs = [{ "txid" : txId, "vout" : vout['n'] }]
+        # Fee 2,000,000 satoshis, (1 - (2000000 sat * 0.00000001 BTC/sat)) = 0.98
+        outputs = { self.nodes[0].getnewaddress() : Decimal("0.98000000") }
+        rawTx = self.nodes[2].createrawtransaction(inputs, outputs)
+        rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx)
+        assert_equal(rawTxSigned['complete'], True)
+        # Fee 2,000,000 satoshis, ~100 b transaction, fee rate should land around 20,000 sat/byte = 0.20000000 BTC/kB
+        # Thus, testmempoolaccept should reject
+        testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']])[0]
+        assert_equal(testres['allowed'], False)
+        assert_equal(testres['reject-reason'], 'max-fee-exceeded')
+        # and sendrawtransaction should throw
+        assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex'])
+        # and the following calls should both succeed
+        testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.20000000')[0]
+        assert_equal(testres['allowed'], True)
+        self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.20000000')
+
+        self.log.info("Test sendrawtransaction/testmempoolaccept with tx already in the chain")
+        self.nodes[2].generate(1)
+        self.sync_blocks()
+        for node in self.nodes:
+            testres = node.testmempoolaccept([rawTxSigned['hex']])[0]
+            assert_equal(testres['allowed'], False)
+            assert_equal(testres['reject-reason'], 'txn-already-known')
+            assert_raises_rpc_error(-27, 'Transaction already in block chain', node.sendrawtransaction, rawTxSigned['hex'])
+
+        self.log.info("Test decoderawtransaction")
+        # witness transaction
+        encrawtx = "010000000001010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f50500000000000102616100000000"
+        decrawtx = self.nodes[0].decoderawtransaction(encrawtx, True) # decode as witness transaction
+        assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000'))
+        assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # force decode as non-witness transaction
+        # non-witness transaction
+        encrawtx = "01000000010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f505000000000000000000"
+        decrawtx = self.nodes[0].decoderawtransaction(encrawtx, False) # decode as non-witness transaction
+        assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000'))
+        # known ambiguous transaction in the chain (see https://github.com/bitcoin/bitcoin/issues/20579)
+        encrawtx = "020000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff4b03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000ffffffff03f4c1fb4b0000000016001497cfc76442fe717f2a3f0cc9c175f7561b6619970000000000000000266a24aa21a9ed957d1036a80343e0d1b659497e1b48a38ebe876a056d45965fac4a85cda84e1900000000000000002952534b424c4f434b3a8e092581ab01986cbadc84f4b43f4fa4bb9e7a2e2a0caf9b7cf64d939028e22c0120000000000000000000000000000000000000000000000000000000000000000000000000"
+        decrawtx = self.nodes[0].decoderawtransaction(encrawtx)
+        decrawtx_wit = self.nodes[0].decoderawtransaction(encrawtx, True)
+        assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # fails to decode as non-witness transaction
+        assert_equal(decrawtx, decrawtx_wit) # the witness interpretation should be chosen
+        assert_equal(decrawtx['vin'][0]['coinbase'], "03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000")
+
+        self.log.info("Test transaction version numbers")
+
+        # Test the minimum transaction version number that fits in a signed 32-bit integer.
+        # As transaction version is unsigned, this should convert to its unsigned equivalent.
+        tx = CTransaction()
+        tx.nVersion = -0x80000000
+        rawtx = tx.serialize().hex()
+        decrawtx = self.nodes[0].decoderawtransaction(rawtx)
+        assert_equal(decrawtx['version'], 0x80000000)
+
+        # Test the maximum transaction version number that fits in a signed 32-bit integer.
+        tx = CTransaction()
+        tx.nVersion = 0x7fffffff
+        rawtx = tx.serialize().hex()
+        decrawtx = self.nodes[0].decoderawtransaction(rawtx)
+        assert_equal(decrawtx['version'], 0x7fffffff)
 
         if not self.options.descriptors:
             self.log.info("Test raw multisig transactions (legacy)")
@@ -381,141 +512,6 @@ class RawTransactionsTest(BitcoinTestFramework):
             self.sync_all()
             assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx
 
-        self.log.info("Test decoderawtransaction")
-        # witness transaction
-        encrawtx = "010000000001010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f50500000000000102616100000000"
-        decrawtx = self.nodes[0].decoderawtransaction(encrawtx, True) # decode as witness transaction
-        assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000'))
-        assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # force decode as non-witness transaction
-        # non-witness transaction
-        encrawtx = "01000000010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f505000000000000000000"
-        decrawtx = self.nodes[0].decoderawtransaction(encrawtx, False) # decode as non-witness transaction
-        assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000'))
-        # known ambiguous transaction in the chain (see https://github.com/bitcoin/bitcoin/issues/20579)
-        encrawtx = "020000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff4b03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000ffffffff03f4c1fb4b0000000016001497cfc76442fe717f2a3f0cc9c175f7561b6619970000000000000000266a24aa21a9ed957d1036a80343e0d1b659497e1b48a38ebe876a056d45965fac4a85cda84e1900000000000000002952534b424c4f434b3a8e092581ab01986cbadc84f4b43f4fa4bb9e7a2e2a0caf9b7cf64d939028e22c0120000000000000000000000000000000000000000000000000000000000000000000000000"
-        decrawtx = self.nodes[0].decoderawtransaction(encrawtx)
-        decrawtx_wit = self.nodes[0].decoderawtransaction(encrawtx, True)
-        assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # fails to decode as non-witness transaction
-        assert_equal(decrawtx, decrawtx_wit) # the witness interpretation should be chosen
-        assert_equal(decrawtx['vin'][0]['coinbase'], "03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000")
-
-        self.log.info("Test signrawtransactionwithwallet")
-        addr = self.nodes[1].getnewaddress()
-        txid = self.nodes[0].sendtoaddress(addr, 10)
-        self.nodes[0].generate(1)
-        self.sync_all()
-        vout = find_vout_for_address(self.nodes[1], txid, addr)
-        rawTx = self.nodes[1].createrawtransaction([{'txid': txid, 'vout': vout}], {self.nodes[1].getnewaddress(): 9.999})
-        rawTxSigned = self.nodes[1].signrawtransactionwithwallet(rawTx)
-        txId = self.nodes[1].sendrawtransaction(rawTxSigned['hex'])
-        self.nodes[0].generate(1)
-        self.sync_all()
-
-        # getrawtransaction tests
-        for n in [0, 3]:
-            self.log.info(f"Test getrawtransaction {'with' if n == 0 else 'without'} -txindex")
-            # 1. valid parameters - only supply txid
-            assert_equal(self.nodes[n].getrawtransaction(txId), rawTxSigned['hex'])
-
-            # 2. valid parameters - supply txid and 0 for non-verbose
-            assert_equal(self.nodes[n].getrawtransaction(txId, 0), rawTxSigned['hex'])
-
-            # 3. valid parameters - supply txid and False for non-verbose
-            assert_equal(self.nodes[n].getrawtransaction(txId, False), rawTxSigned['hex'])
-
-            # 4. valid parameters - supply txid and 1 for verbose.
-            # We only check the "hex" field of the output so we don't need to update this test every time the output format changes.
-            assert_equal(self.nodes[n].getrawtransaction(txId, 1)["hex"], rawTxSigned['hex'])
-
-            # 5. valid parameters - supply txid and True for non-verbose
-            assert_equal(self.nodes[n].getrawtransaction(txId, True)["hex"], rawTxSigned['hex'])
-
-            # 6. invalid parameters - supply txid and invalid boolean values (strings) for verbose
-            for value in ["True", "False"]:
-                assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txid=txId, verbose=value)
-
-            # 7. invalid parameters - supply txid and empty array
-            assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, [])
-
-            # 8. invalid parameters - supply txid and empty dict
-            assert_raises_rpc_error(-1, "not a boolean", self.nodes[n].getrawtransaction, txId, {})
-
-        self.log.info("Test transaction version numbers")
-
-        # Test the minimum transaction version number that fits in a signed 32-bit integer.
-        # As transaction version is unsigned, this should convert to its unsigned equivalent.
-        tx = CTransaction()
-        tx.nVersion = -0x80000000
-        rawtx = tx.serialize().hex()
-        decrawtx = self.nodes[0].decoderawtransaction(rawtx)
-        assert_equal(decrawtx['version'], 0x80000000)
-
-        # Test the maximum transaction version number that fits in a signed 32-bit integer.
-        tx = CTransaction()
-        tx.nVersion = 0x7fffffff
-        rawtx = tx.serialize().hex()
-        decrawtx = self.nodes[0].decoderawtransaction(rawtx)
-        assert_equal(decrawtx['version'], 0x7fffffff)
-
-        self.log.info("Test sendrawtransaction/testmempoolaccept with maxfeerate")
-
-        # Test a transaction with a small fee.
-        txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0)
-        rawTx = self.nodes[0].getrawtransaction(txId, True)
-        vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000'))
-
-        self.sync_all()
-        inputs = [{ "txid" : txId, "vout" : vout['n'] }]
-        # Fee 10,000 satoshis, (1 - (10000 sat * 0.00000001 BTC/sat)) = 0.9999
-        outputs = { self.nodes[0].getnewaddress() : Decimal("0.99990000") }
-        rawTx = self.nodes[2].createrawtransaction(inputs, outputs)
-        rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx)
-        assert_equal(rawTxSigned['complete'], True)
-        # Fee 10,000 satoshis, ~100 b transaction, fee rate should land around 100 sat/byte = 0.00100000 BTC/kB
-        # Thus, testmempoolaccept should reject
-        testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']], 0.00001000)[0]
-        assert_equal(testres['allowed'], False)
-        assert_equal(testres['reject-reason'], 'max-fee-exceeded')
-        # and sendrawtransaction should throw
-        assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000)
-        # and the following calls should both succeed
-        testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0]
-        assert_equal(testres['allowed'], True)
-        self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'])
-
-        # Test a transaction with a large fee.
-        txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0)
-        rawTx = self.nodes[0].getrawtransaction(txId, True)
-        vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000'))
-
-        self.sync_all()
-        inputs = [{ "txid" : txId, "vout" : vout['n'] }]
-        # Fee 2,000,000 satoshis, (1 - (2000000 sat * 0.00000001 BTC/sat)) = 0.98
-        outputs = { self.nodes[0].getnewaddress() : Decimal("0.98000000") }
-        rawTx = self.nodes[2].createrawtransaction(inputs, outputs)
-        rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx)
-        assert_equal(rawTxSigned['complete'], True)
-        # Fee 2,000,000 satoshis, ~100 b transaction, fee rate should land around 20,000 sat/byte = 0.20000000 BTC/kB
-        # Thus, testmempoolaccept should reject
-        testres = self.nodes[2].testmempoolaccept([rawTxSigned['hex']])[0]
-        assert_equal(testres['allowed'], False)
-        assert_equal(testres['reject-reason'], 'max-fee-exceeded')
-        # and sendrawtransaction should throw
-        assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex'])
-        # and the following calls should both succeed
-        testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.20000000')[0]
-        assert_equal(testres['allowed'], True)
-        self.nodes[2].sendrawtransaction(hexstring=rawTxSigned['hex'], maxfeerate='0.20000000')
-
-        self.log.info("Test sendrawtransaction/testmempoolaccept with tx already in the chain")
-        self.nodes[2].generate(1)
-        self.sync_blocks()
-        for node in self.nodes:
-            testres = node.testmempoolaccept([rawTxSigned['hex']])[0]
-            assert_equal(testres['allowed'], False)
-            assert_equal(testres['reject-reason'], 'txn-already-known')
-            assert_raises_rpc_error(-27, 'Transaction already in block chain', node.sendrawtransaction, rawTxSigned['hex'])
-
 
 if __name__ == '__main__':
     RawTransactionsTest().main()

From 7d5cec2e498dc059ff1d74a2b60764db45923264 Mon Sep 17 00:00:00 2001
From: Jon Atack <jon@atack.com>
Date: Mon, 12 Jul 2021 16:04:48 +0200
Subject: [PATCH 8/9] refactor: separate the rpc_rawtransaction tests into
 functions

---
 test/functional/rpc_rawtransaction.py | 212 ++++++++++++++------------
 1 file changed, 114 insertions(+), 98 deletions(-)

diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index a7d86ae4fdf..25e345a8d1b 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -5,11 +5,11 @@
 """Test the rawtransaction RPCs.
 
 Test the following RPCs:
+   - getrawtransaction
    - createrawtransaction
    - signrawtransactionwithwallet
    - sendrawtransaction
    - decoderawtransaction
-   - getrawtransaction
 """
 
 from collections import OrderedDict
@@ -85,7 +85,17 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.nodes[0].generate(5)
         self.sync_all()
 
-        # getrawtransaction tests
+        self.getrawtransaction_tests()
+        self.createrawtransaction_tests()
+        self.signrawtransactionwithwallet_tests()
+        self.sendrawtransaction_tests()
+        self.sendrawtransaction_testmempoolaccept_tests()
+        self.decoderawtransaction_tests()
+        self.transaction_version_number_tests()
+        if not self.options.descriptors:
+            self.raw_multisig_transaction_legacy_tests()
+
+    def getrawtransaction_tests(self):
         addr = self.nodes[1].getnewaddress()
         txid = self.nodes[0].sendtoaddress(addr, 10)
         self.nodes[0].generate(1)
@@ -168,6 +178,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         block = self.nodes[0].getblock(self.nodes[0].getblockhash(0))
         assert_raises_rpc_error(-5, "The genesis block coinbase is not considered an ordinary transaction", self.nodes[0].getrawtransaction, block['merkleroot'])
 
+    def createrawtransaction_tests(self):
         self.log.info("Test createrawtransaction")
         # Test `createrawtransaction` required parameters
         assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction)
@@ -247,6 +258,7 @@ class RawTransactionsTest(BitcoinTestFramework):
             self.nodes[2].createrawtransaction(inputs=[{'txid': TXID, 'vout': 9}], outputs=[{address: 99}, {address2: 99}, {'data': '99'}]),
         )
 
+    def signrawtransactionwithwallet_tests(self):
         for type in ["bech32", "p2sh-segwit", "legacy"]:
             self.log.info(f"Test signrawtransactionwithwallet with missing prevtx info ({type})")
             addr = self.nodes[0].getnewaddress("", type)
@@ -259,12 +271,12 @@ class RawTransactionsTest(BitcoinTestFramework):
             prevtx = dict(txid=TXID, scriptPubKey=pubkey, vout=3, amount=1)
             succ = self.nodes[0].signrawtransactionwithwallet(rawtx, [prevtx])
             assert succ["complete"]
+
             if type == "legacy":
                 del prevtx["amount"]
                 succ = self.nodes[0].signrawtransactionwithwallet(rawtx, [prevtx])
                 assert succ["complete"]
-
-            if type != "legacy":
+            else:
                 assert_raises_rpc_error(-3, "Missing amount", self.nodes[0].signrawtransactionwithwallet, rawtx, [
                     {
                         "txid": TXID,
@@ -295,6 +307,7 @@ class RawTransactionsTest(BitcoinTestFramework):
                 }
             ])
 
+    def sendrawtransaction_tests(self):
         self.log.info("Test sendrawtransaction with missing input")
         inputs  = [{'txid' : TXID, 'vout' : 1}]  # won't exist
         outputs = { self.nodes[0].getnewaddress() : 4.998 }
@@ -302,6 +315,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         rawtx   = self.nodes[2].signrawtransactionwithwallet(rawtx)
         assert_raises_rpc_error(-25, "bad-txns-inputs-missingorspent", self.nodes[2].sendrawtransaction, rawtx['hex'])
 
+    def sendrawtransaction_testmempoolaccept_tests(self):
         self.log.info("Test sendrawtransaction/testmempoolaccept with maxfeerate")
         # Test a transaction with a small fee.
         txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0)
@@ -360,6 +374,7 @@ class RawTransactionsTest(BitcoinTestFramework):
             assert_equal(testres['reject-reason'], 'txn-already-known')
             assert_raises_rpc_error(-27, 'Transaction already in block chain', node.sendrawtransaction, rawTxSigned['hex'])
 
+    def decoderawtransaction_tests(self):
         self.log.info("Test decoderawtransaction")
         # witness transaction
         encrawtx = "010000000001010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f50500000000000102616100000000"
@@ -378,6 +393,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         assert_equal(decrawtx, decrawtx_wit) # the witness interpretation should be chosen
         assert_equal(decrawtx['vin'][0]['coinbase'], "03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000")
 
+    def transaction_version_number_tests(self):
         self.log.info("Test transaction version numbers")
 
         # Test the minimum transaction version number that fits in a signed 32-bit integer.
@@ -395,122 +411,122 @@ class RawTransactionsTest(BitcoinTestFramework):
         decrawtx = self.nodes[0].decoderawtransaction(rawtx)
         assert_equal(decrawtx['version'], 0x7fffffff)
 
-        if not self.options.descriptors:
-            self.log.info("Test raw multisig transactions (legacy)")
-            # The traditional multisig workflow does not work with descriptor wallets so these are legacy only.
-            # The multisig workflow with descriptor wallets uses PSBTs and is tested elsewhere, no need to do them here.
-            # 2of2 test
-            addr1 = self.nodes[2].getnewaddress()
-            addr2 = self.nodes[2].getnewaddress()
+    def raw_multisig_transaction_legacy_tests(self):
+        self.log.info("Test raw multisig transactions (legacy)")
+        # The traditional multisig workflow does not work with descriptor wallets so these are legacy only.
+        # The multisig workflow with descriptor wallets uses PSBTs and is tested elsewhere, no need to do them here.
+        # 2of2 test
+        addr1 = self.nodes[2].getnewaddress()
+        addr2 = self.nodes[2].getnewaddress()
 
-            addr1Obj = self.nodes[2].getaddressinfo(addr1)
-            addr2Obj = self.nodes[2].getaddressinfo(addr2)
+        addr1Obj = self.nodes[2].getaddressinfo(addr1)
+        addr2Obj = self.nodes[2].getaddressinfo(addr2)
 
-            # Tests for createmultisig and addmultisigaddress
-            assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"])
-            self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # createmultisig can only take public keys
-            assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here.
+        # Tests for createmultisig and addmultisigaddress
+        assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"])
+        self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # createmultisig can only take public keys
+        assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here.
 
-            mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address']
+        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address']
 
-            #use balance deltas instead of absolute values
-            bal = self.nodes[2].getbalance()
+        #use balance deltas instead of absolute values
+        bal = self.nodes[2].getbalance()
 
-            # send 1.2 BTC to msig adr
-            txId = self.nodes[0].sendtoaddress(mSigObj, 1.2)
-            self.sync_all()
-            self.nodes[0].generate(1)
-            self.sync_all()
-            assert_equal(self.nodes[2].getbalance(), bal+Decimal('1.20000000')) #node2 has both keys of the 2of2 ms addr., tx should affect the balance
+        # send 1.2 BTC to msig adr
+        txId = self.nodes[0].sendtoaddress(mSigObj, 1.2)
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
+        assert_equal(self.nodes[2].getbalance(), bal+Decimal('1.20000000')) #node2 has both keys of the 2of2 ms addr., tx should affect the balance
 
 
-            # 2of3 test from different nodes
-            bal = self.nodes[2].getbalance()
-            addr1 = self.nodes[1].getnewaddress()
-            addr2 = self.nodes[2].getnewaddress()
-            addr3 = self.nodes[2].getnewaddress()
+        # 2of3 test from different nodes
+        bal = self.nodes[2].getbalance()
+        addr1 = self.nodes[1].getnewaddress()
+        addr2 = self.nodes[2].getnewaddress()
+        addr3 = self.nodes[2].getnewaddress()
 
-            addr1Obj = self.nodes[1].getaddressinfo(addr1)
-            addr2Obj = self.nodes[2].getaddressinfo(addr2)
-            addr3Obj = self.nodes[2].getaddressinfo(addr3)
+        addr1Obj = self.nodes[1].getaddressinfo(addr1)
+        addr2Obj = self.nodes[2].getaddressinfo(addr2)
+        addr3Obj = self.nodes[2].getaddressinfo(addr3)
 
-            mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']])['address']
+        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']])['address']
 
-            txId = self.nodes[0].sendtoaddress(mSigObj, 2.2)
-            decTx = self.nodes[0].gettransaction(txId)
-            rawTx = self.nodes[0].decoderawtransaction(decTx['hex'])
-            self.sync_all()
-            self.nodes[0].generate(1)
-            self.sync_all()
+        txId = self.nodes[0].sendtoaddress(mSigObj, 2.2)
+        decTx = self.nodes[0].gettransaction(txId)
+        rawTx = self.nodes[0].decoderawtransaction(decTx['hex'])
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
 
-            #THIS IS AN INCOMPLETE FEATURE
-            #NODE2 HAS TWO OF THREE KEY AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION
-            assert_equal(self.nodes[2].getbalance(), bal) #for now, assume the funds of a 2of3 multisig tx are not marked as spendable
+        #THIS IS AN INCOMPLETE FEATURE
+        #NODE2 HAS TWO OF THREE KEY AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION
+        assert_equal(self.nodes[2].getbalance(), bal) #for now, assume the funds of a 2of3 multisig tx are not marked as spendable
 
-            txDetails = self.nodes[0].gettransaction(txId, True)
-            rawTx = self.nodes[0].decoderawtransaction(txDetails['hex'])
-            vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('2.20000000'))
+        txDetails = self.nodes[0].gettransaction(txId, True)
+        rawTx = self.nodes[0].decoderawtransaction(txDetails['hex'])
+        vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('2.20000000'))
 
-            bal = self.nodes[0].getbalance()
-            inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "amount" : vout['value']}]
-            outputs = { self.nodes[0].getnewaddress() : 2.19 }
-            rawTx = self.nodes[2].createrawtransaction(inputs, outputs)
-            rawTxPartialSigned = self.nodes[1].signrawtransactionwithwallet(rawTx, inputs)
-            assert_equal(rawTxPartialSigned['complete'], False) #node1 only has one key, can't comp. sign the tx
+        bal = self.nodes[0].getbalance()
+        inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "amount" : vout['value']}]
+        outputs = { self.nodes[0].getnewaddress() : 2.19 }
+        rawTx = self.nodes[2].createrawtransaction(inputs, outputs)
+        rawTxPartialSigned = self.nodes[1].signrawtransactionwithwallet(rawTx, inputs)
+        assert_equal(rawTxPartialSigned['complete'], False) #node1 only has one key, can't comp. sign the tx
 
-            rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx, inputs)
-            assert_equal(rawTxSigned['complete'], True) #node2 can sign the tx compl., own two of three keys
-            self.nodes[2].sendrawtransaction(rawTxSigned['hex'])
-            rawTx = self.nodes[0].decoderawtransaction(rawTxSigned['hex'])
-            self.sync_all()
-            self.nodes[0].generate(1)
-            self.sync_all()
-            assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx
+        rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx, inputs)
+        assert_equal(rawTxSigned['complete'], True) #node2 can sign the tx compl., own two of three keys
+        self.nodes[2].sendrawtransaction(rawTxSigned['hex'])
+        rawTx = self.nodes[0].decoderawtransaction(rawTxSigned['hex'])
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
+        assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx
 
-            # 2of2 test for combining transactions
-            bal = self.nodes[2].getbalance()
-            addr1 = self.nodes[1].getnewaddress()
-            addr2 = self.nodes[2].getnewaddress()
+        # 2of2 test for combining transactions
+        bal = self.nodes[2].getbalance()
+        addr1 = self.nodes[1].getnewaddress()
+        addr2 = self.nodes[2].getnewaddress()
 
-            addr1Obj = self.nodes[1].getaddressinfo(addr1)
-            addr2Obj = self.nodes[2].getaddressinfo(addr2)
+        addr1Obj = self.nodes[1].getaddressinfo(addr1)
+        addr2Obj = self.nodes[2].getaddressinfo(addr2)
 
-            self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address']
-            mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address']
-            mSigObjValid = self.nodes[2].getaddressinfo(mSigObj)
+        self.nodes[1].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address']
+        mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])['address']
+        mSigObjValid = self.nodes[2].getaddressinfo(mSigObj)
 
-            txId = self.nodes[0].sendtoaddress(mSigObj, 2.2)
-            decTx = self.nodes[0].gettransaction(txId)
-            rawTx2 = self.nodes[0].decoderawtransaction(decTx['hex'])
-            self.sync_all()
-            self.nodes[0].generate(1)
-            self.sync_all()
+        txId = self.nodes[0].sendtoaddress(mSigObj, 2.2)
+        decTx = self.nodes[0].gettransaction(txId)
+        rawTx2 = self.nodes[0].decoderawtransaction(decTx['hex'])
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
 
-            assert_equal(self.nodes[2].getbalance(), bal) # the funds of a 2of2 multisig tx should not be marked as spendable
+        assert_equal(self.nodes[2].getbalance(), bal) # the funds of a 2of2 multisig tx should not be marked as spendable
 
-            txDetails = self.nodes[0].gettransaction(txId, True)
-            rawTx2 = self.nodes[0].decoderawtransaction(txDetails['hex'])
-            vout = next(o for o in rawTx2['vout'] if o['value'] == Decimal('2.20000000'))
+        txDetails = self.nodes[0].gettransaction(txId, True)
+        rawTx2 = self.nodes[0].decoderawtransaction(txDetails['hex'])
+        vout = next(o for o in rawTx2['vout'] if o['value'] == Decimal('2.20000000'))
 
-            bal = self.nodes[0].getbalance()
-            inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "redeemScript" : mSigObjValid['hex'], "amount" : vout['value']}]
-            outputs = { self.nodes[0].getnewaddress() : 2.19 }
-            rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs)
-            rawTxPartialSigned1 = self.nodes[1].signrawtransactionwithwallet(rawTx2, inputs)
-            self.log.debug(rawTxPartialSigned1)
-            assert_equal(rawTxPartialSigned1['complete'], False) #node1 only has one key, can't comp. sign the tx
+        bal = self.nodes[0].getbalance()
+        inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "redeemScript" : mSigObjValid['hex'], "amount" : vout['value']}]
+        outputs = { self.nodes[0].getnewaddress() : 2.19 }
+        rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs)
+        rawTxPartialSigned1 = self.nodes[1].signrawtransactionwithwallet(rawTx2, inputs)
+        self.log.debug(rawTxPartialSigned1)
+        assert_equal(rawTxPartialSigned1['complete'], False) #node1 only has one key, can't comp. sign the tx
 
-            rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet(rawTx2, inputs)
-            self.log.debug(rawTxPartialSigned2)
-            assert_equal(rawTxPartialSigned2['complete'], False) #node2 only has one key, can't comp. sign the tx
-            rawTxComb = self.nodes[2].combinerawtransaction([rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']])
-            self.log.debug(rawTxComb)
-            self.nodes[2].sendrawtransaction(rawTxComb)
-            rawTx2 = self.nodes[0].decoderawtransaction(rawTxComb)
-            self.sync_all()
-            self.nodes[0].generate(1)
-            self.sync_all()
-            assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx
+        rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet(rawTx2, inputs)
+        self.log.debug(rawTxPartialSigned2)
+        assert_equal(rawTxPartialSigned2['complete'], False) #node2 only has one key, can't comp. sign the tx
+        rawTxComb = self.nodes[2].combinerawtransaction([rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']])
+        self.log.debug(rawTxComb)
+        self.nodes[2].sendrawtransaction(rawTxComb)
+        rawTx2 = self.nodes[0].decoderawtransaction(rawTxComb)
+        self.sync_all()
+        self.nodes[0].generate(1)
+        self.sync_all()
+        assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx
 
 
 if __name__ == '__main__':

From 387355bb9482a09c1fc9b137bea56745a93b7dfd Mon Sep 17 00:00:00 2001
From: Jon Atack <jon@atack.com>
Date: Mon, 12 Jul 2021 16:38:36 +0200
Subject: [PATCH 9/9] test, refactor: rpc_rawtransaction PEP8

---
 test/functional/rpc_rawtransaction.py | 90 +++++++++++++++------------
 1 file changed, 49 insertions(+), 41 deletions(-)

diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py
index 25e345a8d1b..fbf8c6ef153 100755
--- a/test/functional/rpc_rawtransaction.py
+++ b/test/functional/rpc_rawtransaction.py
@@ -78,9 +78,8 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.sync_all()
         self.nodes[0].generate(COINBASE_MATURITY + 1)
         self.sync_all()
-        self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),1.5)
-        self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),1.0)
-        self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(),5.0)
+        for amount in [1.5, 1.0, 5.0]:
+            self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), amount)
         self.sync_all()
         self.nodes[0].generate(5)
         self.sync_all()
@@ -192,7 +191,8 @@ class RawTransactionsTest(BitcoinTestFramework):
         assert_raises_rpc_error(-1, "JSON value is not an object as expected", self.nodes[0].createrawtransaction, ['foo'], {})
         assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].createrawtransaction, [{}], {})
         assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {})
-        assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", self.nodes[0].createrawtransaction, [{'txid': 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844'}], {})
+        txid = "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844"
+        assert_raises_rpc_error(-8, f"txid must be hexadecimal string (not '{txid}')", self.nodes[0].createrawtransaction, [{'txid': txid}], {})
         assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': TXID}], {})
         assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': TXID, 'vout': 'foo'}], {})
         assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", self.nodes[0].createrawtransaction, [{'txid': TXID, 'vout': -1}], {})
@@ -264,9 +264,9 @@ class RawTransactionsTest(BitcoinTestFramework):
             addr = self.nodes[0].getnewaddress("", type)
             addrinfo = self.nodes[0].getaddressinfo(addr)
             pubkey = addrinfo["scriptPubKey"]
-            inputs  = [ {'txid' : TXID, 'vout' : 3, 'sequence' : 1000}]
-            outputs = { self.nodes[0].getnewaddress() : 1 }
-            rawtx   = self.nodes[0].createrawtransaction(inputs, outputs)
+            inputs = [{'txid': TXID, 'vout': 3, 'sequence': 1000}]
+            outputs = {self.nodes[0].getnewaddress(): 1}
+            rawtx = self.nodes[0].createrawtransaction(inputs, outputs)
 
             prevtx = dict(txid=TXID, scriptPubKey=pubkey, vout=3, amount=1)
             succ = self.nodes[0].signrawtransactionwithwallet(rawtx, [prevtx])
@@ -309,23 +309,25 @@ class RawTransactionsTest(BitcoinTestFramework):
 
     def sendrawtransaction_tests(self):
         self.log.info("Test sendrawtransaction with missing input")
-        inputs  = [{'txid' : TXID, 'vout' : 1}]  # won't exist
-        outputs = { self.nodes[0].getnewaddress() : 4.998 }
-        rawtx   = self.nodes[2].createrawtransaction(inputs, outputs)
-        rawtx   = self.nodes[2].signrawtransactionwithwallet(rawtx)
+        inputs = [{'txid': TXID, 'vout': 1}]  # won't exist
+        outputs = {self.nodes[0].getnewaddress(): 4.998}
+        rawtx = self.nodes[2].createrawtransaction(inputs, outputs)
+        rawtx = self.nodes[2].signrawtransactionwithwallet(rawtx)
         assert_raises_rpc_error(-25, "bad-txns-inputs-missingorspent", self.nodes[2].sendrawtransaction, rawtx['hex'])
 
     def sendrawtransaction_testmempoolaccept_tests(self):
         self.log.info("Test sendrawtransaction/testmempoolaccept with maxfeerate")
+        fee_exceeds_max = "Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)"
+
         # Test a transaction with a small fee.
         txId = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 1.0)
         rawTx = self.nodes[0].getrawtransaction(txId, True)
         vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000'))
 
         self.sync_all()
-        inputs = [{ "txid" : txId, "vout" : vout['n'] }]
+        inputs = [{"txid": txId, "vout": vout['n']}]
         # Fee 10,000 satoshis, (1 - (10000 sat * 0.00000001 BTC/sat)) = 0.9999
-        outputs = { self.nodes[0].getnewaddress() : Decimal("0.99990000") }
+        outputs = {self.nodes[0].getnewaddress(): Decimal("0.99990000")}
         rawTx = self.nodes[2].createrawtransaction(inputs, outputs)
         rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx)
         assert_equal(rawTxSigned['complete'], True)
@@ -335,7 +337,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         assert_equal(testres['allowed'], False)
         assert_equal(testres['reject-reason'], 'max-fee-exceeded')
         # and sendrawtransaction should throw
-        assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000)
+        assert_raises_rpc_error(-25, fee_exceeds_max, self.nodes[2].sendrawtransaction, rawTxSigned['hex'], 0.00001000)
         # and the following calls should both succeed
         testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']])[0]
         assert_equal(testres['allowed'], True)
@@ -347,9 +349,9 @@ class RawTransactionsTest(BitcoinTestFramework):
         vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('1.00000000'))
 
         self.sync_all()
-        inputs = [{ "txid" : txId, "vout" : vout['n'] }]
+        inputs = [{"txid": txId, "vout": vout['n']}]
         # Fee 2,000,000 satoshis, (1 - (2000000 sat * 0.00000001 BTC/sat)) = 0.98
-        outputs = { self.nodes[0].getnewaddress() : Decimal("0.98000000") }
+        outputs = {self.nodes[0].getnewaddress() : Decimal("0.98000000")}
         rawTx = self.nodes[2].createrawtransaction(inputs, outputs)
         rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx)
         assert_equal(rawTxSigned['complete'], True)
@@ -359,7 +361,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         assert_equal(testres['allowed'], False)
         assert_equal(testres['reject-reason'], 'max-fee-exceeded')
         # and sendrawtransaction should throw
-        assert_raises_rpc_error(-25, 'Fee exceeds maximum configured by user (e.g. -maxtxfee, maxfeerate)', self.nodes[2].sendrawtransaction, rawTxSigned['hex'])
+        assert_raises_rpc_error(-25, fee_exceeds_max, self.nodes[2].sendrawtransaction, rawTxSigned['hex'])
         # and the following calls should both succeed
         testres = self.nodes[2].testmempoolaccept(rawtxs=[rawTxSigned['hex']], maxfeerate='0.20000000')[0]
         assert_equal(testres['allowed'], True)
@@ -378,20 +380,22 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.log.info("Test decoderawtransaction")
         # witness transaction
         encrawtx = "010000000001010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f50500000000000102616100000000"
-        decrawtx = self.nodes[0].decoderawtransaction(encrawtx, True) # decode as witness transaction
+        decrawtx = self.nodes[0].decoderawtransaction(encrawtx, True)  # decode as witness transaction
         assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000'))
         assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # force decode as non-witness transaction
         # non-witness transaction
         encrawtx = "01000000010000000000000072c1a6a246ae63f74f931e8365e15a089c68d61900000000000000000000ffffffff0100e1f505000000000000000000"
-        decrawtx = self.nodes[0].decoderawtransaction(encrawtx, False) # decode as non-witness transaction
+        decrawtx = self.nodes[0].decoderawtransaction(encrawtx, False)  # decode as non-witness transaction
         assert_equal(decrawtx['vout'][0]['value'], Decimal('1.00000000'))
         # known ambiguous transaction in the chain (see https://github.com/bitcoin/bitcoin/issues/20579)
-        encrawtx = "020000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff4b03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000ffffffff03f4c1fb4b0000000016001497cfc76442fe717f2a3f0cc9c175f7561b6619970000000000000000266a24aa21a9ed957d1036a80343e0d1b659497e1b48a38ebe876a056d45965fac4a85cda84e1900000000000000002952534b424c4f434b3a8e092581ab01986cbadc84f4b43f4fa4bb9e7a2e2a0caf9b7cf64d939028e22c0120000000000000000000000000000000000000000000000000000000000000000000000000"
+        coinbase = "03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000"
+        encrawtx = f"020000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff4b{coinbase}" \
+                   "ffffffff03f4c1fb4b0000000016001497cfc76442fe717f2a3f0cc9c175f7561b6619970000000000000000266a24aa21a9ed957d1036a80343e0d1b659497e1b48a38ebe876a056d45965fac4a85cda84e1900000000000000002952534b424c4f434b3a8e092581ab01986cbadc84f4b43f4fa4bb9e7a2e2a0caf9b7cf64d939028e22c0120000000000000000000000000000000000000000000000000000000000000000000000000"
         decrawtx = self.nodes[0].decoderawtransaction(encrawtx)
         decrawtx_wit = self.nodes[0].decoderawtransaction(encrawtx, True)
-        assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False) # fails to decode as non-witness transaction
-        assert_equal(decrawtx, decrawtx_wit) # the witness interpretation should be chosen
-        assert_equal(decrawtx['vin'][0]['coinbase'], "03c68708046ff8415c622f4254432e434f4d2ffabe6d6de1965d02c68f928e5b244ab1965115a36f56eb997633c7f690124bbf43644e23080000000ca3d3af6d005a65ff0200fd00000000")
+        assert_raises_rpc_error(-22, 'TX decode failed', self.nodes[0].decoderawtransaction, encrawtx, False)  # fails to decode as non-witness transaction
+        assert_equal(decrawtx, decrawtx_wit)  # the witness interpretation should be chosen
+        assert_equal(decrawtx['vin'][0]['coinbase'], coinbase)
 
     def transaction_version_number_tests(self):
         self.log.info("Test transaction version numbers")
@@ -415,6 +419,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.log.info("Test raw multisig transactions (legacy)")
         # The traditional multisig workflow does not work with descriptor wallets so these are legacy only.
         # The multisig workflow with descriptor wallets uses PSBTs and is tested elsewhere, no need to do them here.
+
         # 2of2 test
         addr1 = self.nodes[2].getnewaddress()
         addr2 = self.nodes[2].getnewaddress()
@@ -424,12 +429,14 @@ class RawTransactionsTest(BitcoinTestFramework):
 
         # Tests for createmultisig and addmultisigaddress
         assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 1, ["01020304"])
-        self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']]) # createmultisig can only take public keys
-        assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1]) # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here.
+        # createmultisig can only take public keys
+        self.nodes[0].createmultisig(2, [addr1Obj['pubkey'], addr2Obj['pubkey']])
+        # addmultisigaddress can take both pubkeys and addresses so long as they are in the wallet, which is tested here
+        assert_raises_rpc_error(-5, "Invalid public key", self.nodes[0].createmultisig, 2, [addr1Obj['pubkey'], addr1])
 
         mSigObj = self.nodes[2].addmultisigaddress(2, [addr1Obj['pubkey'], addr1])['address']
 
-        #use balance deltas instead of absolute values
+        # use balance deltas instead of absolute values
         bal = self.nodes[2].getbalance()
 
         # send 1.2 BTC to msig adr
@@ -437,7 +444,8 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.sync_all()
         self.nodes[0].generate(1)
         self.sync_all()
-        assert_equal(self.nodes[2].getbalance(), bal+Decimal('1.20000000')) #node2 has both keys of the 2of2 ms addr., tx should affect the balance
+        # node2 has both keys of the 2of2 ms addr, tx should affect the balance
+        assert_equal(self.nodes[2].getbalance(), bal + Decimal('1.20000000'))
 
 
         # 2of3 test from different nodes
@@ -459,29 +467,29 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.nodes[0].generate(1)
         self.sync_all()
 
-        #THIS IS AN INCOMPLETE FEATURE
-        #NODE2 HAS TWO OF THREE KEY AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION
-        assert_equal(self.nodes[2].getbalance(), bal) #for now, assume the funds of a 2of3 multisig tx are not marked as spendable
+        # THIS IS AN INCOMPLETE FEATURE
+        # NODE2 HAS TWO OF THREE KEYS AND THE FUNDS SHOULD BE SPENDABLE AND COUNT AT BALANCE CALCULATION
+        assert_equal(self.nodes[2].getbalance(), bal)  # for now, assume the funds of a 2of3 multisig tx are not marked as spendable
 
         txDetails = self.nodes[0].gettransaction(txId, True)
         rawTx = self.nodes[0].decoderawtransaction(txDetails['hex'])
         vout = next(o for o in rawTx['vout'] if o['value'] == Decimal('2.20000000'))
 
         bal = self.nodes[0].getbalance()
-        inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "amount" : vout['value']}]
-        outputs = { self.nodes[0].getnewaddress() : 2.19 }
+        inputs = [{"txid": txId, "vout": vout['n'], "scriptPubKey": vout['scriptPubKey']['hex'], "amount": vout['value']}]
+        outputs = {self.nodes[0].getnewaddress(): 2.19}
         rawTx = self.nodes[2].createrawtransaction(inputs, outputs)
         rawTxPartialSigned = self.nodes[1].signrawtransactionwithwallet(rawTx, inputs)
-        assert_equal(rawTxPartialSigned['complete'], False) #node1 only has one key, can't comp. sign the tx
+        assert_equal(rawTxPartialSigned['complete'], False)  # node1 only has one key, can't comp. sign the tx
 
         rawTxSigned = self.nodes[2].signrawtransactionwithwallet(rawTx, inputs)
-        assert_equal(rawTxSigned['complete'], True) #node2 can sign the tx compl., own two of three keys
+        assert_equal(rawTxSigned['complete'], True)  # node2 can sign the tx compl., own two of three keys
         self.nodes[2].sendrawtransaction(rawTxSigned['hex'])
         rawTx = self.nodes[0].decoderawtransaction(rawTxSigned['hex'])
         self.sync_all()
         self.nodes[0].generate(1)
         self.sync_all()
-        assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx
+        assert_equal(self.nodes[0].getbalance(), bal + Decimal('50.00000000') + Decimal('2.19000000'))  # block reward + tx
 
         # 2of2 test for combining transactions
         bal = self.nodes[2].getbalance()
@@ -502,23 +510,23 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.nodes[0].generate(1)
         self.sync_all()
 
-        assert_equal(self.nodes[2].getbalance(), bal) # the funds of a 2of2 multisig tx should not be marked as spendable
+        assert_equal(self.nodes[2].getbalance(), bal)  # the funds of a 2of2 multisig tx should not be marked as spendable
 
         txDetails = self.nodes[0].gettransaction(txId, True)
         rawTx2 = self.nodes[0].decoderawtransaction(txDetails['hex'])
         vout = next(o for o in rawTx2['vout'] if o['value'] == Decimal('2.20000000'))
 
         bal = self.nodes[0].getbalance()
-        inputs = [{ "txid" : txId, "vout" : vout['n'], "scriptPubKey" : vout['scriptPubKey']['hex'], "redeemScript" : mSigObjValid['hex'], "amount" : vout['value']}]
-        outputs = { self.nodes[0].getnewaddress() : 2.19 }
+        inputs = [{"txid": txId, "vout": vout['n'], "scriptPubKey": vout['scriptPubKey']['hex'], "redeemScript": mSigObjValid['hex'], "amount": vout['value']}]
+        outputs = {self.nodes[0].getnewaddress(): 2.19}
         rawTx2 = self.nodes[2].createrawtransaction(inputs, outputs)
         rawTxPartialSigned1 = self.nodes[1].signrawtransactionwithwallet(rawTx2, inputs)
         self.log.debug(rawTxPartialSigned1)
-        assert_equal(rawTxPartialSigned1['complete'], False) #node1 only has one key, can't comp. sign the tx
+        assert_equal(rawTxPartialSigned1['complete'], False)  # node1 only has one key, can't comp. sign the tx
 
         rawTxPartialSigned2 = self.nodes[2].signrawtransactionwithwallet(rawTx2, inputs)
         self.log.debug(rawTxPartialSigned2)
-        assert_equal(rawTxPartialSigned2['complete'], False) #node2 only has one key, can't comp. sign the tx
+        assert_equal(rawTxPartialSigned2['complete'], False)  # node2 only has one key, can't comp. sign the tx
         rawTxComb = self.nodes[2].combinerawtransaction([rawTxPartialSigned1['hex'], rawTxPartialSigned2['hex']])
         self.log.debug(rawTxComb)
         self.nodes[2].sendrawtransaction(rawTxComb)
@@ -526,7 +534,7 @@ class RawTransactionsTest(BitcoinTestFramework):
         self.sync_all()
         self.nodes[0].generate(1)
         self.sync_all()
-        assert_equal(self.nodes[0].getbalance(), bal+Decimal('50.00000000')+Decimal('2.19000000')) #block reward + tx
+        assert_equal(self.nodes[0].getbalance(), bal + Decimal('50.00000000') + Decimal('2.19000000'))  # block reward + tx
 
 
 if __name__ == '__main__':