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.

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

Jump to bottom

Interaction with Casper smart-contract source code verification service #144

Open
wants to merge 33 commits into
base: dev
Choose a base branch
from
Open

Interaction with Casper smart-contract source code verification service #144

wants to merge 33 commits into from

Conversation

moubctez
Copy link

@moubctez moubctez commented Apr 5, 2024

Allow interaction with Source Code Verification Service.

Check https://github.com/teonite/Casper-SCVS-Verificator for details.

@moubctez moubctez marked this pull request as ready for review April 5, 2024 08:20
@moubctez moubctez changed the title Casper smart contract source code verification service Interaction with Casper smart-contract source code verification service Apr 5, 2024
zacshowa
zacshowa suggested changes Apr 9, 2024
View reviewed changes
Copy link
Contributor

@zacshowa zacshowa left a comment

Choose a reason for hiding this comment

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

Largely looks good.

I would also like to see some discussion surrounding the retry logic in this code.

There are some style nits to address, and this change should be added to the change log entry corresponding to the release that you want this code to go out in.

lib/verification.rs Outdated Show resolved Hide resolved
lib/verification.rs Outdated Show resolved Hide resolved
lib/verification_types.rs Outdated Show resolved Hide resolved
lib/verification.rs Outdated Show resolved Hide resolved
lib/verification.rs Outdated Show resolved Hide resolved
lib/lib.rs Outdated Show resolved Hide resolved
zacshowa
zacshowa reviewed Apr 9, 2024
View reviewed changes
src/verify_contract.rs Outdated Show resolved Hide resolved
zacshowa
zacshowa reviewed Apr 10, 2024
View reviewed changes
src/verify_contract.rs Outdated Show resolved Hide resolved
zacshowa
zacshowa approved these changes Apr 10, 2024
View reviewed changes
Copy link
Contributor

@zacshowa zacshowa left a comment

Choose a reason for hiding this comment

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

I have one additional suggestion for the about message for the command, Other than that this looks good to me.

moubctez and others added 2 commits April 11, 2024 20:11
@moubctez @zacshowa
Update src/verify_contract.rs
3f944b1 
Co-authored-by: Zach Showalter <zacshowa@gmail.com>
@moubctez
cargo fmt
ad4bf50
@moubctez
Copy link
Author

I have one additional suggestion for the about message for the command, Other than that this looks good to me.

Good suggestion. I've applied it.

@zacshowa
Copy link
Contributor

Just wanted to keep you up to date on this PR. Currently it's failing Cargo audit because of a dependency having a security issue,
This is an issue that is currently being fixed in this PR.

Once that PR merges, you should be able to cherry pick the changes into this PR and make it ready to merge.

@moubctez
Copy link
Author

Just wanted to keep you up to date on this PR. Currently it's failing Cargo audit because of a dependency having a security issue, This is an issue that is currently being fixed in this PR.

Once that PR merges, you should be able to cherry pick the changes into this PR and make it ready to merge.

I have upgraded "vergen" to v8 to resolve the issue.

gRoussac
gRoussac requested changes Jun 17, 2024
View reviewed changes
lib/verification.rs Outdated Show resolved Hide resolved
gRoussac
gRoussac reviewed Jun 17, 2024
View reviewed changes
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
lib/verification.rs Show resolved Hide resolved
gRoussac
gRoussac requested changes Jun 21, 2024
View reviewed changes
build.rs Outdated Show resolved Hide resolved
gRoussac
gRoussac requested changes Jun 24, 2024
View reviewed changes
build.rs Outdated Show resolved Hide resolved
@moubctez
Copy link
Author

@gRoussac Now build.rs is the correct one. I had ancient history in dev. My fault.

@gRoussac
Copy link
Contributor

@moubctez You build is failing tokio is an optional dep so it needs to be specified when used
try to add that arround

#[cfg(feature = "tokio")]

44fef2d#file-lib-verification-rs-L11

probably an import issue for second error

44fef2d#file-lib-lib-rs-L577

Thanks

moubctez and others added 4 commits June 26, 2024 21:11
@moubctez
Fix build without default features. Now tokio is required with "time"…
258db32 
… feature. Note that reqwest is not optional and depends on tokio with "net" and "time" features enabled, so there is no additional burden.
@moubctez
Fix wasm build
c047567
@gRoussac
Fix CI-CD tokio not optional
deaa529
@moubctez
Merge pull request #2 from gRoussac/contract-verification-command
f53e85d 
Fix CI-CD tokio not optional
@gRoussac
Copy link
Contributor

When/if merging please choose, "Squash and merge" thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gRoussac gRoussac gRoussac approved these changes

@zacshowa zacshowa zacshowa approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@moubctez @zacshowa @gRoussac @kchudy