Add just some simple way to filter out tokens #398
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
anxolin
commented
Jun 3, 2024
| methodName: string, | ||
| outputFilePath: string, | ||
| tokensToReplace: Record<string, string | null> = {}, | ||
| ): Promise<void> { |
There was a problem hiding this comment.
it had too many arguments, so I decided to change from positional parameters to named parameters
anxolin
commented
Jun 3, 2024
| .tryAggregate(false, getTotalSupplyCalls(bridgedAddresses, provider)) | ||
| .then((results) => parseTotalSupplyResponses(results, bridgedAddresses, chainId)) | ||
|
|
||
| const tokens = tokenFilter ? allTokens.filter(tokenFilter) : allTokens |
There was a problem hiding this comment.
gist of the PR, the function receives now an optional parameter with the actual filter
shoom3301
reviewed
Jun 3, 2024
src/scripts/const/arbitrum.ts
Outdated
| * Future versions of the script should decide using objective on-chain data if a token is liquid or not. | ||
| */ | ||
| export const TOKENS_WITH_LIQUIDITY = [ | ||
| 'ARB', 'cbETH', 'GRT', 'LINK', 'USDC', 'USDT', 'WBTC', 'WETH', |
There was a problem hiding this comment.
Does it mean that we are going to have only 8 tradable tokens on Arbitrum?
Looks a bit poor
There was a problem hiding this comment.
We have liquidity for COW but it's not in the list
There was a problem hiding this comment.
no no, this is a placeholder for tomorrow.
Chen is working on the actual list. There will be many more
shoom3301
approved these changes
Jun 3, 2024
|
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
For Arbitrum launch we have one issue, most of the bridged tokens don't have liquidity.
This creates a bad user experience.
This PR allows the script generators to provide a filtering function that will have the last call on which token should be included and which don't.
In this simplistic implementation, I just allow to cherry-pick a bunch of tokens by token symbol. I know..., token symbols are not unique, but this comes from know lists so its not expected to have duplicates, and the configuration feels simpler to read if I can select the tokens to include by token symbol
Important
This PR should not be merged, and the token selection is not final. Chen is deciding on the final list of tokens to include, tomorrow he will provide them, so we can launch with a filtered list.
Edit 04/06/2024
I did a bunch of enhancements for the launch. Some of them need to be iterated later.
The goal was to reduce the list to a bunch of meaningful tokens (chen provided the list)
The problem, some of these were not in our list
Another problem was that these tokens provided by chen in a CSV didn't have a lot of the information (token name, decimals, etc)
This PR added: