-
Notifications
You must be signed in to change notification settings - Fork 8
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 #8
base: master
Are you sure you want to change the base?
Conversation
Use CSFS in Symmetry Omit activation related details from spec
2024/BIN-2024-0006.md
Outdated
in conjunction with `OP_CHECKSIGFROMSTACK`[^CSFS] and `OP_INTERNALKEY`[^IKEY], | ||
making the script cleaner, and simpler. If `OP_CAT`[^CAT] was used naively, the | ||
contract could be easily broken since `OP_CHECKTEMPLATEVERIFY`[^CTV] is only | ||
defined for 32 byte parameters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be confusing two things? I would have expected "Vector commitments" to be about committing to N>2 elements, via something like:
- witness stack:
[A B C]
(C at top) - script:
PAIRCOMMIT PAIRCOMMIT <PC(A, PC(B, C))> EQUALVERIFY
(That approach is no longer close-to-optimal wrt hashing overhead, so it might be worth explaining why that makes sense)
But instead you're mostly talking about how it compares to a naive use of CAT that would already be broken with N=2?
I would have thought it would be better to compare to an intelligent use of CAT, and to other approaches like Liquid's streaming SHA opcodes (which would have less hashing overhead than repeated PAIRCOMMIT
) or perhaps being able to slide in additional commitments to CHECKSIG
/CSFS
with a POP_SIGDATA
opcode.
Comparison to other approaches probably should be left for a "rationale" section particularly if Motivation is moved up prior to the Spec. I think both POP_SIGDATA
and streaming SHA opcodes would allow reduced hashing overhead compared to PAIRCOMMIT`` when you're committing to more than 2 items, so would be interesting to understand why those aren't better approaches, given you're optimising the hashing overhead when N=2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking more about vector commitments, it actually is impossible to predict what would be more optimal for the user a) leave the items on the stack and only consume the number of items, or b) consume the items from the stack.
So I would probably do <vch1> .. <vchn> <n> VECTORCOMMIT
where n as a signed char can be 2..127
or -2..-127
, and if it's positive the stack elements are left on stack, if negative they are consumed. This would ofc be a different opcode and is not optimal for Symmetry.
SHA256 streaming could be a useful addition if we already have CAT with script restoration. I'm not sure if consensus is possible on that any time soon. In current script it would be too controversial as it naively relaxes the script limitations on what is possible both the limitation of stack element size that can get hashed with CAT only and without CAT it allows for custom construction of 'sighashes' like CTV templates or with CSFS pretty much everything CAT enables in terms of introspection.
PAIRCOMMIT is domain separated and limited on purpose, to hopefully avoid that controversy.
ELSE | ||
<settlement-n-hash> CTV | ||
ENDIF | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these are LN-Symmetry specific examples? There's not really enough context to follow them here though, and I would have thought they'd make more sense in a separate LN/LNHANCE-Symmetry BOLT?
Thanks for the suggestions! Will clean it up! @ajtowns |
Changed OP_PAIRCOMMIT implementation and specification based on feedback from @ajtowns. |
highly optimized for just 2 stack elements. | ||
|
||
```text | ||
# pc-hash = PC(a, PC(b, c)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong but shouldn't this be
PC(PC(c, b), a)
If the stack is
c
b
a
And
PC(PC(a, b), c)
If the stack is
a
b
c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PC takes the top element as vch2 and the one below the top as vch1. Then pushes PC(vch1, vch2) on stack.
const valtype& vch1 = stacktop(-2);
const valtype& vch2 = stacktop(-1);
uint256 hash = PairCommitHash(vch1, vch2);
It should be
<a>
<b>
<c>
# stack: <a> <b> <c>
PC
# stack: <a> <PC(b, c)>
PC
# stack: <PC(a, PC(b, c)>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh I see I had misread the code, this makes sense now thanks for clearing it up!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are the unit tests, you can execute PC only if you check out that branch, build it and run
test_bitcoin --run_test=pchash_tests
pchash_reproduce
on line 68 shows you how to manually reproduce the output, and pchash_tapscript
shows you how you can give it a spin in the interpreter.
https://github.com/lnhance/bitcoin/blob/lnhance-26.x/src/test/pchash_tests.cpp
popstack() has a redundant check @Psifour
No description provided.