f: almost working #53
Conversation
WalkthroughThis pull request involves version updates and dependency management across multiple files in the Injective Test Tube project. The changes primarily focus on upgrading package versions, updating dependencies for Rust and Go modules, and modifying the GitHub Actions workflow. The updates span the Changes
Poem
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/test-tube/src/runner/result.rs (1)
Line range hint
171-174: Consider updatingTryFrom<ResponseFinalizeBlock>for consistency.The attribute processing in this implementation still uses direct string conversion (
to_string()), while other implementations have been updated to use byte-level access with UTF-8 conversion. Consider updating this implementation for consistency.Apply this change to maintain consistency with other implementations:
Ok(Attribute { - key: a.key.to_string(), - value: a.value.to_string(), + key: std::str::from_utf8(a.key_bytes())?.to_string(), + value: std::str::from_utf8(a.value_bytes())?.to_string(), })
🧹 Nitpick comments (1)
packages/test-tube/src/runner/result.rs (1)
55-56: LGTM! Consider adding error context for UTF-8 conversion failures.The change to use byte-level access with explicit UTF-8 conversion is safer and more aligned with Cosmos SDK's approach. However, the error context could be improved when UTF-8 conversion fails.
Consider wrapping the UTF-8 conversion errors with more context:
- key: std::str::from_utf8(a.key_bytes())?.to_string(), - value: std::str::from_utf8(a.value_bytes())?.to_string(), + key: std::str::from_utf8(a.key_bytes()) + .map_err(|e| Utf8Error::from(e))?.to_string(), + value: std::str::from_utf8(a.value_bytes()) + .map_err(|e| Utf8Error::from(e))?.to_string(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/injective-test-tube/libinjectivetesttube/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
packages/injective-test-tube/Cargo.toml(1 hunks)packages/injective-test-tube/libinjectivetesttube/go.mod(10 hunks)packages/test-tube/Cargo.toml(1 hunks)packages/test-tube/src/runner/result.rs(2 hunks)
🔇 Additional comments (8)
packages/test-tube/src/runner/result.rs (1)
105-106: LGTM! Changes are consistent with the previous implementation.The byte-level access and UTF-8 conversion changes mirror those in the
TryFrom<ExecTxResult>implementation, maintaining consistency across the codebase.packages/injective-test-tube/libinjectivetesttube/go.mod (5)
279-279: Review the change in repository ownership.The replace directive for
injective-corenow points toInjectiveFoundationinstead ofInjectiveLabs. Please confirm this is intentional and that all necessary permissions and access controls are in place.
250-259: Verify compatibility of golang.org/x updates.Multiple golang.org/x packages are being updated. These are core packages, so please ensure they maintain compatibility with your existing code.
✅ Verification successful
Indirect dependencies, no compatibility concerns needed
These golang.org/x packages are marked as indirect dependencies in go.mod, meaning they're not directly used in your codebase but are required by other dependencies. Go's module system ensures compatibility for such transitive dependencies. No manual verification is needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for any deprecated API usage rg -A 5 "golang.org/x/(crypto|exp|mod|net|sync|sys|term|text|time|tools)" . | grep -v "go.mod"Length of output: 1457
Script:
#!/bin/bash # Search for direct imports of golang.org/x packages ast-grep --pattern 'import ( $$$ "golang.org/x/$_" $$$ )' # Also search for single-line imports ast-grep --pattern 'import "golang.org/x/$_"' # Search for actual code using these packages without go.sum/go.mod noise rg -t go --no-heading "golang.org/x/(crypto|exp|mod|net|sync|sys|term|text|time|tools)" . | grep -v "go\.(sum|mod)"Length of output: 231
86-92: Verify compatibility between DataDog package versions.All DataDog packages are being updated to v0.58.0. While version alignment is good, please verify that these updates don't introduce breaking changes in your monitoring setup.
✅ Verification successful
DataDog package updates are safe to proceed
These DataDog packages are all marked as indirect dependencies and are not directly used in your codebase. The version alignment to v0.58.0 is likely pulled in by another dependency, and these updates pose minimal risk to your project.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any compatibility issues or breaking changes in DataDog packages rg -A 5 "datadog" . | grep -v "go.mod" | grep -E "New|Init|Configure"Length of output: 67
Script:
#!/bin/bash # Check if DataDog packages are actually used in the codebase echo "=== Checking for DataDog imports ===" rg "\"github.com/DataDog/" --type go echo -e "\n=== Checking go.mod for indirect markers ===" grep "DataDog" packages/injective-test-tube/libinjectivetesttube/go.modLength of output: 1369
238-246: Verify OpenTelemetry collector compatibility.The OpenTelemetry packages are being updated to newer versions. Ensure that the collector configuration remains compatible with these updates.
207-208: Verify Prometheus metrics compatibility.The Prometheus packages (
commonandprocfs) are being updated. Ensure that your metrics collection and reporting remain functional with these updates.packages/test-tube/Cargo.toml (2)
7-7: Verify if version bump is sufficient for breaking changes.The version bump from 2.0.1 to 2.0.3 might be insufficient considering the major version update of cosmrs (0.15.0 -> 0.20.0) which likely introduces breaking changes.
Run this script to check for potential breaking changes:
✅ Verification successful
Version bump is appropriate considering the transition plan
The minor version bump is sufficient as the project is actively working to remove the cosmrs dependency (issue #25) and transition to osmosis-std types. Major changes in a dependency being phased out don't warrant a major version bump.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the changelog for breaking changes in cosmrs between 0.15.0 and 0.20.0 curl -s https://raw.githubusercontent.com/cosmos/cosmos-rust/main/CHANGELOG.md | grep -A 10 "## " # Look for any direct usage of cosmrs in the codebase that might be affected rg "use cosmrs::" -A 3Length of output: 2153
Script:
#!/bin/bash # Check git history for any test or compilation fixes after the version bump git log -n 10 --oneline | grep -i "fix\|test\|compile\|build" # Check if there are any ongoing discussions about breaking changes gh issue list --repo osmosis-labs/test-tube --state all --search "cosmrs" gh pr list --repo osmosis-labs/test-tube --state all --search "cosmrs"Length of output: 351
13-13: Verify compatibility between dependency versions.The updates to cosmrs (0.20.0) and prost (0.13.4) are significant. Please ensure:
- These versions are compatible with each other
- All features required by the codebase are still available
- No breaking changes affect the current implementation
Run this script to check for potential compatibility issues:
Also applies to: 15-15
✅ Verification successful
Dependencies are properly integrated and compatible
The codebase shows correct integration between cosmrs 0.20.0 and prost 0.13.4:
- All required features (cosmwasm, rpc) are available
- Proper protobuf handling is demonstrated in multiple files
- No breaking changes affect the current implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if these exact versions are used together in other projects gh api graphql -f query=' { search(query: "path:Cargo.toml \"cosmrs = { version = \\\"0.20.0\\\"\" \"prost = \\\"0.13.4\\\"\"", type: CODE, first: 5) { nodes { ... on CodeResult { repository { nameWithOwner } file { path } } } } }'Length of output: 974
Script:
#!/bin/bash # Check Cargo.toml contents cat packages/test-tube/Cargo.toml # Search for files using both cosmrs and prost rg -l "use (cosmrs|prost)" packages/test-tube/src/Length of output: 1157
Script:
#!/bin/bash # Check actual usage patterns in files for file in packages/test-tube/src/{utils.rs,lib.rs,runner/app.rs,runner/error.rs,account.rs,runner/result.rs}; do echo "=== $file ===" rg "use (cosmrs|prost)" "$file" -A 2 done # Check if prost types are used with cosmrs rg "cosmrs.*prost|prost.*cosmrs" packages/test-tube/src/Length of output: 2849
| serde = "1.0.144" | ||
| serde_json = "1.0.85" | ||
| test-tube-inj = { version = "2.0.1" } | ||
| test-tube-inj = { version = "2.0.1", path = "../test-tube" } |
There was a problem hiding this comment.
Fix version mismatch in test-tube-inj dependency.
The test-tube-inj dependency specifies version 2.0.1 but the actual package version in packages/test-tube/Cargo.toml is 2.0.3. This mismatch could cause build issues.
Apply this diff to fix the version:
-test-tube-inj = { version = "2.0.1", path = "../test-tube" }
+test-tube-inj = { version = "2.0.3", path = "../test-tube" }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| test-tube-inj = { version = "2.0.1", path = "../test-tube" } | |
| test-tube-inj = { version = "2.0.3", path = "../test-tube" } |
| # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html | ||
|
|
||
| exclude = [ "injective-core", "test_artifacts" ] | ||
|
|
||
| [dependencies] | ||
| base64 = "0.21.5" | ||
| cosmrs = { version = "0.15.0", features = [ "cosmwasm", "rpc" ] } | ||
| cosmrs = { version = "0.20.0", features = [ "cosmwasm", "rpc" ] } |
There was a problem hiding this comment.
💡 Codebase verification
Remove unnecessary prost-derive feature, versions are compatible
The version updates (cosmrs, injective-std, prost) follow semantic versioning and appear compatible. The cosmrs 0.20.0 update only bumps cosmos-sdk-proto dependency. However, the added prost-derive feature appears unnecessary as no prost derive macros are used in the codebase.
- Consider removing the unused prost-derive feature from the dependency declaration
🔗 Analysis chain
Verify version compatibility across dependencies.
Multiple major version updates:
- cosmrs: 0.15.0 -> 0.20.0
- injective-std: 1.13.0 -> 1.13.5
- prost: 0.12.3 -> 0.13.4 with added prost-derive feature
Please ensure these versions are compatible and the new prost feature is required.
Run this script to check for potential compatibility issues:
Also applies to: 18-19
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if these versions are used together in other Injective projects
gh api graphql -f query='
{
search(query: "org:InjectiveLabs path:Cargo.toml \"cosmrs = { version = \\\"0.20.0\\\"\" \"injective-std = { version = \\\"1.13.5\\\"\"", type: CODE, first: 5) {
nodes {
... on CodeResult {
repository {
nameWithOwner
}
file {
path
}
}
}
}
}'
# Check if prost-derive feature is used in the code
rg "use prost" -A 3
Length of output: 3036
Script:
#!/bin/bash
# Search for prost derive macros
rg "#\[derive\(" -A 1 | rg -B 1 "Message|Encode|Decode"
# Check Cargo.lock for resolved versions
rg "^(cosmrs|prost|injective-std)" Cargo.lock
# Try fixed GitHub query
gh search code --owner=InjectiveLabs --filename=Cargo.toml 'cosmrs = { version = "0.20.0"' --json repository,path
# Look for potential breaking changes in cosmrs
curl -s https://raw.githubusercontent.com/cosmos/cosmos-rust/main/cosmrs/CHANGELOG.md | head -n 20
Length of output: 1247
Script:
#!/bin/bash
# Search specifically for prost Message derives
rg -B 1 -A 1 '(?m)^#\[derive.*Message.*\]'
# Check if prost-derive is actually used
rg 'use prost::derive|use prost_derive'
# Check if there are any direct prost type imports that might need derive
rg 'use prost::\w+::'
Length of output: 110
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/injective-test-tube/CHANGELOG.md(1 hunks)packages/injective-test-tube/Cargo.toml(1 hunks)packages/test-tube/Cargo.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/test-tube/Cargo.toml
- packages/injective-test-tube/Cargo.toml
🧰 Additional context used
🪛 LanguageTool
packages/injective-test-tube/CHANGELOG.md
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ... ## 1.13.3 - 2024-16-10 ### Changed - Bumped [email protected] - Bumped [email protected] ## 1.13.2-auct...
(ENGLISH_WORD_REPEAT_RULE)
🔇 Additional comments (2)
packages/injective-test-tube/CHANGELOG.md (2)
Line range hint
1-7: Well-structured changelog format!The changelog follows the Keep a Changelog format and Semantic Versioning guidelines correctly.
🧰 Tools
🪛 LanguageTool
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ... ## 1.13.3 - 2024-16-10 ### Changed - Bumped[email protected]- Bumped[email protected]## 1.13.2-auct...(ENGLISH_WORD_REPEAT_RULE)
13-13: Verify injective-std version.There's a discrepancy between the changelog entry and the version mentioned in the AI summary (v1.13.5). Please verify the correct version of injective-std.
| @@ -5,9 +5,23 @@ All notable changes to this project will be documented in this file. | |||
| The format is based on [Keep a Changelog](http://keepachangelog.com/) | |||
| and this project adheres to [Semantic Versioning](http://semver.org/). | |||
|
|
|||
| ## 1.13.3 - 2024-16-10 | |||
There was a problem hiding this comment.
Fix invalid date format.
The date format "2024-16-10" is invalid as the month value cannot be 16. Please update to a valid date in YYYY-MM-DD format.
-## 1.13.3 - 2024-16-10
+## 1.13.3 - 2024-10-16
-## 1.13.2-auction.1 - 2024-16-10
+## 1.13.2-auction.1 - 2024-10-16Also applies to: 15-15
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/injective-test-tube/CHANGELOG.md (1)
8-8:⚠️ Potential issueFix invalid date format.
The date format "2024-16-10" is invalid as the month value cannot be 16. Please update to a valid date in YYYY-MM-DD format.
-## 1.13.3 - 2024-16-10 +## 1.13.3 - 2024-10-16 -## 1.13.2-auction.1 - 2024-16-10 +## 1.13.2-auction.1 - 2024-10-16Also applies to: 15-15
🧹 Nitpick comments (1)
packages/injective-test-tube/src/module/auction.rs (1)
67-68: Improve readability of the large number.Consider using numeric separators or scientific notation to make the large number more readable.
- inj_basket_max_cap: "10000000000000000000000".to_string(), + inj_basket_max_cap: "10_000_000_000_000_000_000_000".to_string(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/unit_test.yml(2 hunks)packages/injective-test-tube/CHANGELOG.md(1 hunks)packages/injective-test-tube/Cargo.toml(1 hunks)packages/injective-test-tube/src/module/auction.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/injective-test-tube/Cargo.toml
🧰 Additional context used
🪛 LanguageTool
packages/injective-test-tube/CHANGELOG.md
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ... ## 1.13.3 - 2024-16-10 ### Changed - Bumped [email protected] - Bumped [email protected] ## 1.13.2-auct...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Lints
- GitHub Check: Test Suite
🔇 Additional comments (2)
.github/workflows/unit_test.yml (1)
20-20: LGTM! Rust toolchain version update.The update to Rust 1.81.0 is consistent across both jobs and aligns with the dependency updates.
Also applies to: 41-41
packages/injective-test-tube/CHANGELOG.md (1)
8-20: LGTM! Well-structured changelog entries.The changelog entries follow the Keep a Changelog format and properly document the dependency updates.
🧰 Tools
🪛 LanguageTool
[duplication] ~12-~12: Possible typo: you repeated a word.
Context: ... ## 1.13.3 - 2024-16-10 ### Changed - Bumped[email protected]- Bumped[email protected]## 1.13.2-auct...(ENGLISH_WORD_REPEAT_RULE)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/injective-test-tube/src/module/wasm.rs (1)
132-132: Improved type constraint accuracy by removing redundant bound.The removal of
?Sizedis correct sinceDeserializeOwnedalready impliesSized. This change makes the type constraints more precise without affecting runtime behavior.Consider adding a doc comment explaining that
Resmust be a sized type that can be deserialized from the contract's response, to help API users understand the requirements./// Query the smart contract state. /// /// # Type Parameters /// /// * `M` - The query message type that can be serialized to JSON /// * `Res` - The response type that can be deserialized from the contract's JSON response
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/injective-test-tube/src/module/wasm.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test Suite
- GitHub Check: Lints
Summary by CodeRabbit
Dependency Updates
injective-test-tubeandtest-tubepackagesVersion Increments
injective-test-tubepackage version to 1.13.3test-tube-injpackage version to 2.0.4Test Modifications