8000 Add imbued v3 based on template-matching by sdaftuar · Pull Request #29427 · bitcoin/bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add imbued v3 based on template-matching #29427

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

Closed
wants to merge 1 commit into 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
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
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.

< 8000 /div>
@ariard
Copy link
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
59 tasks
@DrahtBot
Copy link
Contributor
DrahtBot commented Jun 9, 2024

⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?

  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@harding
Copy link
Contributor
harding commented Jul 24, 2024

the “must have exactly 2 330-satoshi outputs” i can just add another HTLC p2wsh 330 sats and your template is broken.

I believe @ariard is correct: anchor-style commitment transactions can additionally have 330-sat HTLC outputs.

@DrahtBot
Copy link
Contributor

⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?

  • Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
  • Is it no longer relevant? ➡️ Please close.
  • Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

@sdaftuar
Copy link
Member Author

I believe this PR is no longer relevant, so closing for now.

@sdaftuar sdaftuar closed this Oct 21, 2024
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.

4 participants
0