0.1.1 Backports #3567

TheBlueMatt · 2025-01-28T15:27:14Z

Backports of #3457 #3531 #3534 #3549 #3556 and #3559

tnull · 2025-01-28T16:25:31Z

Seems this would already introduce 2 SemVer violations:

pr/3567 ~/workspace/rust-lightning> cargo semver-checks
    Building lightning v0.1.0 (current)
       Built [   2.802s] (current)
     Parsing lightning v0.1.0 (current)
      Parsed [   0.071s] (current)
     Parsing lightning v0.1.0 (baseline, cached)
      Parsed [   0.072s] (baseline)
    Checking lightning v0.1.0 -> v0.1.0 (no change)
     Checked [   0.130s] 107 checks: 105 pass, 2 fail, 0 warn, 0 skip

--- failure enum_struct_variant_field_added: pub enum struct variant field added ---

Description:
An enum's exhaustive struct variant has a new field, which has to be included when constructing or matching on this variant.
        ref: https://doc.rust-lang.org/reference/attributes/type_system.html#the-non_exhaustive-attribute
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.38.0/src/lints/enum_struct_variant_field_added.ron

Failed in:
  field incoming_cltv_expiry of variant PendingHTLCRouting::Forward in /Users/erohrer/workspace/rust-lightning/lightning/src/ln/channelmanager.rs:168

--- failure pub_module_level_const_missing: pub module-level const is missing ---

Description:
A public const is missing, renamed, or changed from const to static.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.38.0/src/lints/pub_module_level_const_missing.ron

Failed in:
  MIN_RELAY_FEE_SAT_PER_1000_WEIGHT in file /Users/erohrer/.cargo/registry/src/index.crates.io-6f17d22bba15001f/lightning-0.1.0/src/chain/chaininterface.rs:179

We probably need to fix these before moving forward?

TheBlueMatt · 2025-01-28T16:43:04Z

Hmm, yea, the constant rename is easy to fix and should indeed be reverted. The field addition in PendingHTLCRouting::Forward is kinda hard to work around. In theory we could move the new field directly to HTLCSource and then try to figure out how to handle serialization of PendingHTLCRouting, but it'd be a lot of work. PendingHTLCSource is only really public for the onion decoding utilities to be able to return it, and while its definitely possible someone has constructed ones to mock out the onion decoding utilities, as far as I'm aware only one downstream user had any use for the onion decode utilities anyway. Thus, I think its worth eating the semver break here.

TheBlueMatt · 2025-01-28T17:11:35Z

Dropped the rename commit.

Previously, the `feerate_bump` method did not enforce the dust threshold, which could result in us thinking we had raised the fee rate without actually having done so. Instead, `compute_package_output` blindly accepted the updated fee rate while enforcing a non-dust output value, resulting in repeated broadcast attempts of an identical transaction. Conflicts due to removal of a preceding commit resolved in: * lightning/src/chain/package.rs

Bitcoin Core relay policy does not require 16s/vB, which it was previously set to. Trivial conflicts due to removal of a preceding commit resolved in: * lightning/src/chain/chaininterface.rs

Create some tests for various `feerate_bump` scenarios and ensure among other thigns that there are no underflows.

We need to stop passing this Vec by value for the next commit so we can pass it to a different method.

Prior to this patch, if we attempted to send a payment or probe to a buggy route, we would error but continue storing the pending outbound payment forever. Attempts to retry would result in a “duplicate payment” error. In the case of ChannelManager::send_payment, we would also fail to generate a PaymentFailed event, even if the user manually called abandon_payment. This bug is unlikely to have ever been hit in the wild as most users use LDK’s router. Discovered in the course of adding a new send_to_route API. Now, we’ll properly generate events and remove the outbound from storage.

When an outbound payment fails while paying to a route, we need to remove the session_privs for each failed path in the outbound payment. Previously we were sometimes removing in pay_route_internal and sometimes in handle_pay_route_err, so refactor this so we always remove in handle_pay_route_err.

Support more ergonomically sending payments to specific routes. We removed the original version of this API because it was hard to work with, but the concept of sending a payment to a specific route is still useful. Previously, users were able to do this via manually matching the payment id in their router, but that's cumbersome when we could just handle it internally. Trivial `use` conflicts resolved in: * lightning/src/ln/chanmon_update_fail_tests.rs * lightning/src/ln/functional_tests.rs Silent rebase conflicts resolved in: * lightning/src/routing/router.rs

for remote signing of invoices

In a coming commit we'll expire HTLCs backwards even if we haven't yet claimed them on-chain based on their inbound edge being close to causing a channel force-closure. Here we track the incoming edge's CLTV expiry in the pending-routing state so that we can include it in the `HTLCSource` in the next commit. Co-authored-by: Matt Corallo <git@bluematt.me>

In a coming commit we'll expire HTLCs backwards even if we haven't yet claimed them on-chain based on their inbound edge being close to causing a channel force-closure. Here we track and expose the incoming edge's CLTV expiry in the `HTLCSource`, giving `ChannelMonitor` access to it. Co-authored-by: Matt Corallo <git@bluematt.me>

This field was used to test that any HTLC failures didn't come in after an HTLC was fulfilled (indicating, somewhat dubiously, that there may be a bug causing us to fail when we shouldn't have). In the next commit, we'll be failing HTLCs based on on-chain HTLC expiry, but may ultimately receive the preimage thereafter. This would make the `historical_inbound_htlc_fulfills` checks potentially-brittle, so we just remove them as they have dubious value.

Fail inbound HTLCs if they expire within a certain number of blocks from the current height. If we haven't seen the preimage for an HTLC by the time the previous hop's timeout expires, we've lost that HTLC, so we might as well fail it back instead of having our counterparty force-close the channel. Co-authored-by: Matt Corallo <git@bluematt.me>

If we've signed the latest holder tx (i.e. we've force-closed and broadcasted our state), there's not much reason to accept counterparty-transaction-updating `ChannelMonitorUpdate`s, we should make sure the `ChannelManager` fails the channel as soon as possible. This standardizes the failure cases to also match those added to the previous commit, which makes things a bit more readable.

.. previously we just used the 4032 magic number, here we put it in a `pub const` that is reusable elsewhere.

Previously, we would prune tracked descriptors once we see a spend hit `ANTI_REORG_DELAY = 6` confirmations. However, this could lead to a scenario where lingering `ChannelMonitor`s waiting to be archived would still regenerate and replay `Event::SpendableOutput`s, i.e., we would re-add the same (now unspendable due to be actually being already spent) outputs again after having intially pruned them. Here, we therefore keep the tracked descriptors around for longer, in particular at least `ARCHIVAL_DELAY_BLOCKS + ANTI_REORG_DELAY = 4038` confirmations, at which point we assume the lingering monitors to have been likely archived, and it's 'safe' for us to also forget about the descriptors.

valentinewallace · 2025-01-28T19:32:42Z

check_commits is failing

TheBlueMatt · 2025-01-28T19:41:01Z

Yea, that's cause its trying to rebase onto upstream/main, which obv won't work. I'll disable the check on this branch.

TheBlueMatt added this to the 0.1.1 milestone Jan 28, 2025

TheBlueMatt force-pushed the 2025-01-0.1.1-backports branch from 9c1159c to 4ee497b Compare January 28, 2025 17:11

arik-so and others added 16 commits January 28, 2025 18:17

Fix incremental relay fee to be 1s/vB

6d8e18b

Bitcoin Core relay policy does not require 16s/vB, which it was previously set to. Trivial conflicts due to removal of a preceding commit resolved in: * lightning/src/chain/chaininterface.rs

Test fee rate bumping

bc6ae06

Create some tests for various `feerate_bump` scenarios and ensure among other thigns that there are no underflows.

Outbound payments: pass session privs by reference

dd5bec9

We need to stop passing this Vec by value for the next commit so we can pass it to a different method.

RawBolt11Invoice to/from ascii utilities

a31d70d

for remote signing of invoices

Introduce SpendableOutputDescriptor::outpoint accessor

15a567d

Prefactor: Make monior archival delay a pub const

6712b4e

.. previously we just used the 4032 magic number, here we put it in a `pub const` that is reusable elsewhere.

TheBlueMatt force-pushed the 2025-01-0.1.1-backports branch from 4ee497b to 8c49359 Compare January 28, 2025 18:17

valentinewallace approved these changes Jan 28, 2025

View reviewed changes

arik-so approved these changes Jan 28, 2025

View reviewed changes

TheBlueMatt merged commit d8caac4 into lightningdevkit:0.1 Jan 28, 2025
17 of 25 checks passed

tnull mentioned this pull request Jan 29, 2025

Add SemVer compatibility checks to CI #3560

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

0.1.1 Backports #3567

0.1.1 Backports #3567

TheBlueMatt commented Jan 28, 2025

tnull commented Jan 28, 2025 •

edited

Loading

TheBlueMatt commented Jan 28, 2025

TheBlueMatt commented Jan 28, 2025

valentinewallace commented Jan 28, 2025

TheBlueMatt commented Jan 28, 2025

0.1.1 Backports #3567

0.1.1 Backports #3567

Conversation

TheBlueMatt commented Jan 28, 2025

tnull commented Jan 28, 2025 • edited Loading

TheBlueMatt commented Jan 28, 2025

TheBlueMatt commented Jan 28, 2025

valentinewallace commented Jan 28, 2025

TheBlueMatt commented Jan 28, 2025

tnull commented Jan 28, 2025 •

edited

Loading