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

Do not broadcast commitment txn on Permanent mon update failure #1106

Merged

Commits on Sep 15, 2022

  1. Do not broadcast commitment txn on Permanent mon update failure

    See doc updates for more info on the edge case this prevents, and
    there isn't really a strong reason why we would need to broadcast
    the latest state immediately. Specifically, in the case of HTLC
    claims (the most important reason to ensure we have state on chain
    if it cannot be persisted), we will still force-close if there are
    HTLCs which need claiming and are going to expire.
    
    Surprisingly, there were no tests which failed as a result of this
    change, but a new one has been added.
    TheBlueMatt committed Sep 15, 2022
    Configuration menu
    Copy the full SHA
    313810e View commit details
    Browse the repository at this point in the history

Commits on Sep 29, 2022

  1. Rework chain::Watch return types to make async updates less scary

    When a `chain::Watch` `ChannelMonitor` update method is called, the
    user has three options:
     (a) persist the monitor update immediately and return success,
     (b) fail to persist the monitor update immediately and return
         failure,
     (c) return a flag indicating the monitor update is in progress and
         will complete in the future.
    
    (c) is rather harmless, and in some deployments should be expected
    to be the return value for all monitor update calls, but currently
    requires returning `Err(ChannelMonitorUpdateErr::TemporaryFailure)`
    which isn't very descriptive and sounds scarier than it is.
    
    Instead, here, we change the return type used to be a single enum
    (rather than a Result) and rename `TemporaryFailure`
    `UpdateInProgress`.
    TheBlueMatt committed Sep 29, 2022
    Configuration menu
    Copy the full SHA
    12fa0b1 View commit details
    Browse the repository at this point in the history
  2. Add a TODO for an important issue for making async mon updates safe

    If we receive a monitor event from a forwarded-to channel which
    contains a preimage for an HTLC, we have to propogate that preimage
    back to the forwarded-from channel monitor. However, once we have
    that update, we're running in a relatively unsafe state - we have
    the preimage in memory, but if we were to crash the forwarded-to
    channel monitor will not regenerate the update with the preimage
    for us. If we haven't managed to write the monitor update to the
    forwarded-from channel by that point, we've lost the preimage, and,
    thus, money!
    TheBlueMatt committed Sep 29, 2022
    Configuration menu
    Copy the full SHA
    72416b9 View commit details
    Browse the repository at this point in the history
  3. Configuration menu
    Copy the full SHA
    721a858 View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    fb0a481 View commit details
    Browse the repository at this point in the history
  5. Rename APIError::MonitorUpdateFailed to MonitorUpdateInProgress

    This much more accurately represents the error, indicating that a
    monitor update is in progress asynchronously and may complete at a
    later time.
    TheBlueMatt committed Sep 29, 2022
    Configuration menu
    Copy the full SHA
    f961dae View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    52934e0 View commit details
    Browse the repository at this point in the history