We support some languages (okay, just JavaScript) where functions
and fields exist in the same namespace. Sadly, because we map
enums as base classes with child classes that add additional
fields, this requires that fields in enum variants do not have the
same name as functions implemented on that enum.
We violated this in 0.1.1 with
`SpendableOutputDescriptor::outpoint` which aliases the `outpoint`
fields on two `SpendableOutputDescriptor` variants.
Here we rename the new `outpoint` method, which we'll have to carry
on the 0.1-bindings branch in addition to going in 0.2.
e23d32dadd removed support for
reading ancient `Channel`s but left a bit of cleanup for later.
Here we mark a few TLVs as `required` which were always written
in 0.0.113.
21ed477841 changed
ChannelTransactionParameters deserialization to pass in
channel_value_satoshis, so it does not actually need to be overwritten
as was done in an earlier draft of the commit.
The custom derive_channel_signer methods on AnchorDescriptor and
HTLCDescriptor simply delegate to the passed SignerProvider now that
providing ChannelTransactionParameters is no longer needed. Drop these
methods and replace them with the method bodies at the call sites.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, it no longer needs to be used when
deriving a signer. This is a breaking API change, though InMemorySigner
did not make use of channel_value_satoshis when being derived.
Now that the copy of ChannelTransactionParameters is no longer used by
InMemorySigner, remove it and all remaining uses. Since it was set by
ChannelSigner::provide_channel_parameters, this method is no longer
needed and can also be removed.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, InMemorySigner no longer needs a copy.
Remove indirect uses of the copy from TestChannelSigner.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, InMemorySigner no longer needs a copy.
Remove uses of the copy from sign_counterparty_payment_input.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, pass the entire parameters when calling
each method on EcdsaChannelSigner. This will remove the need for
ChannelSigner::provide_channel_parameters. Instead, the parameters from
the FundingScope will be passed in to each method. This simplifies the
interaction with a ChannelSigner when needing to be called for more than
one FundingScope, which will be the case for pending splices and RBF
attempts.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, pass the entire parameters when calling
each method on EcdsaChannelSigner. This will remove the need for
ChannelSigner::provide_channel_parameters. Instead, the parameters from
the FundingScope will be passed in to each method. This simplifies the
interaction with a ChannelSigner when needing to be called for more than
one FundingScope, which will be the case for pending splices and RBF
attempts.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, pass the entire parameters when calling
each method on EcdsaChannelSigner. This will remove the need for
ChannelSigner::provide_channel_parameters. Instead, the parameters from
the FundingScope will be passed in to each method. This simplifies the
interaction with a ChannelSigner when needing to be called for more than
one FundingScope, which will be the case for pending splices and RBF
attempts.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, pass the entire parameters when calling
each method on EcdsaChannelSigner. This will remove the need for
ChannelSigner::provide_channel_parameters. Instead, the parameters from
the FundingScope will be passed in to each method. This simplifies the
interaction with a ChannelSigner when needing to be called for more than
one FundingScope, which will be the case for pending splices and RBF
attempts.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, pass the entire parameters when calling
each method on EcdsaChannelSigner. This will remove the need for
ChannelSigner::provide_channel_parameters. Instead, the parameters from
the FundingScope will be passed in to each method. This simplifies the
interaction with a ChannelSigner when needing to be called for more than
one FundingScope, which will be the case for pending splices and RBF
attempts.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, pass the entire parameters when calling
each method on EcdsaChannelSigner. This will remove the need for
ChannelSigner::provide_channel_parameters. Instead, the parameters from
the FundingScope will be passed in to each method. This simplifies the
interaction with a ChannelSigner when needing to be called for more than
one FundingScope, which will be the case for pending splices and RBF
attempts.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, pass the entire parameters when calling
each method on EcdsaChannelSigner. This will remove the need for
ChannelSigner::provide_channel_parameters. Instead, the parameters from
the FundingScope will be passed in to each method. This simplifies the
interaction with a ChannelSigner when needing to be called for more than
one FundingScope, which will be the case for pending splices and RBF
attempts.
Now that channel_value_satoshis has been moved to
ChannelTransactionParameters, pass the entire parameters when calling
each method on EcdsaChannelSigner. This will remove the need for
ChannelSigner::provide_channel_parameters. Instead, the parameters from
the FundingScope will be passed in to each method. This simplifies the
interaction with a ChannelSigner when needing to be called for more than
one FundingScope, which will be the case for pending splices and RBF
attempts.
Since channel_value_satoshis is needed by the signer and may change for
each new FundingScope, included it in ChannelTransactionParameters so it
can be passed to each signer call in the upcoming commits.
InMemorySigner no longer holds channel_value_satoshis and
channel_parameters. Instead of writing 0 and None, respectively, drop
(de-)serialization support entirely since InMemorySigner hasn't been
serialized since SERIALIZATION_VERSION 2.
Rather than moving relevant fields of ChannelTransactionParameters to
FundingScope, move the entire struct there instead. This prevents API
churn wherever ChannelTransactionParameters is used, which otherwise
would need a FundingScope in addition.
9f9a7a92e1 introduced some changes
to channel state transitions but left one case where we'd leave a
channel in an undefined phase (upon receiving a `commitment_signed`
in the wrong state), which we fix here.
Before this commit, unfunded V2 channels were promoted to `FundedChannel`s in
`PendingV2Channel::funding_tx_constructed`.
Since a monitor is only created upon receipt of an initial
`commitment_signed`, this would cause a crash if the channel was read
from persisted data between those two events.
Consequently, we also need to hold an `interactive_tx_signing_session`
for both of our unfunded V2 channel structs.
We don't yet support contibuting inputs to V2 channels, so we need to
debug_assert and return an error if our signing session somehow has
local inputs.
We also return an error if for some mystical reason, in the no input
contributions case, the input count does not equal the witness count of
zero.
In a following commit we change the state at which a V2 channel is
promoted to a `Channel` (after monitor persistence) to avoid a crash
upon reading a `Channel` with no corresponding monitor persisted.
This means that we will have no need for an optional (based on signing
order) `TxSignatures` being returned when `provide_holder_witnesses` is
called.
The above is also the reason for removing the
`received_commitment_signed` and signing order checks in the body of
`provide_holder_witnesses`.
These checks will only be important when we actually contribute inputs.
Currently, we don't so we always send `tx_signatures` first in
accordance with the Interactive Transaction Construction specification:
https://github.com/lightning/bolts/blob/aa5207aea/02-peer-protocol.md?plain=1#L404
We actually don't need to check if the counterparty had already sent
their `tx_signatures` in `InteractiveTxSigningSession::received_tx_signatures`.
Further, we can get rid of the clone of `funding_tx_opt` in
`FundedChannel::tx_signatures` when setting the `ChannelContext::funding_transaction`
as we don't actually need to propagate it through to
`ChannelManager::internal_tx_complete` as we can use
`ChannelContext::unbroadcasted_funding()` which clones the
`ChannelContext::funding_transaction` anyway.
If a channel is closed on startup, but we find that the
`ChannelMonitor` isn't aware of this, we generate a
`ChannelMonitorUpdate` containing a
`ChannelMonitorUpdateStep::ChannelForceClosed`. This ensures that
the `ChannelMonitor` will not accept any future updates in case we
somehow load up a previous `ChannelManager` (though that really
shouldn't happen).
Previously, we'd apply this update only if we detected that the
`ChannelManager` had not yet informed the `ChannelMonitor` about
the channel's closure, even if the `ChannelMonitor` would already
refuse any other updates because it detected a channel closure
on chain.
This doesn't accomplish anything but an extra I/O write, so we
remove it here.
Further, a user reported that, in regtest, they could:
(a) coop close a channel (not generating a `ChannelMonitorUpdate`)
(b) wait just under 4032 blocks (on regtest, taking only a day)
(c) restart the `ChannelManager`, generating the above update
(d) connect a block or two (during the startup sequence), making
the `ChannelMonitor` eligible for archival,
(d) restart the `ChannelManager` again (without applying the
update from (c), but after having archived the
`ChannelMonitor`, leading to a failure to deserialize as we
have a pending `ChannelMonitorUpdate` for a `ChannelMonitor`
that has been archived.
Though it seems very unlikely this would happen on mainnet, it is
theoretically possible.
`provide_latest_holder_commitment_tx` is used to handle
`ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo` updates
and returns an `Err` if we've set `holder_tx_signed`.
However, later in `ChannelMonitorImpl::update_monitor` (the only
non-test place that `provide_latest_holder_commitment_tx` is
called), we will fail the entire update if `holder_tx_signed` is
(or a few other flags are) are set if the update contained a
`LatestHolderCommitmentTXInfo` (or a few other update types).
Thus, the check in `provide_latest_holder_commitment_tx` is
entirely redundant and can be removed.
In `ChannelMonitorImpl::cancel_prev_commitment_claims` we need to
cancel any claims against a removed commitment transaction. We
were checking if `holder_tx_signed` before checking if either the
current or previous holder commitment transaction had pending
claims against it, but (a) there's no need to do this, there's not
a big performance cost to just always trying to remove claims and
(b) we can't actually rely on `holder_tx_signed`.
`holder_tx_signed` being set doesn't necessarily imply that the
`ChannelMonitor` was persisted (i.e. it may simply be lost in a
poorly-timed restart) but we also (somewhat theoretically) allow
for multiple copies of a `ChannelMonitor` to exist, and a different
one could have signed the commitment transaction which was
confirmed (and then unconfirmed).
Thus, we simply remove the additional check here.
By moving the txid check to the start of `tx_signatures` handling, we
can avoid spurious witness count mismatch errors. Also, it just makes
more sense to check the txid earlier.
Since we now have release branches, and dual-funded channels are expected
in v0.2.0, we can remove the cfg flags. This should also speed up CI a
bit as a bonus.
`legacy` fields in TLV stream reads may be used to read fields
which are later moved into some other field using either
`default_value` or `static_value` "reads". This works fine if the
field supports copy semantics, however if it does not, the
accessing of the field in `_decode_and_build` after the TLV stream
read completes but before the struct is built results in a "use
after move" error.
Instead, here, we drop the attempt to hide unused variable warnings
entirely, dropping the post-TLV-stream access to legacy variables,
allowing their use in move semantics for later fields.
50644df9c7 added a new parameter to
`pay_for_offer` and `pay_for_offer_from_human_readable_name`, but
updated the tests by writing out the verbose parameter
construction. This resulted in some tests blowing up from rustfmt
nonsense, which we fix here.
This update allows users to call `pay_for_offer`,
`pay_for_offer_from_human_readable` and `create_refund_builder` with a
set of parameters they wish to manually set for routing the corresponding
invoice. By accepting `RouteParametersConfig`, users gain greater control
over the routing process.
When Bolt12 payers & builders are called, they creates a new `PendingOutboundPayment`
entry with relevant values that will be used when the corresponding invoice
is received. This update modifies `AwaitingInvoice` & `AwaitingOffer` to include
the entire `RouteParametersConfig` struct instead of just `max_total_routing_fee_msat`.
This change ensures all manual routing parameters are available when finding
payment routes.
Decisions & Reasoning:
**Introduction of `route_params_config` in `InvoiceReceived`:**
This was added for the same reason that `max_total_routing_fee_msat` was originally
introduced in PR #2417. The documentation has been updated to reflect this, based
on [this comment](d7e2ff6220 (r1334619765)).
With the current architecture, Bolt12 payers & builders only allows setting
`max_total_routing_fee_msat` as a route parameter. However, it doesn't
provide users the flexibility to set other important parameters.
This commit introduces a new struct, `RouteParametersConfig`,
that optionally allows users to set additional routing parameters.
In later commits, this struct will be utilized when paying BOLT12 invoices.
