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

bip-341: Commit to all scriptPubKeys in SigMsg #920

Merged
merged 2 commits into from Jun 1, 2020

Conversation

@jonasnick
Copy link
Contributor

jonasnick commented May 15, 2020

@gmaxwell
Copy link
Contributor

gmaxwell commented May 15, 2020

Is there a discussion as to why amounts and scriptpubkey are separately hashed instead of just being one hash interleaved with the amounts? (more similar to the anyonecan pay case)

@jonasnick
Copy link
Contributor Author

jonasnick commented May 15, 2020

Yes, the current design was suggested by @roconnor on the mailing list (https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-May/017808.html) because it saves signers from receiving and hashing the scriptPubKeys if they're just interested in the amounts.

@gmaxwell
Copy link
Contributor

gmaxwell commented May 15, 2020

I can't figure out what use a signer would have for the scriptpubkeys without the values. Values without scriptpubkeys would be useful if the signer is always going to assume all inputs are its own.

@jonasnick
Copy link
Contributor Author

jonasnick commented May 15, 2020

Yeah, scriptPubKeys but not values seems useless. But since values without scriptPubKeys are useful, hashing them separately is better than having a single hash for all TxOuts. Or am I misunderstanding your suggestion?

@gmaxwell
Copy link
Contributor

gmaxwell commented May 16, 2020

Hm. On the basis that scripts without amounts are likely not useful the commitment to scriptpubkeys could be inside the commitment to the amounts, which I think would save some hashing in verifiers and reduce data needing to be sent to signers that don't care about either, but I suppose it doesn't matter much.

@sipa
Copy link
Member

sipa commented May 23, 2020

ACK, also included in bitcoin/bitcoin#17977 now.

@ajtowns
Copy link
Contributor

ajtowns commented May 25, 2020

ACK

2 similar comments
@gmaxwell
Copy link
Contributor

gmaxwell commented May 26, 2020

ACK

@prusnak
Copy link
Contributor

prusnak commented Jun 1, 2020

ACK

@luke-jr luke-jr merged commit 52bd714 into bitcoin:master Jun 1, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.