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/OFAC list #499

Merged
merged 4 commits into from
Sep 6, 2024
Merged

Feat/OFAC list #499

merged 4 commits into from
Sep 6, 2024

Conversation

bangjelkoski
Copy link
Collaborator

@bangjelkoski bangjelkoski commented Sep 6, 2024

Summary by CodeRabbit

  • New Features

    • Introduced a new script to fetch OFAC compliance data.
    • Added validation in the transaction broadcasting process to check Ethereum addresses against the OFAC list.
    • Created a new JSON file containing a list of Ethereum addresses for OFAC compliance.
  • Bug Fixes

    • Enhanced error handling in transaction broadcasting related to restricted addresses.
  • Documentation

    • Updated documentation to reflect new features and compliance processes.
  • Style

    • Minor formatting and stylistic adjustments across various files for improved readability.

Copy link

coderabbitai bot commented Sep 6, 2024

Walkthrough

The changes include the introduction of functionality for fetching and storing a list of Ethereum wallets from a specified URL to enhance compliance with OFAC regulations. A new script has been added to the build process to execute this fetching operation. Modifications have been made to various files to integrate checks against the OFAC list, ensuring that transactions involving restricted addresses are not processed.

Changes

File(s) Change Summary
etc/ofac.js Introduced functionality to fetch and store a JSON file of wallets from a URL. Added error handling and directory management.
packages/sdk-ts/package.json Added a new script fetch:ofac to fetch OFAC data.
packages/sdk-ts/src/broadcaster/MsgBroadcaster.ts Introduced a validation step to check if Ethereum addresses are in the OFAC list, enhancing transaction compliance.
packages/sdk-ts/src/core/modules/tx/broadcaster/MsgBroadcasterWithPk.ts Added compliance checks against a list of restricted wallets in transaction broadcasting methods.
packages/sdk-ts/src/index.ts Added an export statement to include all exports from the json module.
packages/sdk-ts/src/json/index.ts New file that re-exports the ofac module, simplifying imports.
packages/sdk-ts/src/json/ofac.ts New file defining a constant array of Ethereum wallet addresses related to OFAC sanctions.
packages/wallet-ts/src/json/ofac.json New file containing a JSON array of Ethereum addresses for OFAC compliance checks.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Build Script
    participant OFAC Fetch Script
    participant MsgBroadcaster

    User->>Build Script: Initiate Build
    Build Script->>OFAC Fetch Script: Run fetch:ofac
    OFAC Fetch Script-->>Build Script: Fetch Completed
    Build Script->>MsgBroadcaster: Proceed with Build
    MsgBroadcaster->>OFAC JSON: Check Ethereum Address
    OFAC JSON-->>MsgBroadcaster: Address Status
    MsgBroadcaster->>User: Transaction Processed or Blocked
Loading

🐇 In the meadow, I hop and play,
With wallets fetched, come what may!
OFAC's list, we check with care,
For safe transactions, everywhere!
A code so neat, it brings delight,
Hooray for changes, all feels right! 🌼


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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

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

socket-security bot commented Sep 6, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/base64-js@1.5.1 None 0 9.62 kB feross
npm/ieee754@1.2.1 None 0 6.8 kB feross

View full report↗︎

Copy link

@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: 3

Outside diff range, codebase verification and nitpick comments (1)
packages/exceptions/src/types/modules.ts (1)

24-24: Integration of new error category needed.

The Permissions entry in the ChainErrorModule enum is currently not used in the codebase beyond its definition. Please ensure that this new error category is integrated into the error handling mechanisms throughout the application to fully utilize its addition.

Analysis chain

Approve addition and suggest integration check.

The addition of the Permissions entry to the ChainErrorModule enum is appropriate. Ensure that this new error category is properly integrated with the error handling mechanisms throughout the application.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the new error category is used in error handling.

# Test: Search for the new error category usage. Expect: At least one occurrence.
rg --type typescript 'chain-permissions'

Length of output: 75


Script:

#!/bin/bash
# Description: Verify that the new error category is used in error handling.

# Test: Search for the new error category usage. Expect: At least one occurrence.
rg --type ts 'chain-permissions'

Length of output: 109

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between a6386da and 1fbd1e3.

Files selected for processing (22)
  • etc/ofac.js (1 hunks)
  • packages/exceptions/src/types/modules.ts (1 hunks)
  • packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.spec.ts (5 hunks)
  • packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts (7 hunks)
  • packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts (1 hunks)
  • packages/sdk-ts/src/client/chain/types/permissions.ts (1 hunks)
  • packages/sdk-ts/src/core/modules/permissions/index.ts (1 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgClaimVoucher.ts (4 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.spec.ts (1 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts (2 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.spec.ts (1 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.ts (2 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.spec.ts (2 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.ts (3 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.spec.ts (2 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.ts (3 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.spec.ts (1 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.ts (3 hunks)
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.ts (2 hunks)
  • packages/wallet-ts/package.json (1 hunks)
  • packages/wallet-ts/src/broadcaster/MsgBroadcaster.ts (5 hunks)
  • packages/wallet-ts/src/json/ofac.json (1 hunks)
Files skipped from review due to trivial changes (17)
  • packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.spec.ts
  • packages/sdk-ts/src/client/chain/grpc/ChainGrpcPermissionsApi.ts
  • packages/sdk-ts/src/client/chain/transformers/ChainGrpcPermissionsTransformer.ts
  • packages/sdk-ts/src/core/modules/permissions/index.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgClaimVoucher.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.spec.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgCreateNamespace.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.spec.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgDeleteNamespace.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.spec.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgRevokeNamespaceRoles.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.spec.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespace.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.spec.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateNamespaceRoles.ts
  • packages/sdk-ts/src/core/modules/permissions/msgs/MsgUpdateParams.ts
  • packages/wallet-ts/src/json/ofac.json
Additional comments not posted (4)
packages/sdk-ts/src/client/chain/types/permissions.ts (1)

1-56: Approve changes and suggest consistency check.

The updates to the type definitions align with the PR objectives and enhance clarity. Ensure that these changes are consistently applied across the application to avoid integration issues.

Verification successful

Verification successful: New type definitions are consistently used.

The transition to the new type definitions has been successfully implemented, with no occurrences of the old type found. The new type InjectivePermissionsV1Beta1Permissions is consistently used across the codebase, aligning with the PR objectives. No further action is required.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that the new type definitions are consistently used across the application.

# Test: Search for the old type usage. Expect: No occurrences.
rg --type typescript 'InjectivePermissionsV1Beta1Exchange'

Length of output: 93


Script:

#!/bin/bash
# Description: Verify the usage of new and old type definitions across the application.

# Find all TypeScript files and search for old type definitions.
fd --extension ts --exec rg 'InjectivePermissionsV1Beta1Exchange' {}

# Find all TypeScript files and search for new type definitions.
fd --extension ts --exec rg 'InjectivePermissionsV1Beta1Permissions' {}

Length of output: 523

packages/wallet-ts/package.json (2)

25-25: Review the modified build script.

The build script has been updated to include the fetch:ofac operation before proceeding with the existing build steps. This ensures that the OFAC data fetching is completed prior to the build, which is crucial for compliance. The use of yarn to chain these commands is appropriate and ensures that each step must complete successfully before the next begins.


22-22: Review the fetch:ofac script addition.

The addition of the fetch:ofac script is crucial for ensuring compliance with OFAC regulations. The script uses the --experimental-fetch flag, which enables experimental fetch capabilities in Node.js. This is appropriate for making network requests directly from a Node.js script. However, ensure that the relative path ./../../etc/ofac.js correctly points to the intended script file.

Verification successful

Path to ofac.js is correct.

The relative path ./../../etc/ofac.js specified in the fetch:ofac script correctly points to the intended script file. The file exists at the specified location.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the existence of the ofac.js script at the specified path.
fd --type file --full-path 'etc/ofac.js'

Length of output: 52

packages/wallet-ts/src/broadcaster/MsgBroadcaster.ts (1)

61-61: Review the import of the OFAC JSON data.

The import of ofac.json introduces external compliance data into the transaction processing logic. Ensure that the relative path ./../json/ofac.json correctly points to the JSON file containing the OFAC list. This is crucial for the functionality to work as expected.

Verification successful

Import path for OFAC JSON data is correct.

The ofac.json file exists at the specified path, and the import path in MsgBroadcaster.ts is correct. The functionality should work as expected.

  • File: packages/wallet-ts/src/json/ofac.json
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify the existence of the ofac.json file at the specified path.
fd --type file --full-path 'src/json/ofac.json'

Length of output: 85

Comment on lines +5 to +21
const storeJsonFile = (path, data) => {
const dirPath = dirname(path)

if (!existsSync(dirPath)) {
try {
mkdirSync(dirPath, { recursive: true })
} catch (e) {
console.log('❌❌❌ Error creating directory', e)
}
}

try {
writeFileSync(path, data)
} catch (e) {
console.error(`❌❌❌ Error creating JSON file: ${path}`, e)
}
}
Copy link

Choose a reason for hiding this comment

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

Enhance data handling and error messages in storeJsonFile.

The function should ensure that data is properly formatted as JSON before writing. Additionally, the error messages could be more descriptive to aid in debugging. Consider these changes:

- writeFileSync(path, data)
+ writeFileSync(path, JSON.stringify(data, null, 2))  // Adds indentation for readability

- console.log('❌❌❌ Error creating directory', e)
+ console.error('Error creating directory at path:', dirPath, e)

- console.error(`❌❌❌ Error creating JSON file: ${path}`, e)
+ console.error('Error writing to JSON file at path:', path, e)
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.

Suggested change
const storeJsonFile = (path, data) => {
const dirPath = dirname(path)
if (!existsSync(dirPath)) {
try {
mkdirSync(dirPath, { recursive: true })
} catch (e) {
console.log('❌❌❌ Error creating directory', e)
}
}
try {
writeFileSync(path, data)
} catch (e) {
console.error(`❌❌❌ Error creating JSON file: ${path}`, e)
}
}
const storeJsonFile = (path, data) => {
const dirPath = dirname(path)
if (!existsSync(dirPath)) {
try {
mkdirSync(dirPath, { recursive: true })
} catch (e) {
console.error('Error creating directory at path:', dirPath, e)
}
}
try {
writeFileSync(path, JSON.stringify(data, null, 2)) // Adds indentation for readability
} catch (e) {
console.error('Error writing to JSON file at path:', path, e)
}
}

etc/ofac.js Outdated
Comment on lines 23 to 37
const fetchOfac = async () => {
const response = await fetch(
'https://raw.githubusercontent.com/InjectiveLabs/injective-lists/master/wallets/ofac.json',
)
const wallets = await response.text()

try {
storeJsonFile(`src/json/ofac.json`, wallets)

console.log(`✅✅✅ OFAC list fetched`)
} catch (e) {
console.error(`❌❌❌ OFAC list NOT fetched`)
throw e
}
}
Copy link

Choose a reason for hiding this comment

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

Improve error handling and logging in fetchOfac.

While the function generally handles errors, it could benefit from more detailed error logging and potentially retry logic for network failures. Consider these enhancements:

- console.error(`❌❌❌ OFAC list NOT fetched`)
+ console.error('Failed to fetch OFAC list from URL:', e)

+ // Optional: Implement retry logic
+ let attempts = 0;
+ while (attempts < 3) {
+   try {
+     const response = await fetch('https://raw.githubusercontent.com/InjectiveLabs/injective-lists/master/wallets/ofac.json');
+     if (!response.ok) throw new Error(`HTTP status ${response.status}`);
+     break;  // Success
+   } catch (err) {
+     if (attempts >= 2) throw err;  // Rethrow after last attempt
+     attempts++;
+     console.log('Retrying fetch...', attempts);
+   }
+ }

Committable suggestion was skipped due to low confidence.

Comment on lines 148 to 152
if (ofac.includes(txWithAddresses.ethereumAddress)) {
throw new GeneralException(
new Error('You cannot execute this transaction'),
)
}
Copy link

Choose a reason for hiding this comment

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

Review the repeated OFAC checks in transaction broadcasting methods.

The OFAC list checks are implemented in multiple methods (broadcast, broadcastV2, broadcastOld, broadcastWithFeeDelegation) to prevent transactions involving restricted addresses. While the checks are crucial for compliance, the repeated code suggests a potential for refactoring to centralize this logic, which would improve maintainability and reduce the risk of errors in updates.

Consider refactoring the OFAC check into a separate method that can be called by these broadcasting methods. This would centralize the compliance logic and reduce code duplication.

Example refactor:

private checkOFACCompliance(address: string): void {
  if (ofac.includes(address)) {
    throw new GeneralException(new Error('You cannot execute this transaction'));
  }
}

// Usage in broadcasting methods:
this.checkOFACCompliance(txWithAddresses.ethereumAddress);

Also applies to: 182-186, 216-220, 247-251

@bangjelkoski bangjelkoski marked this pull request as draft September 6, 2024 14:48
@bangjelkoski bangjelkoski marked this pull request as ready for review September 6, 2024 17:05
Copy link

@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: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 1fbd1e3 and 0f4bdb1.

Files selected for processing (7)
  • etc/ofac.js (1 hunks)
  • packages/sdk-ts/package.json (1 hunks)
  • packages/sdk-ts/src/core/modules/tx/broadcaster/MsgBroadcasterWithPk.ts (4 hunks)
  • packages/sdk-ts/src/index.ts (1 hunks)
  • packages/sdk-ts/src/json/index.ts (1 hunks)
  • packages/sdk-ts/src/json/ofac.ts (1 hunks)
  • packages/wallet-ts/src/broadcaster/MsgBroadcaster.ts (5 hunks)
Files skipped from review due to trivial changes (2)
  • packages/sdk-ts/src/index.ts
  • packages/sdk-ts/src/json/index.ts
Files skipped from review as they are similar to previous changes (2)
  • etc/ofac.js
  • packages/wallet-ts/src/broadcaster/MsgBroadcaster.ts
Additional comments not posted (1)
packages/sdk-ts/package.json (1)

22-22: Verify the use of experimental features and relative path in the new script.

The script "fetch:ofac" uses the --experimental-fetch flag, which might indicate reliance on unstable features. Verify that this is acceptable for production use. Additionally, the relative path used (./../../etc/ofac.js) assumes a specific working directory structure. Confirm that this path is correct in all expected deployment scenarios.

Comment on lines +1 to +47
export const ofacWallets = [
"0x179f48c78f57a3a78f0608cc9197b8972921d1d2",
"0x1967d8af5bd86a497fb3dd7899a020e47560daaf",
"0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",
"0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",
"0x1da5821544e25c636c1417ba96ade4cf6d2f9b5a",
"0x2f389ce8bd8ff92de3402ffce4691d17fc4f6535",
"0x2f389ce8bd8ff92de3402ffce4691d17fc4f6535",
"0x2f50508a8a3d323b91336fa3ea6ae50e55f32185",
"0x308ed4b7b49797e1a98d3818bff6fe5385410370",
"0x3cbded43efdaf0fc77b9c55f6fc9988fcc9b757d",
"0x3efa30704d2b8bbac821307230376556cf8cc39e",
"0x48549a34ae37b12f6a30566245176994e17c6b4a",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x530a64c0ce595026a4a556b703644228179e2d57",
"0x5512d943ed1f7c8a43f3435c85f7ab68b30121b0",
"0x5a7a51bfb49f190e5a6060a5bc6052ac14a3b59f",
"0x5f48c2a71b2cc96e3f0ccae4e39318ff0dc375b2",
"0x6be0ae71e6c41f2f9d0d1a3b8d0f75e6f6a0b46e",
"0x6f1ca141a28907f78ebaa64fb83a9088b02a8352",
"0x746aebc06d2ae31b71ac51429a19d54e797878e9",
"0x77777feddddffc19ff86db637967013e6c6a116c",
"0x797d7ae72ebddcdea2a346c1834e04d1f8df102b",
"0x8576acc5c05d6ce88f4e49bf65bdf0c62f91353c",
"0x901bb9583b24d97e995513c6778dc6888ab6870e",
"0x961c5be54a2ffc17cf4cb021d863c42dacd47fc1",
"0x97b1043abd9e6fc31681635166d430a458d14f9c",
"0x9c2bc757b66f24d60f016b6237f8cdd414a879fa",
"0x9f4cda013e354b8fc285bf4b9a60460cee7f7ea9",
"0xa7e5d5a720f06526557c513402f2e6b5fa20b008",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xc455f7fd3e0e12afd51fba5c106909934d8a0e4a",
"0xca0840578f57fe71599d29375e16783424023357",
"0xd0975b32cea532eadddfc9c60481976e39db3472",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xe1d865c3d669dcc8c57c8d023140cb204e672ee4",
"0xe7aa314c77f4233c18c6cc84384a9247c0cf367b",
"0xed6e0a7e4ac94d976eebfb82ccf777a3c6bad921",
"0xf3701f445b6bdafedbca97d1e477357839e4120d",
"0xfac583c0cf07ea434052c49115a4682172ab6b4f",
"0xfec8a60023265364d066a1212fde3930f6ae8da7",
"0xffbac21a641dcfe4552920138d90f3638b3c9fba"
Copy link

Choose a reason for hiding this comment

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

Remove duplicate entries in the OFAC wallet list.

The list contains duplicate Ethereum wallet addresses, which could lead to inefficiencies in processing. Consider using a Set to ensure all addresses are unique.

Here's a suggested refactor to ensure uniqueness:

-export const ofacWallets = [
+export const ofacWallets = Array.from(new Set([
   "0x179f48c78f57a3a78f0608cc9197b8972921d1d2",
   "0x1967d8af5bd86a497fb3dd7899a020e47560daaf",
   "0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",
   "0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",  // Duplicate
   "0x1da5821544e25c636c1417ba96ade4cf6d2f9b5a",
   ...
   "0xfec8a60023265364d066a1212fde3930f6ae8da7",
   "0xffbac21a641dcfe4552920138d90f3638b3c9fba"
-]
+]));
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.

Suggested change
export const ofacWallets = [
"0x179f48c78f57a3a78f0608cc9197b8972921d1d2",
"0x1967d8af5bd86a497fb3dd7899a020e47560daaf",
"0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",
"0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",
"0x1da5821544e25c636c1417ba96ade4cf6d2f9b5a",
"0x2f389ce8bd8ff92de3402ffce4691d17fc4f6535",
"0x2f389ce8bd8ff92de3402ffce4691d17fc4f6535",
"0x2f50508a8a3d323b91336fa3ea6ae50e55f32185",
"0x308ed4b7b49797e1a98d3818bff6fe5385410370",
"0x3cbded43efdaf0fc77b9c55f6fc9988fcc9b757d",
"0x3efa30704d2b8bbac821307230376556cf8cc39e",
"0x48549a34ae37b12f6a30566245176994e17c6b4a",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x530a64c0ce595026a4a556b703644228179e2d57",
"0x5512d943ed1f7c8a43f3435c85f7ab68b30121b0",
"0x5a7a51bfb49f190e5a6060a5bc6052ac14a3b59f",
"0x5f48c2a71b2cc96e3f0ccae4e39318ff0dc375b2",
"0x6be0ae71e6c41f2f9d0d1a3b8d0f75e6f6a0b46e",
"0x6f1ca141a28907f78ebaa64fb83a9088b02a8352",
"0x746aebc06d2ae31b71ac51429a19d54e797878e9",
"0x77777feddddffc19ff86db637967013e6c6a116c",
"0x797d7ae72ebddcdea2a346c1834e04d1f8df102b",
"0x8576acc5c05d6ce88f4e49bf65bdf0c62f91353c",
"0x901bb9583b24d97e995513c6778dc6888ab6870e",
"0x961c5be54a2ffc17cf4cb021d863c42dacd47fc1",
"0x97b1043abd9e6fc31681635166d430a458d14f9c",
"0x9c2bc757b66f24d60f016b6237f8cdd414a879fa",
"0x9f4cda013e354b8fc285bf4b9a60460cee7f7ea9",
"0xa7e5d5a720f06526557c513402f2e6b5fa20b008",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xc455f7fd3e0e12afd51fba5c106909934d8a0e4a",
"0xca0840578f57fe71599d29375e16783424023357",
"0xd0975b32cea532eadddfc9c60481976e39db3472",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xe1d865c3d669dcc8c57c8d023140cb204e672ee4",
"0xe7aa314c77f4233c18c6cc84384a9247c0cf367b",
"0xed6e0a7e4ac94d976eebfb82ccf777a3c6bad921",
"0xf3701f445b6bdafedbca97d1e477357839e4120d",
"0xfac583c0cf07ea434052c49115a4682172ab6b4f",
"0xfec8a60023265364d066a1212fde3930f6ae8da7",
"0xffbac21a641dcfe4552920138d90f3638b3c9fba"
export const ofacWallets = Array.from(new Set([
"0x179f48c78f57a3a78f0608cc9197b8972921d1d2",
"0x1967d8af5bd86a497fb3dd7899a020e47560daaf",
"0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",
"0x19aa5fe80d33a56d56c78e82ea5e50e5d80b4dff",
"0x1da5821544e25c636c1417ba96ade4cf6d2f9b5a",
"0x2f389ce8bd8ff92de3402ffce4691d17fc4f6535",
"0x2f389ce8bd8ff92de3402ffce4691d17fc4f6535",
"0x2f50508a8a3d323b91336fa3ea6ae50e55f32185",
"0x308ed4b7b49797e1a98d3818bff6fe5385410370",
"0x3cbded43efdaf0fc77b9c55f6fc9988fcc9b757d",
"0x3efa30704d2b8bbac821307230376556cf8cc39e",
"0x48549a34ae37b12f6a30566245176994e17c6b4a",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x4f47bc496083c727c5fbe3ce9cdf2b0f6496270c",
"0x530a64c0ce595026a4a556b703644228179e2d57",
"0x5512d943ed1f7c8a43f3435c85f7ab68b30121b0",
"0x5a7a51bfb49f190e5a6060a5bc6052ac14a3b59f",
"0x5f48c2a71b2cc96e3f0ccae4e39318ff0dc375b2",
"0x6be0ae71e6c41f2f9d0d1a3b8d0f75e6f6a0b46e",
"0x6f1ca141a28907f78ebaa64fb83a9088b02a8352",
"0x746aebc06d2ae31b71ac51429a19d54e797878e9",
"0x77777feddddffc19ff86db637967013e6c6a116c",
"0x797d7ae72ebddcdea2a346c1834e04d1f8df102b",
"0x8576acc5c05d6ce88f4e49bf65bdf0c62f91353c",
"0x901bb9583b24d97e995513c6778dc6888ab6870e",
"0x961c5be54a2ffc17cf4cb021d863c42dacd47fc1",
"0x97b1043abd9e6fc31681635166d430a458d14f9c",
"0x9c2bc757b66f24d60f016b6237f8cdd414a879fa",
"0x9f4cda013e354b8fc285bf4b9a60460cee7f7ea9",
"0xa7e5d5a720f06526557c513402f2e6b5fa20b008",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xb6f5ec1a0a9cd1526536d3f0426c429529471f40",
"0xc455f7fd3e0e12afd51fba5c106909934d8a0e4a",
"0xca0840578f57fe71599d29375e16783424023357",
"0xd0975b32cea532eadddfc9c60481976e39db3472",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xd882cfc20f52f2599d84b8e8d58c7fb62cfe344b",
"0xe1d865c3d669dcc8c57c8d023140cb204e672ee4",
"0xe7aa314c77f4233c18c6cc84384a9247c0cf367b",
"0xed6e0a7e4ac94d976eebfb82ccf777a3c6bad921",
"0xf3701f445b6bdafedbca97d1e477357839e4120d",
"0xfac583c0cf07ea434052c49115a4682172ab6b4f",
"0xfec8a60023265364d066a1212fde3930f6ae8da7",
"0xffbac21a641dcfe4552920138d90f3638b3c9fba"
]));

Comment on lines +110 to +114
if (ofacWallets.includes(privateKey.toHex())) {
throw new GeneralException(
new Error('You cannot execute this transaction'),
)
}
Copy link

Choose a reason for hiding this comment

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

Refactor repeated OFAC checks and improve error messaging.

The checks against the ofacWallets list are repeated in multiple methods. Consider refactoring this logic into a separate method to reduce code duplication and improve maintainability. Additionally, the error message "You cannot execute this transaction" could be more descriptive, perhaps specifying that the transaction is blocked due to OFAC compliance.

Here's a suggested refactor:

+ private checkOfacCompliance() {
+   const walletHex = this.privateKey.toHex();
+   if (ofacWallets.includes(walletHex)) {
+     throw new GeneralException(
+       new Error('Transaction blocked due to OFAC compliance restrictions.'),
+     );
+   }
+ }

- if (ofacWallets.includes(privateKey.toHex())) {
+ this.checkOfacCompliance();

Also applies to: 132-136, 188-192

@ThomasRalee ThomasRalee merged commit 2ca3eb5 into dev Sep 6, 2024
3 checks passed
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