Hedera ITS / Diff #1
Conversation
- add small doc as guide for testing via Hedera localnet
- check isMinter via token manager instead of the InterchainToken contract
There was a problem hiding this comment.
Pull Request Overview
This PR makes significant updates to support Hedera Token Service (HTS) integration in the Interchain Token Service suite while also updating test cases, deployment scripts, compiler settings, and various token‐manager interfaces. Key changes include new utility functions for nonzero address checks and token manager parameter validation, updated deployment logic (including HTS and WHBAR contracts), and revisions in token creation pricing and token manager contracts.
Reviewed Changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/*.js | Updates to test cases to reflect new HTS-related functions and parameter checks |
| scripts/deploy*.js | Deployment scripts now include HTS and WHBAR library deployments and funding |
| hardhat.config.js | Adjusted default network and optimizer settings |
| contracts/utils/*.sol | New TokenCreationPricing and updated Minter and InterchainTokenDeployer contracts |
| contracts/token-manager/*.sol | Revised TokenManager with new roles and HTS integration |
| contracts/proxies/*.sol and interfaces | Updates to proxy and interface definitions to include HTS and new parameter types |
| contracts/hedera/*.sol | New contracts for WHBAR, HTS library, Hedera interfaces and response codes |
| contracts/InterchainTokenService*.sol | Removed obsolete functions and added setters for token creation price and WHBAR |
| contracts/InterchainTokenFactory.sol | Updated token metadata retrieval to support HTS tokens |
Comments suppressed due to low confidence (4)
test/InterchainTokenServiceFullFlow.js:796
- Passing the string '0x' as the operator parameter in validateTokenManagerParams could be ambiguous. Consider using a well-defined constant like AddressZero for clarity.
)
contracts/utils/Minter.sol:58
- Changing the visibility of isMinter from external to public may affect external interface compatibility. Please ensure this change is intentional and that all external callers are updated accordingly.
function isMinter(address addr) public view returns (bool) {
contracts/utils/TokenCreationPricing.sol:38
- It would be helpful to add an explanation for the '+ 1' adjustment in _tokenCreationPriceTinybars to clarify the rounding logic.
// TODO(hedera) explain why + 1 (for rounding)
contracts/utils/InterchainTokenDeployer.sol:12
- The removal of Create3Fixed and the changes to the minimal proxy bytecode logic should be documented clearly to explain how determinism and deployment address calculation are maintained with the new HTS integration.
*/
not needed because the mintToken can always be called by the service
|
|
||
| New HTS Interchain Tokens will have their Token Manager as the sole Supply Key ("MinterBurner" equivalent in Hedera) and Treasury (the contract that gets the newly minted coins). After minting, the Treasury transfers the tokens to the designated account. Before burning, the tokens are transfered back to the Treasury. Token Managers use typical `allowance` and `transferFrom` to move tokens before burning. Token Manager keeps track of minters and allows for external minting and burning (see `Minter.sol`). Certain ITS features are not supported due to HTS limitations, such as deploying a new Interchain Token with initial supply. | ||
|
|
||
| Since the `createFungibleToken` precompile in Hedera requires a fee to be sent as value, an `WHBAR` contract (`WETH` equivalent) is used to hold the HBAR used for token creation. `InterchainTokenService` transfers certain amount of `WHBAR` to the newly deploying `TokenManagerProxy` contract. The `TokenManagerProxy` contract, during constructor, withdraws HBAR from `WHBAR`, and sends it to `InterchainTokenDeployer`, which finally uses it to pay for the token creation. |
There was a problem hiding this comment.
Sounds like this happens in a single transaction, so why do we need a separate WHBAR? If you can elaborate on it in the README it would be great.
There was a problem hiding this comment.
I added a section of why we don't directly send funds to ITS, but do it via WHBAR.
contracts/hedera/README.md
Outdated
|
|
||
| Since the `createFungibleToken` precompile in Hedera requires a fee to be sent as value, an `WHBAR` contract (`WETH` equivalent) is used to hold the HBAR used for token creation. `InterchainTokenService` transfers certain amount of `WHBAR` to the newly deploying `TokenManagerProxy` contract. The `TokenManagerProxy` contract, during constructor, withdraws HBAR from `WHBAR`, and sends it to `InterchainTokenDeployer`, which finally uses it to pay for the token creation. | ||
|
|
||
| The responsibility of keeping ITS funded on the WHBAR contract lies with the deployer, it is assumed that a top-up mechanism is in place to ensure the contract has enough WHBAR to create new tokens. |
There was a problem hiding this comment.
What if the deployer sends extra WHBAR? Is there a mechanism to take the funds out? How secure is it? Is there a way to revert if someone sends extra HBAR than required?
There was a problem hiding this comment.
I updated the readme to say "contracts deployer (Axelar)" instead of the "deployer" to avoid confusion with the token deployer. I also added explanations to different flows for remote and local deployments.
|
|
||
| Hedera tokens support so-called facades, which allow them to be used as ERC20 tokens. A number of standard methods are supported, like `name`, `balanceOf`, `transfer`, `transferFrom`, `approve`, `allowance`, etc. See [hip-218](https://hips.hedera.com/hip/hip-218) and [hip-376](https://hips.hedera.com/hip/hip-376). `mint` and `burn` are not supported. | ||
|
|
||
| Unlike a regular ERC20 token, HTS tokens don't emit `Transfer` to and from the zero address on mint and burn. |
There was a problem hiding this comment.
Axelerons will need to track this, do we have alternate events emitted in this case so we can pick it up if needed?
There was a problem hiding this comment.
For Mint and Burn there is no event emitted by the token itself. We could emit those from the TokenManager if needed.
contracts/hedera/README.md
Outdated
|
|
||
| ITS contracts in this repo are modified to support Hedera Token Service. All new Interchain Token will be created via HTS, while existing HTS and ERC20 tokens are supported for registration. | ||
|
|
||
| New HTS Interchain Tokens will have their Token Manager as the sole Supply Key ("MinterBurner" equivalent in Hedera) and Treasury (the contract that gets the newly minted coins). After minting, the Treasury transfers the tokens to the designated account. Before burning, the tokens are transfered back to the Treasury. Token Managers use typical `allowance` and `transferFrom` to move tokens before burning. Token Manager keeps track of minters and allows for external minting and burning (see `Minter.sol`). Certain ITS features are not supported due to HTS limitations, such as deploying a new Interchain Token with initial supply. |
There was a problem hiding this comment.
Relayer needs to pull extra funds from gas service when deploying the token on Hedera. gas APi will also need updates.
|
|
||
| However there is an [Automatic Token Associations](https://docs.hedera.com/hedera/core-concepts/accounts/account-properties#automatic-token-associations) feature in Hedera, which allows accounts to approve a number of automatic token associations (airdrops) without needing to explicitly associate with each token. The only way to reliably tell if an account can receive a new token is by reading the property for the account and checking if the value is `-1` (unlimited associations). | ||
|
|
||
| There is an optimistic approach to this, where it is assumed the account has unlimited associations and can receive the token. However if the transaction reverts due to it not being able to receive the token, [gas will be nonetheless charged](https://docs.hedera.com/hedera/core-concepts/smart-contracts/gas-and-fees). This is undesirable, since Hedera charges at minimum 80% of the gas limit. |
There was a problem hiding this comment.
Can the association check be made on ITS FE or on the SDK?
There was a problem hiding this comment.
There's an endpoint that you can query that has max_automatic_token_associations. If it's set to -1 then the account has unlimited associations.
contracts/hedera/IWHBAR.sol
Outdated
| * @return True if successful | ||
| */ | ||
| function transferFrom(address src, address dst, uint wad) external returns (bool); | ||
| } No newline at end of file |
| _interchainTransfer(tokenId, destinationChain, destinationAddress, amount, data, gasValue); | ||
| } | ||
|
|
||
| /******************\ |
There was a problem hiding this comment.
Custom tokens should still be able to use this so probably do not remove it.
contracts/InterchainTokenService.sol
Outdated
| * @notice Used to set the WHBAR contract address. | ||
| * @param whbarAddress_ The new WHBAR contract address. | ||
| */ | ||
| function setWhbarAddress(address whbarAddress_) external onlyOperatorOrOwner { |
There was a problem hiding this comment.
this should probably be immutable since it should never change.
There was a problem hiding this comment.
Updated to not allow changing, it's set during the deployment of the ITS implementation.
- transfer WHBAR from the deploying user to ITS. the user needs to approve the ITF for appropriate amount, which they could get from `tokenCreationPriceTinybars`. - update tests - mention local vs remote deployment in docs
- lint errors - add tests for whbar allowance for ITF - add tests for unsupporting token keys
Fork of the ITS contracts to support Hedera Token Service precompiles.
See
contracts/hedera/README.mdfor an overview of changes and notes. Seecontracts/hedera/TESTING.mdfor instructions on setting up a Hedera local node for testing.What's missing: