Add listunspent() test for spendable/unspendable UTXO #7822

joaopaulofonseca commented at 10:30 AM on April 6, 2016: contributor

This PR adds a test on wallet.py covering the behavior of spendable/unspendable value for UTXO, when calling RPC listunspent.

This value should be different for address or private key import.

laanwj added the label Tests on Apr 6, 2016

in qa/rpc-tests/listunspent.py:None in 0d8581d4a3 outdated

  63 | +        # retrieve that transaction and check if the transaction inputs are not among the unspents
  64 | +        rawtx = self.nodes[0].getrawtransaction(txid, 1)
  65 | +        for txin in rawtx["vin"]:
  66 | +            for utxo in list_unspent_after:
  67 | +                if (txin["txid"] == utxo["txid"] and txin["vout"] == utxo["vout"]):
  68 | +                    raise AssertionError("Fount used output on UTXOs list")

MarcoFalke commented at 3:57 PM on April 6, 2016:

Nit: typo

in qa/rpc-tests/listunspent.py:None in 0d8581d4a3 outdated

  87 | +        assert(not self.nodes[1].listunspent()[0]["spendable"])
  88 | +
  89 | +        try:
  90 | +            self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1)
  91 | +        except JSONRPCException:
  92 | +            print("Impossible to send coins. Available output is unspendable.")

MarcoFalke commented at 3:59 PM on April 6, 2016:

You may want to try assert_raises instead.

in qa/rpc-tests/listunspent.py:None in 0d8581d4a3 outdated

 105 | +        assert_equal(len(self.nodes[1].listunspent()), 1)
 106 | +        assert(self.nodes[1].listunspent()[0]["spendable"])
 107 | +
 108 | +        try:
 109 | +            self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 0.1)
 110 | +        except JSONRPCException, e:

MarcoFalke commented at 4:00 PM on April 6, 2016:

Please use py3 syntax here: as e:

in qa/rpc-tests/listunspent.py:None in 0d8581d4a3 outdated

 106 | +        assert(self.nodes[1].listunspent()[0]["spendable"])
 107 | +
 108 | +        try:
 109 | +            self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 0.1)
 110 | +        except JSONRPCException, e:
 111 | +            raise AssertionError("Not possible to send coins. Possible unsufficient funds.")

MarcoFalke commented at 4:01 PM on April 6, 2016:

Nit: No need to shadow the original exception. You can just remove the try-except block altogether and only leave the sendtoaddress

joaopaulofonseca commented at 5:31 PM on April 6, 2016:

Thanks for the tips. Fixed and rebased.

MarcoFalke commented at 4:01 PM on April 6, 2016: member

Thanks, Concept ACK.

joaopaulofonseca force-pushed on Apr 6, 2016

joaopaulofonseca renamed this:
~~[qa] Add test to RPC listunspent~~
Add test to RPC listunspent
on Apr 7, 2016

laanwj commented at 2:24 PM on April 14, 2016: member

I'm a bit divided whether I should propose to add this into the wallet test, which already uses listunspent. This currently adds some startup/teardown overhead (test takes ~9 seconds) just to test one call. I don't feel strongly about that though. What do you think @MarcoFalke?

MarcoFalke commented at 8:34 AM on April 15, 2016: member

startup/teardown overhead

I think such overhead is rather small. (Note that disablewallet.py runs in less than a second)

The overhead probably is caused by setting up the bidirectional connections and sync_all() calls.

I have not yet looked closely at the test (I try to avoid large diffs right now in the /qa subfolder as each patch may or may not silently conflict with the python3 refactoring work.) @jpdffonseca Would you mind to take a look if this can be neatly squashed into the wallet.py test, as suggested by @laanwj ?

joaopaulofonseca commented at 8:48 AM on April 15, 2016: contributor

@MarcoFalke and @laanwj yes that makes sense. I will try to squash this verifications into wallet.py.

joaopaulofonseca force-pushed on Apr 18, 2016

joaopaulofonseca renamed this:
~~Add test to RPC listunspent~~
Add wallet test to check spendable/unspendable UTXO on RPC listunspent
on Apr 18, 2016

joaopaulofonseca commented at 2:18 PM on April 18, 2016: contributor

@MarcoFalke, after checking that wallet.py already covers some of the tests for listuspent(), I discarded the separate test file and merged the tests regarding the import of bitcoin addresses and private keys into wallet.py. The test execution time should remain the same.

joaopaulofonseca renamed this:
~~Add wallet test to check spendable/unspendable UTXO on RPC listunspent~~
Add listunspent test for spendable/unspendable UTXO
on Apr 18, 2016

joaopaulofonseca renamed this:
~~Add listunspent test for spendable/unspendable UTXO~~
Add listunspent() test for spendable/unspendable UTXO
on Apr 18, 2016

MarcoFalke commented at 7:03 PM on April 18, 2016: member

Thanks. Looks better now.

utACK 3b2eedc

Move method to check matches within arrays on util.py fa942c755a

Add test to check spendable and unspendable UTXO on RPC listunspent 5d217decc1

in qa/rpc-tests/wallet.py:None in 3b2eedcba3 outdated

   5 | @@ -6,6 +6,27 @@
   6 |  from test_framework.test_framework import BitcoinTestFramework
   7 |  from test_framework.util import *
   8 |  
   9 | +def check_array_result(object_array, to_match, expected):

MarcoFalke commented at 7:04 PM on April 18, 2016:

If you copied this function from elsewhere, wouldn't it make sense to move it instead to test_framework/util.py, so the utility function can be used in other tests as well?

joaopaulofonseca commented at 9:26 PM on April 18, 2016:

Indeed.. It is used by multiple tests. I think it comes in handy for current and future test cases. If you guys agree I guess this could be moved to test_framework/util.py to be reused whenever needed..

MarcoFalke commented at 10:21 PM on April 18, 2016:

Sure, go ahead.

laanwj commented at 9:21 AM on April 19, 2016:

Do you intend to do so in this pull? If so I'll wait with merging.

joaopaulofonseca commented at 10:14 AM on April 19, 2016:

Yes, since we are on this topic.

joaopaulofonseca force-pushed on Apr 19, 2016

joaopaulofonseca commented at 11:37 AM on April 19, 2016: contributor

Moved assert_array_result() to test_framework/util.py.

I noticed that the previous method on receivedby.py had a bug when using the flag should_not_find. If the flag is set to True, the validation wouldn't work correctly when the values passed as to_match are indeed found inside object_array, and no assertion is raised.

laanwj commented at 12:57 PM on April 19, 2016: member

utACK 5d217de

laanwj merged this on Apr 19, 2016

laanwj closed this on Apr 19, 2016

laanwj referenced this in commit 0e6fd5e4af on Apr 19, 2016

fixe deleted the branch on Apr 19, 2016

MarcoFalke referenced this in commit 6862627ce6 on Apr 19, 2016

nomnombtc referenced this in commit 5c020a5bcb on Nov 11, 2016

nomnombtc referenced this in commit 47fd3b6eeb on Nov 12, 2016

nomnombtc referenced this in commit 9811199a3e on Nov 13, 2016

sickpig referenced this in commit f5f418a5d9 on Nov 14, 2016

MarcoFalke locked this on Sep 8, 2021