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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 64 additions & 7 deletions cli/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,21 +264,78 @@ impl AsPrettyError for ClientError {
}
}

/// Parse an error code back to a multisig error.
///
/// We need to write this manually, because `multisig::Error::from`
/// unfortunately doesn’t convert back from error codes, it appears
/// to be broken. See also <https://github.com/ChorusOne/solido/issues/177>.
pub fn multisig_error_from_u32(error_code: u32) -> Option<multisig::ErrorCode> {
use multisig::ErrorCode;

let all_errors = [
ErrorCode::InvalidOwner,
ErrorCode::NotEnoughSigners,
ErrorCode::TransactionAlreadySigned,
ErrorCode::Overflow,
ErrorCode::UnableToDelete,
ErrorCode::AlreadyExecuted,
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.

// the `ErrorCode` enum changes in a future version of the multisig program.
// If you ended up here to fix that, please also add the new variant to the
// list above!
match all_errors[0] {
ErrorCode::InvalidOwner => { /* See comment above! */ }
ErrorCode::NotEnoughSigners => { /* See comment above! */ }
ErrorCode::TransactionAlreadySigned => { /* See comment above! */ }
ErrorCode::Overflow => { /* See comment above! */ }
ErrorCode::UnableToDelete => { /* See comment above! */ }
ErrorCode::AlreadyExecuted => { /* See comment above! */ }
ErrorCode::InvalidThreshold => { /* See comment above! */ }
}

for &error in &all_errors {
match ProgramError::from(error) {
ProgramError::Custom(x) if x == error_code => return Some(error),
_ => continue,
}
}

None
}

#[cfg(test)]
mod test {
use crate::error::multisig_error_from_u32;

#[test]
fn test_multisig_error_from_u32() {
// We use `assert!matches!` because `ErrorCode` does not implement `Eq`.
assert!(matches!(
multisig_error_from_u32(0x65),
Some(multisig::ErrorCode::NotEnoughSigners),
));
assert!(matches!(multisig_error_from_u32(u32::MAX), None));
}
}

pub fn print_pretty_error_code(error_code: u32) {
print_key("Error code interpretations:");
println!("\n");
match LidoError::from_u32(error_code) {
Some(err) => println!(" Solido error {} is {:?}", error_code, err),
None => println!(" Error {} is not a known Solido error.", error_code),
}
match multisig::Error::from(ProgramError::Custom(error_code)) {
// Anchor calls it an "ErrorCode", but it's really an enum
// with user-defined errors (as opposed to the Solana ProgramError).
multisig::Error::ErrorCode(custom_error) => {
println!(" Multisig error {} is {:?}", error_code, custom_error);
println!(" {}", custom_error);
match multisig_error_from_u32(error_code) {
Some(multisig_error) => {
println!(
" Multisig error {} is {:?}: {}",
error_code, multisig_error, multisig_error
);
}
_ => {
None => {
println!(" Error {} is not a known Multisig error.", error_code);
}
}
Expand Down
18 changes: 3 additions & 15 deletions tests/test_multisig.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,7 @@
)
except subprocess.CalledProcessError as err:
assert err.returncode != 0
# assert 'Not enough owners signed this transaction' in err.stderr
# TODO(#177) Previously the error included a human-readable message, why does it
# only include the error code now? Something to do with different Anchor
# versions?
assert 'custom program error: 0x65' in err.stdout
assert 'Not enough owners signed this transaction' in err.stdout
new_info = solana_program_show(program_id)
assert new_info == upload_info, 'Program should not have changed.'
print('> Execution failed as expected.')
Expand Down Expand Up @@ -287,11 +283,7 @@
)
except subprocess.CalledProcessError as err:
assert err.returncode != 0
# assert 'The given transaction has already been executed.' in err.stderr
# TODO(#177) Previously the error included a human-readable message, why does it
# only include the error code now? Something to do with different Anchor
# versions?
assert 'custom program error: 0x69' in err.stdout
assert 'The given transaction has already been executed.' in err.stdout
new_info = solana_program_show(program_id)
assert new_info == upgrade_info, 'Program should not have changed.'
print('> Execution failed as expected.')
Expand Down Expand Up @@ -456,11 +448,7 @@
)
except subprocess.CalledProcessError as err:
assert err.returncode != 0
# assert 'The given owner is not part of this multisig.' in err.stderr
# TODO(#177) Previously the error included a human-readable message, why does it
# only include the error code now? Something to do with different Anchor
# versions?
assert 'custom program error: 0x64' in err.stdout
assert 'The given owner is not part of this multisig.' in err.stdout
result = multisig(
'show-transaction',
'--multisig-program-id',
Expand Down