wallet rpc: return final tx hex from walletprocesspsbt if complete #28414

1. 
   pinheadmz commented at 1:03 pm on September 5, 2023: member

   See #28363 (review)

   walletprocesspsbt currently returns a base64-encoded PSBT and a boolean indicating if the tx is “complete”. If it is complete, the base64 PSBT can be finalized with finalizepsbt which returns the hex-encoded transaction suitable for sendrawtransaction.

   With this patch, walletprocesspsbt return object will ALSO include the broadcast-able hex string if the tx is already final. This saves users the extra step of calling finalizepsbt assuming they have already inspected and approve the transaction from earlier steps.

2. 
   DrahtBot commented at 1:04 pm on September 5, 2023: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	ismaelsadeeq, BrandonOdiwuor, ishaanam, Randy808, achow101
   Concept ACK	furszy

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #21283 (Implement BIP 370 PSBTv2 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.

3. wallet rpc: return final tx hex from walletprocesspsbt if complete e3d484b603
4. pinheadmz force-pushed on Sep 5, 2023
5. DrahtBot added the label CI failed on Sep 5, 2023
6. 
   furszy commented at 1:26 pm on September 5, 2023: member

   Concept ACK

   Should mention the API changes in the release notes too.

7. DrahtBot removed the label CI failed on Sep 5, 2023
8. 
   ismaelsadeeq commented at 4:50 pm on September 5, 2023: member

   Tested ACK e3d484b603abff69c6ebfca5cfb78cf82743d090

   I’ve create a psbt on regtest and process it with walletprocesspbst the output now has the hex encoded network tx, which I sendrawtransaction to the node and was accepted.

   I think this is a nice automation, you dont have to call finalizepsbt if the processed psbt is complete.

9. in test/functional/rpc_psbt.py:237 in e3d484b603 outdated

   238+        finalized_psbt_hex = processed_finalized_psbt['hex']
   239+        assert signed_psbt != finalized_psbt
   240+        assert finalized_psbt_hex == finalized_hex
   241 
   242         # Manually selected inputs can be locked:
   243         assert_equal(len(self.nodes[0].listlockunspent()), 0)
   

   ---

   ---

   

   ismaelsadeeq commented at 4:51 pm on September 5, 2023:

   With this PR finalizepsbt rpc calls to get hex encoded tx after a complete walletprocesspsbt in the functional tests are not necessary

   I’ve done that in this commit https://github.com/bitcoin/bitcoin/commit/5b93ed895e5086b35f70c569e9c8f087a21e6d15 feel free to pick it in this PR.

   Or I will follow up after this

   ---

   

   pinheadmz commented at 3:52 pm on September 6, 2023:

   fantastic thanks! I’ll cherry-pick
10. luke-jr referenced this in commit bbd7021a0e on Sep 6, 2023
11. 
    BrandonOdiwuor commented at 10:03 am on September 6, 2023: contributor

    Tested ACK #e3d484b

    The PR will help reduce the steps in creating PSBT transactions by finalizing transactions with walletprocesspsbt if all signatures are already present

    would help improve #28363 - (check discussion)

    0$ ./src/bitcoin-cli -signet walletcreatefundedpsbt '[]' '[{"tb1qteq3qqhsxhzle98t6k3ecpluaapg84an65rw7l": 0.0008}]'
    1  
    2{                                                                                                                                                                                                                  
    3  "psbt": "cHNidP8BAHECAAAAAe+Y/3akTBUt92Mw40C6GvfDGmUpJsClkO2U9IIh4T+CAQAAAAD9////Aj4EDgAAAAAAFgAUgM1wLZlob/pSB96COtrlAxtemAWAOAEAAAAAABYAFF5BEALwNcX8lOvVo5wH/O9Cg9ezAAAAAAABAHECAAAAAduDPeZZtQlruL4IoNEoQbkaS5qT
    47qHcrwVSRqIT1RyAAAAAAAD+////AvMVewEAAAAAFgAUk0uFiw1qYDauGlJX8sCWb9/ZW7RAQg8AAAAAABYAFHNQXZSO8IfJS9hVety+234E0SaEy24CAAEBH0BCDwAAAAAAFgAUc1BdlI7wh8lL2FV63L7bfgTRJoQiBgMoObTjYDiO/v1NA6150z8m1FmklkxZzDvwd4TVUztXbBi
    5yXiGAVAAAgAEAAIAAAACAAAAAAAAAAAAAIgIDo2i8r5PEOyf1Ev0+AFJBQHyCiIcwBO1k7rHDyuaoE/8Ysl4hgFQAAIABAACAAAAAgAEAAAAAAAAAAAA=",                                                                                            
    6  "fee": 0.00001410,                                                                                                                                                                                               
    7  "changepos": 0                                                                                
    8} 
    

     0$ ./src/bitcoin-cli -signet walletprocesspsbt cHNidP8BAHECAAAAAe+Y/3akTBUt92Mw40C6GvfDGmUpJsClkO2U9IIh4T+CAQAAAAD9////Aj4E
     1DgAAAAAAFgAUgM1wLZlob/pSB96COtrlAxtemAWAOAEAAAAAABYAFF5BEALwNcX8lOvVo5wH/O9Cg9ezAAAAAAABAHECAAAAAduDPeZZtQlruL4IoNEoQbkaS5qT7qHcrwVSRqIT1RyAAAAAAAD+////AvMVewEAAAAAFgAUk0uFiw1qYDauGlJX8sCWb9/ZW7RAQg8AAAAAABYAFHN
     2QXZSO8IfJS9hVety+234E0SaEy24CAAEBH0BCDwAAAAAAFgAUc1BdlI7wh8lL2FV63L7bfgTRJoQiBgMoObTjYDiO/v1NA6150z8m1FmklkxZzDvwd4TVUztXbBiyXiGAVAAAgAEAAIAAAACAAAAAAAAAAAAAIgIDo2i8r5PEOyf1Ev0+AFJBQHyCiIcwBO1k7rHDyuaoE/8Ysl4hgF
     3QAAIABAACAAAAAgAEAAAAAAAAAAAA=
     4
     5{
     6  "psbt": "cHNidP8BAHECAAAAAe+Y/3akTBUt92Mw40C6GvfDGmUpJsClkO2U9IIh4T+CAQAAAAD9////Aj4EDgAAAAAAFgAUgM1wLZlob/pSB96COtrlAxtemAWAOAEAAAAAABYAFF5BEALwNcX8lOvVo5wH/O9Cg9ezAAAAAAABAHECAAAAAduDPeZZtQlruL4IoNEoQbkaS5qT
     77qHcrwVSRqIT1RyAAAAAAAD+////AvMVewEAAAAAFgAUk0uFiw1qYDauGlJX8sCWb9/ZW7RAQg8AAAAAABYAFHNQXZSO8IfJS9hVety+234E0SaEy24CAAEBH0BCDwAAAAAAFgAUc1BdlI7wh8lL2FV63L7bfgTRJoQBCGsCRzBEAiAUUXjmuYYiPzB9gLi8DWam9RL6iJ+wG5Mv0+O
     8UeWJPgwIgANOdu3NpxlPzOQ/oC0nkV6IRQ2rCps8x63X+u/Ta0+gBIQMoObTjYDiO/v1NA6150z8m1FmklkxZzDvwd4TVUztXbAAiAgOjaLyvk8Q7J/US/T4AUkFAfIKIhzAE7WTuscPK5qgT/xiyXiGAVAAAgAEAAIAAAACAAQAAAAAAAAAAAA==",
     9  "complete": true,
    10  "hex": "02000000000101ef98ff76a44c152df76330e340ba1af7c31a652926c0a590ed94f48221e13f820100000000fdffffff023e040e000000000016001480cd702d99686ffa5207de823adae5031b5e980580380100000000001600145e411002f035c5fc94e
    11bd5a39c07fcef4283d7b3024730440220145178e6b986223f307d80b8bc0d66a6f512fa889fb01b932fd3e39479624f83022000d39dbb7369c653f3390fe80b49e457a211436ac2a6cf31eb75febbf4dad3e80121032839b4e360388efefd4d03ad79d33f26d459a496
    124c59cc3bf07784d5533b576c00000000"
    13}
    

    0$ ./src/bitcoin-cli -signet sendrawtransaction 02000000000101ef98ff76a44c152df76330e340ba1af7c31a652926c0a590ed94f48221e13
    1f820100000000fdffffff023e040e000000000016001480cd702d99686ffa5207de823adae5031b5e980580380100000000001600145e411002f035c5fc94ebd5a39c07fcef4283d7b3024730440220145178e6b986223f307d80b8bc0d66a6f512fa889fb01b932fd3
    2e39479624f83022000d39dbb7369c653f3390fe80b49e457a211436ac2a6cf31eb75febbf4dad3e80121032839b4e360388efefd4d03ad79d33f26d459a4964c59cc3bf07784d5533b576c00000000
    3
    4176af5a2291ce02add13ec854c817389638d3883427e47ea0e17ca28be177122
    

12. test: remove unnecessary finalizepsbt rpc calls 4614332fc4
13. doc: add release note for PR #28414 2e249b9227
14. 
    pinheadmz commented at 4:13 pm on September 6, 2023: member

    Thanks for the review everyone. I added a release-notes doc and cherrypicked the test cleanup commit from @ismaelsadeeq
15. 
    ismaelsadeeq commented at 8:37 pm on September 6, 2023: member

    re ACK 2e249b922762f19d6ae61edaad062f31bc2849f3
16. DrahtBot requested review from BrandonOdiwuor on Sep 6, 2023
17. 
    BrandonOdiwuor commented at 4:46 am on September 7, 2023: contributor

    re ACK 2e249b9
18. DrahtBot removed review request from BrandonOdiwuor on Sep 7, 2023
19. 
    ishaanam commented at 7:05 pm on September 9, 2023: contributor

    ACK 2e249b922762f19d6ae61edaad062f31bc2849f3

    It would be useful if this was also implemented for descriptorprocesspsbt.

20. maflcko requested review from achow101 on Sep 10, 2023
21. 
    Randy808 commented at 11:21 am on September 12, 2023: contributor

    Tested ACK 2e249b922762f19d6ae61edaad062f31bc2849f3
22. 
    achow101 commented at 4:28 pm on September 12, 2023: member

    ACK 2e249b922762f19d6ae61edaad062f31bc2849f3
23. DrahtBot removed review request from achow101 on Sep 12, 2023
24. achow101 merged this on Sep 12, 2023
25. achow101 closed this on Sep 12, 2023

    ---

26. 
    ismaelsadeeq commented at 4:26 pm on September 15, 2023: member

    ACK 2e249b9

    It would be useful if this was also implemented for descriptorprocesspsbt.

    I think it would be useful for descriptorprocesspsbt also, opened PR for that @ishaanam @pinheadmz @BrandonOdiwuor 28492

27. Frank-GER referenced this in commit 5bd4f23435 on Sep 19, 2023
28. russeree referenced this in commit e1b8ea194a on Sep 20, 2023
29. achow101 referenced this in commit 719cb301e6 on Sep 23, 2023
30. Frank-GER referenced this in commit c264a74f0f on Sep 25, 2023
31. sidhujag referenced this in commit 9ad3a0a5ad on Sep 26, 2023
32. Retropex referenced this in commit b75a024755 on Oct 4, 2023
33. Retropex referenced this in commit 5f62ed90f6 on Oct 4, 2023
34. willcl-ark referenced this in commit 8ec3bb3add on Jun 28, 2024
35. willcl-ark referenced this in commit 26cadb8c0e on Jul 1, 2024
36. bitcoin locked this on Sep 14, 2024

Contributors
pinheadmz DrahtBot furszy ismaelsadeeq BrandonOdiwuor ishaanam Randy808 achow101