Improve createrawtransaction functional tests #11877

pull promag wants to merge 3 commits into bitcoin:master from promag:2017-12-createrawtransaction changing 2 files +55 −1
  1. 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.

  2. rpc: Validate replaceable type in createrawtransaction 320669a363
  3. promag commented at 3:05 pm on December 12, 2017: member
    Relates to #11872 (https://github.com/bitcoin/bitcoin/pull/11872#issuecomment-351024184).
  4. 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.
  5. laanwj commented at 3:09 pm on December 12, 2017: member
    utACK 0df137b
  6. laanwj added the label Tests on Dec 12, 2017
  7. jonasschnelli approved
  8. jonasschnelli commented at 6:47 pm on December 12, 2017: contributor
    utACK 0df137b8c719a904c792240a0e373ca38c334443
  9. 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.
  10. jnewbery commented at 10:07 pm on December 12, 2017: member
    Tested ACK. Great coverage - thanks @promag . One request for an additional comment.
  11. 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.
  12. MarcoFalke commented at 0:13 am on December 13, 2017: member
    Concept ACK.
  13. test: Add multidict to support dictionary with duplicate key (laanwj) 27c6199373
  14. test: Add createrawtransaction functional tests 88af5028ad
  15. promag force-pushed on Dec 13, 2017
  16. 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-----
  17. jnewbery commented at 4:07 pm on December 13, 2017: member
    Tested ACK 88af5028ad3de71c8b86b50cb1c6bdd57c1ba6e5. Comment looks good :slightly_smiling_face:
  18. laanwj merged this on Dec 13, 2017
  19. laanwj closed this on Dec 13, 2017
  20. laanwj referenced this in commit d4991c0cbb on Dec 13, 2017
  21. PastaPastaPasta referenced this in commit 65e657a56c on Jan 17, 2020
  22. PastaPastaPasta referenced this in commit 6fb08431bb on Jan 22, 2020
  23. PastaPastaPasta referenced this in commit 40994d0142 on Jan 22, 2020
  24. PastaPastaPasta referenced this in commit c3469855dc on Jan 29, 2020
  25. PastaPastaPasta referenced this in commit 42a198ec53 on Jan 29, 2020
  26. PastaPastaPasta referenced this in commit 132ad56ca8 on Jan 29, 2020
  27. PastaPastaPasta referenced this in commit 212496c202 on Jan 31, 2020
  28. ckti referenced this in commit cb14997f52 on Mar 28, 2021
  29. DrahtBot locked this on Sep 8, 2021


promag laanwj jonasschnelli jnewbery MarcoFalke

Labels
Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-09-29 01:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me