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.

Sign up for GitHub

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

Jump to bottom

Use sign_holder_htlc_transaction to sign non-anchors holder HTLCs #2667

Merged
merged 6 commits into from
Oct 20, 2023
Merged

Use sign_holder_htlc_transaction to sign non-anchors holder HTLCs #2667

merged 6 commits into from
Oct 20, 2023

Commits on Oct 20, 2023

  1. Support signing non-anchors HTLCs with HTLCDescriptor 

    We plan to use `EcdsaChannelSigner::sign_holder_htlc_transaction` to
also sign holder HTLC transactions on non-anchor outputs channels.
`HTLCDescriptor` was only used in an anchor outputs context, so a few
things needed changing, mostly to handle the different scripts and
feerate.
    @wpaulino
    wpaulino committed Oct 20, 2023
    Configuration menu
    Copy the full SHA
    9510e29 View commit details
    Browse the repository at this point in the history

  2. Provide missing derivation parameters to OnchainTxHandler 

    `OnchainTxHandler` will need to construct `HTLCDescriptor`s for holder
HTLCs, but it did not have access to all of the derivation parameters
that need to be provided.
    @wpaulino
    wpaulino committed Oct 20, 2023
    Configuration menu
    Copy the full SHA
    5958604 View commit details
    Browse the repository at this point in the history

  3. Use sign_holder_htlc_transaction to sign non-anchors holder HTLCs 

    We want to ensure we use fresh random signatures to prevent certain
classes of transaction replacement attacks at the bitcoin P2P layer.
This was already covered for commitment transactions and zero fee holder
HTLC transactions, but was missing for holder HTLC transactions on
non-anchors channels.

We can easily do this by reusing the existing
`EcdsaChannelSigner::sign_holder_htlc_transaction` method and
circumventing the existing `holder_htlc_sigs/prev_holder_htlc_sigs`
caches, which will be removed in a later commit anyway.
    @wpaulino
    wpaulino committed Oct 20, 2023
    Configuration menu
    Copy the full SHA
    03ec746 View commit details
    Browse the repository at this point in the history

  4. Remove caching of holder HTLC signatures 

    Since we want our holder HTLC signatures to be randomly generated and
not reused, our existing caches are useless now, so we opt to remove
them.
    @wpaulino
    wpaulino committed Oct 20, 2023
    Configuration menu
    Copy the full SHA
    a9d9d26 View commit details
    Browse the repository at this point in the history

  5. Don't sign holder HTLCs along with holder commitments 

    `sign_holder_commitment_and_htlcs` never really made sense. Unlike
`sign_counterparty_commitment`, the signatures for holder HTLC
transactions may be required much later than the commitment
transaction's. While it was nice for us to only reach the signer once to
obtain all holder signatures, it's not really ideal anymore as we want
our signatures to be random and not reused.

We no longer return all holder HTLC signatures and instead defer to
obtaining them via `EcdsaChannelSigner::sign_holder_htlc_transaction`.
    @wpaulino
    wpaulino committed Oct 20, 2023
    Configuration menu
    Copy the full SHA
    aae4e7c View commit details
    Browse the repository at this point in the history

  6. Move HTLCDescriptor to sign module 

    Now that `HTLCDescriptor` is no longer specific to anchors, it doesn't
make sense for it to live in the `bump_transaction` module anymore.
    @wpaulino
    wpaulino committed Oct 20, 2023
    Configuration menu
    Copy the full SHA
    b06a652 View commit details
    Browse the repository at this point in the history