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

Refactor transaction validation checks into separate functions #2460

Closed
jvff opened this issue Jul 7, 2021 · 0 comments
Closed

Refactor transaction validation checks into separate functions #2460

jvff opened this issue Jul 7, 2021 · 0 comments
Labels
C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement

Comments

@jvff
Copy link
Contributor

jvff commented Jul 7, 2021

Motivation

PRs #2302, #2371, and #2419 included some refactorings of the transaction::Verifier, splitting some code into separate methods so that they can be reused. It might be beneficial to further split the methods so that the consensus rules can be properly quoted in the documentation, instead of in the code comments. The split code could be either new methods, or new functions in the transaction::check module, where other consensus rule checks are implemented.

Specifications

The consensus rules that are affected and quoted in the code comments are:

The joinSplitSig MUST represent a valid signature, under joinSplitPubKey, of the sighash.

https://zips.z.cash/protocol/protocol.pdf#sproutnonmalleability

Consensus rule: cv and rk MUST NOT be of small order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]rk MUST NOT be 𝒪_J.

The proof π_ZKSpend MUST be valid given a primary input formed from the other fields except spendAuthSig.

The spend authorization signature MUST be a valid SpendAuthSig signature over SigHash using rk as the validating key.

https://zips.z.cash/protocol/protocol.pdf#spenddesc

cv and wpk MUST NOT be of small order, i.e. [h_J]cv MUST NOT be 𝒪_J and [h_J]wpk MUST NOT be 𝒪_J.

The proof π_ZKOutput MUST be valid given a primary input formed from the other fields except C^enc and C^out.

https://zips.z.cash/protocol/protocol.pdf#outputdesc

The consensus rule affected by the code but not quoted in any nearby comment:

bindingSigSapling MUST represent a valid signature under the transaction binding validating key bvk^Sapling of SigHash

https://zips.z.cash/protocol/protocol.pdf#txnconsensus

The consensus rules that are likely relevant but not yet implemented are:

The spend authorization signature MUST be a valid SpendAuthSigOrchard signature over SigHash using rk as the validating key. [...] validation of the R component of the signature prohibits non-canonical encodings.

Tracking issue: #2317
Open PR: #2442

The proof 𝜋 MUST be valid given a primary input (cv, rt^Orchard, nf, rk, cm_x, enableSpends, enableOutputs)

Tracking issue: #2105

https://zips.z.cash/protocol/protocol.pdf#actiondesc

bindingSigOrchard MUST represent a valid signature under the transaction binding validating key bvk^Orchard of SigHash

Tracking issue: #2103

https://zips.z.cash/protocol/protocol.pdf#txnconsensus

Related Work

#1981 was the issue that tracked part of the refactor when initial support for V5 transactions was implemented.

@jvff jvff added C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage labels Jul 7, 2021
@mpguerra mpguerra added C-cleanup Category: This is a cleanup P-Low labels Jul 9, 2021
@jvff jvff closed this as completed Mar 18, 2022
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Sep 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: This is a cleanup C-enhancement Category: This is an improvement
Projects
None yet
Development

No branches or pull requests

2 participants