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

Interaction with Casper smart-contract source code verification service for feat-2.0 branch #184

Conversation

wojcik91
Copy link
Collaborator

Port of changes from https://github.com/casper-ecosystem/casper-client-rs/pull/144/files adjusted to be merged into the branch tracking Node version 2.0

@gRoussac
Copy link
Contributor

gRoussac commented Sep 9, 2024

@wojcik91 can you please pull feat-track-2.0 to remove that removes vergen dependency ?

lib/cli.rs Outdated Show resolved Hide resolved
lib/lib.rs Outdated Show resolved Hide resolved
build.rs Outdated Show resolved Hide resolved
Maciej Wójcik added 2 commits September 12, 2024 10:10
@wojcik91
Copy link
Collaborator Author

@gRoussac I applied your review suggestions.

@gRoussac
Copy link
Contributor

@wojcik91 I need to checkout your branch to see what clippy complains about

error: package `clap_lex v0.7.2` cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.73.0
Error: Either upgrade to rustc 1.74 or newer, or use
cargo update -p clap_lex@0.7.2 --precise ver
where `ver` is the latest version of `clap_lex` supporting rustc 1.73.0

@gRoussac
Copy link
Contributor

gRoussac commented Sep 12, 2024

@wojcik91 I think the best would be to bump the toolchain as per casper-node setting

Please make this change reviewed by @mpapierski

teonite#3

@wojcik91
Copy link
Collaborator Author

@wojcik91 I think the best would be to bump the toolchain as per casper-node setting

Please make this change reviewed by @mpapierski

teonite#3

I merged the update and it seems that locally it works now.

Just out of curiosity - is there a specific reason why this project is not using a lockfile? Previously I did not notice this issue because this specific dependency was at 0.6.0 in my Cargo.lock.

@wojcik91
Copy link
Collaborator Author

I see the test failure, will resolve.

@gRoussac
Copy link
Contributor

gRoussac commented Sep 12, 2024

Just out of curiosity - is there a specific reason why this project is not using a lockfile? Previously I did not notice this issue because this specific dependency was at 0.6.0 in my Cargo.lock.

it's a long debate I have no opinion, I think committing lock files either rust or npm has pros and cons. Right now we have it on casper-node, not on the client, the client is more simple it should just pass CI/CD with cargo update in any case. We can not decide this for your PR but it might be considered I guess.

@wojcik91
Copy link
Collaborator Author

Just out of curiosity - is there a specific reason why this project is not using a lockfile? Previously I did not notice this issue because this specific dependency was at 0.6.0 in my Cargo.lock.

it's a long debate I have no opinion, I think committing lock files either rust or npm has pros and cons. Right now we have it on casper-nod, not on the client, the client is more simple it should just pass CI/CD with cargo update in any case. We can not decide this for your PR but it might be considered I guess.

Sure, I certainly don't aim to convince anyone to make any sweeping changes as part of this PR :)
Just thought there was some project-specific reason I did not consider since it seems unorthodox.

@gRoussac
Copy link
Contributor

@wojcik91 Please review/merge this PR teonite#5

@devendran-m devendran-m requested review from zajko and removed request for alsrdn, rafal-ch and jacek-casper October 23, 2024 16:29
@zajko zajko merged commit 08bf74d into casper-ecosystem:feat-track-node-2.0 Oct 28, 2024
1 check passed
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 Source Code Verification option
6 participants