-
Notifications
You must be signed in to change notification settings - Fork 77
Move the Default Payjoin FFI API to primitives #1191
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move the Default Payjoin FFI API to primitives #1191
Conversation
|
Thanks for sharing your draft in public early before it's ready for review |
Pull Request Test Coverage Report for Build 20280641768Details
💛 - Coveralls |
8a25f4f to
ec56cdc
Compare
|
Ready for review @spacebear21 @DanGould |
|
Assigned @spacebear21 to this one
|
|
Thanks Armin - I agree this will be much easier to review once it's rebased and commits are organized. |
|
will do |
c4baa41 to
da7a4d2
Compare
|
@arminsabouri and @spacebear21 squashed much of the git history to make things much cleaner |
benalleng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks pretty good just the small nit about the uniffi-downgrade and upgrade one after the other.
I was able to test python but couldn't build dart or javascript properly to test
spacebear21
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! I also think the three "adapt harness" commits could be squashed into one since they kind of all do the same thing.
I think the last two commits may also be squashed, which would resolve @benalleng 's comment.
5ae6fef to
229a2ba
Compare
ec32660 to
f83f814
Compare
f6cae3c to
e86ef0c
Compare
This introduces PlainTx* records and Weight for the primitive FFI surface, adds ReceiverBuilderError, AddressParseError, InputPairError, FeeRateError, and provides conversions without altering existing signatures.
e86ef0c to
1c43f78
Compare
This migrates the FFI layer to the new primitive types and serialized PSBTs as we move away from importing bitcoin_ffi.
After migrating to primitive types on the FFI layer we no longer need to import the bitcoin_ffi package. Also cleans up any API changes made during the transition.
1c43f78 to
3187533
Compare
benalleng
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack 3187533 those are some clean commit messages 👀
spacebear21
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 3187533, great work on the rebase and commit organization!
Following up on rust-payjoin#738 to make the FFI boundary primitive-first. Strong types at the edge forced downstreams to depend on
bitcoin_ffiand broke whenever wrappers drifted. We now expose strings/bytes/integers with runtime validation.Changes
ReceiverBuilder::newtakes an address string; errors surface asReceiverBuilderError/AddressParseError. AddedPlainTxIn/TxOut/OutPoint/PsbtInput/WeightplusInputPairErrorfor the rest of the receiver typestates (script callbacks, output substitution, input contribution, PSBT parsing).SenderSessionOutcometo expose success/failure/cancel plus base64 PSBT.ProvisionalProposal::psbt_to_signnow returns base64, so wallets can consume it without wrappers. Fee-rate setters validate rawu64and report overflow.Notes