-
Notifications
You must be signed in to change notification settings - Fork 115
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
Update cosmossdk.io/math to v1.4.0 #2588
Conversation
WalkthroughThe changes in this pull request focus on updating the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
🧹 Outside diff range and nitpick comments (1)
protocol/go.mod (1)
Line range hint
171-177
: Documentation update may be neededThe replace directives are well organized into logical blocks with clear TODO comments. However, consider adding a comment about the math version update and its security implications for future reference.
Add a comment before the replace blocks:
+// Updated cosmossdk.io/math to v1.4.0 to address security advisory GHSA-7225-m954-23v7 replace ( // Block for dep upgrades that would have been pulled in via Slinky ... )
Also applies to: 178-189, 190-197
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
protocol/go.sum
is excluded by!**/*.sum
📒 Files selected for processing (1)
protocol/go.mod
(1 hunks)
🔇 Additional comments (2)
protocol/go.mod (2)
7-7
: Verify the security fix for GHSA-7225-m954-23v7
The update to cosmossdk.io/math v1.4.0
aligns with the PR objective to address the security advisory. Let's verify this version contains the fix.
✅ Verification successful
✅ Security fix confirmed in cosmossdk.io/math v1.4.0
The GitHub Security Advisory data confirms that v1.4.0 properly addresses the high-severity vulnerability (ASA-2024-010) related to mismatched bit-length validation. The vulnerability affects versions <= 1.3.0, and v1.4.0 is explicitly listed as the first patched version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if v1.4.0 of cosmossdk.io/math addresses the security advisory
# Check the changelog or release notes for v1.4.0
gh api repos/cosmos/cosmos-sdk/releases | jq -r '.[] | select(.tag_name | contains("math/v1.4.0")) | .body'
# Check if there are any known vulnerabilities for this version
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: GO, package: "cosmossdk.io/math") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 728
Line range hint 171-177
: Verify dependency compatibility with replaced modules
The replace directives show that several core Cosmos SDK components are using dYdX forks. We should verify that math v1.4.0
is compatible with these forked versions.
✅ Verification successful
Math v1.4.0 dependency is compatible with the codebase
The verification shows that:
- The codebase is using
cosmossdk.io/math v1.4.0
directly - The replace directives don't override the math package version
- The math package is widely used across the codebase in various modules (assets, sending, vault, etc.)
- No conflicting versions or compatibility issues are found in the replace directives
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential compatibility issues between math v1.4.0 and forked dependencies
# Check go.mod files in forked repos for their math dependency version
for repo in "cosmos-sdk" "cometbft" "iavl"; do
echo "Checking $repo..."
gh api repos/dydxprotocol/$repo/contents/go.mod | jq -r '.content' | base64 -d | grep -A 5 "cosmossdk.io/math"
done
Length of output: 818
Script:
#!/bin/bash
# Let's try a different approach to check dependency compatibility
# First, let's check the current go.mod for any replace directives and math version
echo "Checking current go.mod file..."
cat go.mod | grep -A 1 "cosmossdk.io/math"
echo -e "\nChecking replace directives..."
cat go.mod | grep "replace"
# Let's also check if math is used in any critical parts
echo -e "\nChecking usage of math package..."
rg "cosmossdk.io/math" --type go
Length of output: 5173
Script:
#!/bin/bash
# Let's check the protocol/go.mod file specifically since we can see it exists from the imports
echo "Checking protocol/go.mod for math version and replace directives..."
cat protocol/go.mod | grep -A 1 "cosmossdk.io/math"
echo -e "\nChecking replace directives in protocol/go.mod..."
cat protocol/go.mod | grep -A 1 "replace"
Length of output: 910
@@ -470,7 +470,7 @@ replace ( | |||
// Use dYdX fork of CometBFT | |||
github.com/cometbft/cometbft => github.com/dydxprotocol/cometbft v0.38.6-0.20241106081823-31609289f401 | |||
// Use dYdX fork of Cosmos SDK | |||
github.com/cosmos/cosmos-sdk => github.com/dydxprotocol/cosmos-sdk v0.50.6-0.20241022180223-cc8c850952c5 | |||
github.com/cosmos/cosmos-sdk => github.com/dydxprotocol/cosmos-sdk v0.50.6-0.20241120185835-38650041ec4d |
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.
Maps to this commit.
@@ -468,9 +468,9 @@ replace ( | |||
// Use dYdX fork of Cosmos SDK/store | |||
cosmossdk.io/store => github.com/dydxprotocol/cosmos-sdk/store v1.0.3-0.20240326192503-dd116391188d | |||
// Use dYdX fork of CometBFT | |||
github.com/cometbft/cometbft => github.com/dydxprotocol/cometbft v0.38.6-0.20241106081823-31609289f401 | |||
github.com/cometbft/cometbft => github.com/dydxprotocol/cometbft v0.38.6-0.20241120221529-56316dc17261 |
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.
Maps to this commit.
(cherry picked from commit aa9ff70) # Conflicts: # protocol/go.mod # protocol/go.sum
Changelist
Rationale
Test Plan
N/A
Author/Reviewer Checklist
state-breaking
label.indexer-postgres-breaking
label.PrepareProposal
orProcessProposal
, manually add the labelproposal-breaking
.feature:[feature-name]
.backport/[branch-name]
.refactor
,chore
,bug
.Summary by CodeRabbit
New Features
Bug Fixes