wallet rpc: return final tx hex from walletprocesspsbt if complete #28414
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
b0cfc37 to
e3d484b
Compare
furszy
reviewed
Sep 5, 2023
Member
furszy
left a comment
There was a problem hiding this comment.
Concept ACK
Should mention the API changes in the release notes too.
ismaelsadeeq
reviewed
Sep 5, 2023
Member
ismaelsadeeq
left a comment
There was a problem hiding this comment.
Tested ACK e3d484b
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.
ismaelsadeeq
reviewed
Sep 5, 2023
| assert finalized_psbt_hex == finalized_hex | ||
|
|
||
| # Manually selected inputs can be locked: | ||
| assert_equal(len(self.nodes[0].listlockunspent()), 0) |
Member
There was a problem hiding this comment.
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 5b93ed8
feel free to pick it in this PR.
Or I will follow up after this
Member
Author
There was a problem hiding this comment.
fantastic thanks! I'll cherry-pick
luke-jr
pushed a commit
to luke-jr/bitcoin
that referenced
this pull request
Sep 6, 2023
Github-Pull: bitcoin#28414 Rebased-From: e3d484b
Contributor
BrandonOdiwuor
left a comment
There was a problem hiding this comment.
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)
$ ./src/bitcoin-cli -signet walletcreatefundedpsbt '[]' '[{"tb1qteq3qqhsxhzle98t6k3ecpluaapg84an65rw7l": 0.0008}]'
{
"psbt": "cHNidP8BAHECAAAAAe+Y/3akTBUt92Mw40C6GvfDGmUpJsClkO2U9IIh4T+CAQAAAAD9////Aj4EDgAAAAAAFgAUgM1wLZlob/pSB96COtrlAxtemAWAOAEAAAAAABYAFF5BEALwNcX8lOvVo5wH/O9Cg9ezAAAAAAABAHECAAAAAduDPeZZtQlruL4IoNEoQbkaS5qT
7qHcrwVSRqIT1RyAAAAAAAD+////AvMVewEAAAAAFgAUk0uFiw1qYDauGlJX8sCWb9/ZW7RAQg8AAAAAABYAFHNQXZSO8IfJS9hVety+234E0SaEy24CAAEBH0BCDwAAAAAAFgAUc1BdlI7wh8lL2FV63L7bfgTRJoQiBgMoObTjYDiO/v1NA6150z8m1FmklkxZzDvwd4TVUztXbBi
yXiGAVAAAgAEAAIAAAACAAAAAAAAAAAAAIgIDo2i8r5PEOyf1Ev0+AFJBQHyCiIcwBO1k7rHDyuaoE/8Ysl4hgFQAAIABAACAAAAAgAEAAAAAAAAAAAA=",
"fee": 0.00001410,
"changepos": 0
} $ ./src/bitcoin-cli -signet walletprocesspsbt cHNidP8BAHECAAAAAe+Y/3akTBUt92Mw40C6GvfDGmUpJsClkO2U9IIh4T+CAQAAAAD9////Aj4E
DgAAAAAAFgAUgM1wLZlob/pSB96COtrlAxtemAWAOAEAAAAAABYAFF5BEALwNcX8lOvVo5wH/O9Cg9ezAAAAAAABAHECAAAAAduDPeZZtQlruL4IoNEoQbkaS5qT7qHcrwVSRqIT1RyAAAAAAAD+////AvMVewEAAAAAFgAUk0uFiw1qYDauGlJX8sCWb9/ZW7RAQg8AAAAAABYAFHN
QXZSO8IfJS9hVety+234E0SaEy24CAAEBH0BCDwAAAAAAFgAUc1BdlI7wh8lL2FV63L7bfgTRJoQiBgMoObTjYDiO/v1NA6150z8m1FmklkxZzDvwd4TVUztXbBiyXiGAVAAAgAEAAIAAAACAAAAAAAAAAAAAIgIDo2i8r5PEOyf1Ev0+AFJBQHyCiIcwBO1k7rHDyuaoE/8Ysl4hgF
QAAIABAACAAAAAgAEAAAAAAAAAAAA=
{
"psbt": "cHNidP8BAHECAAAAAe+Y/3akTBUt92Mw40C6GvfDGmUpJsClkO2U9IIh4T+CAQAAAAD9////Aj4EDgAAAAAAFgAUgM1wLZlob/pSB96COtrlAxtemAWAOAEAAAAAABYAFF5BEALwNcX8lOvVo5wH/O9Cg9ezAAAAAAABAHECAAAAAduDPeZZtQlruL4IoNEoQbkaS5qT
7qHcrwVSRqIT1RyAAAAAAAD+////AvMVewEAAAAAFgAUk0uFiw1qYDauGlJX8sCWb9/ZW7RAQg8AAAAAABYAFHNQXZSO8IfJS9hVety+234E0SaEy24CAAEBH0BCDwAAAAAAFgAUc1BdlI7wh8lL2FV63L7bfgTRJoQBCGsCRzBEAiAUUXjmuYYiPzB9gLi8DWam9RL6iJ+wG5Mv0+O
UeWJPgwIgANOdu3NpxlPzOQ/oC0nkV6IRQ2rCps8x63X+u/Ta0+gBIQMoObTjYDiO/v1NA6150z8m1FmklkxZzDvwd4TVUztXbAAiAgOjaLyvk8Q7J/US/T4AUkFAfIKIhzAE7WTuscPK5qgT/xiyXiGAVAAAgAEAAIAAAACAAQAAAAAAAAAAAA==",
"complete": true,
"hex": "02000000000101ef98ff76a44c152df76330e340ba1af7c31a652926c0a590ed94f48221e13f820100000000fdffffff023e040e000000000016001480cd702d99686ffa5207de823adae5031b5e980580380100000000001600145e411002f035c5fc94e
bd5a39c07fcef4283d7b3024730440220145178e6b986223f307d80b8bc0d66a6f512fa889fb01b932fd3e39479624f83022000d39dbb7369c653f3390fe80b49e457a211436ac2a6cf31eb75febbf4dad3e80121032839b4e360388efefd4d03ad79d33f26d459a496
4c59cc3bf07784d5533b576c00000000"
}$ ./src/bitcoin-cli -signet sendrawtransaction 02000000000101ef98ff76a44c152df76330e340ba1af7c31a652926c0a590ed94f48221e13
f820100000000fdffffff023e040e000000000016001480cd702d99686ffa5207de823adae5031b5e980580380100000000001600145e411002f035c5fc94ebd5a39c07fcef4283d7b3024730440220145178e6b986223f307d80b8bc0d66a6f512fa889fb01b932fd3
e39479624f83022000d39dbb7369c653f3390fe80b49e457a211436ac2a6cf31eb75febbf4dad3e80121032839b4e360388efefd4d03ad79d33f26d459a4964c59cc3bf07784d5533b576c00000000
176af5a2291ce02add13ec854c817389638d3883427e47ea0e17ca28be177122
Member
Author
|
Thanks for the review everyone. I added a release-notes doc and cherrypicked the test cleanup commit from @ismaelsadeeq |
Member
|
re ACK 2e249b9 |
Contributor
|
re ACK 2e249b9 |
Contributor
|
Tested ACK 2e249b9 |
Member
|
ACK 2e249b9 |
Member
I think it would be useful for |
Frank-GER
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Sep 19, 2023
…sspsbt if complete 2e249b9 doc: add release note for PR bitcoin#28414 (Matthew Zipkin) 4614332 test: remove unnecessary finalizepsbt rpc calls (ismaelsadeeq) e3d484b wallet rpc: return final tx hex from walletprocesspsbt if complete (Matthew Zipkin) Pull request description: See bitcoin#28363 (comment) `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. ACKs for top commit: ismaelsadeeq: re ACK 2e249b9 BrandonOdiwuor: re ACK 2e249b9 Randy808: Tested ACK 2e249b9 achow101: ACK 2e249b9 ishaanam: ACK 2e249b9 Tree-SHA512: 229c1103265a9b4248f080935a7ad5607c3be3f9a096a9ab6554093b2cd8aa8b4d1fa55b1b97d3925ba208dbc3ccba4e4d37c40e1491db0d27ba3d9fe98f931e
achow101
added a commit
to bitcoin-core/gui
that referenced
this pull request
Sep 23, 2023
… encoded tx if complete a99e9e6 doc: add release note (ismaelsadeeq) 2b4edf8 test: check `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq) c405207 rpc: `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq) Pull request description: Coming from [#28414 comment](bitcoin/bitcoin#28414 (review)) Same thing also for `descriptorprocesspsbt`. Before this PR `descriptorprocesspsbt` returns a boolean `complete` which indicates that the psbt is final, users then have to call `finalizepsbt` to get the hex encoded network transaction. In this PR if the psbt is complete the return object also has the hex encoded network transaction ready for broadcast with `sendrawtransaction`. This save users calling `finalizepsbt` with the descriptor, if it is already complete. ACKs for top commit: achow101: ACK a99e9e6 pinheadmz: ACK a99e9e6 ishaanam: ACK a99e9e6 Tree-SHA512: c3f1b1391d4df05216c463127cd593f8703840430a99febb54890bc66fadabf9d9530860605f347ec54c1694019173247a0e7a9eb879d3cbb420f9e8d9839b75
Frank-GER
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Sep 25, 2023
… tx if complete a99e9e6 doc: add release note (ismaelsadeeq) 2b4edf8 test: check `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq) c405207 rpc: `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq) Pull request description: Coming from [bitcoin#28414 comment](bitcoin#28414 (review)) Same thing also for `descriptorprocesspsbt`. Before this PR `descriptorprocesspsbt` returns a boolean `complete` which indicates that the psbt is final, users then have to call `finalizepsbt` to get the hex encoded network transaction. In this PR if the psbt is complete the return object also has the hex encoded network transaction ready for broadcast with `sendrawtransaction`. This save users calling `finalizepsbt` with the descriptor, if it is already complete. ACKs for top commit: achow101: ACK a99e9e6 pinheadmz: ACK a99e9e6 ishaanam: ACK a99e9e6 Tree-SHA512: c3f1b1391d4df05216c463127cd593f8703840430a99febb54890bc66fadabf9d9530860605f347ec54c1694019173247a0e7a9eb879d3cbb420f9e8d9839b75
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Sep 26, 2023
… tx if complete a99e9e6 doc: add release note (ismaelsadeeq) 2b4edf8 test: check `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq) c405207 rpc: `descriptorprocesspsbt` return hex encoded tx (ismaelsadeeq) Pull request description: Coming from [bitcoin#28414 comment](bitcoin#28414 (review)) Same thing also for `descriptorprocesspsbt`. Before this PR `descriptorprocesspsbt` returns a boolean `complete` which indicates that the psbt is final, users then have to call `finalizepsbt` to get the hex encoded network transaction. In this PR if the psbt is complete the return object also has the hex encoded network transaction ready for broadcast with `sendrawtransaction`. This save users calling `finalizepsbt` with the descriptor, if it is already complete. ACKs for top commit: achow101: ACK a99e9e6 pinheadmz: ACK a99e9e6 ishaanam: ACK a99e9e6 Tree-SHA512: c3f1b1391d4df05216c463127cd593f8703840430a99febb54890bc66fadabf9d9530860605f347ec54c1694019173247a0e7a9eb879d3cbb420f9e8d9839b75
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #28363 (comment)
walletprocesspsbtcurrently returns a base64-encoded PSBT and a boolean indicating if the tx is "complete". If it is complete, the base64 PSBT can be finalized withfinalizepsbtwhich returns the hex-encoded transaction suitable forsendrawtransaction.With this patch,
walletprocesspsbtreturn object will ALSO include the broadcast-able hex string if the tx is already final. This saves users the extra step of callingfinalizepsbtassuming they have already inspected and approve the transaction from earlier steps.