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

[Bug] debug::print fails module verification #3590

Closed
jamescarter-le opened this issue Jul 28, 2022 · 3 comments
Closed

[Bug] debug::print fails module verification #3590

jamescarter-le opened this issue Jul 28, 2022 · 3 comments
Assignees
Labels
Milestone

Comments

@jamescarter-le
Copy link

jamescarter-le commented Jul 28, 2022

Steps to Reproduce Issue

When using debug::print in a module, it will fail move prove checks.

Please see very simple example repo: https://github.com/jamescarter-le/sui_bytecode_fail

image

Expected Result

debug::print should not fail move prove checks, or the compiler should raise this as an error.

Actual Result

sui client publish returns:
Move Bytecode Verification Error. Please run the Bytecode Verifier for more information. from sui start
RPC call failed: ErrorObject { code: ServerError(-32000), message: "Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.", data: None } from the current devnet.

PS SuiDev\my_move_package> sui move test
INCLUDING DEPENDENCY MoveStdlib
INCLUDING DEPENDENCY Sui
BUILDING MyGame         
Running Move unit tests
....
Test result: OK. Total tests: 21; passed: 21; failed: 0
PS SuiDev\my_move_package> sui move build                      
PS SuiDev\my_move_package> sui client publish --gas-budget 3000
Move Bytecode Verification Error. Please run the Bytecode Verifier for more information.

System Information

  • OS: Windows 10
  • Compiler: sui 0.6.2
@jamescarter-le jamescarter-le changed the title [Bug] devnet 0.6.2 does not produce valid Move bytecode [Bug] devnet sui 0.6.2 does not produce valid Move bytecode Jul 28, 2022
@jamescarter-le jamescarter-le changed the title [Bug] devnet sui 0.6.2 does not produce valid Move bytecode [Bug] debug::print fails move prove Jul 29, 2022
@bholc646 bholc646 added the move label Aug 11, 2022
@awelc awelc changed the title [Bug] debug::print fails move prove [Bug] debug::print fails module verification Aug 15, 2022
@awelc
Copy link
Contributor

awelc commented Aug 16, 2022

I have looked into it and the first thing to note (I also mentioned this on Discord) is that this is NOT a Move prover problem but rather an issue related to module verification (and, apparently, not even that as described below).

The real issue here is that it's not the verifier that fails but rather the linker as we do not include the debug module in the set of Move stdlib modules we utilize in Sui (at least outside of test mode):

#[cfg(not(test))]

If I understand it correctly, the debug module is simply not included in the devnet's genesis image.

Of course one of the problems here is that the error message that is passed to the user is quite opaque. Furthermore, running the verifier will (at this point) not help here as this is the linker problem. It's unfortunately not easy to pass the detailed error message to the user as the same messages are store on-chain and have to be succinct, generic, and stable in the face of future changes (@tnowacki should be able to provide more context here if need be). Still, there are some solutions that can help mitigate this problem, in particular (as discussed with @tnowacki):

  • we made the debug module [#test_only] (either upstream or just in Sui) to prevent using debug::print in "regular" code altogether
  • we modify Sui module building procedure (used by both sui move build and sui client publish) to report an error in the specific case of the code from the debug module being included in the code being built/published
  • we add the debug module to devnet's genesis image (the native method implementing debug::print is a no-op outside of test mode anyways - we could still charge some outrageous amount of gas for "executing" it on-chain to discourage people leaving debug statements in their code)

It would be great to get some feedback on this (@sblackshear, @lxfind, @oxade? - please add others to the discussion if need be).

@tnowacki
Copy link
Contributor

+1 for making the module test_only, even on the main move-lang repo.

The native function has this check of #[cfg(feature = "testing")] to make it a no-op in a live repo. This was really a hack before the unit testing framework was around IMO. I think it makes everything feel a bit more consistent to just slap on a test_only annotation

@awelc
Copy link
Contributor

awelc commented Sep 8, 2022

Resolved in #4356

@awelc awelc closed this as completed Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants