Its a bit strange to return a reference to a `Vec` in Rust, when a
slice is really intended as the way to do so. Worse, the bindings
don't know how to map a reference to a `Vec` (but do have code to
map a slice of `Clone`able objects).
Here, we move `NodeAnnouncementInfo::addresses` to return a slice,
though to do so we have to adapt the `WithoutLength` `Writeable`
impl to support slices as well.
The PaymentFailureReason variants for invoice request failures will
cause downgrades to break. Instead, use a new TLV for the reason and
continue to write the old TLV, only use None for the new reasons.
Instead of re-using PaymentFailureReason::RecipientRejected, define a
new InvoiceRequestRejected variant for when an InvoiceError is received
instead of a Bolt12Invoice. This allows user to differentiate the cause
of the failure.
Now that Event::PaymentFailed has an option payment_hash, it can be used
in replace of Event::InvoiceRequestFailed. This allows for including a
reason when abandoning a payment before an invoice is received.
When abandoning a BOLT12 payment before a Bolt12Invoice is received, an
Event::InvoiceRequestFailed is generated and the abandonment reason is
lost. Make payment_hash optional in Event::PaymentFailed so that
Event::InvoiceRequestFailed can be removed in favor of it.
When handling a BOLT12 invoice, and invoice error is sent if the invoice
contains unknown required features. However, since the payment is still
in state AwaitingInvoice, abandoning it results in losing the reason
since an InvoiceRequestFailed event would be generated. Move the check
to PendingOutboundPayments such that the payment is first moved to state
InvoiceReceived so that a PaymentFailed event is generated instead.
Refunds are typically communicated via QR code, where a smaller size is
desirable. Make the HMAC in OutboundPayment data optional such that it
is elided from blinded paths used in refunds. This prevents abandoning
refunds if the reader sends an invoice_error instead of an invoice
message. However, this use case isn't necessary as the corresponding
outbound payment will either timeout when the refund expires or can be
explicitly abandoned by the creator.
A BOLT12 payment may be abandoned when handling the invoice or when
receiving an InvoiceError message. When abandoning the payment, don't
use UserAbandoned as the reason since that is meant for when the user
calls ChannelManager::abandon_payment.
When making an outbound BOLT12 payment, multiple invoices may be
received for the same payment id. Instead of abandoning the payment when
a duplicate invoice received, simply ignore it without responding with
an InvoiceError. This prevents abandoning in-progress payments and
sending unnecessary onion messages.
Before abandoning a payment when receiving an InvoiceError, verify that
the PaymentId included in the OffersContext with the included HMAC. This
prevents a malicious actor sending an InvoiceError with a known payment
id from abandoning our payment.
When receiving an InvoiceError in response to an InvoiceRequest, the
corresponding payment should be abandoned. Add an HMAC to
OffersContext::OutboundPayment such that the payment ID can be
authenticated prior to abandoning the payment.
An HMAC needs to be included in OffersContext::OutboundPayment to
authenticate the included PaymentId. Implement Readable and Writeable to
allow for this.
When receiving an InvoiceError in response to an InvoiceRequest, the
corresponding payment should be abandoned. Add functions for
constructing and verifying an HMAC over a Payment ID to allow for this.
The past handful of commits were mostly moving code around, so to
aid reviewers violated our `rustfmt` rules. Here we rectify that by
`rustfmt`'ing the newly-added files.
In the next commit we'll `rustfmt` newly-added files, but before
we do so we clean up some code so that the resulting files won't be
quite as absurd. We also exclude the new `invoice_utils.rs` file,
as it needs quite substantial cleanups.
Now that the `lightning` crate depends on the `lightning-invoice`
crate, there's no reason to have the `sign_invoice` method take raw
base32 field elements as we can now give it a real
`RawBolt11Invoice`, which we do here.
This simplifies the interface and avoids a
serialization-deserialization roundtrip when signing invoices in a
validating signer.
FIxes#3227
`lightning-invoice` previously had a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.
Here we finally rectify this issue, swapping the dependency order
and making `lightning` depend on `lightning-invoice` rather than
the other way around.
This moves various utilities which were in `lightning-invoice` but
relied on `lightning` payment types to make payments to where they
belong (the `lightning` crate), but doesn't bother with integrating
them well in their new home.
`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.
This takes one more step, moving the `UntrustedString` and
`PrintableString` types to `lightning-types`.
`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.
This takes one more step, moving the `Features` types from
`lightning` to `lightning-types`.
It turns out all the places we use `Features::is_subset` we could
as well be using `Features::requires_unknown_bits_from`. Further,
in the next commit `Features` will move to a different crate so any
methods which the `lightning` crate uses will need to be public. As
the `is_subset` API is prety confusing (it doesn't consider
optional/required bits, only whether the bits themselves are
strictly a subset) it'd be nice to not have to expose it, which is
enabled here.
`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.
This takes one more step, moving the routing types
`lightning-invoice` uses into `lightning-types`.
`lightning-invoice` currently has a dependency on the entire
`lightning` crate just because it wants to use some of the useful
types from it. This is obviously backwards and leads to some
awkwardness like the BOLT 11 invoice signing API in the `lightning`
crate taking a `[u5]` rather than a `Bolt11Invoice`.
This is the first step towards fixing that - moving the common
types we need into a new `lightning-types` crate which both can
depend on.
Since we're using a new crate and can't depend on the existing
`lightning` hex utility to implement `Display`, we also take this
opportunity to switch to the new `Display` impl macro in
`hex_conservative`.
If we're gonna push users towards using `Balance` to determine
their current balances, we really need to provide more information,
including msat balances.
Here we add rounded-out msat balances to the pre-close balance
information
`Balance::ClaimableOnChannelClose` excludes the commitment
transaction fee, which makes it hard to use for current balance
calculation. Here we add it, setting the value to zero for inbound
channels (i.e. ones for which we don't pay the fee).
When the user is fetching their current balances after forwarding a
payment (before it clears), they'll see a
`MaybePreimageClaimableHTLC` and a `MaybeTimeoutClaimableHTLC` but
if they sum up their balance using
`Balance::claimable_amount_satoshis` neither will be included.
Obviously, exactly one of the two balances should be included - one
of the two resolutions should happen in our favor. This causes our
visible balance to fluctuate up and down by the full value of any
HTLCs we're in the middle of forwarding, which is incredibly
confusing to see. If we want to stop the fluctuations, we need to
pick one of the two balances to include. The obvious candidate is
`MaybeTimeoutClaimableHTLC` as it is the lower of the two, and
represents our balance without the fee we'd receive from the
forward.
Sadly, if we always include it, we'll end up also including any
HTLCs which we've sent but which haven't yet been claimed by their
recipient, which is the wrong behavior.
Luckily, we have access to the `Option<HTLCSource>` while walking
HTLCs, which allows us to add an `outbound_payment` flag to
`MaybeTimeoutClaimableHTLC`. This allows us to only include
forwarded payments in `claimable_amount_satoshis`.
Sadly, even with this in place our balance still fluctuates by the
changes in the commitment transaction fees we have to pay during
forwarding, but addressing that is left for later.
While there's not really much harm in requiring a `Clone`able
reference (they almost always are), it does make our bindings
struggle a bit as they don't support multi-trait bounds (as it
would require synthesizing a new C trait, which the bindings don't
do automatically). Luckily, there's really no reason for it, and we
can just call the `DefaultMessageRouter` directly when we want to
route a message.
We've carried this patch for a while on the bindings branch, but
there's not a strong reason it can't go upstream.
Use the `hex-conservative` crate directly from `bitcoin` instead of from
`hashes`. Although it makes no real difference it is slightly more clear
and more terse.
The `hex` crate is re-exported by `rust-bitcoin` so we can get it from
there instead of explicitly depending on it. Doing so reduces the
maintenance burden and helps reduce the likelyhood of getting two
versions in the dependency graph.
The original default value of 0 was inconsistent with the minimum requirement
of 1000 satoshis in ChannelHandshakeConfig::their_channel_reserve_proportional_millionths.
