Get Payment #476
Get Payment #476
Conversation
dangeross
force-pushed
the
savage-payment-by-destination
branch
from
7677be3
to
27bab47
Compare
lib/core/src/persist/mod.rs
Outdated
| .get_connection()? | ||
| .query_row( | ||
| &self.select_payment_query( | ||
| Some("(rs.invoice = ?1 OR ss.invoice = ?1 OR cs.claim_address = ?1 OR pd.destination = ?1)"), |
There was a problem hiding this comment.
It seems that a payment may have multiple destinations? This means that get_payment_by_destination may return same result for different destinations? for example for invoice and bip21 that was parsed as input?
There was a problem hiding this comment.
@roeierez Currently there's no case where this can happen. Specifically, here's the cases:
- Receive Swap/Chain Swap: No payment details set, no conflict can occur
- Send swap using Boltz: No payment details set, no conflict can occur
- Direct Send/Send "swap" with invoice MRH: We persist data as a Liquid payment, therefore there is no data in the swap tables
- Generic non-SDK payment (as of Set
PaymentDetailsas mandatory, default to LiquidAddress #474): We persist the tx's output's script pubkey in the payment details. We have no data in the swap tables.
So in all cases the swap/details are mutually exclusive, meaning there's no way for two inputs to return the same payment.
Note: We may want to change the MRH behavior in the future (since we were considering to store also the invoice so we can index by it). So if that's planned then I agree we will have a conflict
There was a problem hiding this comment.
I see, so I guess only removing the claim address is needed? @dangeross
There was a problem hiding this comment.
@dangeross do you think it makes sense instead of a string to use an enum so the query will be more intuitive and search will look up for the specific identifier?
PaymentDestination {
Lightning(bolt11),
Liquid(address),
Bitcoin(address)
}
There was a problem hiding this comment.
@roeierez Yeah, it does make it more intuitive and fits in with other functions of the SDK.
dangeross
force-pushed
the
savage-payment-by-destination
branch
2 times, most recently
from
40e92fd
to
e20fc03
Compare
lib/core/src/sdk.rs
Outdated
| pub async fn payment_by_destination( | ||
| &self, | ||
| destination: &PaymentDestination, | ||
| ) -> Result<Option<Payment>, PaymentError> { | ||
| self.ensure_is_started().await?; | ||
|
|
||
| Ok(self.persister.get_payment_by_destination(destination)?) | ||
| } |
There was a problem hiding this comment.
Looking back on the changes, wouldn't it be better to name this get_payment_by_destination like the persister method? I'm afraid this could be misinterpreted as executing a payment by destination.
There was a problem hiding this comment.
Agreed, I've since added payment_hash as a query option so get_payment() makes more sense now
dangeross
force-pushed
the
savage-payment-by-destination
branch
from
fcea4a5
to
74ed1dd
Compare
This PR adds a method to lookup a payment using payment query arguments, which can be one of:
Fixes #473