Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: add ARC-1 proposal #690

Closed
wants to merge 1 commit into from
Closed

docs: add ARC-1 proposal #690

wants to merge 1 commit into from

Conversation

milapsheth
Copy link
Member

Description

Todos

  • Unit tests
  • Manual tests
  • Documentation
  • Connect epics/issues

Steps to Test

Expected Behaviour

Other Notes

@milapsheth milapsheth requested a review from a team as a code owner November 13, 2024 19:02
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.61%. Comparing base (ada9e92) to head (fa79831).
Report is 39 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #690   +/-   ##
=======================================
  Coverage   93.61%   93.61%           
=======================================
  Files         238      238           
  Lines       35565    35565           
=======================================
  Hits        33294    33294           
  Misses       2271     2271           

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


ITS Hub can use the source and destination token decimals to determine the appropriate scaling factor to apply when intercepting this message. ITS edge contract can populate the source token decimals by reading the token metadata, however the destination token decimals will need to be provided by the deployer.

Setting this incorrectly however can be lead to the ITS hub recording the deployment incorrectly, while the deployment tx on the destination ITS will fail due to mismatching decimals, which isn’t recoverable.
Copy link
Contributor

Choose a reason for hiding this comment

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

why will it fail on the destination? Because the decimals would be greater than the chain supports? Can't we check this in the ITS hub?

Copy link
Member Author

Choose a reason for hiding this comment

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

ITS hub doesn't know about the destination token's decimals. Solution 2 addresses this


Setting this incorrectly however can be lead to the ITS hub recording the deployment incorrectly, while the deployment tx on the destination ITS will fail due to mismatching decimals, which isn’t recoverable.

One potential mitigation is to allow the deployer to resubmit the msg but with a different destination decimals, and have ITS hub overwrite the recorded deployment. ITS Hub can allow this overwrite until the first Transfer msg is encountered to allow the user to fix their deployment. This adds some more complexity in ITS hub setup.
Copy link
Contributor

Choose a reason for hiding this comment

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

this overwrite until transfer logic sounds a bit complicated, would prefer to avoid. Can the successful deployment send back some sort of confirmation to the hub?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, a modified version of Solution 2 is to send a GMP call back on deploy token manager going through with the decimals of the destination token but I find it more complex to have a 2-stage setup in ITS Hub

2. ITS Hub calculates scaling factor and records deployment
3. Destination chain deploys token manager with the corresponding token address (extracted from the params)

#### Solution 2: Token Registration and Linking
Copy link
Contributor

Choose a reason for hiding this comment

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

this solution seems much better. Is there a downside to this approach?

Copy link
Member Author

Choose a reason for hiding this comment

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

Extra complexity and worse UX for a developer setting up their token link. Although, I think it's acceptable for supporting custom tokens. The common flow of deploying a token is still very simple

}

struct LinkToken {
bytes32 tokenId;
Copy link
Contributor

Choose a reason for hiding this comment

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

how/when is this tokenId generated?

@haiyizxx
Copy link
Contributor

haiyizxx commented Dec 4, 2024

moved to ARCs repo

@haiyizxx haiyizxx closed this Dec 4, 2024
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.

3 participants