RPC: error code changes and prevent crash with walletpassphrase #3137

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:RPC changing 3 files +10 −5

Diapolo commented at 2:26 PM on October 23, 2013: none
- fix crash with walletpassphrase by checking if RPC server is running and give a friendly error message how to fix this (fixes #3100)
- add 3 new RPCErrorCodes RPC_SERVER_NOT_STARTED, RPC_NODE_ALREADY_ADDED and RCP_NODE_NOT_ADDED (I checked the source to not use a number already in use for RPC_SERVER_NOT_STARTED)
- use the new codes where needed / missing
- add missing use of RPC_INVALID_PARAMETER
BitcoinPullTester commented at 3:04 PM on October 23, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/5730193ba2a00c5b62321e91e45a339301ed3103 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
gavinandresen commented at 1:15 AM on October 24, 2013: contributor

Haven't tested, but looks good to me.

in src/bitcoinrpc.h:None in 5730193ba2 outdated

  64 | @@ -64,6 +65,10 @@ enum RPCErrorCode
  65 |      RPC_WALLET_WRONG_ENC_STATE      = -15, // Command given in wrong wallet encryption state (encrypting an encrypted wallet etc.)
  66 |      RPC_WALLET_ENCRYPTION_FAILED    = -16, // Failed to encrypt the wallet
  67 |      RPC_WALLET_ALREADY_UNLOCKED     = -17, // Wallet is already unlocked
  68 | +
  69 | +    // Node errors

laanwj commented at 6:14 AM on October 24, 2013:

These should be under P2P client errors and prefixed with RPC_CLIENT (as they have to do with P2P client control, not wallet control/status)

Diapolo commented at 6:51 AM on October 24, 2013:

Will change this, thanks.

in src/bitcoinrpc.h:None in 5730193ba2 outdated
```
  29 | @@ -30,7 +30,7 @@ enum HTTPStatusCode
  30 |      HTTP_INTERNAL_SERVER_ERROR = 500,
  31 |  };
  32 |  
  33 | -// Bitcoin RPC error codes
  34 | +// Bitcoin RPC error codes (next free/unused code is -19)
```
laanwj commented at 6:25 AM on October 24, 2013:

Don't mention this: it is better to skip unused codes and simply take MIN - 1 when allocating new error codes. The currently unused codes may have been used for some purpose in the past and that can cause confusion.

in src/rpcnet.cpp:None in 5730193ba2 outdated

 162 | @@ -163,7 +163,7 @@ Value getaddednodeinfo(const Array& params, bool fHelp)
 163 |                  break;
 164 |              }
 165 |          if (laddedNodes.size() == 0)
 166 | -            throw JSONRPCError(-24, "Error: Node has not been added.");
 167 | +            throw JSONRPCError(RCP_NODE_NOT_ADDED, "Error: Node has not been added.");

laanwj commented at 6:27 AM on October 24, 2013:

RCP? :smile_cat:

Diapolo commented at 6:53 AM on October 24, 2013:

OMG, LOL... I needed 2 minutes to get it ^^.

laanwj commented at 6:28 AM on October 24, 2013: member

ACK apart from minor nits mentioned

RPC: error code changes and prevent crash with walletpassphrase

- fix crash with walletpassphrase by checking if RPC server is running and
  give a friendly error message how to fix this (fixes #3100)
- add 3 new RPCErrorCodes RPC_SERVER_NOT_STARTED, RPC_NODE_ALREADY_ADDED
  and RCP_NODE_NOT_ADDED (I checked the source to not use a number already
  in use for RPC_SERVER_NOT_STARTED)
- use the new codes where needed / missing
- add missing use of RPC_INVALID_PARAMETER

4315ec1a72

Diapolo commented at 7:03 AM on October 24, 2013: none

Updated: Fixed @laanwj's nits.
laanwj referenced this in commit 9d580b82c5 on Oct 24, 2013
laanwj merged this on Oct 24, 2013
laanwj closed this on Oct 24, 2013
Diapolo deleted the branch on Oct 24, 2013
Bushstar referenced this in commit dcdf1f3a68 on Apr 8, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#3100 [qt] using walletpassphrase command in debug console crashes client