Skip to content

Add sighash versioning specification to ZIPs 246 and 230 - #1056

Merged
nuttycom merged 7 commits into
mainfrom
sighash-updates
Aug 19, 2025
Merged

Add sighash versioning specification to ZIPs 246 and 230#1056
nuttycom merged 7 commits into
mainfrom
sighash-updates

Conversation

@arya2

@arya2 arya2 commented Jul 22, 2025

Copy link
Copy Markdown
Collaborator

No description provided.

Progresses ZIP 246.

Co-authored-by: Jack Grigg <jack@electriccoin.co>
Co-authored-by: Daira-Emma Hopwood <daira-emma@jacaranda.org>
Comment thread zips/zip-0246.rst Outdated
Comment thread zips/zip-0230.rst Outdated
daira
daira previously approved these changes Jul 22, 2025

@daira daira left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

utACK with suggestion.

Co-authored-by: Daira-Emma Hopwood <daira@jacaranda.org>
@nuttycom nuttycom changed the title Updates to ZIP 246 and 230 Add sighash versioning specification to ZIP 246 and 230 Jul 29, 2025
@nuttycom nuttycom changed the title Add sighash versioning specification to ZIP 246 and 230 Add sighash versioning specification to ZIPs 246 and 230 Jul 29, 2025
nuttycom
nuttycom previously approved these changes Jul 29, 2025

@nuttycom nuttycom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ACK with @daira @str4d @arya2 @SamHSmith modulo QEDit's confirmation that this will work for atomic swaps. cc/ @vivek-arte

@vivek-arte

vivek-arte commented Aug 11, 2025

Copy link
Copy Markdown
Contributor

As mentioned in our discussion with @str4d last week, this approach should work for the Swaps use case as well.

However, as we discussed, there is a need to add sigHashInfo to the issuance authorization signatures as well -- to allow for the "update to the algorithm in case of a bug" setting (and similarly, to the key rotation bundles).
I believe the ZIP Editors will make a further pass through this in the near future, update for the above, and fill out the remaining parts. We will revisit this after those updates.

…at includes a `SighashInfo`.

Co-authored-by: Kris Nuttycombe <nuttycom@users.noreply.github.com>
Co-authored-by: Daira-Emma Hopwood <daira@users.noreply.github.com>
Comment thread zips/zip-0230.rst Outdated
Comment thread zips/zip-0246.rst Outdated
Co-authored-by: Daira-Emma Hopwood <daira@jacaranda.org>
@ConstanceBeguier

ConstanceBeguier commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

It is not specified how SighashInfo is incorporated into the digests.
I think we only need to update auth_digest and specifically:

  • transparent_scripts_digest
  • spend_auth_sigs (A.2b) in sapling_auth_digest
  • binding_sig (A.2d) in sapling_auth_digest
  • bindingSigOrchard (A.3b) in orchard_auth_digest
  • spendAuthSigsOrchard (A.3a.ii) in orchard_auth_digest
  • issueAuthSig (A.4a) in issuance_auth_digest

For each of these signatures, the signature should be replaced by (SighashInfo || signature) where SighashInfo = sighash_version || associated_data.

Do you agree?

Comment thread zips/zip-0230.rst
Comment thread zips/zip-0230.rst Outdated
@daira

daira commented Aug 19, 2025

Copy link
Copy Markdown
Collaborator

@ConstanceBeguier wrote:

For each of these signatures, the signature should be replaced by (SighashInfo || signature) where SighashInfo = sighash_version || associated_data.

Yes. I think I was confused in the meeting today between the sighash_version and the signature version byte. Your suggestion is completely fine as-is.

@nuttycom nuttycom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

utACK afce126

@nuttycom
nuttycom merged commit 381f666 into main Aug 19, 2025
1 check passed
@nuttycom
nuttycom deleted the sighash-updates branch August 20, 2025 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants