fix(its): validate token manager address from token id

[ahramy]

AXE-4705

• validate token manager address from token id
• coverage remains the same

----------------------------------------|----------|----------|----------|----------|----------------|
File                                    |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
----------------------------------------|----------|----------|----------|----------|----------------|
  InterchainTokenFactory.sol            |      100 |      100 |      100 |      100 |                |
  InterchainTokenService.sol            |      100 |      100 |      100 |      100 |                |

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.70%. Comparing base (b680103) to head (0b790eb).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #284   +/-   ##
=======================================
  Coverage   97.70%   97.70%           
=======================================
  Files          19       19           
  Lines         609      611    +2     
  Branches      125      126    +1     
=======================================
+ Hits          595      597    +2     
  Misses          5        5           
  Partials        9        9           

Flag	Coverage Δ
	97.70% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Foivos]

The reason why those checks were not there is because they cost gas, and the call will revert anyway if the TokenManager does not exist. Furthermore, the way you are doing it the tokenManagerAddress is calculated twice, once for the check, and once in the TokenHandler. If we think we need the checks for sanity I would add them in the TokenHandler after the tokenManagerAddress is calculated. This would also avoid codesize bloating the service. If you want to make the check in the service then the tokenManagerAddress should be passed to the TokenHandler instead of the tokenId to avoid recalculating it.

[ahramy]

The reason why those checks were not there is because they cost gas, and the call will revert anyway if the TokenManager does not exist. Furthermore, the way you are doing it the tokenManagerAddress is calculated twice, once for the check, and once in the TokenHandler. If we think we need the checks for sanity I would add them in the TokenHandler after the tokenManagerAddress is calculated. This would also avoid codesize bloating the service. If you want to make the check in the service then the tokenManagerAddress should be passed to the TokenHandler instead of the tokenId to avoid recalculating it.

Had offline discussion with Milap as well, I think we want to have a long-term solution for this. i.e. migration. Hence, closing this issue for now.