Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented May 3, 2022

RPCs handling output scripts (addresses, scriptPubKeys, and output script descriptors) should not be placed in a file called misc.cpp, so move them out, then rename misc.cpp.

MacroFake added 2 commits May 3, 2022 08:59
Can be reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
-BEGIN VERIFY SCRIPT-
 git mv src/rpc/misc.cpp src/rpc/node.cpp
 sed -i 's@rpc/misc.cpp@rpc/node.cpp@g' $(git grep -l misc.cpp)
 sed -i 's,RegisterMiscRPCCommands,RegisterNodeRPCCommands,g' $(git grep -l RegisterMiscRPCCommands)
-END VERIFY SCRIPT-
@DrahtBot
Copy link
Contributor

DrahtBot commented May 3, 2022

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #24162 (rpc: add require_checksum flag to deriveaddresses by kallewoof)
  • #24161 (rpc/doc: describe using combo(privkey) to get checksum and then list … by kallewoof)
  • #22838 (descriptors: Be able to specify change and receiving in a single descriptor string by achow101)

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.

@pk-b2
Copy link

pk-b2 commented May 3, 2022

ACK fa758f9

Testing: Built locally, verified RPCs are still working

Agree with the renaming to rpc/node.cpp

Copy link

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK fa758f9

@fanquake fanquake merged commit c290249 into bitcoin:master May 4, 2022
@maflcko maflcko deleted the 2205-rpc-out-script-📂 branch May 4, 2022 09:12
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 4, 2022
@bitcoin bitcoin locked and limited conversation to collaborators May 4, 2023
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