rpc: sendmany and sendtoaddress return PSBT for wallets without private keys #18201

1. 
   Sjors commented at 4:34 pm on February 24, 2020: member

   Similar to what #16373 did for bumpfee, this PR lets sendmany and sendtoaddress return a PSBT for wallets without private keys:

   0Result:
   1"txid"                  (string) The transaction id.
   2{
   3  "psbt":    "psbt"      (string) The base64-encoded unsigned PSBT of the new transaction. Only returned when wallet private keys are disabled.
   4}
   

   I consolidated code between these two RPC calls, since sendtoaddress is essentially sendmany with 1 destination. Unless I overlooked something, the only behaviour change is that some sendtoaddress error codes changed from -4 to -6. The release note mentions this.

   The PSBT code is prepared for hardware wallet support (e.g. #16546). It checks if the PSBT is complete, which currently can’t happen with watch-only wallets.

   Closes #18180. Depends on #18115.

2. Deduplicate the message verifying code

   The logic of verifying a message was duplicated in 2 places:
   
   src/qt/signverifymessagedialog.cpp
     SignVerifyMessageDialog::on_verifyMessageButton_VM_clicked()
   
   src/rpc/misc.cpp
     verifymessage()
   
   with the only difference being the result handling. Move the logic into
   a dedicated
   
   src/util/message.cpp
     MessageVerify()
   
   which returns a set of result codes, call it from the 2 places and just
   handle the results differently in the callers.

   ea0e98b5ff
3. Deduplicate the message signing code

   The logic of signing a message was duplicated in 3 places:
   
   src/qt/signverifymessagedialog.cpp
     SignVerifyMessageDialog::on_signMessageButton_SM_clicked()
   
   src/rpc/misc.cpp
     signmessagewithprivkey()
   
   src/wallet/rpcwallet.cpp
     signmessage()
   
   Move the logic into
   
   src/util/message.cpp
     MessageSign()
   
   and call it from all the 3 places.

   4b9d620f6c
4. Refactor message hashing into a utility function

   And add unit test for it.
   
   The purpose of using a preamble or "magic" text as part of signing and
   verifying a message was not given when the code was repeated in a few
   locations. Make a test showing how it is used to prevent inadvertently
   signing a transaction.

   cb764403a3
5. Refactor rawtransaction's SignTransaction into generic SignTransaction function 8b865bef58
6. Add SignTransaction function to ScriptPubKeyMan and LegacyScriptPubKeyMan 5ec7ec37fb
7. Implement CWallet::SignTransaction using ScriptPubKeyMan::SignTransaction 1bb7939d3e
8. Use CWallet::SignTransaction in CreateTransaction and signrawtransactionwithwallet

   Instead of duplicating signing code, just use the function we already
   have.

   6be1281f73
9. Move FillPSBT to be a member of CWallet e4314bf986
10. Move key and script filling and signing from CWallet::FillPSBT to ScriptPubKeyMan::FillPSBT

    Instead of fetching a SigningProvider from ScriptPubKeyMan in order
    to fill and sign the keys and scripts for a PSBT, just pass that
    PSBT to a new FillPSBT function that does all that for us.

    c5ae320bc6
11. Move direct calls to MessageSign into new SignMessage functions in CWallet and ScriptPubKeyMan

    Instead of getting a SigningProvider and then going to MessageSign,
    have ScriptPubKeyMan handle the message signing internally.

    d7df7b8243
12. Replace GetSigningProvider with GetSolvingProvider

    Not all ScriptPubKeyMans will be able to provide private keys,
    but pubkeys and scripts should be. So only provide public-only
    SigningProviders, i.e. ones that can help with Solving.

    f2bd919cfd
13. Clear any input_errors for an input after it is signed

    Make sure that there are no errors set for an input after it is signed.
    This is useful for when there are multiple ScriptPubKeyMans. Some may
    fail to sign, but one may be able to sign, and after it does, we don't
    want there to be any more errors there.

    5cab890b91
14. [rpc] refactor: consolidate sendmany and sendtoaddress code

    The only new behavior is some error codes are changed from -4 to -6.

    7061c2c5c9
15. [rpc] sendmany, sendtoaddress: PSBT for wallets without privkeys

    In addition we check to see if the PSBT is complete, in preperation for a ScriptPubKeyMan that can use an external signer.
    
    PSBT for sendtoaddress

    7421500a56
16. Sjors requested review from instagibbs on Feb 24, 2020
17. in test/functional/wallet_watchonly.py:101 in 7421500a56

     97@@ -94,6 +98,14 @@ def run_test(self):
     98         assert_equal("psbt" in result, True)
     99         assert_raises_rpc_error(-4, "Insufficient funds", wo_wallet.walletcreatefundedpsbt, inputs, outputs, 0, no_wo_options)
    100 
    101+        self.log.info('Testing sendmany watch-only')
    

    ---

    ---

    

    instagibbs commented at 5:08 pm on February 24, 2020:

    you should test that wallet balances aren’t changing here(accidentally committing the transaction internally or something) and that it would enter the mempool properly
18. 
    instagibbs commented at 5:09 pm on February 24, 2020: member

    concept ACK, will more closely review once it’s not 3 PRs high :)
19. 
    luke-jr commented at 5:35 pm on February 24, 2020: member

    Concept NACK, I think: don’t we want such cases to call an external signer and behave as normally?
20. 
    Sjors commented at 6:34 pm on February 24, 2020: member

    After some IRC discussion, it seems better to create a new RPC after all, so I resurrected #16378
21. Sjors closed this on Feb 24, 2020

    ---

22. meshcollider referenced this in commit 4db44acf2d on Jul 12, 2020
23. sidhujag referenced this in commit 97ea73b8c3 on Jul 12, 2020
24. meshcollider referenced this in commit ffaac6e614 on Sep 15, 2020
25. sidhujag referenced this in commit 2d9d86f6b8 on Sep 15, 2020
26. DrahtBot locked this on Feb 15, 2022

Contributors
Sjors instagibbs luke-jr