-
Notifications
You must be signed in to change notification settings - Fork 46
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
Feat/sync dev with v1.54.1 #267
Conversation
WalkthroughThe changes in this pull request involve the removal of the Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)level=warning msg="[lintersdb] The name "gas" is deprecated. The linter has been renamed to: gosec." 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
Documentation and Community
|
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: 2
🧹 Outside diff range and nitpick comments (1)
client/chain/ofac.go (1)
Line range hint
1-92
: Consider implementing data integrity verificationThe OFAC list is critical for compliance. Consider adding checksum verification when downloading the list to ensure data integrity.
Here's a suggested implementation:
func DownloadOfacList() error { resp, err := http.Get(DefaultOfacListURL) if err != nil { return err } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return fmt.Errorf("failed to download OFAC list, status code: %d", resp.StatusCode) } + // Read response body + body, err := io.ReadAll(resp.Body) + if err != nil { + return err + } + + // Verify content integrity + expectedHash := resp.Header.Get("X-Content-Hash") + if expectedHash != "" { + actualHash := fmt.Sprintf("%x", sha256.Sum256(body)) + if actualHash != expectedHash { + return fmt.Errorf("OFAC list content integrity check failed") + } + } if err := os.MkdirAll(OfacListPath, 0755); err != nil { return err } outFile, err := os.Create(GetOfacListPath()) if err != nil { return err } defer outFile.Close() - _, err = io.Copy(outFile, resp.Body) + _, err = outFile.Write(body) if err != nil { return err } _, err = outFile.WriteString("\n") if err != nil { return err } return nil }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
client/chain/injective_data/ofac.json
(0 hunks)client/chain/ofac.go
(1 hunks)injective_data/ofac.json
(1 hunks)
💤 Files with no reviewable changes (1)
- client/chain/injective_data/ofac.json
🔇 Additional comments (1)
client/chain/ofac.go (1)
13-13
: Verify the new OFAC list URL endpoint
The URL change from ofac.json
to ofacAndRestricted.json
suggests a more comprehensive restricted list. This is a critical security-related change.
Let's verify the endpoint availability and data format consistency:
✅ Verification successful
Let me gather more information about the data structure of both endpoints to ensure complete compatibility.
Let me check the actual content structure of both endpoints to ensure the data format is compatible with how it's being used in the code.
URL change to ofacAndRestricted.json is compatible and functional
The verification confirms:
- Both endpoints are accessible and return valid JSON
- Data format is identical (arrays of wallet addresses as strings)
- New endpoint contains all entries from the old list (157 entries) plus additional addresses (160 total)
- The code's JSON parsing implementation remains compatible with the new endpoint
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new OFAC list endpoint and compare data formats
# Check if the new endpoint is accessible and returns valid JSON
curl -s "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofacAndRestricted.json" | jq . > /dev/null
if [ $? -eq 0 ]; then
echo "New endpoint returns valid JSON"
else
echo "Error: New endpoint might be inaccessible or returns invalid JSON"
fi
# Compare structure with old endpoint to ensure compatibility
curl -s "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json" | jq 'type' > old_type.txt
curl -s "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofacAndRestricted.json" | jq 'type' > new_type.txt
diff old_type.txt new_type.txt
Length of output: 521
Script:
#!/bin/bash
# Description: Compare the structure of both JSON files in detail
# Get the keys and structure of old endpoint
echo "Old endpoint structure:"
curl -s "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json" | jq 'keys'
echo -e "\nNew endpoint structure:"
curl -s "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofacAndRestricted.json" | jq 'keys'
# Check how the data is used in the codebase
echo -e "\nCode usage context:"
rg -A 5 "DefaultOfacListURL" --type go
Length of output: 3058
Script:
#!/bin/bash
# Get a sample entry from both endpoints to compare format
echo "Old endpoint sample entry:"
curl -s "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json" | jq '.[0]'
echo -e "\nNew endpoint sample entry:"
curl -s "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofacAndRestricted.json" | jq '.[0]'
# Check how the JSON is parsed in the code
echo -e "\nJSON parsing context:"
rg -B 5 -A 10 "DefaultOfacListURL" --type go
Length of output: 2050
"0xffbac21a641dcfe4552920138d90f3638b3c9fba", | ||
"0xc5801cd781d168e2d3899ad9c39d8a2541871298", | ||
"0x0992E2D17e0082Df8a31Bf36Bd8Cc662551de68B", | ||
"0x8aa07899eb940f40e514b8efdb3b6af5d1cf7bb" |
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.
Fix: Invalid Ethereum address length
The address 0x8aa07899eb940f40e514b8efdb3b6af5d1cf7bb
is missing one character to be a valid Ethereum address (39 characters instead of 40 after '0x').
Please verify and correct the address. A valid Ethereum address should be 42 characters long (including '0x' prefix).
"0xc5801cd781d168e2d3899ad9c39d8a2541871298", | ||
"0x0992E2D17e0082Df8a31Bf36Bd8Cc662551de68B", | ||
"0x8aa07899eb940f40e514b8efdb3b6af5d1cf7bb" |
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.
💡 Codebase verification
Invalid Ethereum address format detected
The address 0x8aa07899eb940f40e514b8efdb3b6af5d1cf7bb
is invalid as it contains only 38 hexadecimal characters after the '0x' prefix instead of the required 40 characters. No duplicates were found in the file.
0x8aa07899eb940f40e514b8efdb3b6af5d1cf7bb
needs to be corrected to a valid 40-character Ethereum address
🔗 Analysis chain
Verify format of new Ethereum addresses
The newly added addresses need verification for correct Ethereum address format and checksums.
Let's verify the address format and check for duplicates:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Ethereum address format and check for duplicates
# Check if addresses follow the Ethereum address format (0x followed by 40 hex characters)
for addr in "0xc5801cd781d168e2d3899ad9c39d8a2541871298" "0x0992E2D17e0082Df8a31Bf36Bd8Cc662551de68B" "0x8aa07899eb940f40e514b8efdb3b6af5d1cf7bb"; do
if ! echo "$addr" | grep -qE "^0x[0-9a-fA-F]{40}$"; then
echo "Invalid address format: $addr"
fi
done
# Check for duplicates in the entire file
jq -r 'group_by(.) | map(select(length > 1)) | .[]' injective_data/ofac.json
Length of output: 911
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.
Changes look good to me
PR to sync the dev branch with the changes introduced in master for the v1.54.1 release
Summary by CodeRabbit
New Features
Bug Fixes
Chores