Skip to content

Conversation

mkflow27
Copy link
Contributor

Copy link
Contributor Author

@mkflow27 mkflow27 left a comment

Choose a reason for hiding this comment

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

Since wrapped is only used for NativeAssets, maybe we can have a BaseToken (without wrapped) and NativeToken (which extends BaseToken and has wrapped as required)?

For now I have not made the change to the Token. Having the Token include an optional wrapped sounds ok to me, but can do a NativeToken as well. As you can see right now, the change from Token -> BaseToken created quite a big dif with many changes simply being the replacement & import. But there are also some functional changes. Which I need to thoroughly review again. They are:

  • src/entities/addLiquidityNested/addLiquidityNestedV2/validateInputs.ts
    Change: a.token.isUnderlyingEqual(NATIVE_ASSETS[chainId]) → a.token.isSameAddress(NATIVE_ASSETS[chainId].wrapped)

  • src/entities/utils/getValue.ts
    Change: a.token.isUnderlyingEqual(NATIVE_ASSETS[a.token.chainId]) → a.token.isSameAddress(NATIVE_ASSETS[a.token.chainId].wrapped)

  • src/entities/utils/replaceWrapped.ts
    Change: token.isUnderlyingEqual(NATIVE_ASSETS[chainId]) → token.isSameAddress(NATIVE_ASSETS[chainId].wrapped)

  • src/entities/swap/swaps/v2/auraBalSwaps/joinPool.ts
    Change: token.isUnderlyingEqual(NATIVE_ASSETS[ChainId.MAINNET]) → token.isSameAddress(NATIVE_ASSETS[ChainId.MAINNET].wrapped)

  • test/entities/swaps/v2/auraBalSwaps/auraBal.integration.test.ts
    Change:
    tokenIn.isUnderlyingEqual(NATIVE_ASSETS[ChainId.MAINNET]) → tokenIn.isSameAddress(NATIVE_ASSETS[ChainId.MAINNET].wrapped)
    tokenOut.isUnderlyingEqual(NATIVE_ASSETS[ChainId.MAINNET]) → tokenOut.isSameAddress(NATIVE_ASSETS[ChainId.MAINNET].wrapped)

  • test/lib/utils/removeLiquidityNestedHelper.ts
    Change: a.token.isSameAddress(NATIVE_ASSETS[chainId].wrapped) (this was already using isSameAddress)

  • test/v3/removeLiquidityBoosted/removeLiquidityPartialBoosted.integration.test.ts
    Change: a.token.isSameAddress(NATIVE_ASSETS[chainId].wrapped) (this was already using isSameAddress)

  • src/entities/removeLiquidityNested/removeLiquidityNestedV2/validateInputs.ts
    Change: a.token.isSameAddress(NATIVE_ASSETS[input.chainId].wrapped) (this was already using isSameAddress)

Besides these changes that could "get overlooked" The main changes are:

  • src/entities/baseToken.ts - New core class
  • src/entities/token.ts - Refactored to extend BaseToken
  • src/entities/tokenAmount.ts - Core type changes

I am interested in @MattPereira 's opinion of still needing the "Token" as it is currently defined with optional wrapped. or if we could potentially think about removing the optional wrapped from the Token and leave naming as is and introduce a NativeToken with wrapped being mandatory.

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.

1 participant