refactor: split Token into BaseToken & Token

[mkflow27]

Related to: balancer/backend#830 (comment)

[mkflow27]

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.

[MattPereira]

im unfamiliar with the big picture here involving the SOR but from a code quality perspective removing optional wrapped property in favor of explicit NativeToken class does sound good as long as the knock on effects of refactor arent too onerous

[mkflow27]

im unfamiliar with the big picture here involving the SOR but from a code quality perspective removing optional wrapped property in favor of explicit NativeToken class does sound good as long as the knock on effects of refactor arent too onerous

Thanks @MattPereira . I was mostly trying to understand what implications the removal of the Token class would have for the frontend. As the sdk exports it.

[brunoguerios]

Hey Mk 👋
Since most changes are simply replacement and import, why don't you keep the variable named as Token instead of renaming them to BaseToken? It should cause less downstream changes to the FE as well.
Regarding updating isUnderlyingEqual to isSameAddress, I'd do it differently. If you keep isUnderlyingEqual implemented for NativeToken, all you'd need to do is call something like NATIVE_ASSETS[ChainId.MAINNET].isUnderlyingEqual(tokenIn).

[mkflow27]

Hey Mk 👋 Since most changes are simply replacement and import, why don't you keep the variable named as Token instead of renaming them to BaseToken? It should cause less downstream changes to the FE as well. Regarding updating isUnderlyingEqual to isSameAddress, I'd do it differently. If you keep isUnderlyingEqual implemented for NativeToken, all you'd need to do is call something like NATIVE_ASSETS[ChainId.MAINNET].isUnderlyingEqual(tokenIn).

Thanks @brunoguerios for your suggestions. I agree that keeping it named as token and reducing the functionality of that class would create a smaller dif. My main reasoning was that this could be an issue for other sdk users whenever they update the dependency (and are using the wrapped functionality of the Token). I am fine with keeping it named Token and reduce the class functionality.

• I'll refactor it that way.
• I'll also take a look at your NativeToken suggestion
  Thanks

[MattPereira]

The FE only uses SDK's Token class in in 3 files that are mostly irrelevant / test related

just give us a ping whenever this is released so we can make sure 🙏

[mkflow27]

Hey @brunoguerios. I might be missing something but with the changes to the Token (removing wrapped and isUnderlyingEqual) it won't be possible do use what you suggested as both needs to be NativeToken and I won't be able to pass in a Token as isUnderlyingEqual tried to access the wrapped property.

[brunoguerios]

Hey @brunoguerios. I might be missing something but with the changes to the Token (removing wrapped and isUnderlyingEqual) it won't be possible do use what you suggested as both needs to be NativeToken and I won't be able to pass in a Token as isUnderlyingEqual tried to access the wrapped property.

True - for a moment I thought it was comparing wrapped against address 😅 - we should be fine with isSameAddress comparison 👍