-
Notifications
You must be signed in to change notification settings - Fork 149
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
chore(verifier): bump actix-prost related dependencies #1233
Conversation
…dependencies to v0.12; bump prost dependencies to v0.13; bump blockscout-service-launcher to v0.17.0
WalkthroughThis pull request implements multiple updates within the Rust workspace of the smart-contract-verifier. Dependency specifications in several Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
smart-contract-verifier/smart-contract-verifier-server/src/services/vyper_verifier.rs (1)
85-87
: Consider handling conversion errors gracefully.While switching to
try_from
is a good practice, immediately unwrapping the result could lead to panics. Since this is debug logging, you might want to handle the error case more gracefully:-bytecode_type = BytecodeType::try_from(request.bytecode_type) - .unwrap() - .as_str_name(), +bytecode_type = BytecodeType::try_from(request.bytecode_type) + .map(|t| t.as_str_name()) + .unwrap_or("invalid"),Also applies to: 156-158
smart-contract-verifier/smart-contract-verifier-server/tests/integration/types/batch_solidity.rs (1)
286-291
: Add test cases for invalid enum values.Since we're now using
try_from
for enum conversions, we should add test cases to verify that invalid values are handled correctly. Consider adding tests for:
- Invalid compiler values
- Invalid language values
- Invalid match type values
Example test case structure:
#[test] fn test_invalid_compiler_value() { let invalid_value = 999; // Value not defined in the enum let result = proto::contract_verification_success::compiler::Compiler::try_from(invalid_value); assert!(result.is_err()); }Also applies to: 311-315
smart-contract-verifier/sig-provider-extension/src/lib.rs (1)
36-46
: Consider reusing the gRPC client to improve performanceIn the
create_signatures
method, a newSignatureServiceClient
is created on each call, which may introduce overhead due to establishing a new connection every time. To improve performance, consider creating the client once during initialization and reusing it for subsequent calls.Apply the following changes to store the client in
SigProviderImpl
:struct SigProviderImpl { - uri: Uri, + client: SignatureServiceClient<tonic::transport::Channel>, } impl SigProvider { pub async fn new(config: Config) -> Result<Self, tonic::transport::Error> { + let client = match SignatureServiceClient::connect(config.url.clone()).await { + Ok(client) => client, + Err(err) => { + tracing::error!( + "error connecting to signature service; uri={}, err={}", + config.url, + err + ); + return Err(err); + } + }; Ok(Self { - inner: Arc::new(SigProviderImpl { uri: config.url }), + inner: Arc::new(SigProviderImpl { client }), }) } } impl SigProviderImpl { async fn create_signatures(&self, abi: String) { - let mut client = match SignatureServiceClient::connect(self.uri.to_string()).await { - Ok(client) => client, - Err(err) => { - tracing::error!( - "error connecting to signature service; uri={}, err={}", - self.uri, - err - ); - return; - } - }; + let mut client = self.client.clone(); let _ = client .create_signatures(CreateSignaturesRequest { abi }) .await; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
smart-contract-verifier/Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (6)
smart-contract-verifier/Cargo.toml
(3 hunks)smart-contract-verifier/sig-provider-extension/Cargo.toml
(1 hunks)smart-contract-verifier/sig-provider-extension/src/lib.rs
(2 hunks)smart-contract-verifier/smart-contract-verifier-server/src/services/solidity_verifier.rs
(2 hunks)smart-contract-verifier/smart-contract-verifier-server/src/services/vyper_verifier.rs
(2 hunks)smart-contract-verifier/smart-contract-verifier-server/tests/integration/types/batch_solidity.rs
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Linting / lint
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (3)
smart-contract-verifier/smart-contract-verifier-server/src/services/solidity_verifier.rs (1)
99-101
: Same error handling improvement needed as in vyper_verifier.rs.The same pattern of unwrapping
try_from
result is used here. Consider applying the same error handling improvement suggested for the Vyper verifier.Also applies to: 171-173
smart-contract-verifier/sig-provider-extension/Cargo.toml (1)
15-15
: Dependency addition aligns with logging requirementsAdding
tracing = { workspace = true }
is appropriate, given the logging functionality introduced inlib.rs
.smart-contract-verifier/Cargo.toml (1)
16-18
: Verify compatibility with updated dependenciesSeveral dependencies have been updated to new versions:
actix-prost
to version "0.1.0"blockscout-service-launcher
to "0.17.0"prost
andprost-build
to "0.13"tonic
andtonic-build
to "0.12"Ensure that these updates do not introduce breaking changes affecting the codebase.
Please check for any breaking changes or required code adjustments in the updated dependencies:
Also applies to: 26-26, 47-48, 69-70
bump actix-prost dependencies to v0.1.0; bump tonic dependencies to v0.12; bump prost dependencies to v0.13; bump blockscout-service-launcher to v0.17.0
Summary by CodeRabbit
Chores
Refactor
Tests