-
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
fix(eth-bytecode-db): bump verification-common to support non-map 'compilation_artifacts.sources' values #1216
fix(eth-bytecode-db): bump verification-common to support non-map 'compilation_artifacts.sources' values #1216
Conversation
…compilation_artifacts.sources' values
WalkthroughThe pull request encompasses changes across multiple files in the eth-bytecode-db project. The primary modifications include updating the Possibly related PRs
Poem
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 (1)
eth-bytecode-db/eth-bytecode-db-server/src/services/database.rs (1)
363-366
: LGTM! Consider enhancing the error context with structured fields.The addition of detailed error logging before converting to tonic::Status improves error observability, which is crucial for debugging issues with invalid compilation artifacts.
Consider adding structured fields to provide more context in the logs:
- tracing::error!("error while retrieving alliance sources: {err:#?}"); + tracing::error!( + error.details = ?err, + error.display = %err, + "error while retrieving alliance sources" + );This structured approach:
- Separates the detailed debug representation (
?err
) from the display representation (%err
)- Makes it easier to query and filter logs
- Maintains the same level of detail while improving log parsing
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
eth-bytecode-db/Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (4)
eth-bytecode-db/Cargo.toml
(1 hunks)eth-bytecode-db/eth-bytecode-db-server/src/services/database.rs
(1 hunks)eth-bytecode-db/verifier-alliance-database/tests/integration/internal_compiled_contracts.rs
(2 hunks)eth-bytecode-db/verifier-alliance-database/tests/integration/verified_contracts.rs
(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Unit, doc and integration tests
🔇 Additional comments (3)
eth-bytecode-db/verifier-alliance-database/tests/integration/internal_compiled_contracts.rs (1)
33-33
: LGTM! Verify JSON structure matches schema.The change from
SourceId
to a JSON-based structure simplifies the source mapping representation while maintaining the same semantic meaning.✅ Verification successful
Source mapping JSON structure is consistent and valid ✅
The source mapping structure
{"src/Counter.sol": { "id": 0 }}
is consistently used across the codebase and maintains a clear, simple schema mapping source files to their IDs.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the JSON structure for sources matches the schema across the codebase. # Search for similar source mapping patterns rg -A 2 '"sources":\s*Some\(from_json!\('Length of output: 43
Script:
#!/bin/bash # Search for from_json! macro usage and source-related patterns echo "=== Searching for from_json! macro usage ===" rg "from_json!\(" -A 3 echo -e "\n=== Searching specifically in test files ===" fd "test.*\.rs$" --exec rg -l "sources.*from_json" echo -e "\n=== Looking for source-related structures ===" rg "sources.*Some\(" -A 2Length of output: 8671
eth-bytecode-db/verifier-alliance-database/tests/integration/verified_contracts.rs (1)
194-194
: LGTM! Consistent with other test files.The change aligns with the updated source mapping representation in
internal_compiled_contracts.rs
.eth-bytecode-db/Cargo.toml (1)
78-78
: Verify the new verification-common commit.The dependency update to commit
8d9cf0e
supports the new JSON-based source mapping structure.✅ Verification successful
✓ Commit 8d9cf0e is valid and contains the expected changes
The commit implements the required JSON-based source mapping structure changes through a more flexible CompilationArtifacts.sources implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new verification-common commit exists and check for breaking changes. # Check if the commit exists in the repository gh api repos/blockscout/blockscout-rs/commits/8d9cf0e --jq '.sha' || echo "Commit not found" # Compare the commits to identify breaking changes gh api repos/blockscout/blockscout-rs/compare/5481500...8d9cf0e --jq '.commits[] | {sha: .sha, message: .commit.message}'Length of output: 6868
Summary by CodeRabbit
Dependency Updates
verification-common
library to a new commit versionBug Fixes
Refactor