Skip to content

[move-prover] Print warning about the level of Prover support for Sui #14348

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.

Sign up for GitHub

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

Merged
awelc merged 4 commits into from
Oct 27, 2023
Merged

[move-prover] Print warning about the level of Prover support for Sui #14348

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ed37013
[move-prover] Print warning about the level of Prover support for Sui
awelc Oct 19, 2023
c735cf4
Added Prover warning message to the help prompt
awelc Oct 19, 2023
e541fc8
Finessed the warning message
awelc Oct 25, 2023
3e83b17
Further finessed the warning message
awelc Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/sui-move/src/prove.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use sui_types::sui_framework_address_concat_string;

const SUI_NATIVE_TEMPLATE: &[u8] = include_bytes!("sui-natives.bpl");

/// Run the Move Prover on the package at `path` (Warning: Move Prover support for Sui is currently limited)
#[derive(Parser)]
#[group(id = "sui-move-prover")]
pub struct Prover {
Expand Down Expand Up @@ -92,6 +93,7 @@ impl Prover {
),
);

eprintln!("WARNING: the level of Move Prover support for Sui is currently limited; use at your own risk");
Copy link
Copy Markdown
Contributor

@tnowacki tnowacki Oct 25, 2023

Choose a reason for hiding this comment

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

Could we maybe give a block of descriptions of what things might or might not work? And with what amount of work they might need to do?

Copy link
Copy Markdown
Contributor Author

@awelc awelc Oct 25, 2023

Choose a reason for hiding this comment

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

The idea makes sense but I am worried that this will change in time. Actually, I am not 100% confident what what does and does not work at this point and I feel like a more generic message would be sufficient to trigger further investigation on the side of a person that still wants to go for it despite the warning.

Copy link
Copy Markdown
Contributor

@tnowacki tnowacki Oct 25, 2023

Choose a reason for hiding this comment

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

Maybe then we could just explain out what "at your own risk" entails? Like just explaining: "Not everything is guaranteed to work. Please file an issue if an update breaks your usage"
Something to that effect. Like I want people to know that it might not work, but we should also be receptive if we break something that was working

Copy link
Copy Markdown
Contributor Author

@awelc awelc Oct 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I finessed the warning message a bit but in reality we don't have enough resources to encourage people to file issues so I did not include that part...

let prover_result = std::thread::spawn(move || {
prove::run_move_prover(
build_config,
Expand Down