Skip to content

Conversation

meshcollider
Copy link
Contributor

@meshcollider meshcollider commented Dec 2, 2021

As part of an effort to split rpcwallet as per #23622, this moves rpcdump.cpp into the new wallet/rpc directory as well as moving backup and encryption RPCs out of rpcwallet.

@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 2, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23497 (Add src/node/ and src/wallet/ code to node:: and wallet:: namespaces by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@meshcollider
Copy link
Contributor Author

meshcollider commented Dec 2, 2021

Too many conflicts, going to try just the first three commits.

@maflcko
Copy link
Member

maflcko commented Dec 2, 2021

No conflicts as of last run.

🥳

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK acc8d32 🌐

Show signature

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK acc8d3286b823c4c7117ed0a96955473e9531231 🌐
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUjiZgv9GnY7+5aMLX9txfbszQ6ca/bHKp+JdJdyFIThs/nHExTJtj5pf7Vs4nAl
zyVHJ88tGcryN3znQ9PXgH0SJkJoEKs8/lj5D6fCuUhZxpiWo3Pkwr1+m9/Fzmhp
myyg0hfqHO/hKscHbqBymhnUBjLde+QxvvMubrGRcH7ttX2DXhf0CjkgOu1Cy66m
OkwPa3gzsrRAmZgfccmQgakffQMCKvmbO7gZKLMp/ADq1xd7U1xMu3esU45aivLo
/00J5R66WRU5yvSsu8q3Gbd+2cHSGCgJ1tqTdnfKu5LNFe0HHxzJZAb3+KZe+juv
mWuMdK5K2ibQstTTJzvL7ufx7u4DCiQ17ZiBXnFKRzoHzfri/rbD2tpZ37pm53Et
vimX1QJjuH3Q/EvkVo/mESFuX4WImUgZ824TI6LP5g+9ZzUy5xb1Ykjxu2CWaRmP
NphiK+7GO14/vQ24fuzSA4tS2sO0Nubb4FGgh6bLCIHbQLzRd+/O2M6fj7YchfFT
hDUbWhhl
=GyxK
-----END PGP SIGNATURE-----


Span<const CRPCCommand> GetWalletRPCCommands();

std::tuple<std::shared_ptr<CWallet>, std::vector<bilingual_str>> LoadWalletHelper(WalletContext& context, UniValue load_on_start_param, const std::string wallet_name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would happen if this was moved to wallet/rpc/util?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I was thinking about it but also conscious of @ryanofsky's idea to potentially split further into load.cpp and thought it would be better there. I can move it in a follow-up if preferred.

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK acc8d32. Confirmed this is move-only (other than making RPC methods non-static)

@meshcollider meshcollider changed the title Split rpcwallet into multiple smaller parts MOVEONLY: Move wallet backup and encryption RPCs out of rpcwallet Dec 2, 2021
@meshcollider
Copy link
Contributor Author

meshcollider commented Dec 2, 2021

Rebased and removed the unnecessary #include <util/translation.h>

Also added a new commit moving LoadWalletHelper to util.

Copy link
Member

@maflcko maflcko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 5b2167f 🎭

Show signature

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 5b2167fd30ea4384b93a0226e9fbef4650aa9438 🎭
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUjWSAwAwn/XIKyHQa7CB5HTs5LLGzTptpL/Vw/TxqapygYFQvpyNrH+n08poK27
hx0S9VROa7RQcR6V1g+qG6bpCFTPuXTVAH+hsfmB8wbgUXFrD2HRFGzvZKAMpd4v
+kM0KG9Tk6s46rebOVeCqrm66HT9CAW5oU/+E1QBs6zhJwy/2pYqAkY/yeDyajmW
wSNjxfKHyklREyX+EgZZubTmLdi8FbVr7mb76Fzg12T2kvx9Eo1SUymNcseZngbx
O437ifxvr4g8mTAIeUocm1/o95AGU8PgV7UPXJRCbLbUBufzN6qfMk2/6VZrOb4l
ucTX1XiPDPpu2dMoC1MNMjd2XxHaO7IIko0yX54wOl3BTZQDH915er0eA25sCIm3
c+9J/CNmYHcVwwcXcOsn3t6SOPW5yZQOGAJHa16GBpfTBCun0exFpvFO9aTl9vJi
XsruLAh/BdY7GbQg8yuoeztD36SzAKelU5ygJ/EMWzmo9BEaI94lTaCk42trJTq4
gHsqzbHN
=Y20I
-----END PGP SIGNATURE-----

maflcko pushed a commit to bitcoin-core/gui that referenced this pull request Dec 3, 2021
…ion RPCs out of rpcwallet

5b2167f MOVEONLY: Move LoadWalletHelper to wallet/rpc/util (Samuel Dobson)
8b73640 MOVEONLY: Move wallet encryption RPCs to encrypt.cpp (Samuel Dobson)
803b305 MOVEONLY: Move backupwallet and restorewallet to rpc/backup.cpp (Samuel Dobson)
3a9d393 MOVEONLY: Move rpcdump.cpp to wallet/rpc/backup.cpp (Samuel Dobson)

Pull request description:

  As part of an effort to split rpcwallet as per #23622, this moves `rpcdump.cpp` into the new wallet/rpc directory as well as moving backup and encryption RPCs out of rpcwallet.

ACKs for top commit:
  MarcoFalke:
    ACK 5b2167f 🎭

Tree-SHA512: aa8054767927fa56b5c51edc91a2d94fe9f1cca198e1b2cac1ebd464f6956a89c782a7b6de4409361adca6ca1377272b6e2af660b737c4849ee323f899945ad9
@DrahtBot
Copy link
Contributor

DrahtBot commented Dec 3, 2021

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

@fanquake
Copy link
Member

fanquake commented Dec 3, 2021

This was merged.

@fanquake fanquake closed this Dec 3, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Dec 3, 2021
RandyMcMillan pushed a commit to RandyMcMillan/mempool-tab that referenced this pull request Dec 23, 2021
…ion RPCs out of rpcwallet

ca1a014 MOVEONLY: Move LoadWalletHelper to wallet/rpc/util (Samuel Dobson)
b6856d7 MOVEONLY: Move wallet encryption RPCs to encrypt.cpp (Samuel Dobson)
79bc58a MOVEONLY: Move backupwallet and restorewallet to rpc/backup.cpp (Samuel Dobson)
9ad90aa MOVEONLY: Move rpcdump.cpp to wallet/rpc/backup.cpp (Samuel Dobson)

Pull request description:

  As part of an effort to split rpcwallet as per #23622, this moves `rpcdump.cpp` into the new wallet/rpc directory as well as moving backup and encryption RPCs out of rpcwallet.

ACKs for top commit:
  MarcoFalke:
    ACK ca1a014 🎭

Tree-SHA512: aa8054767927fa56b5c51edc91a2d94fe9f1cca198e1b2cac1ebd464f6956a89c782a7b6de4409361adca6ca1377272b6e2af660b737c4849ee323f899945ad9
@bitcoin bitcoin locked and limited conversation to collaborators Dec 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants