OnionMessageProvider is a super-trait of OnionMessageHandler, but they
don't need to be used separately. Additionally, the former is misplaced
in the events module. Remove OnionMessageProvider and add it's only
method, next_onion_message_for_peer, into OnionMessageHandler.
We'd previously ignore the existing amount transactions were already
attempting to spend when deciding whether we should add more inputs
throughout coin selection. This would result in us attaching more inputs
than necessary to satisfy our target amount. In the case of HTLC
transactions, we'd burn the HTLC amount completely, since the pre-signed
transaction has zero fee (input amount == output amount).
Along the way, we also fix the slight overpayment in anchor
transactions. We now properly account for the fees the transaction
already paid for, simply by pretending the fees are part of the anchor
input amount.
When an invoice is requested but either receives an error or never
receives a response, surface an event to indicate to the user that the
corresponding future payment has failed.
This will make it possible to
link between SpendableOuts and ChannelMonitor
- change channel_id to option so we dont break upgrade
- remove unused channel_id
- document channel_id
- extract channel id dynamically to pass test
- use contains to check channel_id in test as the events are not ordered
- update docs framing
- specify ldk version channel_id will be introduced in
Co-authored-by: Elias Rohrer <dev@tnull.de>
Update lightning/src/events/mod.rs
Co-authored-by: Elias Rohrer <dev@tnull.de>
Creates a new `events::ClaimedHTLC` struct that contains the relevant
information about a claimed HTLC; e.g., the channel it arrived on, its ID, the
amount of the HTLC, the overall amount of the payment, etc. Adds appropriate
serialization support.
Adds a `Vec<events::ClaimedHTLC>` to the `ClaimingPayment`
structure. Populates this when creating the struct by converting the
`payment.htlcs` (which are `ClaimingHTLC` structs) into `event::ClaimedHTLC`
structs. This is a straightforward transformation.
Adds a `Vec<events::ClaimedHTLC>` to the `events::Event::PaymentClaimed`
enum. This is populated directly from the `ClaimingPayment`'s `htlcs` vec.
Fixes#2477.
In 0ad1f4c943 we fixed a nasty bug
where a failure to persist a `ChannelManager` faster than a
`ChannelMonitor` could result in the loss of a `PaymentSent` event,
eventually resulting in a `PaymentFailed` instead!
As noted in that commit, there's still some risk, though its been
substantially reduced - if we receive an `update_fulfill_htlc`
message for an outbound payment, and persist the initial removal
`ChannelMonitorUpdate`, then respond with our own
`commitment_signed` + `revoke_and_ack`, followed by receiving our
peer's final `revoke_and_ack`, and then persist the
`ChannelMonitorUpdate` generated from that, all prior to completing
a `ChannelManager` persistence, we'll still forget the HTLC and
eventually trigger a `PaymentFailed` rather than the correct
`PaymentSent`.
Here we fully fix the issue by delaying the final
`ChannelMonitorUpdate` persistence until the `PaymentSent` event
has been processed and document the fact that a spurious
`PaymentFailed` event can still be generated for a sent payment.
The original fix in 0ad1f4c943 is
still incredibly useful here, allowing us to avoid blocking the
first `ChannelMonitorUpdate` until the event processing completes,
as this would cause us to add event-processing delay in our general
commitment update latency. Instead, we ultimately race the user
handling the `PaymentSent` event with how long it takes our
`revoke_and_ack` + `commitment_signed` to make it to our
counterparty and receive the response `revoke_and_ack`. This should
give the user plenty of time to handle the event before we need to
make progress.
Sadly, because we change our `ChannelMonitorUpdate` semantics, this
change requires a number of test changes, avoiding checking for a
post-RAA `ChannelMonitorUpdate` until after we process a
`PaymentSent` event. Note that this does not apply to payments we
learned the preimage for on-chain - ensuring `PaymentSent` events
from such resolutions will be addressed in a future PR. Thus, tests
which resolve payments on-chain switch to a direct call to the
`expect_payment_sent` function with the claim-expected flag unset.
Create a new table in 'peer_state' to maintain unaccepted inbound
channels; i.e., a channel for which we've received an 'open_channel'
message but that user code has not yet confirmed for acceptance. When
user code accepts the channel (e.g. via 'accept_inbound_channel'),
create the channel object and as before.
Currently, the 'open_channel' message eagerly creates an
InboundV1Channel object before determining if the channel should be
accepted. Because this happens /before/ the channel has been assigned
a user identity (which happens in the handler for OpenChannelRequest),
the channel is assigned a random user identity. As part of the
creation process, the channel's cryptographic material is initialized,
which then uses this randomly generated value for the user's channel
identity e.g. in SignerProvider::generate_channel_keys_id.
By delaying the creation of the InboundV1Channel until /after/ the
channel has been accepted, we ensure that we defer cryptographic
initialization until we have given the user the opportunity to assign
an identity to the channel.
Because we don't know which custom TLV type numbers the user is
expecting (and it would be cumbersome for them to tell us), instead of
failing unknown even custom TLVs on deserialization, we accept all
custom TLVs, and pass them to the user to check whether they recognize
them and choose to fail back if they don't. However, a user may not
check for custom TLVs, in which case we should reject any even custom
TLVs as unknown.
This commit makes sure a user must explicitly accept a payment with
even custom TLVs, by (1) making the default
`ChannelManager::claim_funds` fail if the payment had even custom TLVs
and (2) adding a new function
`ChannelManager::claim_funds_with_known_custom_tlvs` that accepts them.
This commit also refactors our custom TLVs test and updates various
documentation to account for this.
The current ChannelClosed event does not let
you know the counterparty during a channel close
event. This change adds the counterparty_node_id
and the channel_capacity to the ChannelClosed event.
This helps users to have more context during a
channel close event. Solves #2343
We already hold them in a vec, so there's no cost to passing them
by ownership vs making it a slice. Further, this helps bindings as
we can't represent slices to non-pointers in a sensible way.
In bindings we can't practically pass a mutable transaction, and
instead need to pass an owned transaction and have the sign method
return a signed copy. We do this here for all build modes as the
API is roughly equivalent also to Rust users.
The C bindings generation currently has issues looking through a
generic associated type. While this should be fixed in the bindings
generator, its easy to fix here for now and we can revisit it
later.
With anchors, we've yet to change the frequency or aggressiveness of
feerate updates, so it's likely that commitment transactions have a
good enough feerate to confirm on its own. In any case, when producing a
child anchor transaction, we should already take into account the fees
paid by the commitment transaction itself, allowing the user to save
some satoshis. Unfortunately, in its current form, this will still
result in overpaying by a small margin at the expense of making the coin
selection API more complex.
There's no need to yield such an event when the commitment transaction
already meets the target feerate on its own, so we can simply broadcast
it without an anchor child transaction. This may be a common occurrence
until we are less aggressive about feerate updates.
* `PaymentPathFailed::path` was added without an optional compat
wrapper in e5310dd5f0.
* `PaymentPathSuccessful::path` has always been written since the
event was added in 2c4f16d5e3.
* `Probe{Failed,Successful}::path` have always been written since
the events were added in eb8bce0d16.
Now that all of the core functionality for anchor outputs has landed,
we're ready to remove the config flag that was temporarily hiding it
from our API.
This change modifies six structs that were keeping
track of anchors features with an `opt_anchors` field,
as well as another field keeping track of nonzero-fee-
anchor-support.
