create, mint, burn, transfer erc20 tokens should have separate scopes

[rnbguy]

We should create, mint, burn, transfer an ERC20 token in separate steps. There shouldn't be any overlap between them in the ERC20 contract.

1. create should only happen with zero supply. This is mainly to set up the token metadata and initialize ownerships.
2. mint/burn should only happen to/from ICS20 contract. It shouldn't mint/burn to/from a non-ICS20 contract or account.
3. transfer should only happen between ICS20 contract and another non-ICS20 contract.

For example, the following mint_execute method has a create_token(name, amount) -- which calls the token constructor with non-zero initial supply.

ibc-starknet/cairo-contracts/packages/apps/src/transfer/components/transfer.cairo

Lines 671 to 688 in 1414bc8

	fn mint_execute(
	ref self: ComponentState<TContractState>,
	account: ContractAddress,
	denom: PrefixedDenom,
	amount: u256,
	) {
	let mut token = self.get_token(denom.key());
	if token.is_non_zero() {
	token.mint(get_contract_address(), amount);
	} else {
	let name = denom.base.hosted().unwrap();
	token = self.create_token(name, amount);
	self.record_ibc_token(denom, token.address);
	}
	token.transfer(account, amount);
	}

This should be separated by a constructor call with zero supply and a consecutive non-zero mint. So it should look something similar to this,

        fn mint_execute(
            ref self: ComponentState<TContractState>,
            account: ContractAddress,
            denom: PrefixedDenom,
            amount: u256,
        ) {
            let mut token = self.get_token(denom.key());
            if token.address.is_zero() {
                let name = denom.base.hosted().unwrap();
                token = self.create_token(name);
                self.record_ibc_token(denom, token.address);
            }

            token.mint(get_contract_address(), amount);
            token.transfer(account, amount);
        }

The current code is probably correct. But this suggestion is to separate concerns for security reasons and code coverage during testing. The context of minting tokens during a ERC20 constructor is different from minting tokens a second time (without going through the constructor).

Example for security concern: We found out that, ICS20 contract can burn tokens from a user wallet without user approval -- but it did require an approval when we separated that process into transfer-to-self and then burn-from-self (#296 (comment)).

Example for better code coverage: When we transfer tokens from Osmosis to Starknet for the first time, it uses syscall to deploy a new contract

ibc-starknet/cairo-contracts/packages/apps/src/transfer/erc20_call.cairo

Lines 51 to 54 in 1414bc8

	(name, symbol, decimals, amount, recipient, owner).serialize(ref call_data);
	let (address, _) = deploy_syscall(class_hash, salt, call_data.span(), false)
	.unwrap_syscall();

We have to perform another transfer of the same token to use the permissioned_mint.

ibc-starknet/cairo-contracts/packages/apps/src/transfer/erc20_call.cairo

Lines 69 to 71 in 1414bc8

	fn mint(self: @ERC20Contract, recipient: ContractAddress, amount: u256) {
	self.mintable_dispatcher().permissioned_mint(recipient, amount)
	}

[Farhad-Shabani]

@rnbguy, why this matters?

[rnbguy]

Sorry for not giving more context before. I updated my comment above.