Skip to content

Conversation

laanwj
Copy link
Member

@laanwj laanwj commented Mar 30, 2022

When getrawtransaction is successfully used on a coinbase transaction, there is an assertion error. This is very unlikely but happens in the interface_usdt_utxocache.py test in #24358.

This does the following:

  • Add missing "coinbase" documentation.

  • Synchronize documentation between getrawtransaction and decoderawtransaction, the two users of TxToUniv that have detailed documentation. decodepsbt and getblock also uses it but fortunately elides this block.

  • Change "vout[].amount" to STR_AMOUNT for consistency.

  • Add maintainer comment to keep the two places synchronized. It might be possible to get smarter with deduplication, but there are some extra fields that prevent the obvious way.

When `getrawtransaction` is successfully used on a coinbase transaction,
there is an assertion error. This is very unlikely but happens in the
test in bitcoin#24358.

This does the following:

- Add missing "coinbase" documentation.

- Synchronize documentation between `getrawtransaction` and
  `decoderawtransaction`, the two users of `TxToUniv` that have detailed
  documentation. `decodepsbt` also uses it but fortunately elides this block.

- Change "vout[].amount" to `STR_AMOUNT` for consistency.

- Add maintainer comment to keep the two places synchronized. It might
  be possible to get smarter with deduplication, but there are some
  extra fields that prevent the obvious way.
Copy link
Member

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 71038a1

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24718 (rpc: getblock/getrawtransaction/decode*/gettxout fixups by jonatack)
  • #23319 (rpc: Return fee and prevout (utxos) to getrawtransaction by dougEfresh)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@maflcko
Copy link
Member

maflcko commented Mar 31, 2022

Rendered diff:

diff --git a/decoderawtransaction b/decoderawtransaction
index 276cb46..6acb235 100644
--- a/decoderawtransaction
+++ b/decoderawtransaction
@@ -1,58 +1,58 @@
 decoderawtransaction "hexstring" ( iswitness )
 
 Return a JSON object representing the serialized, hex-encoded transaction.
 
 Arguments:
 1. hexstring    (string, required) The transaction hex string
 2. iswitness    (boolean, optional, default=depends on heuristic tests) Whether the transaction hex is a serialized witness transaction.
                 If iswitness is not present, heuristic tests will be used in decoding.
                 If true, only witness deserialization will be tried.
                 If false, only non-witness deserialization will be tried.
                 This boolean should reflect whether the transaction has inputs
                 (e.g. fully valid, or on-chain transactions), if known by the caller.
 
 Result:
 {                             (json object)
   "txid" : "hex",             (string) The transaction id
   "hash" : "hex",             (string) The transaction hash (differs from txid for witness transactions)
-  "size" : n,                 (numeric) The transaction size
+  "size" : n,                 (numeric) The serialized transaction size
   "vsize" : n,                (numeric) The virtual transaction size (differs from size for witness transactions)
-  "weight" : n,               (numeric) The transaction's weight (between vsize*4 - 3 and vsize*4)
+  "weight" : n,               (numeric) The transaction's weight (between vsize*4-3 and vsize*4)
   "version" : n,              (numeric) The version
   "locktime" : xxx,           (numeric) The lock time
   "vin" : [                   (json array)
     {                         (json object)
-      "coinbase" : "hex",     (string, optional)
-      "txid" : "hex",         (string, optional) The transaction id
-      "vout" : n,             (numeric, optional) The output number
-      "scriptSig" : {         (json object, optional) The script
+      "coinbase" : "hex",     (string, optional) The coinbase value (only if coinbase transaction)
+      "txid" : "hex",         (string, optional) The transaction id (if not coinbase transaction)
+      "vout" : n,             (numeric, optional) The output number (if not coinbase transaction)
+      "scriptSig" : {         (json object, optional) The script (if not coinbase transaction)
         "asm" : "str",        (string) asm
         "hex" : "hex"         (string) hex
       },
       "txinwitness" : [       (json array, optional)
         "hex",                (string) hex-encoded witness data (if any)
         ...
       ],
       "sequence" : n          (numeric) The script sequence number
     },
     ...
   ],
   "vout" : [                  (json array)
     {                         (json object)
       "value" : n,            (numeric) The value in BTC
       "n" : n,                (numeric) index
       "scriptPubKey" : {      (json object)
         "asm" : "str",        (string) the asm
         "desc" : "str",       (string) Inferred descriptor for the output
         "hex" : "hex",        (string) the hex
         "type" : "str",       (string) The type, eg 'pubkeyhash'
         "address" : "str"     (string, optional) The Bitcoin address (only if a well-defined address exists)
       }
     },
     ...
   ]
 }
 
 Examples:
 > bitcoin-cli decoderawtransaction "hexstring"
 > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "decoderawtransaction", "params": ["hexstring"]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
diff --git a/getrawtransaction b/getrawtransaction
index fbbe77c..e1a266d 100644
--- a/getrawtransaction
+++ b/getrawtransaction
@@ -1,75 +1,76 @@
 getrawtransaction "txid" ( verbose "blockhash" )
 
 Return the raw transaction data.
 
 By default, this call only returns a transaction if it is in the mempool. If -txindex is enabled
 and no blockhash argument is passed, it will return the transaction if it is in the mempool or any block.
 If a blockhash argument is passed, it will return the transaction if
 the specified block is available and the transaction is in that block.
 
 Hint: Use gettransaction for wallet transactions.
 
 If verbose is 'true', returns an Object with information about 'txid'.
 If verbose is 'false' or omitted, returns a string that is serialized, hex-encoded data for 'txid'.
 
 Arguments:
 1. txid         (string, required) The transaction id
 2. verbose      (boolean, optional, default=false) If false, return a string, otherwise return a json object
 3. blockhash    (string, optional) The block in which to look for the transaction
 
 Result (if verbose is not set or set to false):
 "str"    (string) The serialized, hex-encoded data for 'txid'
 
 Result (if verbose is set to true):
 {                                    (json object)
   "in_active_chain" : true|false,    (boolean, optional) Whether specified block is in the active chain or not (only present with explicit "blockhash" argument)
   "hex" : "hex",                     (string) The serialized, hex-encoded data for 'txid'
   "txid" : "hex",                    (string) The transaction id (same as provided)
   "hash" : "hex",                    (string) The transaction hash (differs from txid for witness transactions)
   "size" : n,                        (numeric) The serialized transaction size
   "vsize" : n,                       (numeric) The virtual transaction size (differs from size for witness transactions)
   "weight" : n,                      (numeric) The transaction's weight (between vsize*4-3 and vsize*4)
   "version" : n,                     (numeric) The version
   "locktime" : xxx,                  (numeric) The lock time
   "vin" : [                          (json array)
     {                                (json object)
-      "txid" : "hex",                (string) The transaction id
-      "vout" : n,                    (numeric) The output number
-      "scriptSig" : {                (json object) The script
+      "coinbase" : "hex",            (string, optional) The coinbase value (only if coinbase transaction)
+      "txid" : "hex",                (string, optional) The transaction id (if not coinbase transaction)
+      "vout" : n,                    (numeric, optional) The output number (if not coinbase transaction)
+      "scriptSig" : {                (json object, optional) The script (if not coinbase transaction)
         "asm" : "str",               (string) asm
         "hex" : "hex"                (string) hex
       },
-      "sequence" : n,                (numeric) The script sequence number
       "txinwitness" : [              (json array, optional)
         "hex",                       (string) hex-encoded witness data (if any)
         ...
-      ]
+      ],
+      "sequence" : n                 (numeric) The script sequence number
     },
     ...
   ],
   "vout" : [                         (json array)
     {                                (json object)
       "value" : n,                   (numeric) The value in BTC
       "n" : n,                       (numeric) index
       "scriptPubKey" : {             (json object)
         "asm" : "str",               (string) the asm
         "desc" : "str",              (string) Inferred descriptor for the output
-        "hex" : "str",               (string) the hex
+        "hex" : "hex",               (string) the hex
         "type" : "str",              (string) The type, eg 'pubkeyhash'
         "address" : "str"            (string, optional) The Bitcoin address (only if a well-defined address exists)
       }
     },
     ...
   ],
   "blockhash" : "hex",               (string, optional) the block hash
   "confirmations" : n,               (numeric, optional) The confirmations
   "blocktime" : xxx,                 (numeric, optional) The block time expressed in UNIX epoch time
   "time" : n                         (numeric, optional) Same as "blocktime"
 }
 
 Examples:
 > bitcoin-cli getrawtransaction "mytxid"
 > bitcoin-cli getrawtransaction "mytxid" true
 > curl --user myusername --data-binary '{"jsonrpc": "1.0", "id": "curltest", "method": "getrawtransaction", "params": ["mytxid", true]}' -H 'content-type: text/plain;' http://127.0.0.1:8332/
 > bitcoin-cli getrawtransaction "mytxid" false "myblockhash"
 > bitcoin-cli getrawtransaction "mytxid" true "myblockhash"

@maflcko maflcko merged commit 1a54c06 into bitcoin:master Mar 31, 2022
@maflcko
Copy link
Member

maflcko commented Mar 31, 2022

Follow up in #24721

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 3, 2022
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request May 21, 2022
When `getrawtransaction` is successfully used on a coinbase transaction,
there is an assertion error. This is very unlikely but happens in the
test in bitcoin#24358.

This does the following:

- Add missing "coinbase" documentation.

- Synchronize documentation between `getrawtransaction` and
  `decoderawtransaction`, the two users of `TxToUniv` that have detailed
  documentation. `decodepsbt` also uses it but fortunately elides this block.

- Change "vout[].amount" to `STR_AMOUNT` for consistency.

- Add maintainer comment to keep the two places synchronized. It might
  be possible to get smarter with deduplication, but there are some
  extra fields that prevent the obvious way.

Github-Pull: bitcoin#24716
Rebased-From: 71038a1
@bitcoin bitcoin locked and limited conversation to collaborators Mar 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants