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 verify command #696

Closed
wants to merge 21 commits into from
Closed

Add verify command #696

wants to merge 21 commits into from

Conversation

HCastano
Copy link
Contributor

@HCastano HCastano commented Aug 18, 2022

Will close the verify subcommand part of #525.

Note that this command does not necessarily work across different operating systems
or CPU architectures (see the discussion in #525 for more details). As such it should
only be used in the context of a well known Docker container à la srtool.

Current built on top of #680 since I want to make sure that we have all the correct info
in the metadata.

@HCastano HCastano changed the base branch from master to hc-add-build-info August 18, 2022 21:00
src/cmd/verify.rs Outdated Show resolved Hide resolved
@HCastano HCastano force-pushed the hc-add-verify-command branch 3 times, most recently from bf18ec4 to 23a5d82 Compare September 29, 2022 21:46
@HCastano HCastano marked this pull request as ready for review September 29, 2022 21:50
Copy link
Collaborator

@ascjones ascjones left a comment

Choose a reason for hiding this comment

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

Possibly a job for a follow up, but it would be really nice to enhance this by querying a node for the contract code deployed at an account and comparing it with the built contract.

found at {}.",
format!("`{}`", metadata.contract.name).bright_white(),
format!("{:?}", manifest_path.as_ref()).bright_white()).bright_red()
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be super useful to print out the SourceCompiler properties of both reference_wasm and built_wasm in case of non matching wasm.

It's useful to know whether they are the same or different for figuring out why verification failed.

And while we are at it, any other properties we have available in Source e.g. SourceLanguage to catch different versions of ink!.

let fs_wasm = std::fs::read(built_wasm_path)?;
let built_wasm = SourceWasm::new(fs_wasm);

if reference_wasm != built_wasm {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe we should also compare the contract ABI metadata itself? They should be identical too, and if not it might give clues as to what (if anything) is different.

} else {
anyhow::bail!(
"\nThe metadata for the reference contract does not contain a Wasm binary,\n\
therefore we are unable to verify the contract."
Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case couldn't we just grab the code hash and then compare that?

Since releases of `cargo-contract` are now bundled with a `wasm-opt`
library we can assume that equal versions of `cargo-contract` contain
equal versions of `wasm-opt`.
@SkymanOne SkymanOne mentioned this pull request Aug 29, 2023
@SkymanOne
Copy link
Contributor

Closing in favour of #1306

@SkymanOne SkymanOne closed this Aug 29, 2023
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.

3 participants