Improve createrawtransaction functional tests

promag commented at 3:01 pm on December 12, 2017: member

This was motivated by the Invalid parameter, duplicated address test.

Credit to @laanwj for multidict implementation.

rpc: Validate replaceable type in createrawtransaction 320669a363

promag commented at 3:05 pm on December 12, 2017: member

Relates to #11872 (https://github.com/bitcoin/bitcoin/pull/11872#issuecomment-351024184).

in test/functional/rawtransactions.py:86 in 0df137b8c7 outdated

72+        assert_raises_rpc_error(-3, "Expected type object", self.nodes[0].createrawtransaction, [], 'foo')
73+        assert_raises_rpc_error(-8, "Data must be hexadecimal string", self.nodes[0].createrawtransaction, [], {'data': 'foo'})
74+        assert_raises_rpc_error(-5, "Invalid Bitcoin address", self.nodes[0].createrawtransaction, [], {'foo': 0})
75+        assert_raises_rpc_error(-3, "Invalid amount", self.nodes[0].createrawtransaction, [], {address: 'foo'})
76+        assert_raises_rpc_error(-3, "Amount out of range", self.nodes[0].createrawtransaction, [], {address: -1})
77+        assert_raises_rpc_error(-8, "Invalid parameter, duplicated address: %s" % address, self.nodes[0].createrawtransaction, [], multidict([(address, 1), (address, 1)]))

laanwj commented at 3:06 pm on December 12, 2017:

So there is already a check for duplicated address? That’s great!

promag commented at 3:08 pm on December 12, 2017:

Yes, createrawtransaction and sendmany RPC’s check for duplicated address.

laanwj commented at 3:09 pm on December 12, 2017:

Indeed! I hadn’t expected a check here because passing this is tricky in JSON object format.

laanwj commented at 3:09 pm on December 12, 2017: member

utACK 0df137b

laanwj added the label Tests on Dec 12, 2017

jonasschnelli approved

jonasschnelli commented at 6:47 pm on December 12, 2017: contributor

utACK 0df137b8c719a904c792240a0e373ca38c334443

in test/functional/rawtransactions.py:19 in 0df137b8c7 outdated

14@@ -15,6 +15,16 @@
15 from test_framework.test_framework import BitcoinTestFramework
16 from test_framework.util import *
17 
18+
19+class multidict(dict):

jnewbery commented at 10:06 pm on December 12, 2017:

Can you add a comment for this please? Something like:

0"""Dictionary that allows duplicate keys.
1
2Constructed with a list of (key, value) tuples. When dumped by the json module, will output invalid json with repeated keys, eg:
3>>> json.dumps(multidict([(1,2),(1,2)])
4'{"1": 2, "1": 2}'
5
6Used to test calls to rpc methods with invalid repeated keys in the json object."""

laanwj commented at 5:53 am on December 13, 2017:

Yeah submitting JSON objects in deterministic order and with potentially duplicated keys is the only thing it’s good for, it’s not quite a traditional multidict implementation.

promag commented at 2:52 pm on December 13, 2017:

Done.

jnewbery commented at 10:07 pm on December 12, 2017: member

Tested ACK. Great coverage - thanks @promag . One request for an additional comment.

in test/functional/rawtransactions.py:66 in 0df137b8c7 outdated

48@@ -39,6 +49,41 @@ def run_test(self):
49         self.nodes[0].generate(5)
50         self.sync_all()
51 
52+        # Test `createrawtransaction` required parameters
53+        assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction)
54+        assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, [])
55+
56+        # Test `createrawtransaction` invalid extra parameters
57+        assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, [], {}, 0, False, 'foo')

MarcoFalke commented at 0:11 am on December 13, 2017:

nit: My personal style preference is to use named args when passing unnamed parameters. Not sure what others think.

promag commented at 9:52 am on December 13, 2017:

In this case I’m not sure, because that will throw Unknown named parameter. But for other cases you mean?

0assert_raises_rpc_error(-1, "createrawtransaction", self.nodes[0].createrawtransaction, inputs=[], outputs={}, locktime=0, replaceable=False)

MarcoFalke commented at 3:06 pm on December 13, 2017:

Ah, right. Won’t work in this case.

MarcoFalke commented at 0:13 am on December 13, 2017: member

Concept ACK.

test: Add multidict to support dictionary with duplicate key (laanwj) 27c6199373

test: Add createrawtransaction functional tests 88af5028ad

promag force-pushed on Dec 13, 2017

MarcoFalke commented at 3:23 pm on December 13, 2017: member

According to what others reviewed, only change should be adding a comment, so trivial to re-ACK. (ping @jnewbery )

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3utACK 88af5028ad3de71c8b86b50cb1c6bdd57c1ba6e5
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQIcBAEBCgAGBQJaMUWGAAoJENLqSFDnUoslcNUP/Ar1eK4SOmsoA/SvpfhbpwHE
 74jMFEqkKhef4F1AYQIDaJAvh4HKUpyTX2o8IA9XRvrHLjTJz0vFMd/jMMuKbnB7M
 86Ie1mmykyNRmFg3qvYJDa4XD6FdLEpR7Nwk4YYlAJemBm9+G24vAY1jL6r3Devle
 9g6z90/2iYBYOam6ihkhfd7luRMEFKcIi8fjIVsPXK+g8TLOlS4M9wTjRqQ+6RvC4
10npkANcAsaHcF3f3d3VOggij3TrEUq40fiir5dtW/ViU27fT03YFZKJKfuSRjFxCf
11NRGL1IDlgw4HFKnBLdC/Boq25Lx3f5w7PDUBU4C60YL/j6n5CehxjA3TAOHzDvh+
12l2wGjk3y9TGTnrknZGmwmP7GRbM1a3bVPN0UsZoaKMBzwGnAe+UMpf/rkR+jxjhO
13Ge73jZyowtpv0hPJotVb89kGcWCBaWWOXE6Btr1t8bc/NJB7UYJdjh5/uCHRpeOn
14VeZOZAxS839xUphtgf9yaBF/sjM7lLO/TFIg5OTZBQd/Rs1aV8wijy/P3IeSn+zd
15ZFAmBcqUYep9JQorRDNHzav4a0KIeL10kEJ3BYWezmJSPIkpdXJHX+f/vS8sbkNA
16FMyJi1htT2dwffLal6YleHHEOmpkO2RF0WSzsV0Hs8f3tEImu/5Tgk47gTxo5Nvy
17FqUd3vncrAM1cc5IyU4G
18=IryX
19-----END PGP SIGNATURE-----

jnewbery commented at 4:07 pm on December 13, 2017: member

Tested ACK 88af5028ad3de71c8b86b50cb1c6bdd57c1ba6e5. Comment looks good :slightly_smiling_face:

laanwj merged this on Dec 13, 2017

laanwj closed this on Dec 13, 2017

laanwj referenced this in commit d4991c0cbb on Dec 13, 2017

PastaPastaPasta referenced this in commit 65e657a56c on Jan 17, 2020

PastaPastaPasta referenced this in commit 6fb08431bb on Jan 22, 2020

PastaPastaPasta referenced this in commit 40994d0142 on Jan 22, 2020

PastaPastaPasta referenced this in commit c3469855dc on Jan 29, 2020

PastaPastaPasta referenced this in commit 42a198ec53 on Jan 29, 2020

PastaPastaPasta referenced this in commit 132ad56ca8 on Jan 29, 2020

PastaPastaPasta referenced this in commit 212496c202 on Jan 31, 2020

ckti referenced this in commit cb14997f52 on Mar 28, 2021

DrahtBot locked this on Sep 8, 2021

Improve createrawtransaction functional tests #11877