Update sample for 0.0.103 #40
Conversation
jkczyz
force-pushed
the
2021-11-release-103
branch
from
956a237 to
16779b1
Compare
jkczyz
force-pushed
the
2021-11-release-103
branch
from
16779b1 to
e31773b
Compare
| println!("ERROR: failed to send payment: {:?}", e); | ||
| print!("> "); | ||
| HTLCStatus::Failed |
There was a problem hiding this comment.
Why do we treat no-route differently from all-routes-failed-immediately?
There was a problem hiding this comment.
Not sure... was just trying to maintain the same behavior. ChannelManager never gets called in this case, so maybe there's no need to update the payment storage? (cc: @valentinewallace)
There was a problem hiding this comment.
Hm yeah I don’t think either would make sense to put in PaymentStorage. Don’t recall my exact reasoning tho.
There was a problem hiding this comment.
Yea, I think don't bother to store them? I'm not even sure if we need to store outbound payments anyway at this point?
There was a problem hiding this comment.
I'm not even sure if we need to store outbound payments anyway at this point?
list_payments currently allows you to see whether an outbound payment is pending vs completed/failed, due to storing them here. Could be exposed from ChannelManager instead tho
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah, yeah and right now neither inbound nor outbound payments are persisted in the sample. They're just used in list_payments. I can drop them entirely. Just want to note that it seems independent of the PR. Happy to do it here in a follow-up.
There was a problem hiding this comment.
Fair, lets do it in a followup and land this for now.
There was a problem hiding this comment.
Hmm... so won't ChannelManager eventually remove the payments after they've been fulfilled? The sample would still needs to keep track of these, right? The idea of list_payments is basically a transaction log, IIUC.
There was a problem hiding this comment.
Hmm... so won't
ChannelManagereventually remove the payments after they've been fulfilled? The sample would still needs to keep track of these, right? The idea oflist_paymentsis basically a transaction log, IIUC.
I second this, though payments are currently not persisted to disk in the sample. So IMO inbound/outbound tracking shouldn’t be removed
| let payee = Payee::for_keysend(payee_pubkey); | ||
| let params = RouteParameters { payee, final_value_msat: amt_msat, final_cltv_expiry_delta: 40 }; | ||
|
|
||
| let route = match router::find_route( |
There was a problem hiding this comment.
Hmmmmmm, we really need to get InvoicePayer to support spontaneous payments eventually.
There was a problem hiding this comment.
Yeah, just opened issue lightningdevkit/rust-lightning#1156. Happy to take it on.
Update the sample node to use APIs from the 0.0.103 release.
InvoicePayerfor routing and sending paymentsfind_routefor keysend paymentsScorerinstance and persist it periodically