Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup PackageTemplatea bit #3297

Merged
merged 8 commits into from
Oct 18, 2024

Commits on Oct 16, 2024

  1. Rename claim cleaning match bool for accuracy

    We don't actually care if a confirmed transaction claimed other
    outputs, only that it claimed a superset of the outputs in the
    pending claim we're looking at. Thus, the variable to detect that
    is renamed `is_claim_subset_of_tx` instead of `are_sets_equal`.
    TheBlueMatt committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    2f549ee View commit details
    Browse the repository at this point in the history
  2. Rename PackageTemplate::timelock counteraprty_spendable_height

    This function was very confusing - its used to determine by when
    we have to stop aggregating this claim with others as it starts to
    be at risk of pinning due to the counterparty's ability to spend
    the output.
    
    It is not ever used as a timelock for a transaction, and thus its
    name is very confusing.
    
    Instead we rename it `counterparty_spendable_height`.
    TheBlueMatt committed Oct 16, 2024
    Configuration menu
    Copy the full SHA
    a19edbc View commit details
    Browse the repository at this point in the history

Commits on Oct 17, 2024

  1. Drop unused PackageTemplate::height_original

    This has never been used, and its set to a fixed value of zero for
    HTLCs on local commitment transactions making it impossible to rely
    on so might as well remove it.
    TheBlueMatt committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    8c4794d View commit details
    Browse the repository at this point in the history
  2. Stop passing current height to PackageTemplate::build_package

    Now that we don't store the confirmation height of the inputs
    being spent, passing the current height to
    `PackageTemplate::build_package` is useless - we only use it to set
    the height at which we should next bump the fee, but we just want
    it to be "next block", so we might as well use `0` and avoid the
    extra argument. Further, in one case we were already passing `0`,
    so passing the argument is just confusing as we can't rely on it
    being set.
    
    Note that this does remove an assertion that we never merge
    packages that were crated at different heights, and in the future
    we may wish to do that (as there's no specific reason not to), but
    we do not currently change the behavior.
    TheBlueMatt committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    303a0c9 View commit details
    Browse the repository at this point in the history
  3. Clean up PackageTemplate::get_height_timer to consider type

    `PackageTemplate::get_height_timer` is used to decide when to next
    bump our feerate on claims which need to make it on chain within
    some window. It does so by comparing the current height with some
    deadline and increasing the bump rate as the deadline approaches.
    
    However, the deadline used is the `counterparty_spendable_height`,
    which is the height at which the counterparty might be able to
    spend the same output, irrespective of why. This doesn't make sense
    for all output types, for example outbound HTLCs are spendable by
    our counteraprty immediately (by revealing the preimage), but we
    don't need to get our HTLC timeout claims confirmed immedaitely,
    as we actually have `MIN_CLTV_EXPIRY` blocks before the inbound
    edge of a forwarded HTLC becomes claimable by our (other)
    counterparty.
    
    Thus, here, we adapt `get_height_timer` to look at the type of
    output being claimed, and adjust the rate at which we bump the fee
    according to the real deadline.
    TheBlueMatt committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    be91587 View commit details
    Browse the repository at this point in the history
  4. Rename soonest_conf_deadline to counterparty_spendable_height

    This renames the field in `PackageTemplate` which describes the
    height at which a counterparty can make a claim to an output to
    match its actual use.
    
    Previously it had been set based on when a counterparty can claim
    an output but also used for other purposes. In the previous commit
    we cleaned up its use for fee-bumping-rate, so here we can rename
    it as it is now only used as the `counteraprty_spendable_height`.
    TheBlueMatt committed Oct 17, 2024
    Configuration menu
    Copy the full SHA
    d557334 View commit details
    Browse the repository at this point in the history

Commits on Oct 18, 2024

  1. Set correct counterparty_spendable_height for outb local HTLCs

    For outbound HTLCs, the counterparty can spend the output
    immediately. This fixes the `counterparty_spendable_height` in the
    `PackageTemplate` claiming outbound HTLCs on local commitment
    transactions, which was previously spuriously set to the HTLC
    timeout (at which point *we* can claim the HTLC).
    TheBlueMatt committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    6ae33dc View commit details
    Browse the repository at this point in the history
  2. Add a test for the fee-bump rate of timeout HTLC claims on cp txn

    In a previous commit we updated the fee-bump-rate of claims against
    HTLC timeouts on counterparty commitment transactions so that
    instead of immediately attempting to bump every block we consider
    the fact that we actually have at least `MIN_CLTV_EXPIRY_DELTA`
    blocks to do so, and bumping at the appropriate rate given that.
    
    Here we test that by adding an extra check to an existing test
    that we do not bump in the very next block after the HTLC timeout
    claim was initially broadcasted.
    TheBlueMatt committed Oct 18, 2024
    Configuration menu
    Copy the full SHA
    20db790 View commit details
    Browse the repository at this point in the history