fix(interchain-token-service): use destination max target decimals for scaling #679
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #679 +/- ##
=======================================
Coverage 93.52% 93.53%
=======================================
Files 237 237
Lines 35293 35290 -3
=======================================
- Hits 33009 33007 -2
+ Misses 2284 2283 -1 ☔ View full report in Codecov by Sentry. |
| @@ -366,7 +364,7 @@ fn destination_token_decimals( | |||
| { | |||
| source_chain_decimals | |||
| } else { | |||
| source_chain_config | |||
| destination_chain_config | |||
There was a problem hiding this comment.
Is this right? I thought the original code was right, where the source chain sets the default rounding value. Though I do think this makes a little more sense, where the destination chain sets the max decimals that it supports.
Do we need a check for the case where the source chain max uint is greater than the destination chain max uint, but the source decimals is less than the destination chain max target decimals? Otherwise, this computation could actually scale the decimals up, if max target decimals is greater than source decimals.
There was a problem hiding this comment.
How does it make more sense for the Sui config to decide it's OK to lose 12 decimals of ether from Ethereum? When we deploy a chain, we can't possibly know what tokens will be deployed to it and therefore calculate the max_target_decimals to make sense. We do have a rough idea of the tokens we could possibly deploy from that chain.
| @@ -366,7 +364,7 @@ fn destination_token_decimals( | |||
| { | |||
| source_chain_decimals | |||
| } else { | |||
| source_chain_config | |||
| destination_chain_config | |||
There was a problem hiding this comment.
How does it make more sense for the Sui config to decide it's OK to lose 12 decimals of ether from Ethereum? When we deploy a chain, we can't possibly know what tokens will be deployed to it and therefore calculate the max_target_decimals to make sense. We do have a rough idea of the tokens we could possibly deploy from that chain.
Description
Todos
Steps to Test
Expected Behaviour
Other Notes