[RFC] Splitting up rpcwallet

meshcollider commented at 1:05 am on November 29, 2021: contributor

This relates to PR #23602.

rpcwallet is the file that takes longest to compile, especially with sanitizers enabled it can take several 10s of seconds.

Allow faster incremental and parallel builds by starting to split it up. First, split off signmessage, which is unrelated to other stuff such as wallet file handling, wallet encryption, tx creation, or wallet status/info.

That PR moves signmessage and the send* RPCs to separate files in wallet/rpc/. I suggest we go even further with this split and do something like the following:

~Move wallet/rpcdump.cpp => wallet/rpc/backup.cpp and combine with backupwallet and restorewallet RPCs.~ (Done in #23647).
~Introduce wallet/rpc/util.cpp and move the helper functions there, like GetWalletForJSONRPCRequest and EnsureWalletIsUnlocked, etc.~ (Done in #23640).
Introduce wallet/rpc/addresses.cpp and move getnewaddress, getrawchangeaddress, setlabel, listaddressgroupings, addmultisigaddress, keypoolrefill, newkeypool, getaddressinfo, getaddressesbylabel, listlabels, and walletdisplayaddress there.
Introduce wallet/rpc/transactions.cpp and move listreceivedbyaddress, listreceivedbylabel, listtransactions, listsinceblock, gettransaction, abandontransaction, and rescanblockchain there.
Introduce wallet/rpc/utxos.cpp and move getreceivedbyaddress, getreceivedbylabel, getbalance, getunconfirmedbalance, lockunspent, listlockunspent, getbalances, and listunspent there.
~Move the walletpassphrase, walletpassphrasechange, walletlock, and encryptwallet RPCs to wallet/rpc/encrypt.cpp.~ (Done in #23647).
Move the spending / signing / transaction-creation RPCs to wallet/rpc/spend.cpp.
This would leave rpcwallet.cpp with only the getwalletinfo, listwalletdir, listwallets, loadwallet, setwalletflag, createwallet, unloadwallet, sethdseed , and upgradewallet RPCs, which are the “core” wallet management RPCs, so it makes sense. It could be moved to the wallet/rpc/ directory though under a different name.

Obviously this would be a lot of movement, so it would be good to get thoughts on it first.

I’ll open PRs in stages for this if there is agreement to proceed.

meshcollider added the label Brainstorming on Nov 29, 2021

meshcollider renamed this:
~~Splitting up rpcwallet~~
[RFC] Splitting up rpcwallet
on Nov 29, 2021

MarcoFalke commented at 8:13 am on November 29, 2021: member

ACK. Was planning to split this mostly in the same way.

ryanofsky commented at 6:40 pm on November 29, 2021: member

This seems like a nice change. Few thoughts I had were

Maybe rpc/coins instead of rpc/utxos Or maybe not. I don’t know which direction usage is shifting, but “coins” always seemed more friendly and comprehensible to me.
Maybe rename rpcwallet to rpc/wallets after all the moves, or split further into rpc/general rpc/load. In any case it would be nice to have all wallet RPC methods in the rpc/ directory.
Personally I think this would be nicest to do in one MOVEONLY PR that did not change any existing code. That way any conflicting PR will have to be rebased at most one time, instead of possibly multiple times.

meshcollider commented at 1:43 am on November 30, 2021: contributor

@MarcoFalke would you prefer to continue splitting it yourself? Or would you like me to implement these changes on top of #23602?

MarcoFalke commented at 12:40 pm on November 30, 2021: member

Shouldn’t matter too much, but I think it is fine to do in multiple pull requests (first the ones without conflicts and then the ones with conflicts). For example #23602 didn’t have any conflicts and I expect util.cpp and encrypt.cpp to have no conflicts either. So I’d attempt to move those next, but I won’t be working on this myself for the next week, so feel free to take over in the meantime.

meshcollider commented at 4:14 am on December 1, 2021: contributor

Alright I have moved everything as suggested in a branch on my repo, and will slowly open the moves up in stages as PRs.

meshcollider commented at 3:15 am on December 2, 2021: contributor

@MarcoFalke is there a way to ask @DrahtBot which PRs would conflict without just opening a PR and waiting?

MarcoFalke referenced this in commit af1067c4b7 on Dec 2, 2021

MarcoFalke commented at 7:43 am on December 2, 2021: member

Not trivially, though the conflict detection also runs on draft pull requests and I can bump it up the scheduler queue, if needed.

meshcollider commented at 8:11 am on December 2, 2021: contributor

Alright, I’ve opened a draft of the whole split just to see how many conflicts it has: #23647

MarcoFalke referenced this in commit 8b4d53e4d6 on Dec 3, 2021

fanquake referenced this in commit fa3fb46b81 on Dec 8, 2021

meshcollider commented at 3:49 am on December 8, 2021: contributor

All done in #23667

meshcollider closed this on Dec 8, 2021

DrahtBot locked this on Dec 8, 2022

[RFC] Splitting up rpcwallet #23622