Send Payjoin #365
Merged
Send Payjoin #365
Conversation
This file contains 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
|
rebased on upstream/main |
|
Thanks for the rebase. |
|
There is a bug in here where after multiple sends the sender will throw: flutter: BdkError.electrum(field0: Electrum server error: "sendrawtransaction RPC error -26: {"code":-26,"message":"mandatory-script-verify-flag-failed (Witness program hash mismatch)"}") which must be from the broadcast call. My hunch is that the sender is trying to spend a UTXO that's already been spent but the wallet history hasn't updated, so the spend check during broadcast fails. |
DanGould
force-pushed
the
pj
branch
8 times, most recently
from
4b49339 to
373616c
Compare
|
The remaining UI related items can be left out of this PR |
Parse the payjoin endpoint as a URI and add it to SendState.
Sends payjoin in place using the build, send, confirm flow. There are 2 hacks to get this to work that I'm sure must be worked out. 1. BIP21 Uris are mangled by dart's Uri struct since BIP21 and form URI encoding are different specifications. This requires some reencoding of bip21 values before the sender gets initialized. In the future, the bip21 implementation here must be made spec compliant, perhaps by depending on the rust crate so as to use the same types in bdk, payjoin-flutter, etc. 2. TransactionDetails used to display historical data are missing. BDK defines a TransactionDetails type which is now deprecated in BDK 1.0 which includes details like 'address', 'sent' amount, and 'received' amount. This PR makes no attempt to follow this paradigm. Somehow historical data must be populated so that history is accurately populated and the confirm screen and confirmed screen displays correct information.
This also toggles the text on the Confirm screen to give user state context.
This still ignores UI updates from the spawned isolate and doesn't even have a proper splash screen for a confirmed send. It has also yet to be smoke tested
|
ACK! |
|
I'll patch the linting issues after merge so we can move forward. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a basic sender implementation that may be merged so we can build off of it. It does not include any isolate behavior or do accounting properly. I would like help getting the accounting right by taking data from a signed payjoin and pushing it into the Transaction model @ethicnology.
This separates the signing and "confirmed" stages and marshals successful payjoins to the final screen. I don't think a new payjoin handshake screen is merited until the isolates can allow that behavior to happen outside of the send cubit.
Please see the commit log for hacks taken to get this working.
For a payjoin send:
I ask reviewers to confirm errors are handled appropriately, since this flutter paradigm is new to me.