[test] Add test for gettxout to wallet.py #10256

  61 |          # Send 21 BTC from 0 to 2 using sendtoaddress call.
  62 |          self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 11)
  63 | -        self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 10)
  64 | +        mempool_txid = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 10)
  65 | +
  66 | +        # test gettxout

jnewbery commented at 9:28 PM on April 28, 2017:

nit: my personal preference is to make this kind of 'heading' comment into a log, so test runners have some feedback as they're running the test.

  62 |          self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 11)
  63 | -        self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 10)
  64 | +        mempool_txid = self.nodes[0].sendtoaddress(self.nodes[2].getnewaddress(), 10)
  65 | +
  66 | +        # test gettxout
  67 | +        # confirmed tx should be visible if you exclude mempool

jnewbery commented at 9:29 PM on April 28, 2017:

This comment isn't quite right. We're not looking at a confirmed transaction here, we're looking at a transaction output who's spend is unconfirmed. That's why if include_mempool is set to False, we still think we own this output

  68 | +        # but invisible if you include mempool
  69 | +        confirmed_txid, confirmed_index = utxos[0]["txid"], utxos[0]["vout"]
  70 | +        txout = self.nodes[0].gettxout(confirmed_txid, confirmed_index, False)
  71 | +        assert_equal(txout['value'], 50)
  72 | +        txout = self.nodes[0].gettxout(confirmed_txid, confirmed_index, True)
  73 | +        assert(txout is None)

jnewbery commented at 9:31 PM on April 28, 2017:

no need for brackets here. assert is a statement, not a function.

This should just be: assert not txout

  78 | +        assert(txout is None)
  79 | +        txout = self.nodes[0].gettxout(mempool_txid, mempool_index, True)
  80 | +        # note the mempool tx will have randomly assigned indices
  81 | +        # but 10 will go to node2 and the rest will go to node0
  82 | +        balance = self.nodes[0].getbalance()
  83 | +        assert(txout['value'] in (10, balance))

jnewbery commented at 9:34 PM on April 28, 2017:

I think the intent is clearer if you collect both txouts above, and then test that one of them is equal to 10 and the other is equal to balance. Something like:

txout0 = self.nodes[0].gettxout(mempool_txid, 0, True)
txout1 = self.nodes[0].gettxout(mempool_txid, 1, True)

balance = self.nodes[0].getbalance()
assert_equal(set(10, balance), set(txout0['value'], txout1['value']))

jimmysong commented at 9:49 PM on April 28, 2017:

good idea

  65 | +
  66 | +        # test gettxout
  67 | +        # confirmed tx should be visible if you exclude mempool
  68 | +        # but invisible if you include mempool
  69 | +        confirmed_txid, confirmed_index = utxos[0]["txid"], utxos[0]["vout"]
  70 | +        txout = self.nodes[0].gettxout(confirmed_txid, confirmed_index, False)

jnewbery commented at 9:36 PM on April 28, 2017:

consider using named arguments when the positional arguments are just bools. It makes it difficult for the reader to know what the argument is for (alright, I know you've explained it in the comment above, but the reader still needs to trust that you're right!)

  74 | +        # new utxo from mempool should be invisible if you exclude mempool
  75 | +        # but visible if you include mempool
  76 | +        txout = self.nodes[0].gettxout(mempool_txid, 0, False)
  77 | +        assert txout is None
  78 | +        txout1 = self.nodes[0].gettxout(mempool_txid, 0, True)
  79 | +        txout2 = self.nodes[0].gettxout(mempool_txid, 1, True)

MarcoFalke commented at 12:45 PM on April 29, 2017:

nit: In the future you might use named args for arguments that are unnamed boolean or integers of a function that takes more than one arg.

gettxout(txid=mempool_txid, n=1,include_mempool= True)