In #797, we stopped enforcing that read/sent node_announcements
had their addresses sorted. While this is fine in practice, we
should still make a best-effort to sort them to comply with the
spec's forward-compatibility requirements, which we do here in the
ChannelManager.
Since InvoiceFeatures are an implementation detail of InvoiceBuilder, an
explicit call is needed to support the basic_mpp feature. Since it is
dependent on the payment_secret feature, conditionally define the
builder's method only when payment_secret has been set.
Instead of relying on users to set an invoice's features correctly,
enforce the semantics inside InvoiceBuilder. For instance, if the user
sets a PaymentSecret then InvoiceBuilder should ensure the appropriate
feature bits are set. Thus, for this example, the TaggedField
abstraction can be retained while still ensuring BOLT 11 semantics at
the builder abstraction.
Current Bitcoin Core's policy will reject a p2wsh as a dust if it's
under 330 satoshis. A typical p2wsh output is 43 bytes big to which
Core's `GetDustThreshold()` sums up a minimal spend of 67 bytes (even
if a p2wsh witnessScript might be smaller). `dustRelayFee` is set
to 3000 sat/kb, thus 110 * 3000 / 1000 = 330. As all time-sensitive
outputs are p2wsh, a value of 330 sat is the lower bound desired
to ensure good propagation of transactions. We give a bit margin to
our counterparty and pick up 660 satoshis as an accepted
`dust_limit_satoshis` upper bound.
As this reasoning is tricky and error-prone we hardcode it instead of
letting the user picking up a non-sense value.
Further, this lower bound of 330 sats is also hardcoded as another constant
(MIN_DUST_LIMIT_SATOSHIS) instead of being dynamically computed on
feerate (derive_holder_dust_limit_satoshis`). Reducing risks of
non-propagating transactions in casee of failing fee festimation.
While this is less readable, I spent way too long trying to adapt
the bindings generation code to handle glob imports and concluded
it would take refactoring almost the entire import-resolution
logic. While this may be a good refactor to do eventually, its
probably not worth it today.
The C bindings generator now looks to default generic types as the
way to map a struct or enum parameter. Because SignOrCreationError
is only used directly with an error type of `()`, we set that to
the default and assume no other error types are needed.
The ChannelSigner bounds are specified both in `impl<>` and in the
`where` clause, which the C bindings generator doesn't like. There
is no reason to have them specified twice.
This prevents aliasing the global secp256k1::Signature name in C
bindings and also makes it a little more explicit that the object
is different from other signature types.
There is generally never a reason to return a non-mutable reference
to a u64 vs just copying it, same applies here. It makes the API
slightly less consistent, but is easier to map in bindings and just
makes more sense.
For users who get PaymentPreimages via
`get_payment_secret_preimage`, they need to provide the
PaymentPreimage back in `claim_funds` but they aren't actually
given the preimage anywhere.
This commit gives users the PaymentPreimage in the
`PaymentReceived` event.
