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

Fix various issues found in full_stack_target fuzzing #2808

Merged

Commits on Dec 24, 2023

  1. Fix Feature eq + hash to ignore excess zero bytes

    If we get a `Feature` object which has excess zero bytes, we
    shouldn't consider it a different `Feature` from another with the
    same bits set, but no excess zero bytes. Here we fix both the
    `Hash` and `PartialEq` implementation for `Features` to ignore
    excess zero bytes.
    TheBlueMatt committed Dec 24, 2023
    Configuration menu
    Copy the full SHA
    df1f981 View commit details
    Browse the repository at this point in the history

Commits on Dec 29, 2023

  1. Stop including dust values in feerate affordability checks

    When we or our counterparty are updating the fees on the channel,
    we currently check that the resulting balance is sufficient not
    only to meet the reserve threshold, but also not push it below
    dust. This isn't required in the BOLTs and may lead to spurious
    force-closures (which would be a bit safer, but reserve should
    always exceed the dust threshold).
    
    Worse, the current logic is broken - it compares the output value
    in *billionths of satoshis* to the dust limit in satoshis. Thus,
    the code is borderline dead anyway, but can overflow for channels
    with several million Bitcoin, causing the fuzzer to get mad (and
    lead to spurious force-closures for few-billion-dollar channels).
    TheBlueMatt committed Dec 29, 2023
    Configuration menu
    Copy the full SHA
    ddb54fc View commit details
    Browse the repository at this point in the history
  2. Fix REVOKEABLE_REDEEMSCRIPT_MAX_LENGTH for contest delays >0x7fff

    When contest delays are >= 0x8000, script pushes require an extra
    byte to avoid being interpreted as a negative int. Thus, for
    channels with CSV delays longer than ~7.5 months we may generate
    transactions with slightly too little fee. This isn't really a huge
    deal, but we should prefer to be conservative here, and slightly
    too high fee in the general case is better than slightly too little
    fee in other cases.
    TheBlueMatt committed Dec 29, 2023
    Configuration menu
    Copy the full SHA
    c946edb View commit details
    Browse the repository at this point in the history
  3. Fix debug assertion on opening a channel with a disconnected peer

    If we try to open a channel with a peer that is disconnected (but
    with which we have some other channels), we'll end up with an
    unfunded channel which will lead to a panic when the peer
    reconnects. Here we drop this debug assertion without bother to add
    a new test, given this behavior will change in a PR very soon.
    TheBlueMatt committed Dec 29, 2023
    Configuration menu
    Copy the full SHA
    5d8cd5a View commit details
    Browse the repository at this point in the history
  4. Fix dust buffer feerate calculation overflow

    If a peer provides a feerate which nears `u32::MAX`, we may
    overflow calculating the dust buffer feerate, leading to spuriously
    keeping non-anchor channels open when they should be force-closed.
    TheBlueMatt committed Dec 29, 2023
    Configuration menu
    Copy the full SHA
    3b6a361 View commit details
    Browse the repository at this point in the history

Commits on Jan 8, 2024

  1. Fix reachable unwrap on non-channel_type manual channel acceptance

    If we receive an `OpenChannel` message without a `channel_type`
    with `manually_accept_inbound_channels` set, we will `unwrap()`
    `None`.
    
    This is uncommon these days as most nodes support `channel_type`,
    but sadly is rather trivial for a peer to hit for those with manual
    channel acceptance enabled.
    
    Reported in and fixes lightningdevkit#2804. Luckily, the updated
    `full_stack_target` has no issue reaching this issue quickly.
    TheBlueMatt committed Jan 8, 2024
    Configuration menu
    Copy the full SHA
    7f24e83 View commit details
    Browse the repository at this point in the history