getrawtransaction: inform about blockhash argument when lookup fails #16217

darosior · 2019-06-14T21:13:11Z

Just 4 words added on getrawtransaction lookup error to fix #16142

promag · 2019-06-16T23:11:44Z

src/rpc/rawtransaction.cpp

@@ -196,7 +196,7 @@ static UniValue getrawtransaction(const JSONRPCRequest& request)
            }
            errmsg = "No such transaction found in the provided block";
        } else if (!g_txindex) {
-            errmsg = "No such mempool transaction. Use -txindex to enable blockchain transaction queries";
+            errmsg = "No such mempool transaction. Use -txindex or provide a block hash to enable blockchain transaction queries";


No strong opinion, but generally I think error messages shouldn't mention arguments or configurations, for that we have the help message.

Is that really a guideline? This seems very helpful, especially as the underlying API changes(no more UTXO lookup)

For cli usage this might be useful, but for the developer what really matters IMO is the documentation and error code in the response.

I understand the issue regarding the API change, but I'd rather see a documentation improvement than having "hints" in the errors.

maflcko · 2019-06-17T11:15:59Z

ACK, could you also update the help text, so that this is not only shown on error?

darosior · 2019-06-17T12:27:04Z

@MarcoFalke it's already in the help text :

bitcoin/src/rpc/rawtransaction.cpp

Lines 78 to 80 in c59e3a3

    
           "\nBy default this function only works for mempool transactions. When called with a blockhash\n" 
        
           "argument, getrawtransaction will return the transaction if the specified block is available and\n" 
        
           "the transaction is found in that block. When called without a blockhash argument, getrawtransaction\n"

… lookup fails c59e3a3 getrawtransaction: inform about blockhash argument when lookup fails (darosior) Pull request description: Just 4 words added on `getrawtransaction` lookup error to fix #16142 ACKs for commit c59e3a: Tree-SHA512: 2219099c1240667527a9b1498a58818b5ff1c2ef366c498d2bb57963e828b3c87fa3e6b94be7e6463bd289ceabc13f9c9b1082134641594ba335ac400e6d63aa

Summary: This is a backport of Core [[bitcoin/bitcoin#16217 | PR16217]] Test Plan: ``` $ src/bitcoin-cli getrawtransaction 1df224c31dbfbfcefe3d3de6671e53e73d6cd123628d491db9db3f1558f3110a error code: -5 error message: No such mempool transaction. Use -txindex or provide a block hash to enable blockchain transaction queries. Use gettransaction for wallet transactions. ``` Reviewers: #bitcoin_abc, majcosta Reviewed By: #bitcoin_abc, majcosta Differential Revision: https://reviews.bitcoinabc.org/D8366

…nt when lookup fails c59e3a3 getrawtransaction: inform about blockhash argument when lookup fails (darosior) Pull request description: Just 4 words added on `getrawtransaction` lookup error to fix bitcoin#16142 ACKs for commit c59e3a: Tree-SHA512: 2219099c1240667527a9b1498a58818b5ff1c2ef366c498d2bb57963e828b3c87fa3e6b94be7e6463bd289ceabc13f9c9b1082134641594ba335ac400e6d63aa

getrawtransaction: inform about blockhash argument when lookup fails

c59e3a3

fanquake added the RPC/REST/ZMQ label Jun 14, 2019

promag reviewed Jun 16, 2019

View reviewed changes

maflcko merged commit c59e3a3 into bitcoin:master Jun 17, 2019

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

getrawtransaction: inform about blockhash argument when lookup fails #16217

getrawtransaction: inform about blockhash argument when lookup fails #16217

darosior commented Jun 14, 2019

promag Jun 16, 2019

instagibbs Jun 17, 2019

promag Jun 17, 2019

maflcko commented Jun 17, 2019

darosior commented Jun 17, 2019

getrawtransaction: inform about blockhash argument when lookup fails #16217

getrawtransaction: inform about blockhash argument when lookup fails #16217

Conversation

darosior commented Jun 14, 2019

promag Jun 16, 2019

Choose a reason for hiding this comment

instagibbs Jun 17, 2019

Choose a reason for hiding this comment

promag Jun 17, 2019

Choose a reason for hiding this comment

maflcko commented Jun 17, 2019

darosior commented Jun 17, 2019