refactor: transfer to ICS20 first then burn #296
Conversation
| // approve ics20 contract to spend the tokens for `address_starknet_b` | ||
| { | ||
| let call_data = cairo_encoding.encode(&product![ | ||
| ics20_contract_address, | ||
| U256::from(transfer_quantity) | ||
| ])?; | ||
|
|
||
| let call = Call { | ||
| to: *ics20_token_address, | ||
| selector: selector!("approve"), | ||
| calldata: call_data, | ||
| }; | ||
|
|
||
| let execution = starknet_account_b.execute_v3(vec![call]); | ||
|
|
||
| let tx_hash = execution.send().await?.transaction_hash; | ||
|
|
||
| starknet_chain.poll_tx_response(&tx_hash).await?; | ||
| } |
There was a problem hiding this comment.
hmm. this is a very interesting ! So, the token owner can burn tokens from anyone's balance, but token owner can't transfer it to their own balance ?
There was a problem hiding this comment.
Good question. This got me yesterday too. Should add a check in the ERC20Mintable contract.
In the standard ERC-20, since the supply is fixed, minting and burning don’t matter, so ownership isn’t really a concern. But in our case, we’re working with an extended version of ERC20 where only the owner (ICS20) can mint and burn tokens. This part of code was originally written at the start of the project just to get the MVP working. Now that it looks like we have no choice but to go with our own ERC20 version, it's good to review the implementation there and ensure it’s fully secure by adding some unit tests.
In continuation of #275
Closes: #276