-
Notifications
You must be signed in to change notification settings - Fork 446
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
Implement seal_debug_message
#792
Conversation
Let's be honest: It was about 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.
all in all LGTM
Co-authored-by: Robin Freyler <robin.freyler@gmail.com>
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.
LGTM, we can merge soon.
I personally wouldn't introduce yet another crate feature and instead document the expected behavior when using this SEAL macro/function without respecting the SEAL unstable feature upon compilation. |
#[cfg(feature = "ink-debug")] | ||
/// Call `seal_debug_message` with the supplied UTF-8 encoded message. | ||
/// | ||
/// If debug message recording is disabled in the contracts pallet, the first call to will |
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.
/// If debug message recording is disabled in the contracts pallet, the first call to will | |
/// If debug message recording is disabled in the contracts pallet, the first call will |
/// Call `seal_debug_message` with the supplied UTF-8 encoded message. | ||
/// | ||
/// If debug message recording is disabled in the contracts pallet, the first call to will | ||
/// return LoggingDisabled error, and further calls will be a no-op to avoid the cost of calling |
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.
please make it LoggingDisabled
. somehow I cannot make a suggestion here.
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 sorry because I merged it already 🙈
Companion PR for paritytech/substrate#8773, which replaces
seal_println
withseal_debug_message
.I've added some convenience macros
debug_print!
anddebug_println!
, let me know what you think.todo
println
ink-docs#19seal_debug_message
is now anunstable
feature in the contracts pallet, should we hide it behindink-unstable
? Or just add a comment that the node needs to compiled with"pallet-contracts/unstable-interface"
enabled.ink-debug
feature which makesdebug_message
and thedebug_print*
macro a noop. Could modifycargo-contract
to add/remove this feature from the contract/