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

OP_PAIRCOMMIT #1699

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

OP_PAIRCOMMIT #1699

wants to merge 5 commits into from

Conversation

moonsettler
Copy link

@moonsettler moonsettler commented Nov 11, 2024

OP_PAIRCOMMIT is the newest member of the LNhance family of opcodes. It provides limited vector commitment functionality in tapscript.

When evaluated, the OP_PAIRCOMMIT instruction:

  • pops the top two values off the stack,
  • takes the "PairCommit" tagged SHA256 hash of the stack elements,
  • pushes the resulting commitment on the top of the stack.

Discussion: https://delvingbitcoin.org/t/op-paircommit-as-a-candidate-for-addition-to-lnhance/1216/12

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This document has a few formatting issues, please make sure that the preamble matches the BIP 2 requirements and take a look at the rich diff to see whether it looks the way you intend.

Please note that the BIPs repository also accepts markdown files.

@moonsettler
Copy link
Author

moonsettler commented Nov 13, 2024

Switched back to markdown. Header now in BIP-2 format.

@moonsettler
Copy link
Author

The original create date of OP_PAIRCOMMIT is 2024-03-15 this is the latest revision based on feedback from Anthony Towns.
https://gist.github.com/moonsettler/d7f1fb88e3e54ee7ecb6d69ff126433b/revisions
What date should go to the header?

@jonatack
Copy link
Member

Added a discussion link to the PR description.

The original create date of OP_PAIRCOMMIT is 2024-03-15 this is the latest revision based on feedback from Anthony Towns.
gist.github.com/moonsettler/d7f1fb88e3e54ee7ecb6d69ff126433b/revisions
What date should go to the header?

Perhaps add a changelog with the revision based on Anthony Towns' feedback followed by the initial version. Or use the date of the current draft revision as your starting point.

@murchandamus
Copy link
Contributor

According to BIP 2:

The Created header records the date that the BIP was assigned a number, […]

@moonsettler moonsettler marked this pull request as ready for review November 14, 2024 15:56
@murchandamus
Copy link
Contributor

Has this proposal been sent to the mailing list?

@moonsettler
Copy link
Author

moonsettler commented Nov 14, 2024

Has this proposal been sent to the mailing list?

Not yet. Wanted to get it into an acceptable shape before I post it there.

Proposed to the mailing list, waiting for feedback.

README.mediawiki Outdated Show resolved Hide resolved
README.mediawiki Outdated Show resolved Hide resolved
bip-PC.md Outdated
Comment on lines 35 to 39
If `OP_CAT` was available, it could be used to combine multiple stack elements,
that get verified with `OP_CHECKSIGFROMSTACK` as a valid state update.

`OP_PAIRCOMMIT` solves this specific problem without introducing a wide range
of potentially controversial new behaviors, such as novel 2-way peg mechanisms.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sounds like OP_PAIRCOMMIT is closely related to CAT and CSFS. Could you perhaps expand on the related work and design decisions in a Rationale section?

Copy link
Author

@moonsettler moonsettler Nov 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatives we discussed:

  • OP_CAT
  • Merkle operation opcodes
  • SHA256 streaming opcodes
  • 'Kitty' CAT (result or inputs limited in size to try disable introspection and arithmetic extension uses)
  • OP_CTV also commiting to the taproot annex in tapscript
  • OP_CHECKSIGFROMSTACK variant on n elements as message instead of 1
  • OP_VECTORCOMMIT (decoupling above behavior)

Finally after weighing everything OP_PAIRCOMMIT was the simplest addition that got what we needed exactly in the most efficient way. It's a minimal code change, very easy to reason about. Therefore we expect it to be the least controversial option.

Sadly a lot of the discussion is all over the place and on unsearchable mediums.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s why I am suggesting that this proposal should collect some of that information.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to keep it simple and to the point. Added a more brief rationale section. Could do a more in depth recollection on what we learned and why certain alternatives fell out of favor on a delving thread we link from here, if people are actually curious.

Copy link
Author

@moonsettler moonsettler Nov 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expanded on the rationale behind OP_PAIRCOMMIT on delvingbitcoin.

Remove style, Correct title
bip-PC.md Show resolved Hide resolved
@moonsettler moonsettler force-pushed the paircommit branch 3 times, most recently from 59249d9 to dfb0670 Compare November 15, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants