RPC: gettxout: Slightly improve doc and tests #10859

jtimon commented at 12:28 AM on July 18, 2017: contributor

Slightly related to #10822 in the sense that I felt the documentation and testing wasn't as good as it could be while writing it.

Ping @sipa since we discussed this on IRC.

in src/rpc/blockchain.cpp:950 in 2bf90e1f63 outdated

 948 | -            "2. n              (numeric, required) vout number\n"
 949 | -            "3. include_mempool  (boolean, optional) Whether to include the mempool\n"
 950 | +            "1. \"txid\"             (string, required) The transaction id\n"
 951 | +            "2. \"n\"                (numeric, required) vout number\n"
 952 | +            "3. \"include_mempool\"  (boolean, optional) Whether to include the mempool. Default: true."
 953 | +            "     Note that an unspent output that is spent in the memepool won't appear.\n"

sipa commented at 12:44 AM on July 18, 2017:

Wow, such meme, many pool.

jtimon commented at 12:52 AM on July 18, 2017:

hahaha, yeah let's leave a memepool structure for another PR. fixed

jtimon force-pushed on Jul 18, 2017

fanquake added the label RPC/REST/ZMQ on Jul 18, 2017

jtimon commented at 2:34 AM on August 26, 2017: contributor

btw, hopefully the nit was solved

sipa commented at 7:42 PM on August 27, 2017: member

utACK 7ec368927480c809d27de186690f9d6b6c2e9a9f

in test/functional/wallet.py:62 in 7ec3689274 outdated

  55 | @@ -56,6 +56,15 @@ def run_test(self):
  56 |          assert_equal(len(self.nodes[1].listunspent()), 1)
  57 |          assert_equal(len(self.nodes[2].listunspent()), 0)
  58 |  
  59 | +        self.log.info("test gettxout")
  60 | +        confirmed_txid, confirmed_index = utxos[0]["txid"], utxos[0]["vout"]
  61 | +        # First, outputs that are unspent both in the chain and in the
  62 | +        # mainpool should appear with or without include_mempool

MarcoFalke commented at 10:06 PM on August 27, 2017:

mempool

MarcoFalke commented at 10:07 PM on August 27, 2017: member

utACK 7ec368927480c809d27de186690f9d6b6c2e9a9f after fixup

RPC: gettxout: Slightly improve doc and tests 6d2d2eb493

jtimon force-pushed on Aug 28, 2017

jtimon commented at 10:59 PM on August 28, 2017: contributor

Fixed @MarcoFalke 's nit

MarcoFalke merged this on Aug 28, 2017

MarcoFalke closed this on Aug 28, 2017

in test/functional/wallet.py:62 in 6d2d2eb493

  55 | @@ -56,6 +56,15 @@ def run_test(self):
  56 |          assert_equal(len(self.nodes[1].listunspent()), 1)
  57 |          assert_equal(len(self.nodes[2].listunspent()), 0)
  58 |  
  59 | +        self.log.info("test gettxout")
  60 | +        confirmed_txid, confirmed_index = utxos[0]["txid"], utxos[0]["vout"]
  61 | +        # First, outputs that are unspent both in the chain and in the
  62 | +        # mempool should appear with or without include_mempool

promag commented at 11:49 PM on August 28, 2017:

Nit, missing period.

MarcoFalke referenced this in commit 91e49c51f1 on Aug 28, 2017

in test/functional/wallet.py:67 in 6d2d2eb493

  62 | +        # mempool should appear with or without include_mempool
  63 | +        txout = self.nodes[0].gettxout(txid=confirmed_txid, n=confirmed_index, include_mempool=False)
  64 | +        assert_equal(txout['value'], 50)
  65 | +        txout = self.nodes[0].gettxout(txid=confirmed_txid, n=confirmed_index, include_mempool=True)
  66 | +        assert_equal(txout['value'], 50)
  67 | +

MarcoFalke commented at 11:50 PM on August 28, 2017:

nit: There is trailing whitespace, causing issues with some editors. We don't have this rule in the developer notes, but good to keep in mind for the future.

jtimon commented at 9:41 PM on August 29, 2017:

Perhaps it is time to add it to the notes. Is is save right now to have your editor configured to remove all trailing whitespaces in any file you save in the project now? I remember when that wouldn't be save in the sense that your +14-5 commit would turn into +60-50 or something.

MarcoFalke commented at 11:50 PM on August 28, 2017: member

re-utACK 6d2d2eb

in src/rpc/blockchain.cpp:948 in 6d2d2eb493

 943 | @@ -944,9 +944,10 @@ UniValue gettxout(const JSONRPCRequest& request)
 944 |              "gettxout \"txid\" n ( include_mempool )\n"
 945 |              "\nReturns details about an unspent transaction output.\n"
 946 |              "\nArguments:\n"
 947 | -            "1. \"txid\"       (string, required) The transaction id\n"
 948 | -            "2. n              (numeric, required) vout number\n"
 949 | -            "3. include_mempool  (boolean, optional) Whether to include the mempool\n"
 950 | +            "1. \"txid\"             (string, required) The transaction id\n"
 951 | +            "2. \"n\"                (numeric, required) vout number\n"

promag commented at 11:57 PM on August 28, 2017:

Nit, vout index?

in src/rpc/blockchain.cpp:950 in 6d2d2eb493

 948 | -            "2. n              (numeric, required) vout number\n"
 949 | -            "3. include_mempool  (boolean, optional) Whether to include the mempool\n"
 950 | +            "1. \"txid\"             (string, required) The transaction id\n"
 951 | +            "2. \"n\"                (numeric, required) vout number\n"
 952 | +            "3. \"include_mempool\"  (boolean, optional) Whether to include the mempool. Default: true."
 953 | +            "     Note that an unspent output that is spent in the mempool won't appear.\n"

promag commented at 12:02 AM on August 29, 2017:

Mixed feelings when reading the note, I thought an unspent output is never spent weather the transaction is in the mempool or not, otherwise is not an unspent output.

Suggestion, remove unspent: Note that an output that is spent in the mempool isn't listed.

promag commented at 10:05 PM on August 29, 2017:

@MarcoFalke it's merged but WDYT?

MarcoFalke commented at 6:36 AM on August 30, 2017:

"unspent output" refers to the chainstate, whereas "spent in the mempool" has no effect on the chainstate. Imo it is fine.

jtimon deleted the branch on Aug 31, 2017

MarcoFalke referenced this in commit 2a5d099e47 on Oct 3, 2017

attilaaf referenced this in commit 9184637847 on May 25, 2019

PastaPastaPasta referenced this in commit 535d1985d8 on Sep 19, 2019

PastaPastaPasta referenced this in commit 1e0a196127 on Sep 23, 2019

PastaPastaPasta referenced this in commit bfbafdf29f on Sep 24, 2019

codablock referenced this in commit 74325db11a on Sep 24, 2019

barrystyle referenced this in commit b5221c6846 on Jan 22, 2020

MarcoFalke locked this on Sep 8, 2021