Drop JSONRPCRequest constructors after #21366 #21574

1. 
   ryanofsky commented at 5:42 pm on April 2, 2021: member

   This just makes an additional simplification after #21366 replaced util::Ref with std::any. It was originally suggested #21366 (comment) but delayed for a followup. It would have prevented usage bug #21572.
2. DrahtBot added the label RPC/REST/ZMQ on Apr 2, 2021
3. DrahtBot added the label Wallet on Apr 2, 2021
4. 
   theStack commented at 7:42 pm on April 2, 2021: member

   Concept ACK

   Considering the bug fixed in #21572, I think it would also make sense to deduplicate the two loops in WalletClientImpl::registerRpcs(), as their bodies contain exactly the same code. (Doesn’t necessarily have to be in this PR though; if not, I’m happy to do a follow-up).

5. 
   ryanofsky commented at 8:25 pm on April 2, 2021: member

   I think it would also make sense to deduplicate the two loops in WalletClientImpl::registerRpcs(), as their bodies contain exactly the same code. (Doesn’t necessarily have to be in this PR though; if not, I’m happy to do a follow-up).

   Thanks for reviewing and suggesting the improvement. #21467 is removing the second loop though, so best not to consolidate these.

6. 
   DrahtBot commented at 1:26 am on April 3, 2021: member

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #21467 (Move external signer out of wallet module by Sjors)

   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.

7. 
   meshcollider commented at 5:04 am on April 6, 2021: contributor

   Code review ACK 2b9871142607159bb102ac58edd51d62b413a050
8. theStack approved
9. 
   theStack commented at 10:39 pm on April 6, 2021: member

   Code-review ACK 2b9871142607159bb102ac58edd51d62b413a050 Nice simplification!
10. 
    promag commented at 7:43 am on April 7, 2021: member

    Code review ACK 2b9871142607159bb102ac58edd51d62b413a050
11. Drop JSONRPCRequest constructors after #21366

    This just makes an additional simplification after #21366 replaced
    util::Ref with std::any. It was originally suggested
    https://github.com/bitcoin/bitcoin/pull/21366#issuecomment-792044351 but
    delayed for a followup. It would have prevented usage bug
    https://github.com/bitcoin/bitcoin/pull/21572.

    9044522ef7
12. DrahtBot added the label Needs rebase on Apr 7, 2021
13. 
    MarcoFalke commented at 9:07 am on April 7, 2021: member

    I think you forgot to rebase this on 937fd4a66f048780bffc5e714d0c800de987ce93 when opening the pr. Git can’t figure out that this doesn’t conflict, otherwise.
14. ryanofsky force-pushed on Apr 7, 2021
15. 
    ryanofsky commented at 12:58 pm on April 7, 2021: member

    I think you forgot to rebase this on 937fd4a when opening the pr. Git can’t figure out that this doesn’t conflict, otherwise.

    Thanks! Rebased now.

    ---

    Rebased 2b9871142607159bb102ac58edd51d62b413a050 -> 9044522ef76f880760165d98fab024802ccfc062 (pr/simpreq.1 -> pr/simpreq.2, compare) due to conflict with #21572

16. 
    promag commented at 1:35 pm on April 7, 2021: member

    ACK 9044522ef76f880760165d98fab024802ccfc062, fixed conflict in src/wallet/interfaces.cpp.
17. DrahtBot removed the label Needs rebase on Apr 7, 2021
18. MarcoFalke merged this on Apr 8, 2021
19. MarcoFalke closed this on Apr 8, 2021

    ---

20. sidhujag referenced this in commit 3d28753d27 on Apr 8, 2021
21. Fabcien referenced this in commit 5ae548070b on Jun 7, 2022
22. DrahtBot locked this on Aug 18, 2022

Contributors
ryanofsky theStack DrahtBot meshcollider promag MarcoFalke

Labels
Wallet RPC/REST/ZMQ