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

feat: specify which account is missing/has an invalid signature on signature verification failure #2002

Merged
merged 6 commits into from
Jan 5, 2024

Conversation

cryptopapi997
Copy link
Contributor

In my experience, an annoying error to get because it doesn't give you the information you really want (which account has the problem) even though it clearly can. This PR fixes this.

Also seems I'm not the only that finds this annoying: https://twitter.com/redacted_noah/status/1742943786299191741

@cryptopapi997
Copy link
Contributor Author

Just saw there's an official issue for this, so linking this here too: #2001

@mergify mergify bot added the community label Jan 5, 2024
@mergify mergify bot requested a review from a team January 5, 2024 01:30
Copy link
Collaborator

@steveluscher steveluscher left a comment

Choose a reason for hiding this comment

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

Awesome!

This code used to, and still does, have bail-out behaviour – it returns upon finding the first bad signature.

Let's go further and make it all-the-way useful. Can you refactor this PR so that it unconditionally verifies all of the signatures, and then logs the addresses of all the failed signers?

@cryptopapi997
Copy link
Contributor Author

cryptopapi997 commented Jan 5, 2024

Sounds good, changed it to aggregate the errors & added another test to check that the aggregation works as expected. Feel free to take another look @steveluscher

@steveluscher steveluscher added the automerge Merge this Pull Request automatically once CI passes label Jan 5, 2024
@steveluscher
Copy link
Collaborator

cat-cute

@steveluscher steveluscher changed the title Specify which account is missing/has an invalid signature on signature verification failure feat: Specify which account is missing/has an invalid signature on signature verification failure Jan 5, 2024
@steveluscher steveluscher changed the title feat: Specify which account is missing/has an invalid signature on signature verification failure feat: specify which account is missing/has an invalid signature on signature verification failure Jan 5, 2024
@steveluscher steveluscher merged commit 109615d into solana-labs:master Jan 5, 2024
6 checks passed
@steveluscher steveluscher linked an issue Jan 5, 2024 that may be closed by this pull request
Copy link
Contributor

github-actions bot commented Jan 5, 2024

🎉 This PR is included in version 1.88.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link
Contributor

Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Merge this Pull Request automatically once CI passes community released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signature verification failed does not specify which signature
2 participants