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

Try verifying signatures against the last consensus branch ID on failure #4260

Open
str4d opened this issue Dec 11, 2019 · 6 comments
Open
Labels
I-error-handling Problems and improvements related to error handling I-performance Problems and improvements with respect to performance usability

Comments

@str4d
Copy link
Contributor

str4d commented Dec 11, 2019

Currently, if a transaction is received that is signed with a different consensus branch ID, the error returned is either a plain signature failure (if fully-shielded), or more commonly a rather inscrutable failure in the scripting system (for transparent signatures). If we encounter this error, we could try re-verifying with the previous epoch's consensus branch ID. If that succeeds, then we can return a more helpful error to the sending node / user. The downside to this is that it would double signature verification load for failure cases (both normal and adversarial), but if we localise it to just the signature verification itself (i.e. for transparent signatures, trying both inside the script stack) then we aren't fully-doubling the load (as we don't need to re-evaluate the script).

@str4d str4d added I-performance Problems and improvements with respect to performance usability I-error-handling Problems and improvements related to error handling labels Dec 11, 2019
@tromer
Copy link
Contributor

tromer commented Feb 21, 2020

Example of a user encountering the cryptic error:
https://discordapp.com/channels/669694001464737815/671029188353851393/679969630156357662

@rex4539
Copy link
Contributor

rex4539 commented Feb 21, 2020

Another example: #4345

@rex4539
Copy link
Contributor

rex4539 commented Feb 21, 2020

@str4d str4d added this to the Core Sprint 2020-07 milestone Feb 21, 2020
@str4d str4d added the S-scratching-an-itch Status: Something we haven't planned for a sprint but are doing anyway label Feb 21, 2020
@str4d str4d self-assigned this Feb 21, 2020
@str4d str4d removed this from the Core Sprint 2020-07 milestone Feb 21, 2020
@str4d str4d removed the S-scratching-an-itch Status: Something we haven't planned for a sprint but are doing anyway label Feb 21, 2020
@str4d str4d removed their assignment Feb 21, 2020
@str4d
Copy link
Contributor Author

str4d commented Feb 21, 2020

#4371 implements this for transparent and JoinSplit signatures. Sapling signatures are checked inside the Rust codebase along with the proofs, and so we need a larger refactor before we can efficiently implement the same test.

@rex4539
Copy link
Contributor

rex4539 commented Mar 10, 2020

One more: #4392

zkbot added a commit that referenced this issue Apr 9, 2020
Check failing transparent and JoinSplit signatures against the previous network upgrade

This change improves usability across network upgrades, by informing
users when their new transactions are being created with the consensus
branch ID from the previous epoch.

We only check failing signatures against the previous epoch to minimise
the extra computational load on nodes.

A future refactor is needed to similarly check Sapling signatures.

Part of #4260.
@str4d
Copy link
Contributor Author

str4d commented Jun 20, 2022

v5 transactions commit directly to the consensus branch ID, so this is not an issue for them. The only remaining area is Sapling signatures on v4 transactions as described above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-error-handling Problems and improvements related to error handling I-performance Problems and improvements with respect to performance usability
Projects
None yet
Development

No branches or pull requests

3 participants