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

[move] Enabled debug printing in code published on chain (+ Move version bump) #4356

Merged
merged 5 commits into from
Sep 8, 2022

Conversation

awelc
Copy link
Contributor

@awelc awelc commented Aug 29, 2022

Being able to use debug printing in local testing but being forced to remove it for publishing has been a source of confusion for a while now (e.g., #3590).

We have discussed different solutions and the current consensus is that we should allow publishing code with debug prints on chain but have it being a no-op unless used for testing. This is already implemented on core Move side where the printing code is gated with the testing feature in the native function implementation (otherwise the native function's printing part is a no-op): https://github.com/move-language/move/blob/bc57bf2df7aee3f639ec670af5c6ff5341d89a9d/language/move-stdlib/src/natives/debug.rs#L39

This could be enhanced by additional (linter) warnings on Sui side, but we don't have a linting framework at the moment. We could also increase the gas cost of debug printing functions but let's delegate it to a potential future PR.

@awelc awelc changed the title [move] Enabled debug printing in code published on chain [move] Enabled debug printing in code published on chain (+ Move version bump) Sep 7, 2022
Copy link
Collaborator

@sblackshear sblackshear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this Adam!!

@awelc awelc merged commit c8812ff into main Sep 8, 2022
@awelc awelc deleted the aw/debug-print branch September 8, 2022 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants