SOR - Evaluate if Token still needs to differentiate address/wrapped

[brunoguerios]

Token attributes address and wrapped are used in the SOR/SDK to handle native assets and their wrapped addresses (e.g. ETH/WETH).

After so many changes, such as moving the SOR to the backend repo and also introducing the wethIsEth flag, it would interesting to evaluate if wrapped concept is still needed.

Removing that will likely simplify both SOR and SDK codebases since Token is used everywhere.

Note: this is likely more relevant for the SOR code, but would be nice to double check if SDK still uses it somewhere.

[mkflow27]

Hey @bruno Can we talk about this issue during our standup today. I have a writeup of my thoughts here (https://www.notion.so/Pr-to-check-if-Token-wrapped-27ae395e228180eaaab7cb00e5239fd7?source=copy_link) but tl;dr:

• the wrapped? optional constructor arguments is not used in the SOR at all. (Neither as part of getToken (which is native asset handling I think) nor the poolImplementations where the pool specific token is handled (extends the Token)

• Therefor I believe we can safely replace any token.wrapped with token.address pretty much. I have opened up a draft pr for this here.

The main question for me now is though that I believe simply replacing wrapped with addressdoes not simplify the SOR in any way I believe.

From the issue description I get that potentially changing the Token in the sdk could be a way forward. Depending on backwards compatibility requirements. I am thinking something like:

• Split the Token in the SDK into something like BaseToken and Token. Leave Token as is to not break backwards compatibility for other consumers, and then I could simply import BaseToken as Token in the SOR. I believe this approach would introduce the least changes in the codebase.

I think this could be a good approach for this issue. Let me know what you think

[brunoguerios]

I think your suggested changes make sense.

The main idea is to simplify the SOR codebase by removing the overhead of having to understand a concept that is not used there.
If we get to make the SDK codebase easier to understand and a bit more type safe, that's a plus.
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)?
Not completely sure of the impact this would cause on the SDK though.

Regarding backwards compatibility, I think all we need to worry about is the FE. If we're not breaking anything there, I'd say it's ok.
Open for different opinions in case the team thinks otherwise.

[mkflow27]

Copying from the pr on the backend

@brunoguerios I am slowly progressing through replacing references to Token with BaseToken (working name). Commenting here as I had to refactor to use functionality of the BaseToken here now instead of Token.

There are other opportunities for me to replace Token with BaseToken (think TokenAmount). However, if I do these changes in the sdk. (TokenAmount can either accept Token or BaseToken in the constructor I am getting alot of downstream build errors in the sdk.

So, the question here is:

Would a "perfect" refactor also need to change how TokenAmount is used?