-
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.53.1 #257
Conversation
…the TX in the mainpool if it has been rejected by the node during broadcast
…c_broadcast_before_await fix/check_tx_result_in_sync_broadcast_before_await
…ps to be able to read/download the file
…ved the test to use a temp test directory
…th_relative Fix/make OFAC list file path relative
… of the file after the update
fix/ofac_list_url'
… feat/sync_dev_with_v1_53_1
WalkthroughThe changes in this pull request involve updates to 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-lintlevel=warning msg="[lintersdb] The name "gas" is deprecated. The linter has been renamed to: gosec." 🪧 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: 0
🧹 Outside diff range and nitpick comments (1)
client/chain/ofac.go (1)
13-13
: Consider version pinning for better stability.The URL currently points to the
master
branch, which could lead to unexpected changes if the OFAC list is updated. Consider pinning to a specific commit hash or tag for better stability and control over updates.You might want to:
- Pin to a specific commit hash
- Implement version checking before updates
- Add a mechanism to verify the integrity of downloaded data
Example approach:
-DefaultOfacListURL = "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json" +// Pin to specific version for stability +DefaultOfacListURL = "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/<commit-hash>/json/wallets/ofac.json"
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
client/chain/ofac.go
(1 hunks)injective_data/ofac.json
(1 hunks)
🔇 Additional comments (4)
client/chain/ofac.go (1)
13-13
: Verify the new OFAC list URL accessibility and data format.
The URL structure has been updated to include the refs/heads/master/json
path segment. Let's verify the endpoint's accessibility and data format.
Consider adding a comment explaining the URL structure and its significance:
+// DefaultOfacListURL points to the OFAC blacklist maintained in the injective-lists repository.
+// The list contains Ethereum addresses that are blocked due to OFAC compliance requirements.
DefaultOfacListURL = "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json"
✅ Verification successful
URL change verified - endpoint is accessible and returns valid data format
The new OFAC list URL is working as expected:
- Returns HTTP 200 status code, confirming accessibility
- Returns a valid JSON array of strings, matching the expected format for Ethereum addresses
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new OFAC list URL accessibility and JSON format
# Test 1: Check if the URL is accessible and returns 200
echo "Testing URL accessibility..."
response_code=$(curl -s -o /dev/null -w "%{http_code}" "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json")
if [ "$response_code" != "200" ]; then
echo "Error: URL returned status code $response_code"
exit 1
fi
# Test 2: Verify JSON format and structure
echo "Verifying JSON format..."
curl -s "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json" | jq 'if type == "array" and (.[0] | type == "string") then "Valid JSON array of strings" else "Invalid format" end'
Length of output: 577
injective_data/ofac.json (3)
1-157
: JSON structure looks good
The file maintains a valid JSON array structure with proper formatting.
2-157
: All Ethereum addresses are properly formatted
Each address follows the standard Ethereum address format (0x prefix followed by 40 hexadecimal characters).
1-157
: Verify OFAC compliance updates
The changes to the sanctioned addresses list should be properly documented and verified against official OFAC updates.
Let's verify these changes:
Consider adding a comment in the file or updating documentation to reference:
- The OFAC announcement/bulletin number
- The date of the update
- A brief description of the changes
master
for the v1.53.1 releaseSummary by CodeRabbit