Add uniffi bindings #59
Conversation
ok300
force-pushed
the
ok300-consolidate-rev-swap-onchain-amt
branch
from
1324935 to
7e5772e
Compare
|
I also added a |
| "PersistError", | ||
| "InvalidPreimage", | ||
| "AlreadyClaimed", | ||
| "BoltzError", |
There was a problem hiding this comment.
It's broader than that, it covers error kinds that can be thrown by the Boltz client (not the Boltz service) , like electrum errors, key parsing errors, HTTP errors, blinding/unblinding errors, etc.
BoltzError looks like a good enough umbrella-term IMO, until we start handling the sub-types differently.
There was a problem hiding this comment.
But it leaks implementation details. When we move to other swappers it won't fit in there.
There was a problem hiding this comment.
Then maybe it can default to a PaymentError::Generic and have the relevant sub-types be mapped to other relevant types.
| dictionary WalletInfo { | ||
| u64 balance_sat; | ||
| string pubkey; | ||
| string active_address; |
There was a problem hiding this comment.
What is the active_address and why do we need it?
There was a problem hiding this comment.
It is the address currently in use by the wallet, since we could also provide the interface for changing the derivation index. Can for now be renamed to address as that is not implemented yet, and I don't see a strong case as to it being necessary in the future.
There was a problem hiding this comment.
No need, but in the future we could allow multiple addresses within the same wallet, handling swaps from each one. Not a priority for now.
There was a problem hiding this comment.
Yes, even if we allow multiple addresses I think the user doesn't need to know about it. We only want to expose the lightning ATM experience hiding the onchain experience as much as we can.
Since this is a first version, let's adapt and aggressive minimalistic approach so we will not regret later on exposing logic we didn't intend to.
There was a problem hiding this comment.
Then maybe it's better to not expose it until then? Makes the interface cleaner.
There was a problem hiding this comment.
@hydra-yse would you agree? Is there any other reason to keep it?
| string active_address; | ||
| }; | ||
|
|
||
| dictionary PreparePaymentResponse { |
There was a problem hiding this comment.
What is the purpose of PreparePaymentResponse and how the user gets it? I don't see any API to retrieve it.
There was a problem hiding this comment.
Maybe you looked at an intermediary commit in the PR branch.
Here are all repo instances: https://github.com/search?q=repo%3Abreez%2Fbreez-sdk-liquid%20PreparePaymentResponse&type=code
PreparePaymentResponse is used as part of prepare_payment:
| }; | ||
|
|
||
| dictionary ReceivePaymentRequest { | ||
| u64? invoice_amount_sat; |
|
|
||
| dictionary ReceivePaymentRequest { | ||
| u64? invoice_amount_sat; | ||
| u64? onchain_amount_sat; |
There was a problem hiding this comment.
I think onchain shouldn't be here. perhaps receiver_amount ?
This PR:
Part of #9
Binding generation can be tested with
cd lib/ls-sdk-bindings cargo run --features=uniffi/cli --bin uniffi-bindgen generate src/ls_sdk.udl --no-format --language kotlin -o ffi/kotlin cargo run --features=uniffi/cli --bin uniffi-bindgen generate src/ls_sdk.udl --no-format --language python -o ffi/python cargo run --features=uniffi/cli --bin uniffi-bindgen generate src/ls_sdk.udl --no-format --language swift -o ffi/swiftwhich should generate bindings in
lib/ls-sdk-bindings/ffi