Skip to content
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.7.2 #357

Merged
merged 3 commits into from
Nov 13, 2024
Merged

Feat/sync dev with v1.7.2 #357

merged 3 commits into from
Nov 13, 2024

Conversation

aarmoa
Copy link
Collaborator

@aarmoa aarmoa commented Nov 13, 2024

  • PR to sync the dev branch with the changes introduced in master for the v1.7.2 release

Summary by CodeRabbit

  • New Features
    • Updated the OFAC list with 82 new Ethereum addresses and removed 4 outdated addresses.
  • Bug Fixes
    • Corrected the link to the official OFAC JSON file and refreshed the local copy.
  • Documentation
    • Updated changelog to reflect changes for version 1.7.2.

Copy link
Contributor

coderabbitai bot commented Nov 13, 2024

Walkthrough

The changes in this pull request include an update to the changelog for version 1.7.2, which documents a correction to the link for the ofac.json file and notes a refresh of the local copy. Additionally, the ofac.json file has been modified to add 82 new Ethereum addresses and remove 4 existing ones. The OFAC_LIST_URL constant in the ofac.py file has also been updated to reflect a new structured path for accessing the OFAC list.

Changes

File Change Summary
CHANGELOG.md Added version entry for 1.7.2, corrected link to ofac.json, noted refresh of local copy.
pyinjective/ofac.json Added 82 new Ethereum addresses and removed 4 existing addresses.
pyinjective/ofac.py Updated OFAC_LIST_URL to a new structured path for accessing the ofac.json file.

Possibly related PRs

  • Implement OFAC list address check #346: The changes in this PR involve the introduction of an ofac.json file that lists Ethereum addresses for compliance checks, which is directly related to the updates made in the main PR regarding the ofac.json file link correction and refresh.
  • fix/sync_dev_after_v1_7_1 #352: This PR includes a fix for obtaining the absolute path of the ofac.json file, which is relevant to the main PR's updates on the changelog and the handling of the ofac.json file.

Suggested reviewers

  • PavelInjective

🐰 In the code we hop and play,
New addresses join the fray!
A link corrected, fresh and bright,
Our OFAC list is now just right!
With every change, we take a leap,
In the world of code, our secrets keep! 🐇


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (3)
pyinjective/ofac.py (2)

7-9: Consider adding URL fallback mechanism

Since this is a critical security feature (OFAC compliance), consider implementing a fallback mechanism in case the primary URL is unavailable.

Consider this implementation:

-OFAC_LIST_URL = (
-    "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json"
-)
+# Primary URL with new structure
+OFAC_LIST_URL_PRIMARY = (
+    "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json"
+)
+# Fallback URL with old structure
+OFAC_LIST_URL_FALLBACK = (
+    "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/master/wallets/ofac.json"
+)
+OFAC_LIST_URL = OFAC_LIST_URL_PRIMARY

Then update the download_ofac_list method to attempt the fallback URL if the primary fails:

@classmethod
async def download_ofac_list(cls):
    async with aiohttp.ClientSession() as session:
        # Try primary URL first
        try:
            async with session.get(OFAC_LIST_URL_PRIMARY) as response:
                response.raise_for_status()
                text_content = await response.text()
                ofac_list = json.loads(text_content)
                cls._save_ofac_list(ofac_list)
                return
        except Exception as primary_error:
            # Try fallback URL
            try:
                async with session.get(OFAC_LIST_URL_FALLBACK) as response:
                    response.raise_for_status()
                    text_content = await response.text()
                    ofac_list = json.loads(text_content)
                    cls._save_ofac_list(ofac_list)
                    return
            except Exception as fallback_error:
                raise Exception(
                    f"Error fetching OFAC list: Primary: {primary_error}, Fallback: {fallback_error}"
                )

@classmethod
def _save_ofac_list(cls, ofac_list):
    ofac_file_path = cls.get_ofac_list_path()
    with open(ofac_file_path, "w") as f:
        json.dump(ofac_list, f, indent=2)
        f.write("\n")

Line range hint 31-37: Enhance error handling with specific exception types

The current error handling catches all exceptions generically. Consider separating the handling of different error types for better error reporting and debugging.

Here's a suggested implementation:

     @classmethod
     async def download_ofac_list(cls):
         async with aiohttp.ClientSession() as session:
             try:
                 async with session.get(OFAC_LIST_URL) as response:
                     response.raise_for_status()
                     text_content = await response.text()
-                    ofac_list = json.loads(text_content)
-                    ofac_file_path = cls.get_ofac_list_path()
-                    with open(ofac_file_path, "w") as f:
-                        json.dump(ofac_list, f, indent=2)
-                        f.write("\n")
-                    return
-            except Exception as e:
-                raise Exception(f"Error fetching OFAC list: {e}")
+                    try:
+                        ofac_list = json.loads(text_content)
+                    except json.JSONDecodeError as e:
+                        raise ValueError(f"Invalid JSON in OFAC list response: {e}")
+                    
+                    try:
+                        ofac_file_path = cls.get_ofac_list_path()
+                        with open(ofac_file_path, "w") as f:
+                            json.dump(ofac_list, f, indent=2)
+                            f.write("\n")
+                    except IOError as e:
+                        raise IOError(f"Failed to write OFAC list to file: {e}")
+                    return
+            except aiohttp.ClientError as e:
+                raise ConnectionError(f"Failed to fetch OFAC list: {e}")
pyinjective/ofac.json (1)

1-157: Consider implementing version tracking for OFAC list updates

To improve maintainability and auditability, consider adding:

  1. A version identifier in the JSON file
  2. A last updated timestamp
  3. A reference to the source OFAC list version

Example structure:

 [
+  {
+    "version": "1.7.2",
+    "last_updated": "2024-11-13",
+    "source_reference": "OFAC_SDN_LIST_20241113",
+    "addresses": [
       "0x01e2919679362dfbc9ee1644ba9c6da6d6245bb1",
       ...
     ]
+  }
 ]
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 9f7b8e3 and be9cc3f.

📒 Files selected for processing (3)
  • CHANGELOG.md (1 hunks)
  • pyinjective/ofac.json (1 hunks)
  • pyinjective/ofac.py (1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md

[duplication] ~6-~6: Possible typo: you repeated a word
Context: ...this file. ## [1.7.2] - 2024-11-13 ### Fixed - Fixed link to official ofac.json file - Ref...

(ENGLISH_WORD_REPEAT_RULE)

🔇 Additional comments (1)
pyinjective/ofac.py (1)

7-9: Verify the accessibility and validity of the new OFAC list URL

The URL structure has been updated to include additional path segments. Let's verify that the new URL is accessible and returns valid JSON data.

✅ Verification successful

URL change is valid but old URL is no longer accessible

The verification results show:

  • New URL is accessible (HTTP 200) and returns valid JSON with 156 entries
  • Old URL structure is no longer valid (returns invalid JSON)
  • The repository has been reorganized to use the new path structure with /refs/heads/master/json/ prefix

This confirms that the URL update in the code is necessary and correct, as it aligns with the new repository structure.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the new OFAC list URL is accessible and returns valid JSON

# Test 1: Check if the URL is accessible
echo "Testing URL accessibility..."
curl -s -f -I "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json" || echo "URL not accessible"

# Test 2: Verify JSON validity
echo "Testing JSON validity..."
curl -s "https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json" | jq . >/dev/null 2>&1 || echo "Invalid JSON"

# Test 3: Compare with the old URL structure to ensure no data loss
echo "Comparing with old URL structure..."
NEW_URL="https://raw.githubusercontent.com/InjectiveLabs/injective-lists/refs/heads/master/json/wallets/ofac.json"
OLD_URL="https://raw.githubusercontent.com/InjectiveLabs/injective-lists/master/wallets/ofac.json"

curl -s "$NEW_URL" | jq length
curl -s "$OLD_URL" | jq length

Length of output: 1844

Comment on lines 1 to 157
"0x58e8dcc13be9780fc42e8723d8ead4cf46943df2",
"0x5a14e72060c11313e38738009254a90968f58f51",
"0x5a7a51bfb49f190e5a6060a5bc6052ac14a3b59f",
"0x5cab7692d4e94096462119ab7bf57319726eed2a",
"0x5efda50f22d34f262c29268506c5fa42cb56a1ce",
"0x5f48c2a71b2cc96e3f0ccae4e39318ff0dc375b2",
"0x5f6c97c6ad7bdd0ae7e0dd4ca33a4ed3fdabd4d7",
"0x610b717796ad172b316836ac95a2ffad065ceab4",
"0x653477c392c16b0765603074f157314cc4f40c32",
"0x67d40ee1a85bf4a4bb7ffae16de985e8427b6b45",
"0x6be0ae71e6c41f2f9d0d1a3b8d0f75e6f6a0b46e",
"0x6bf694a291df3fec1f7e69701e3ab6c592435ae7",
"0x6f1ca141a28907f78ebaa64fb83a9088b02a8352",
"0x722122df12d4e14e13ac3b6895a86e84145b6967",
"0x723b78e67497e85279cb204544566f4dc5d2aca0",
"0x72a5843cc08275c8171e582972aa4fda8c397b2a",
"0x743494b60097a2230018079c02fe21a7b687eaa5",
"0x746aebc06d2ae31b71ac51429a19d54e797878e9",
"0x756c4628e57f7e7f8a459ec2752968360cf4d1aa",
"0x76d85b4c0fc497eecc38902397ac608000a06607",
"0x776198ccf446dfa168347089d7338879273172cf",
"0x77777feddddffc19ff86db637967013e6c6a116c",
"0x797d7ae72ebddcdea2a346c1834e04d1f8df102b",
"0x7db418b5d567a4e0e8c59ad71be1fce48f3e6107",
"0x7f19720a857f834887fc9a7bc0a0fbe7fc7f8102",
"0x7f367cc41522ce07553e823bf3be79a889debe1b",
"0x7ff9cfad3877f21d41da833e2f775db0569ee3d9",
"0x8281aa6795ade17c8973e1aedca380258bc124f9",
"0x833481186f16cece3f1eeea1a694c42034c3a0db",
"0x83e5bc4ffa856bb84bb88581f5dd62a433a25e0d",
"0x84443cfd09a48af6ef360c6976c5392ac5023a1f",
"0x8576acc5c05d6ce88f4e49bf65bdf0c62f91353c",
"0x8589427373d6d84e98730d7795d8f6f8731fda16",
"0x88fd245fedec4a936e700f9173454d1931b4c307",
"0x901bb9583b24d97e995513c6778dc6888ab6870e",
"0x910cbd523d972eb0a6f4cae4618ad62622b39dbf",
"0x931546d9e66836abf687d2bc64b30407bac8c568",
"0x94a1b5cdb22c43faab4abeb5c74999895464ddaf",
"0x94be88213a387e992dd87de56950a9aef34b9448",
"0x94c92f096437ab9958fc0a37f09348f30389ae79",
"0x961c5be54a2ffc17cf4cb021d863c42dacd47fc1",
"0x97b1043abd9e6fc31681635166d430a458d14f9c",
"0x983a81ca6fb1e441266d2fbcb7d8e530ac2e05a2",
"0x9ad122c22b14202b4490edaf288fdb3c7cb3ff5e",
"0x9c2bc757b66f24d60f016b6237f8cdd414a879fa",
"0x9f4cda013e354b8fc285bf4b9a60460cee7f7ea9",
"0xa0e1c89ef1a489c9c7de96311ed5ce5d32c20e4b",
"0xa160cdab225685da1d56aa342ad8841c3b53f291",
"0xa5c2254e4253490c54cef0a4347fddb8f75a4998",
"0xa60c772958a3ed56c1f15dd055ba37ac8e523a0d",
"0xa7e5d5a720f06526557c513402f2e6b5fa20b008",
"0xaeaac358560e11f52454d997aaff2c5731b6f8a6",
"0xaf4c0b70b2ea9fb7487c7cbb37ada259579fe040",
"0xaf8d1839c3c67cf571aa74b5c12398d4901147b3",
"0xb04e030140b30c27bcdfaafffa98c57d80eda7b4",
"0xb1c8094b234dce6e03f10a5b673c1d8c69739a00",
"0xb20c66c4de72433f3ce747b58b86830c459ca911",
"0xb541fc07bc7619fd4062a54d96268525cbc6ffef",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xba214c1c1928a32bffe790263e38b4af9bfcd659",
"0xbb93e510bbcd0b7beb5a853875f9ec60275cf498",
"0xc2a3829f459b3edd87791c74cd45402ba0a20be3",
"0xc455f7fd3e0e12afd51fba5c106909934d8a0e4a",
"0xca0840578f57fe71599d29375e16783424023357",
"0xcc84179ffd19a1627e79f8648d09e095252bc418",
"0xcee71753c9820f063b38fdbe4cfdaf1d3d928a80",
"0xd0975b32cea532eadddfc9c60481976e39db3472",
"0xd21be7248e0197ee08e0c20d4a96debdac3d20af",
"0xd47438c816c9e7f2e2888e060936a499af9582b3",
"0xd4b88df4d29f5cedd6857912842cff3b20c8cfa3",
"0xd5d6f8d9e784d0e26222ad3834500801a68d027d",
"0xd691f27f38b395864ea86cfc7253969b409c362d",
"0xd692fd2d0b2fbd2e52cfa5b5b9424bc981c30696",
"0xd82ed8786d7c69dc7e052f7a542ab047971e73d2",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xd8d7de3349ccaa0fde6298fe6d7b7d0d34586193",
"0xd90e2f925da726b50c4ed8d0fb90ad053324f31b",
"0xd96f2b1c14db8458374d9aca76e26c3d18364307",
"0xdcbeffbecce100cce9e4b153c4e15cb885643193",
"0xdd4c48c0b24039969fc16d1cdf626eab821d3384",
"0xdf231d99ff8b6c6cbf4e9b9a945cbacef9339178",
"0xdf3a408c53e5078af6e8fb2a85088d46ee09a61b",
"0xe1d865c3d669dcc8c57c8d023140cb204e672ee4",
"0xe7aa314c77f4233c18c6cc84384a9247c0cf367b",
"0xe950dc316b836e4eefb8308bf32bf7c72a1358ff",
"0xed6e0a7e4ac94d976eebfb82ccf777a3c6bad921",
"0xedc5d01286f99a066559f60a585406f3878a033e",
"0xefe301d259f525ca1ba74a7977b80d5b060b3cca",
"0xf3701f445b6bdafedbca97d1e477357839e4120d",
"0xf4b067dd14e95bab89be928c07cb22e3c94e0daa",
"0xf60dd140cff0706bae9cd734ac3ae76ad9ebc32a",
"0xf67721a2d8f736e75a49fdd7fad2e31d8676542a",
"0xf7b31119c2682c88d88d455dbb9d5932c65cf1be",
"0xfac583c0cf07ea434052c49115a4682172ab6b4f",
"0xfd8610d20aa15b7b2e3be39b396a1bc3516c7144",
"0xfec8a60023265364d066a1212fde3930f6ae8da7",
"0xffbac21a641dcfe4552920138d90f3638b3c9fba"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Address marked for removal is still present in the OFAC list

The verification revealed that the address 0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff that was supposed to be removed is still present in the list. Additionally, the script verification showed:

  • Total addresses: 156
  • All addresses follow valid Ethereum address format
  • No duplicate addresses found
🔗 Analysis chain

Verify OFAC list synchronization with official source

The JSON array is properly formatted and contains valid Ethereum addresses. However, given the security-critical nature of this file for compliance purposes, we should verify:

  1. The completeness of the list
  2. The accuracy of additions and removals
  3. The synchronization with the official OFAC SDN list

Please ensure that:

  1. The changes align with the latest official OFAC SDN list
  2. The implementation in ofac.py is updated to handle any new address patterns
  3. Comprehensive testing is performed to verify blocking functionality

Would you like me to help generate test cases for the updated list?

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the OFAC list changes and synchronization

# 1. Verify the removed addresses are not present
echo "Checking for removed addresses..."
for addr in "0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff"
do
    if grep -q "$addr" pyinjective/ofac.json; then
        echo "WARNING: Removed address $addr is still present"
    fi
done

# 2. Count total addresses for verification
echo "Total addresses in list:"
jq length pyinjective/ofac.json

# 3. Verify address format
echo "Checking address format..."
jq -r '.[]' pyinjective/ofac.json | while read -r addr; do
    if ! [[ $addr =~ ^0x[a-fA-F0-9]{40}$ ]]; then
        echo "Invalid address format: $addr"
    fi
done

# 4. Check for duplicates
echo "Checking for duplicates..."
jq -r '.[]' pyinjective/ofac.json | sort | uniq -d

Length of output: 14600

@aarmoa aarmoa merged commit 74a6433 into dev Nov 13, 2024
12 checks passed
@aarmoa aarmoa deleted the feat/sync_dev_with_v1_7_2 branch November 13, 2024 17:45
@coderabbitai coderabbitai bot mentioned this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants