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

Add Debug module to move stdlib deps #660

Merged
merged 2 commits into from
Mar 5, 2022
Merged

Add Debug module to move stdlib deps #660

merged 2 commits into from
Mar 5, 2022

Conversation

lxfind
Copy link
Contributor

@lxfind lxfind commented Mar 4, 2022

This PR imports the Debug module from move stdlib nursery to our deps.
Added a verifier to make sure we don't ship any call to Debug module when we publish them onchain.
This closes #652

@lxfind lxfind requested review from awelc, sblackshear and damirka March 4, 2022 20:06
@@ -0,0 +1,52 @@
// Copyright (c) 2022, Mysten Labs, Inc.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this verifier pass? As long as we keep the Debug module out of genesis, any attempt to publish code with Debug's will fail with a linker error.

This perhaps isn't the best UX (although a linker error does give you the name of the module that the linker didn't find), but it does save authorities from needing to run a special verifier pass for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we want to enable the use of Debug when running e2e tests though, like when running authority tests. So we need the module in the genesis.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Added it to denylist only under non-test mode. This should allow us to run tests with it.
Kept the changes in module_builder as they are useful.

@@ -124,6 +129,30 @@ impl ModuleBuilder {
)
}

pub fn add_generic_function(
Copy link
Contributor

Choose a reason for hiding this comment

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

n00b question: why is this function defined but not used anywhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function was used in the first version of this PR. I have iterated and deleted the use. But this function is still useful utility to keep, so I didn't delete it.

@lxfind lxfind merged commit 48dbe19 into main Mar 5, 2022
@lxfind lxfind deleted the add-debug branch March 5, 2022 22:24
mwtian pushed a commit that referenced this pull request Sep 12, 2022
Preserve order of batches
mwtian pushed a commit to mwtian/sui that referenced this pull request Sep 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Debug module to Move stdlib
3 participants