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

Fix parsing of multisig errors #360

Merged
merged 1 commit into from
Aug 13, 2021
Merged

Fix parsing of multisig errors #360

merged 1 commit into from
Aug 13, 2021

Conversation

ruuda
Copy link
Contributor

@ruuda ruuda commented Aug 12, 2021

#177 was starting to bug me, “fix” it. This is a bit of an ugly fix; the bug is in Anchor and it really should be fixed there (this turned out to be the problem), but until then, we can work around it here.

@ruuda ruuda requested a review from enriquefynn August 12, 2021 15:22
Copy link
Member

@enriquefynn enriquefynn left a comment

Choose a reason for hiding this comment

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

LGTM! This would have saved me some hours of debugging 😬

I just have a question about a comment.

ErrorCode::InvalidThreshold,
];

// The purpose of the match statement below is to trigger a compile error if
Copy link
Member

Choose a reason for hiding this comment

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

This is the match below the below match, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It’s about the match statement directly below the comment, on line 289. As you can see, it does absolutely nothing at all, all of the match arms are empty. The reason it’s here, is to trigger the compiler’s exhaustiveness check. Suppose the multisig program adds ErrorCode::CanOnlyApproveOnTuesdays in a future version, and we did not have this match. Then we might miss the addition, and forget to add it to the all_errors list above. Then an CanOnlyApproveOnTuesdays error occurs, and our code would claim it’s not a known multisig error. Therefore I added the match here. If a new version of the multisig program adds a variant, then that will cause a compile error here, saying that the case ErrorCode::CanOnlyApproveOnTuesdays is not covered. When a programmer then goes on to add the case, they will hopefully notice the comment (or else a reviewer should), and add the new case to the all_errors list above as well.

This is broken in Anchor, but we can do it by hand then ...
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.

3 participants