feat: manual blacklist - IL-264 #147
Conversation
WalkthroughThe changes in this pull request introduce a new job step in the GitHub Actions workflow for generating a restriction list, along with the addition of new constants for restricted countries and blacklisted addresses. A new script for generating the restriction list is also created, which updates relevant JSON files. The formatting of the country codes JSON file has been improved for readability. The modifications enhance the functionality related to restrictions and compliance management within the project. Changes
Possibly related PRs
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: 4
🧹 Outside diff range and nitpick comments (4)
ts-scripts/generateRestrictionList.ts (2)
1-3: Consider adding type definitions for imported data.For better type safety and code maintainability, consider defining and importing types for
restrictedCountriesandblacklistedAddresses.+import { RestrictedCountry } from './types/geo' +import { BlacklistedAddress } from './types/ofac' import { readJSONFile, updateJSONFile } from './helper/utils' -import { restrictedCountries } from './data/geo' -import { blacklistedAddresses } from './data/ofac' +import { restrictedCountries } from './data/geo' as RestrictedCountry[] +import { blacklistedAddresses } from './data/ofac' as BlacklistedAddress[]
1-21: Enhance security and compliance features.Given that this code handles sensitive compliance data (OFAC lists and geographical restrictions), consider implementing:
- Audit logging for all operations
- Cryptographic verification of data sources
- Backup mechanism before updates
- Rate limiting for file operations
- Configuration validation
Would you like assistance in implementing these security enhancements?
ts-scripts/generateOfacList.ts (1)
Line range hint
6-8: Enhance error handling and logging for compliance operations.Since this is compliance-critical code fetching data from an external source, consider these security improvements:
- Add explicit error handling for network failures
- Log changes for audit purposes
- Verify SSL certificate
Consider this enhancement:
+ import { Logger } from '@injectivelabs/utils' + + const logger = new Logger('OFAC-List-Generator') async function generateOFACList() { + try { const response = await new HttpClient( 'https://www.treasury.gov/ofac/downloads/sanctions/1.0/', + { + timeout: 10000, + rejectUnauthorized: true + } ).get<{}, { data: string }>('sdn_advanced.xml') + + logger.info('Successfully fetched OFAC data') // ... rest of the function ... + + const oldList = require('../json/wallets/ofac.json') + const newList = [...result, ...blacklistedAddresses] + logger.info(`OFAC list updated: ${oldList.length} → ${newList.length} addresses`) + } catch (error) { + logger.error(`Failed to update OFAC list: ${error.message}`) + throw error + } }.github/workflows/master.yaml (1)
70-75: Consider additional security measures for compliance data.Since this step handles sensitive compliance data (blacklists and restrictions):
- Consider adding a validation step after generation
- Add checksum verification for critical files
- Consider implementing approval gates for restriction list changes
Consider adding a validation step after generation:
- name: Generate restriction list uses: borales/actions-yarn@v4 with: cmd: restriction:generate dir: "ts-scripts" + - name: Validate restriction list + uses: borales/actions-yarn@v4 + with: + cmd: restriction:validate + dir: "ts-scripts"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
.github/workflows/master.yaml(1 hunks)json/geo/countries.json(1 hunks)ts-scripts/data/geo.ts(1 hunks)ts-scripts/data/ofac.ts(1 hunks)ts-scripts/generateOfacList.ts(2 hunks)ts-scripts/generateRestrictionList.ts(1 hunks)ts-scripts/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- json/geo/countries.json
- ts-scripts/data/geo.ts
- ts-scripts/data/ofac.ts
🔇 Additional comments (4)
ts-scripts/generateOfacList.ts (1)
3-3: Verify the structure and content of blacklistedAddresses.
Since this is compliance-related code dealing with OFAC restrictions, we should ensure the imported data structure is properly typed and validated.
✅ Verification successful
The imported blacklistedAddresses is properly typed and safely used
The implementation shows:
blacklistedAddressesis correctly typed asstring[]inofac.ts- It's properly used in
generateOFACList()to append additional addresses to the OFAC list - The addresses are sorted and deduplicated before being written to the JSON file
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the structure and content of blacklistedAddresses
# Check the source file content
echo "Checking source file content:"
cat ts-scripts/data/ofac.ts
# Check if TypeScript types are defined
echo -e "\nChecking for type definitions:"
rg -A 3 "type.*blacklistedAddresses" ts-scripts/data/ofac.ts
Length of output: 287
Script:
#!/bin/bash
# Let's verify how blacklistedAddresses is used in the codebase
# Check the implementation in generateOfacList.ts
echo "Checking generateOfacList.ts implementation:"
cat ts-scripts/generateOfacList.ts
# Find any tests or validation logic
echo -e "\nChecking for related test files:"
fd -e test.ts -e spec.ts generateOfacList
# Look for any TypeScript interfaces or types related to OFAC
echo -e "\nChecking for OFAC-related types:"
rg -i "interface|type.*ofac" --type ts
Length of output: 1769
ts-scripts/package.json (2)
25-25: LGTM! The script follows the established naming convention.
The new restriction:generate script is well-positioned and follows the project's established pattern of category:action naming convention.
Let's verify that the referenced TypeScript file exists:
✅ Verification successful
The generateRestrictionList.ts file exists at the expected location ✅
The script restriction:generate correctly references the TypeScript file located at ts-scripts/generateRestrictionList.ts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence of the generateRestrictionList.ts file
# Expected: File should exist in the ts-scripts directory
fd -t f "generateRestrictionList.ts" ts-scripts/
Length of output: 86
25-25: Consider adding this script to the tokens:generate chain.
Since this is a new restriction list generator, consider whether it should be part of the tokens:generate script chain, especially if the restrictions are token-related.
Let's check if other restriction-related tasks exist in the workflow:
.github/workflows/master.yaml (1)
70-75: Implementation looks good, but needs testing verification.
The new restriction list generation step follows the established pattern and is correctly positioned in the workflow. However, given this handles compliance-related data:
- Ensure the
restriction:generatescript has been thoroughly tested - Verify the generated restriction list format matches expected schema
- Confirm the changes are properly validated before being committed
Let's verify the related scripts and configurations:
✅ Verification successful
Implementation verified and properly handles sensitive compliance data
The restriction list generation implementation has been verified and shows:
- Proper handling of restricted countries list (
CU,IR,KP,SY) - Correct management of OFAC blacklisted addresses
- Secure file operations with proper error handling
- Consistent data structure in output files (JSON)
The implementation follows security best practices by:
- Using constant data sources
- Implementing proper file system checks
- Maintaining atomic updates
- Following established JSON schema
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the restriction generation setup
# Check if the script exists and is properly configured
echo "Checking restriction generation script..."
cat ts-scripts/package.json | grep "restriction:generate"
# Verify the JSON schema/structure
echo "Checking JSON files structure..."
find json/ -name "*.json" -type f -exec echo "Found JSON file: {}" \;
Length of output: 2738
Script:
#!/bin/bash
# Let's examine the restriction generation script and related files
# Check the implementation of generateRestrictionList
echo "Checking restriction list generator implementation..."
cat ts-scripts/generateRestrictionList.ts
# Look for any test files related to restrictions
echo -e "\nChecking for test files..."
fd -t f -e test.ts -e spec.ts restriction ts-scripts/
# Check if there's a restriction list output file
echo -e "\nLooking for restriction list output..."
fd -t f restriction -e json json/
Length of output: 976
Script:
#!/bin/bash
# Let's check the data sources and utility functions
# Check the data sources
echo "Checking geo data source..."
cat ts-scripts/data/geo.ts
echo -e "\nChecking OFAC data source..."
cat ts-scripts/data/ofac.ts
# Check utility functions
echo -e "\nChecking utility functions..."
cat ts-scripts/helper/utils.ts
# Check current content of output files
echo -e "\nChecking current countries.json..."
cat json/geo/countries.json
echo -e "\nChecking current ofac.json..."
cat json/wallets/ofac.json
Length of output: 12731
| updateOfacList() | ||
| generateRestrictedCountries() |
There was a problem hiding this comment.
Implement proper Promise handling for atomic operations.
The current implementation could lead to partial updates if one operation fails. Consider wrapping the executions in a main function with proper error handling.
-updateOfacList()
-generateRestrictedCountries()
+async function main() {
+ try {
+ await Promise.all([
+ updateOfacList(),
+ generateRestrictedCountries()
+ ])
+ console.log('Successfully updated restriction lists')
+ } catch (error) {
+ console.error('Failed to update restriction lists:', error)
+ process.exit(1)
+ }
+}
+
+main()📝 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.
| updateOfacList() | |
| generateRestrictedCountries() | |
| async function main() { | |
| try { | |
| await Promise.all([ | |
| updateOfacList(), | |
| generateRestrictedCountries() | |
| ]) | |
| console.log('Successfully updated restriction lists') | |
| } catch (error) { | |
| console.error('Failed to update restriction lists:', error) | |
| process.exit(1) | |
| } | |
| } | |
| main() |
| export const generateRestrictedCountries = async () => { | ||
| await updateJSONFile('json/geo/countries.json', restrictedCountries) | ||
| } |
There was a problem hiding this comment.
Add error handling and data validation.
The function should include:
- Error handling for file operations
- Validation of the
restrictedCountriesdata - Atomic write operation to prevent file corruption
export const generateRestrictedCountries = async () => {
- await updateJSONFile('json/geo/countries.json', restrictedCountries)
+ try {
+ if (!Array.isArray(restrictedCountries) || restrictedCountries.length === 0) {
+ throw new Error('Invalid restricted countries data')
+ }
+
+ await updateJSONFile('json/geo/countries.json', restrictedCountries)
+ } catch (error) {
+ console.error('Failed to generate restricted countries:', error)
+ throw error
+ }
}📝 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.
| export const generateRestrictedCountries = async () => { | |
| await updateJSONFile('json/geo/countries.json', restrictedCountries) | |
| } | |
| export const generateRestrictedCountries = async () => { | |
| try { | |
| if (!Array.isArray(restrictedCountries) || restrictedCountries.length === 0) { | |
| throw new Error('Invalid restricted countries data') | |
| } | |
| await updateJSONFile('json/geo/countries.json', restrictedCountries) | |
| } catch (error) { | |
| console.error('Failed to generate restricted countries:', error) | |
| throw error | |
| } | |
| } |
| export const updateOfacList = async () => { | ||
| const wallets = readJSONFile({ | ||
| path: 'json/wallets/ofac.json' | ||
| }) | ||
|
|
||
| await updateJSONFile('json/wallets/ofac.json', [ | ||
| ...wallets, | ||
| ...blacklistedAddresses | ||
| ]) | ||
| } |
There was a problem hiding this comment.
Enhance OFAC list update logic with validation and deduplication.
Current implementation has several potential issues:
- No deduplication of addresses
- Missing address format validation
- No error handling
- Potential memory issues with large lists
export const updateOfacList = async () => {
+ try {
+ if (!Array.isArray(blacklistedAddresses) || blacklistedAddresses.length === 0) {
+ throw new Error('Invalid blacklisted addresses data')
+ }
+
const wallets = readJSONFile({
path: 'json/wallets/ofac.json'
})
+ // Validate wallet addresses format
+ const isValidAddress = (addr: string) => /^0x[a-fA-F0-9]{40}$/.test(addr)
+ if (!blacklistedAddresses.every(isValidAddress)) {
+ throw new Error('Invalid wallet address format detected')
+ }
+
+ // Deduplicate addresses using Set
+ const uniqueAddresses = Array.from(new Set([
+ ...wallets,
+ ...blacklistedAddresses
+ ]))
+
await updateJSONFile('json/wallets/ofac.json', [
- ...wallets,
- ...blacklistedAddresses
+ ...uniqueAddresses
])
+ } catch (error) {
+ console.error('Failed to update OFAC list:', error)
+ throw error
+ }
}📝 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.
| export const updateOfacList = async () => { | |
| const wallets = readJSONFile({ | |
| path: 'json/wallets/ofac.json' | |
| }) | |
| await updateJSONFile('json/wallets/ofac.json', [ | |
| ...wallets, | |
| ...blacklistedAddresses | |
| ]) | |
| } | |
| export const updateOfacList = async () => { | |
| try { | |
| if (!Array.isArray(blacklistedAddresses) || blacklistedAddresses.length === 0) { | |
| throw new Error('Invalid blacklisted addresses data') | |
| } | |
| const wallets = readJSONFile({ | |
| path: 'json/wallets/ofac.json' | |
| }) | |
| // Validate wallet addresses format | |
| const isValidAddress = (addr: string) => /^0x[a-fA-F0-9]{40}$/.test(addr) | |
| if (!blacklistedAddresses.every(isValidAddress)) { | |
| throw new Error('Invalid wallet address format detected') | |
| } | |
| // Deduplicate addresses using Set | |
| const uniqueAddresses = Array.from(new Set([ | |
| ...wallets, | |
| ...blacklistedAddresses | |
| ])) | |
| await updateJSONFile('json/wallets/ofac.json', [ | |
| ...uniqueAddresses | |
| ]) | |
| } catch (error) { | |
| console.error('Failed to update OFAC list:', error) | |
| throw error | |
| } | |
| } |
| await updateJSONFile('json/wallets/ofac.json', [ | ||
| ...result.sort((a, b) => a.localeCompare(b)), | ||
| ...blacklistedAddresses | ||
| ]) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve data consistency and validation in the combined list.
The current implementation has several potential issues:
- No deduplication after combining the arrays
- Sorting is only applied to OFAC addresses, not the final list
- No validation of blacklisted addresses format
Consider applying this improvement:
await updateJSONFile('json/wallets/ofac.json', [
- ...result.sort((a, b) => a.localeCompare(b)),
- ...blacklistedAddresses
+ ...[...result, ...blacklistedAddresses]
+ .filter((address) => /^0x[a-fA-F0-9]{40}$/i.test(address))
+ .map((address) => address.toLowerCase())
+ .filter((address, index, self) => self.indexOf(address) === index)
+ .sort((a, b) => a.localeCompare(b))
])This change will:
- Validate all addresses using a regex pattern
- Convert all addresses to lowercase
- Remove duplicates from the combined list
- Sort the final list consistently
📝 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.
| await updateJSONFile('json/wallets/ofac.json', [ | |
| ...result.sort((a, b) => a.localeCompare(b)), | |
| ...blacklistedAddresses | |
| ]) | |
| await updateJSONFile('json/wallets/ofac.json', [ | |
| ...[...result, ...blacklistedAddresses] | |
| .filter((address) => /^0x[a-fA-F0-9]{40}$/i.test(address)) | |
| .map((address) => address.toLowerCase()) | |
| .filter((address, index, self) => self.indexOf(address) === index) | |
| .sort((a, b) => a.localeCompare(b)) | |
| ]) |
This PR
Summary by CodeRabbit
New Features
Improvements
Bug Fixes