doc: Fix invalid txid in `gettransaction` RPC example

l0rinc commented at 11:51 am on January 6, 2025: contributor

The RPC examples previously used an invalid 63-character txid, causing errors such as:

0build/src/bitcoin-cli gettransaction "1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d" true
1error code: -8
2error message:
3txid must be of length 64 (not 63, for '1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d')

This commit replaces the invalid txid with a valid one from https://github.com/bitcoin/bitcoin/blob/df5c643f92d4a6b1ef4dfe4b9c54f902990bb54b/src/validation.cpp#L2582

You can verify the fix by checking the following:

The original txid does not exist:

https://mempool.space/tx/1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d.

The new txid is valid:

https://mempool.space/tx/d4f7fbbf92f4a3014a230b2dc70b8058d02eb36ac06b4a0736d9d60eaa9e8781.

Or use the commands below to validate locally:

 0# Build the Bitcoin Core binaries
 1cmake -B build && cmake --build build -j$(nproc)
 2
 3# Start bitcoind with an up-to-date datadir and txindex enabled
 4build/src/bitcoind -datadir="~/your-up-to-date-bitcoin-datadir" -daemon -txindex=1 
 5
 6# Wait a few seconds, then test the txids
 7build/src/bitcoin-cli getrawtransaction "1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d" # invalid
 8build/src/bitcoin-cli getrawtransaction "d4f7fbbf92f4a3014a230b2dc70b8058d02eb36ac06b4a0736d9d60eaa9e8781" # valid
 9
10# Stop bitcoind to clean up
11build/src/bitcoin-cli stop

scripted-diff: Replace invalid txid in RPC examples with valid one

The examples previously used a 63-character txid, which caused RPC errors such as:
  - error code: -8, error message: txid must be of length 64

This commit replaces the invalid txid with a valid one, taken from https://github.com/bitcoin/bitcoin/blob/df5c643f92d4a6b1ef4dfe4b9c54f902990bb54b/src/validation.cpp#L2582.

-BEGIN VERIFY SCRIPT-
sed -i \
    's/1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d/d4f7fbbf92f4a3014a230b2dc70b8058d02eb36ac06b4a0736d9d60eaa9e8781/g' \
    $(git ls-files src/)
-END VERIFY SCRIPT-

ba901eecfa

DrahtBot commented at 11:51 am on January 6, 2025: contributor

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31610.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

DrahtBot added the label Refactoring on Jan 6, 2025

l0rinc renamed this:
~~scripted-diff: Replace invalid txid in RPC examples with valid one~~
RPC: Fix invalid txid in RPC help examples
on Jan 6, 2025

l0rinc renamed this:
~~RPC: Fix invalid txid in RPC help examples~~
RPC: Fix invalid txid in `gettransaction` example
on Jan 6, 2025

jonatack commented at 4:50 pm on January 7, 2025: member

A similar discussion came up a few years ago to prefer invalid addresses in RPC examples; see #17819 and the EXAMPLE_ADDRESS constant in src/rpc/util.h#L60-64. Do we have RPC examples with valid txids?

l0rinc commented at 5:30 pm on January 7, 2025: contributor

I understand the rationale for using placeholders instead of hard-coded values—especially for dangerous commands—and perhaps even correctly formatted ones that clearly don’t point to anything real. However, using slightly incorrect examples as guidance for harmless queries seems really counterproductive. Why would we want to deliberately mislead users? What’s the harm in providing a real example so they can see how a valid transaction should behave?

jonatack commented at 6:06 pm on January 7, 2025: member

Valid addresses in RPC helps may be potentially more harmful than txids due to the accidental risk of losing funds, but in general, I believe the idea is to allow the RPC caller to invoke it with an address or transaction that is relevant to them / the use case of the software client, rather than an arbitrary bikesheddable real-life one, and by default to see an example of the RPC result with an invalid address or transaction provided.

l0rinc commented at 6:12 pm on January 7, 2025: contributor

I don’t see how “id should be 64 chars long not 63” is helpful to the users - but I agree that placeholders could be more in-line with other more dangerous commands.

jonatack commented at 10:03 pm on January 7, 2025: member

One idea could be to update the RPCExamples struct with an added string member (or RPCExamples::ToDescriptionString to take a std::optional<string> arg) to enable optionally printing something like the following, instead of just “Examples:”

0Examples (replace the txid with a valid value):
1> bitcoin-cli gettransaction "1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d"

luke-jr commented at 4:11 am on January 8, 2025: member

As an example, it might be nice if it “just worked” when the user tries it. Maybe just pick a random txid out of the current mempool?

l0rinc commented at 4:58 am on January 8, 2025: contributor

@jonatack, we could likely achieve something similar by adding a different hash for every example instead - the users should be able to extrapolate. @luke-jr, I agree that the users should see a working example - I got this “random” txid from the source, see the commit message.

luke-jr commented at 5:18 am on January 8, 2025: member

An old txid will only work with txindex enabled, though

l0rinc commented at 10:03 am on January 9, 2025: contributor

An old txid will only work with txindex enabled, though

This is also true for the new one - please refer to the example in the PR’s description. @luke-jr, @jonatack, do you have any concerns about the current approach?

luke-jr commented at 4:22 pm on January 9, 2025: member

That’s why I was suggesting pulling a txid from the current mempool

l0rinc commented at 7:21 pm on January 19, 2025: contributor

@luke-jr, what’s wrong with the current approach?

luke-jr commented at 7:38 pm on January 19, 2025: member

@luke-jr, what’s wrong with the current approach?

It won’t work without txindex enabled

l0rinc commented at 7:24 pm on January 20, 2025: contributor

@luke-jr How would using an unconfirmed, ephemeral transaction work in practice here? By the time this is merged, it could be gone or still require -txindex=1 - or am I missing something here? Also, a Use -txindex error seems more helpful than the current must be of length 64 (not 63 error (in case they’ve started without -txindex). Do you have a more reliable approach?

l0rinc renamed this:
~~RPC: Fix invalid txid in `gettransaction` example~~
doc: Fix invalid txid in `gettransaction` RPC example
on Jan 20, 2025

luke-jr commented at 8:40 pm on January 20, 2025: member

My suggestion was to pull a txid from the mempool at runtime.

l0rinc commented at 9:07 pm on January 20, 2025: contributor

That’s an interesting idea for the command line, though it seems a bit outside the current scope. Do we currently have any dynamic examples like that? Also, how would this approach address static RPC reference pages, such as https://developer.bitcoin.org/reference/rpc/gettransaction.html?

sipa commented at 9:09 pm on January 20, 2025: member

@luke-jr That sounds like overkill for a simple example.

doc: Fix invalid txid in gettransaction RPC example #31610

Code Coverage & Benchmarks

Reviews

doc: Fix invalid txid in `gettransaction` RPC example #31610