wallet: Precompute Txdata after setting PSBT inputs’ UTXOs #25590

1. 
   achow101 commented at 6:53 pm on July 11, 2022: member

   If we are given a PSBT that is missing one or more input UTXOs, our PrecomputedTransactionData will be incorrect and missing information that it should otherwise have, and therefore we may not produce a signature when we should. To avoid this problem, we can do the precomputation after we have set the UTXOs the wallet is able to set for the PSBT.

   Also adds a test for this behavior.

2. in test/functional/rpc_psbt.py:773 in 12f5f8e0b7 outdated

   764@@ -764,5 +765,15 @@ def test_psbt_input_keys(psbt_input, keys):
   765             psbt = self.nodes[0].walletprocesspsbt(psbt)["psbt"]
   766             self.nodes[0].sendrawtransaction(self.nodes[0].finalizepsbt(psbt)["hex"])
   767 
   768+            self.log.info("Test that walletprocesspsbt both updates and signs a non-updated psbt containing Taproot inputs")
   769+            addr = self.nodes[0].getnewaddress("", "bech32m")
   770+            txid = self.nodes[0].sendtoaddress(addr, 1)
   771+            vout = find_vout_for_address(self.nodes[0], txid, addr)
   772+            psbt = self.nodes[0].createpsbt([{"txid": txid, "vout": vout}], [{self.nodes[0].getnewaddress(): 0.9999}])
   773+            signed = self.nodes[0].walletprocesspsbt(psbt)
   

   ---

   ---

   

   instagibbs commented at 6:55 pm on July 11, 2022:

   for reviewers: without the fix, this would have to be called twice in a row
3. in test/functional/wallet_taproot.py:301 in 12f5f8e0b7 outdated

   297@@ -298,6 +298,7 @@ def do_test_psbt(self, comment, pattern, privmap, treefn, keys_pay, keys_change)
   298             self.generatetoaddress(self.nodes[0], 1, self.boring.getnewaddress(), sync_fun=self.no_op)
   299             test_balance = int(self.psbt_online.getbalance() * 100000000)
   300             ret_amnt = random.randrange(100000, test_balance)
   301+
   

   ---

   ---

   

   instagibbs commented at 6:57 pm on July 11, 2022:

   errant newline

   ---

   

   achow101 commented at 10:08 pm on July 11, 2022:

   oops, removed.
4. instagibbs approved
5. 
   instagibbs commented at 6:57 pm on July 11, 2022: member

   ACK 12f5f8e0b726373109fae9356be2fcb5ea3c608e

   fixes the issue I was having

6. DrahtBot added the label Wallet on Jul 11, 2022
7. wallet: Precompute Txdata after setting PSBT inputs' UTXOs

   If we are given a PSBT that is missing one or more input UTXOs, our
   PrecomputedTransactionData will be incorrect and missing information
   that it should otherwise have, and therefore we may not produce a
   signature when we should. To avoid this problem, we can do the
   precomputation after we have set the UTXOs the wallet is able to set for
   the PSBT.
   
   Also adds a test for this behavior.

   d2ed97656b
8. achow101 force-pushed on Jul 11, 2022
9. 
   instagibbs commented at 12:47 pm on July 12, 2022: member

   reACK https://github.com/bitcoin/bitcoin/pull/25590/commits/d2ed97656bba050051cfc677f1fa7eb3fc633f7d
10. 
    Sjors commented at 4:03 pm on July 14, 2022: member

    ACK d2ed97656bba050051cfc677f1fa7eb3fc633f7d
11. 
    DrahtBot commented at 3:58 pm on July 16, 2022: contributor

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #25625 (test: add test for decoding PSBT with per-input preimage types by theStack)

    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.

12. 
    aureleoules commented at 8:33 am on July 19, 2022: member

    The test does not fail without the fix, is this normal?
13. 
    Sjors commented at 8:39 am on July 19, 2022: member

    @aureleoules I’m fairly sure I tested that the test did fail without the fix (don’t forget to recompile after removing the fix). Can you try again?
14. 
    aureleoules commented at 8:49 am on July 19, 2022: member

    My bad @Sjors, I didn’t run the test with --descriptors.
15. 
    aureleoules commented at 8:53 am on July 19, 2022: member

    ACK d2ed97656bba050051cfc677f1fa7eb3fc633f7d. This change does fix the behavior of the test.
16. MarcoFalke merged this on Jul 19, 2022
17. MarcoFalke closed this on Jul 19, 2022

    ---

18. sidhujag referenced this in commit fad1d2bb0e on Jul 19, 2022
19. bitcoin locked this on Jul 19, 2023

Contributors
achow101 instagibbs Sjors DrahtBot aureleoules

Labels
Wallet