feat(token-manager): add token manager for ITS #215

milapsheth · 2025-01-26T11:52:38Z

codecov-commenter · 2025-01-26T12:05:16Z

Codecov Report

Attention: Patch coverage is 90.56604% with 15 lines in your changes missing coverage. Please review.

Project coverage is 95.09%. Comparing base (03d1a48) to head (7ba8813).

Files with missing lines	Patch %	Lines
...llar-interchain-token-service/src/token_manager.rs	50.00%	12 Missing ⚠️
...llar-interchain-token-service/src/token_handler.rs	71.42%	2 Missing ⚠️
contracts/stellar-token-manager/src/contract.rs	95.45%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #215      +/-   ##
==========================================
- Coverage   95.17%   95.09%   -0.09%     
==========================================
  Files          61       65       +4     
  Lines        3423     3548     +125     
==========================================
+ Hits         3258     3374     +116     
- Misses        165      174       +9

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

ahramy · 2025-01-26T22:29:22Z

contracts/stellar-token-manager/src/tests/test.rs

+#[test]
+fn execute_fails_unauthorized() {
+    let (env, client, target) = setup();
+    let unauthorized = Address::generate(&env);


Suggested change

let unauthorized = Address::generate(&env);

let unauthorized_user = Address::generate(&env);

or not_owner ?

ahramy · 2025-01-26T22:29:30Z

contracts/stellar-token-manager/src/tests/test.rs

+    let unauthorized = Address::generate(&env);
+
+    assert_auth_err!(
+        unauthorized,


Suggested change

unauthorized,

unauthorized_user,

ahramy · 2025-01-26T23:09:14Z

contracts/stellar-interchain-token-service/src/contract.rs

@@ -35,6 +35,7 @@ const ITS_HUB_CHAIN_NAME: &str = "axelar";
 const PREFIX_INTERCHAIN_TOKEN_ID: &str = "its-interchain-token-id";
 const PREFIX_INTERCHAIN_TOKEN_SALT: &str = "interchain-token-salt";


Suggested change

const PREFIX_INTERCHAIN_TOKEN_SALT: &str = "interchain-token-salt";

const PREFIX_INTERCHAIN_TOKEN_SALT: &str = "its-interchain-token-salt";

this is mimicking EVM
https://github.com/axelarnetwork/interchain-token-service/blob/main/contracts/InterchainTokenFactory.sol#L27

ahramy · 2025-01-26T23:10:54Z

contracts/stellar-interchain-token-service/src/contract.rs

@@ -35,6 +35,7 @@ const ITS_HUB_CHAIN_NAME: &str = "axelar";
 const PREFIX_INTERCHAIN_TOKEN_ID: &str = "its-interchain-token-id";
 const PREFIX_INTERCHAIN_TOKEN_SALT: &str = "interchain-token-salt";
 const PREFIX_CANONICAL_TOKEN_SALT: &str = "canonical-token-salt";
+const PREFIX_TOKEN_MANAGER: &str = "token-manager-id";


Just to double check, in EVM the contract id looks like token-manager: https://github.com/axelarnetwork/interchain-token-service/blob/cc6ef7282e18c6b2842cbf1098b06161e38ea32e/contracts/token-manager/TokenManager.sol#L30

if we're going to use token-manager-id, then variable name could be PREFIX_TOKEN_MANAGER_ID for code readability

Unlike token id, there's no token manager id. Still I'm going to revisit the prefixes in a follow up PR

ahramy · 2025-01-26T23:13:41Z

contracts/stellar-interchain-token-service/src/contract.rs

@@ -35,6 +35,7 @@ const ITS_HUB_CHAIN_NAME: &str = "axelar";
 const PREFIX_INTERCHAIN_TOKEN_ID: &str = "its-interchain-token-id";
 const PREFIX_INTERCHAIN_TOKEN_SALT: &str = "interchain-token-salt";
 const PREFIX_CANONICAL_TOKEN_SALT: &str = "canonical-token-salt";
+const PREFIX_TOKEN_MANAGER: &str = "token-manager-id";
 const EXECUTE_WITH_TOKEN: &str = "execute_with_interchain_token";


EXECUTE_WITH_INTERCHAIN_TOKEN?

ahramy · 2025-01-26T23:26:41Z

contracts/stellar-interchain-token-service/src/contract.rs

        Self::set_token_id_config(
            env,
            token_id,
            TokenIdConfigValue {
-                token_address: deployed_address,
+                token_address,
+                token_manager: token_manager_address,


Suggested change

token_manager: token_manager_address,

token_manager_address,

ahramy · 2025-01-26T23:28:32Z

contracts/stellar-interchain-token-service/src/event.rs

+    pub token_manager_address: Address,
+    pub token_address: Address,


Suggested change

pub token_manager_address: Address,

pub token_address: Address,

pub token_address: Address,

pub token_manager_address: Address,

ahramy · 2025-01-26T23:35:21Z

contracts/stellar-interchain-token-service/src/token_handler.rs

-        TokenManagerType::LockUnlock => {
-            token.transfer(sender, &env.current_contract_address(), &amount)
-        }
+        TokenManagerType::LockUnlock => token.transfer(sender, &token_manager, &amount),
    }


Do we need to check unsupported token manager type? e.g.

_ => { return Err(ContractError::UnsupportedTokenManagerType); }

the enum is only defined for these two variants, so another option isn't possible

axelar-cgp-stellar/contracts/stellar-interchain-token-service/src/types.rs

Line 46 in f7daf9c

pub enum TokenManagerType {

ahramy · 2025-01-26T23:35:55Z

contracts/stellar-interchain-token-service/src/token_handler.rs

-            &env.current_contract_address(),
-            recipient,
-            &amount,
-        ),
    }


same question as above

ahramy · 2025-01-26T23:39:01Z

contracts/stellar-token-manager/src/contract.rs

+        let res: Val = env.invoke_contract(&contract, &func, args);
+
+        extend_instance_ttl(env);
+


Just to check, does event need to be emitted for the contract call?

Nah, unnecessary gas cost for normal operations

milapsheth added 6 commits January 25, 2025 21:12

feat(token-manager): add token manager for ITS

c8ab214

token manager dep

4bf8185

add wasm builds

93eac19

cleanup deps

4c0dd3f

switch minter to token manager

19d4bbc

fix tests

b739aa8

milapsheth requested a review from a team as a code owner January 26, 2025 11:52

cargo toml

f7daf9c

ahramy reviewed Jan 26, 2025

View reviewed changes

milapsheth added 2 commits January 27, 2025 02:27

address comments

e5c92ba

update tests

7ba8813

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(token-manager): add token manager for ITS #215

feat(token-manager): add token manager for ITS #215

milapsheth commented Jan 26, 2025 •

edited by jira bot

Loading

codecov-commenter commented Jan 26, 2025 •

edited

Loading

ahramy Jan 26, 2025

ahramy Jan 26, 2025

ahramy Jan 26, 2025

milapsheth Jan 27, 2025

ahramy Jan 26, 2025

milapsheth Jan 27, 2025

ahramy Jan 26, 2025

milapsheth Jan 27, 2025

ahramy Jan 26, 2025

ahramy Jan 26, 2025

ahramy Jan 26, 2025

milapsheth Jan 27, 2025

ahramy Jan 26, 2025

ahramy Jan 26, 2025

milapsheth Jan 27, 2025

	let unauthorized = Address::generate(&env);
	let unauthorized_user = Address::generate(&env);

		@@ -35,6 +35,7 @@ const ITS_HUB_CHAIN_NAME: &str = "axelar";
		const PREFIX_INTERCHAIN_TOKEN_ID: &str = "its-interchain-token-id";
		const PREFIX_INTERCHAIN_TOKEN_SALT: &str = "interchain-token-salt";

		pub token_manager_address: Address,
		pub token_address: Address,

		let res: Val = env.invoke_contract(&contract, &func, args);

		extend_instance_ttl(env);

feat(token-manager): add token manager for ITS #215

Are you sure you want to change the base?

feat(token-manager): add token manager for ITS #215

Conversation

milapsheth commented Jan 26, 2025 • edited by jira bot Loading

codecov-commenter commented Jan 26, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

milapsheth commented Jan 26, 2025 •

edited by jira bot

Loading

codecov-commenter commented Jan 26, 2025 •

edited

Loading