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

Add imbued v3 based on template-matching #29427

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sdaftuar
Copy link
Member

This attempts to imbue v3 semantics on transactions spending outputs from a transaction that looks like a LN commitment transaction.

Opening this as a draft now so that LN folks can see what this would look like, and concept ACK/NACK as appropriate. If we want to go down this route, I can add tests and comments indicating that this behavior should be deprecated at some point in the future.

See #29319 for context, and I wrote up a summary of some statistics I gathered from analyzing data from 2023 here.

This attempts to imbue v3 semantics on transactions spending outputs from a
transaction that looks like a LN commitment transaction.
@DrahtBot
Copy link
Contributor

DrahtBot commented Feb 13, 2024

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #29325 (consensus: Store transaction nVersion as uint32_t by achow101)
  • #29306 (policy: enable sibling eviction for v3 transactions by glozow)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@ariard
Copy link
Member

ariard commented Feb 17, 2024

one design feedback, look on how any imbuance mechanism would have to fit with dynamic upgrades, whatever the bitcoin use-cases. especially matters for time-sensitive flows: lightning/bolts#1117

@ariard
Copy link
Member

ariard commented Feb 19, 2024

can you precise where the imbuance mechanism should be applied? like in AcceptSingleTransaction or AcceptMultipleTransaction or even earlier at the transaction-relay level in ProcessMessages ?
i think it’s good thing to have a generic imbuance mechanism though i would say the earlier in the net processing stack the better.

hardcoding a template in the imbuance mechanism is a dumb idea even for today Lightning, the “must have exactly 2 330-satoshi outputs” i can just add another HTLC p2wsh 330 sats and your template is broken.

@ariard
Copy link
Member

ariard commented Feb 20, 2024

If the design goal of this imbuance mechanism is to apply "novel" transaction-relay policy in a backward fashion on pre-signed transactions in the context of multi-party applications and contracting protocols, I think there should be a cryptographic opt-in of one of the transaction issuer itself.

I think the template approach is a dead-end as not only in practice each multi-party applications and contracting protocols have inherent malleability affecting the chain of transaction (amounts, scriptpubkeys types, inputs / outputs ordering), though even when there is a standard, each implementation and operations are applying their own policy. E.g for lightning on the lowest-value HTLC accepted, which is loosely documented (cf. max_dust_htlc_exposure_msat).

Even assuming a standadization of the source of commitment transaction malleability on the LN-side, there is no certainty that old off-chain commitment transaction cannot be used to neutralize the pinning risk low reduction brought by v3. The only way to invalidate old state (in a no eltoo world) is an on-chain operation.

I think a better imbuance approach is a new p2p extension, e.g enhanced_wtxid relay, where the tx-relay identifier commits to the policy effect or version applied e.g wtxid || nversion || nsequence || wildcard, where nversion/nsequence field bits can be used to commit to a new deployed policy not integrated by a multi-party applications or contracting protocol pre-signed transaction yet, in a non-interactive fashion.

The enhanced_wtxid could be signed by at least 1 owner of the chain of transaction (e.g taproot key-path or internal pubkey of the first spent input of the transaction) to minimize on-path tampering by a transaction-relay node, e.g avoid MEV attacks on commitment transaction anchor outputs.

Such more generic imbuance mechanism could be deployed on top of bip331 package-relay by upgrading the version number of sendpackages or as an encapsulated change of its own like it's has been done with bip339.

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

@glozow glozow mentioned this pull request Apr 16, 2024
56 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants