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

Add logic to output ofac-sanctioned-digital-currency-addresses.json #81

Merged
merged 5 commits into from
Oct 17, 2023

Conversation

nvonpentz
Copy link
Member

@nvonpentz nvonpentz commented Aug 30, 2023

Generates a new file ofac-sanctioned-digital-currency-addresses.json, that looks like this:

{
   "addresses": [<address1>, <address2>]
}

It's just a flat list of addresses to not send transfers to, and includes every coin type (e.g. bitcoin, EVM, and Solana addresses are all included in the one array).

The list is generated by parsing the lists branch of this repo https://github.com/brave-intl/ofac-sanctioned-digital-currency-addresses/tree/lists. That repo has a daily job that scrapes the digital currency addresses from the OFAC website, and outputs them in sanctioned_addresses_*.json files at the top level of the github repo.

In order to fetch those files from our github repo, the code in this change uses the github API, which requires an authentication token GITHUB_TOKEN , which must be added as an environment variable to this repo.

That list is parsed clientside and checked prior to regular transfers and token transfers in brave-core in this pull request.

Security review - https://github.com/brave/reviews/issues/1273.

We don't know which chain IDs or coinTypes will be added by OFAC
upstream, and it's difficult given an arbitrary symbol to identify which
chain ID / CoinType it is. So it's easier to just put all the banned
addresses in a flat namespace. There's only 540 so it's not really a
performance hit to check all of them for an Ethereum transaction instead
of just the ETH ones.
GITHUB_TOKEN -> API_AUTH_TOKEN_GITHUB

The IT asked for a more descriptive name. Also, apparently you variables
cannot start with the prefix 'GITHUB'.
Copy link
Member

@onyb onyb left a comment

Choose a reason for hiding this comment

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

LGTM

@nvonpentz
Copy link
Member Author

Merging even though the build failed since the build failed due to a missing GITHUB_API_AUTH_TOKEN. That secret is set, I just think it's only exposed from the main branch.

@nvonpentz nvonpentz merged commit ec9e230 into main Oct 17, 2023
5 of 6 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.

4 participants