-
Notifications
You must be signed in to change notification settings - Fork 5
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
chore: return the oracle info struct in kormir-server #27
Conversation
83eed80
to
a54cc26
Compare
kormir-server/src/routes.rs
Outdated
} | ||
|
||
Ok(event.attestation().unwrap()) |
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.
Should bubble up the error here instead of unwrap()
kormir-server/src/json_models.rs
Outdated
serde_json::to_string(ann).unwrap() | ||
} | ||
|
||
pub fn att_as_json(ann: &OracleAttestation) -> String { |
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.
NIT: Rename to att
or attestation
kormir-server/src/json_models.rs
Outdated
} | ||
} | ||
|
||
pub fn ann_as_json(ann: &OracleAnnouncement) -> String { |
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.
I'm not sure if we want to have these functions that unwrap. Even though the ann & att are validated in kormir, I still don't like an unwrap especially when just used for logging.
Since they are just for logging, should we just print the event id & outcome for attestations?
kormir-server/src/routes.rs
Outdated
Json(body): Json<crate::routes::CreateNumericEvent>, | ||
) -> Result<Json<String>, (StatusCode, String)> { | ||
Json(body): Json<CreateNumericEventRequest>, | ||
) -> Result<Json<OracleAnnouncement>, (StatusCode, String)> { | ||
if body.num_digits.is_some() && body.num_digits.unwrap() == 0 { |
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.
Could this be unwrap_or(0) so that the route does not panic?
@@ -34,30 +34,82 @@ pub async fn list_events( | |||
"Failed to list events".to_string(), | |||
) | |||
})?; | |||
match Format::from_query(¶ms) { | |||
Err(_) => Err((StatusCode::BAD_REQUEST, "Invalid format".into())), | |||
Ok(format) => match format { |
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.
NIT: return an error here of Result<Value, Error> to properly handle panic in the format match
a54cc26
to
f66fdeb
Compare
f66fdeb
to
bfd73c5
Compare
bfd73c5
to
2849721
Compare
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.
tACK, LGTM
I added a JSON pubkey response as well. Thank you!
Fixes #24
Sample output of
/create-enum
and/announcement
Sample output of
/sign-enum
and/attestation
Sample output of
/create-numeric
and/announcement
Sample output of
/sign-numeric
and/attestation