RPC: gettxout: Slightly improve doc and tests #10859
Conversation
src/rpc/blockchain.cpp
Outdated
There was a problem hiding this comment.
hahaha, yeah let's leave a memepool structure for another PR. fixed
jtimon
force-pushed
the
b15-rpc-gettxout-mempool
branch
from
2bf90e1 to
7ec3689
Compare
|
btw, hopefully the nit was solved |
|
utACK 7ec368927480c809d27de186690f9d6b6c2e9a9f |
test/functional/wallet.py
Outdated
|
utACK 7ec368927480c809d27de186690f9d6b6c2e9a9f after fixup |
jtimon
force-pushed
the
b15-rpc-gettxout-mempool
branch
from
7ec3689 to
6d2d2eb
Compare
|
Fixed @MarcoFalke 's nit |
6d2d2eb RPC: gettxout: Slightly improve doc and tests (Jorge Timón) Pull request description: 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. Tree-SHA512: a0b3ffdac65245a0429e772fc2d8bcc1e829b02c70fb2af6ee0b7578cae46683f6c51a824b4d703d4dc3f99b6f03a658d6bbc818bf32981516f24124249a211d
| assert_equal(txout['value'], 50) | ||
| txout = self.nodes[0].gettxout(txid=confirmed_txid, n=confirmed_index, include_mempool=True) | ||
| assert_equal(txout['value'], 50) | ||
|
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
re-utACK 6d2d2eb |
| self.log.info("test gettxout") | ||
| confirmed_txid, confirmed_index = utxos[0]["txid"], utxos[0]["vout"] | ||
| # First, outputs that are unspent both in the chain and in the | ||
| # mempool should appear with or without include_mempool |
| "2. n (numeric, required) vout number\n" | ||
| "3. include_mempool (boolean, optional) Whether to include the mempool\n" | ||
| "1. \"txid\" (string, required) The transaction id\n" | ||
| "2. \"n\" (numeric, required) vout number\n" |
| "1. \"txid\" (string, required) The transaction id\n" | ||
| "2. \"n\" (numeric, required) vout number\n" | ||
| "3. \"include_mempool\" (boolean, optional) Whether to include the mempool. Default: true." | ||
| " Note that an unspent output that is spent in the mempool won't appear.\n" |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
"unspent output" refers to the chainstate, whereas "spent in the mempool" has no effect on the chainstate. Imo it is fine.
Github-Pull: bitcoin#10859 Rebased-From: 6d2d2eb
6d2d2eb RPC: gettxout: Slightly improve doc and tests (Jorge Timón) Pull request description: Slightly related to bitcoin#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. Tree-SHA512: a0b3ffdac65245a0429e772fc2d8bcc1e829b02c70fb2af6ee0b7578cae46683f6c51a824b4d703d4dc3f99b6f03a658d6bbc818bf32981516f24124249a211d
5 participants
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.