Push down RPC reqWallet flag #5992

pull jonasschnelli wants to merge 3 commits into bitcoin:master from jonasschnelli:2015/04/remove_reqWallet changing 6 files +328 −208

jonasschnelli commented at 8:00 AM on April 10, 2015: contributor

No description provided.
sipa commented at 9:30 AM on April 11, 2015: member

Concept ACK.
jgarzik commented at 3:33 PM on April 12, 2015: contributor

ut ACK

Would prefer code movement in a first commit, separated out from the rest.
[move] move listunspent to wallet/rpcwallet.cpp 0b9dc9c8f5
jonasschnelli force-pushed on Apr 12, 2015
jonasschnelli commented at 3:57 PM on April 12, 2015: contributor

separated out the code moving
jgarzik commented at 4:06 PM on April 12, 2015: contributor

Looks good. This probably wants a release note, as it's a slight RPC API-visible change.
sipa commented at 4:22 PM on April 12, 2015: member

How is it visible?
jgarzik commented at 4:47 PM on April 12, 2015: contributor

An exception is no longer thrown in all cases. Value::null return value comes through.
Push down RPC reqWallet flag b9fb692d04
jonasschnelli force-pushed on Apr 12, 2015
jonasschnelli commented at 5:38 PM on April 12, 2015: contributor

This should not be the case. Value::null should only come through for the help string generation (in -disablewallet mode the cmd should not be listed under help). I tried to avoid any, even slightly, API changes.
[squashme] fix listunspent code indentation ea9e82df73
in src/wallet/rpcwallet.cpp:None in b9fb692d04 outdated
```
2302 | +    assert(pwalletMain != NULL);
2303 | +    LOCK2(cs_main, pwalletMain->cs_wallet);
2304 | +    pwalletMain->AvailableCoins(vecOutputs, false);
2305 | +    BOOST_FOREACH(const COutput& out, vecOutputs) {
2306 | +        if (out.nDepth < nMinDepth || out.nDepth > nMaxDepth)
2307 | +        continue;
```
laanwj commented at 12:54 PM on April 13, 2015:

Something weird happened to the indentation of this function, for example these continues are not indented.

jonasschnelli commented at 12:55 PM on April 13, 2015:

IDE issue! Will fix this.

jonasschnelli commented at 1:04 PM on April 13, 2015:

Fixed.
in src/rpcserver.cpp:None in b9fb692d04 outdated
```
 192 | @@ -193,11 +193,6 @@ string CRPCTable::help(string strCommand) const
 193 |              continue;
 194 |          if ((strCommand != "" || pcmd->category == "hidden") && strMethod != strCommand)
 195 |              continue;
 196 | -#ifdef ENABLE_WALLET
```
laanwj commented at 1:05 PM on April 13, 2015:

This means that the wallet commands will be listed in help, even if the wallet is disabled. Is that on purpose?

jonasschnelli commented at 1:09 PM on April 13, 2015:

No. the !pwalletMain check (wallet disabled at runtime yes/no) will be done within the new function EnsureWalletIsAvailable(bool) : https://github.com/bitcoin/bitcoin/pull/5992/files#diff-df7d84ff2f53fcb2a0dc15a3a51e55ceR40

laanwj commented at 1:20 PM on April 13, 2015:

Ok, fair enough. Forget my other remark then.
laanwj commented at 1:07 PM on April 13, 2015: member

Value::null should only come through for the help string generation (in -disablewallet mode the cmd should not be listed under help). I

~~The help flag parameter is set to true when a full help message is requested, in case of help <command>. In this case throwing an exception is fine.~~
laanwj commented at 9:05 AM on April 15, 2015: member

Tested ACK, also verified that listunspent is move-only.
laanwj merged this on Apr 15, 2015
laanwj closed this on Apr 15, 2015
laanwj referenced this in commit 4c4f1b4721 on Apr 15, 2015
random-zebra referenced this in commit 7a849ca06a on Jul 1, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors