-
Notifications
You must be signed in to change notification settings - Fork 42
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
Support logging debug events via the guest interface #464
Conversation
soroban-env-host/src/host.rs
Outdated
let fmt: String = self | ||
.visit_obj(fmt, move |hv: &Vec<u8>| Ok(String::from_utf8(hv.clone())))? | ||
// TODO: Remove unwrap. | ||
.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.
This allocates a String, but only when debug-assertions are enabled. This code will not be compiled in releases used in stellar-core.
@@ -87,13 +87,13 @@ impl Display for DebugArg { | |||
/// [host::Host::debug_event](crate::host::Host::debug_event) for normal use. | |||
#[derive(Clone, Debug)] | |||
pub struct DebugEvent { | |||
pub msg: Option<&'static str>, | |||
pub msg: Option<Cow<'static, str>>, |
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 think this is the right way to do the Cow so it can hold either the static str or the String. I'm not overly familiar with Cow though. @graydon Am I using this right?
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.
This is ready for review.
Related PRs are here:
I'm not sure how to rebuild the wasm files. I had a go using the Makefile, but then none of them executed successfully after that.
I updated the PR to use a feature instead of debug-assertions, because if we ever wanted to use debug-assertions to debug the host, we'd be enabling new functionality for contracts which could be dangerous. |
@graydon requesting a post-merge review on this one when you have a moment. |
What
Support logging debug events via the guest interface, gated behind the debug-assertions build flag.
Why
Close #447
The reason I've used the debug-assertions build flag and not a feature is because the Rust reference describes the flag as something that can be used to gate extra debug code, and I think we'll use it in the SDK for debug logs since it is a pretty huge advantage to not have to ask every contract developer to layer adebug
feature into their contract just to get logging. Having that consistency between SDK and env is convenient.https://doc.rust-lang.org/stable/reference/conditional-compilation.html?highlight=debug_assertions#debug_assertionsStatus
This is a WIP.