-
Notifications
You must be signed in to change notification settings - Fork 209
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
Node auth error messages - take 3 #397
Changes from 2 commits
87ed65a
05787ba
ea81f5a
2b40171
9d73d5a
c6d835e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,12 @@ pub enum Error { | |
#[cfg(feature = "backtraces")] | ||
backtrace: snafu::Backtrace, | ||
}, | ||
#[snafu(display("Error calling the enclave: {}", msg))] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't an error calling the enclave specifically. Also if this is going to be printed to the log we can probably just print There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also this is the 3rd variant of the name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, so just writing is as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So why not call it GoEnclaveErr or something? Duplicate names will cause all sorts of fun when trying to use find and replace, or find in path (not to mention the potential There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not even in Go yet 😅 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. יאללה קניתי There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
EnclaveErr { | ||
msg: String, | ||
#[cfg(feature = "backtraces")] | ||
backtrace: snafu::Backtrace, | ||
}, | ||
} | ||
|
||
impl Error { | ||
|
@@ -60,6 +66,13 @@ impl Error { | |
.build() | ||
} | ||
|
||
pub fn enclave_err<S: ToString>(msg: S) -> Self { | ||
EnclaveErr { | ||
msg: msg.to_string(), | ||
} | ||
.build() | ||
} | ||
|
||
pub fn out_of_gas() -> Self { | ||
OutOfGas {}.build() | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we could add errors for specific isvEnclaveQuoteStatus? Like, return "failed to perform registration. Invalid quote status "KEY_REVOKED" " or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm... can you point me to the place in the code where this error originates? I'd probably have to fiddle with 1-3 functions to bubble this specific case out to the context where i instantiate variants of this type.
Note that the way i use this type is really high level, and unrelated to how most of the related code just uses
sgx_status_t
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/enigmampc/SecretNetwork/blob/05787bae238375af0e95662e3006ba56451f9590/cosmwasm/packages/wasmi-runtime/src/registration/cert.rs#L360
If it's not something you can do in 15 minutes or less we can leave it for some other time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this will be somewhat involved.
I'm not sure I understand what exactly all the structs and enums mean, and the specific error you're referring to happens a couple layers into functions that obfuscate the specific errors that happen. If I change that, I'd need to add a couple of error types, and modify a bunch of other functions that use the same functions that I needed to change in the first place.