docs: add arc-2 proposal

[cjcobb23]

No description provided.

[milapsheth]

Suggested change

	supply: Option<Uint256>, // None if not tracked for this chain
	supply: TokenSupply,

perhaps, so usage is clear

[cgorenflo]

I also prefer a dedicated enum. This is a public interface, so I want to convey clearly the meaning of parameters, and

enum TokenSupply{
   Tracked(uint256),
   Untracked
}

is absolutely straight forward about what it's doing

[milapsheth]

this isn't being stored in the state

[cgorenflo]

Next steps: seems like there is still quite a bit of confusion around deployment types and managers

[milapsheth]

Suggested change

	deployed_chain: ChainNameRaw,
	origin_chain: ChainNameRaw,
	token_id: TokenId,
	decimals: u8,
	chain: ChainNameRaw,
	token_id: TokenId,
	decimals: u8,
	origin_chain: ChainNameRaw,

maybe chain is sufficient with a different field order

[cgorenflo]

instance_chain? I'm all for concise naming, but chain might be a bit too confusing

[cjcobb23]

i think chain is too vague when there are two chain fields in this struct. instance_chain is fine, or target_chain

[milapsheth]

Suggested change

	RegisterToken {
	RegisterTokenInstance {

avoids confusion with the RegisterToken ITS message type

[haiyizxx]

I feel like the name needs to convey the purpose that link p2p token to the hub. RegisterTokenInstance is still opaque

[cgorenflo]

This command is very specific for this purpose and will be obsolete once we have migrated all tokens, right? So why not literally go with something like MigrateP2pTokenInstance?

[cjcobb23]

I don't like the Migrate prefix, because the command itself is not actually migrating anything. Would prefer RegisterP2pTokenInstance

[milapsheth]

Suggested change

	be registered with the hub and to be processed through the hub. At the edge contracts, these tokens will need to be
	migrated from p2p mode to hub mode.
	be registered with the hub and to be processed through the hub. After the tokens are registered on the ITS hub, the ITS edge contracts can switch their connection from p2p to hub mode.

The ITS tokens themselves don't need any migration on edge contracts, just need to switch connection type

[milapsheth]

Suggested change

	Only the EVM ITS edge contracts using the Axelar consensus protocol are using p2p mode in production.

just to clarify that this doesn't affect Amplifier deployments

[milapsheth]

allow providing a list of token instances so batching doesn't require multiple proposals even if governance is used

[cgorenflo]

I understand the need for batching, but as an implementation approach, I'd rather we have RegisterToken (or whatever we're going to call it) and then create a command RegisterTokens that takes a vector of RegisterToken messages. I'm fine adding batching to goals though

[milapsheth]

hmm. i'm fine with having a separate method to update supply so both implementations are cleaner

[milapsheth]

i think we can validate these values off-chain so we don't need to overwrite. the supply is the trickier one.

[haiyizxx]

override the supply is an interesting idea, we still need a clearer plan about how to set the accurate supply

[haiyizxx]

I feel like the name needs to convey the purpose that link p2p token to the hub. RegisterTokenInstance is still opaque

[haiyizxx]

override the supply is an interesting idea, we still need a clearer plan about how to set the accurate supply

[haiyizxx]

can you explain in doc why the command will fails in these 3 scenarios?

[cgorenflo]

I'd like to iterate over the document some more, but not necessarily in this initial PR. I've marked comments as "next steps" that I want to see marked as open questions in the document, but that can be worked on in the next iteration instead of indefinitely blocking the merge

[cgorenflo]

This command is very specific for this purpose and will be obsolete once we have migrated all tokens, right? So why not literally go with something like MigrateP2pTokenInstance?

[cgorenflo]

I understand the need for batching, but as an implementation approach, I'd rather we have RegisterToken (or whatever we're going to call it) and then create a command RegisterTokens that takes a vector of RegisterToken messages. I'm fine adding batching to goals though

[cgorenflo]

instance_chain? I'm all for concise naming, but chain might be a bit too confusing

[cgorenflo]

I also prefer a dedicated enum. This is a public interface, so I want to convey clearly the meaning of parameters, and

enum TokenSupply{
   Tracked(uint256),
   Untracked
}

is absolutely straight forward about what it's doing

[cgorenflo]

Next steps: seems like there is still quite a bit of confusion around deployment types and managers

[cgorenflo]

Next steps: This seems like an almost impossible task, especially with governance delay. I think we need to get stakeholder input on what is an acceptable solution

[cgorenflo]

Suggested change

	The command will fail if the token instance already exists with a different decimals, origin_chain or
	The command will fail if the token instance already exists with different decimals, origin_chain or

[cgorenflo]

I think once we called the migration once, if it went through, any adjustment should be done with a universal admin/governance function that works for all tokens, not just migrated ones. I don't like to treat them separately ones they are known to the hub

[cgorenflo]

Next steps: describe this rescue functionality and what happens to transactions to tokens with failed migrations (i.e. migrated but wrong data)

[cgorenflo]

is that always true, i.e. are there no deployed p2p tokens that we cannot track (e.g. due to existing minters)?
If it is, can we codify this instead of allowing an input that will always fail?

[cjcobb23]

Actually, I think there must be deployed p2p tokens with custom minters, so we probably should fail this case. @milapsheth

[cgorenflo]

Next steps: Can we mitigate this? Do we have a plan to help with the information gathering?