-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add HTS Fungible Token Standard Collection #1240
base: main
Are you sure you want to change the base?
Conversation
Test Results 18 files + 1 110 suites +7 13m 3s ⏱️ - 3m 21s For more details on these failures, see this check. Results for commit f77bcfb. ± Comparison against base commit 3aa1542. This pull request removes 37 and adds 18 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results. |
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
5937416
to
925d1c9
Compare
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Signed-off-by: Nana Essilfie-Conduah <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should remove this.
function _createHTSToken(string memory _name, string memory _symbol, int64 _initialTotalSupply, uint8 _decimals) internal virtual { | ||
IHederaTokenService.HederaToken memory tokenInfo = _setupHTSToken(_name, _symbol); | ||
|
||
(bool success, bytes memory result) = htsSystemContractAddress.call{value : msg.value}( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a specific reason to use .call
here? should we call it using IHederaTokenService(0x167).createFungibleToken(...)
instead?
|
||
(int responseCode, address tokenAddress) = success ? abi.decode(result, (int32, address)) : (HederaResponseCodes.UNKNOWN, address(0)); | ||
|
||
require(responseCode == HederaResponseCodes.SUCCESS, "Failed to create HTS token"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this pattern masks the responseCode
in case of error, making it difficult to debug. Should we introduce a custom error, such as HtsError(int)
, so at least the caller can see the error code?
|
||
require(responseCode == HederaResponseCodes.SUCCESS, "Failed to create HTS token"); | ||
|
||
htsTokenAddress = tokenAddress; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd moved this into the constructor so it's clear that htsTokenAddress
is only assigned once.
import "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol"; | ||
import "../../IHTSStructs.sol"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: list imported symbols
Description:
Currently developers need to have a deep understanding of
IHederaTokenService.sol
to expose all the features for fungible tokens and management contracts.Add a set of standard contracts a developer can simple inherit from and deploy that offer expected ERC20 functions
IHtsFungibleToken.sol
that cover HTS token + ERC20 behaviour that can be operated on a token via a deployed contractIHtsFungibleTokenProxy.sol
that extendsIHtsFungibleToken.sol
behaviour to support interaction with an already existing token that can be operated on a token directly via an EOAHtsFungibleToken.sol
abstract contract that implements creation flow of a new fungible token and allows key delegated transfer related behaviourExampleHtsFungibleToken.sol
that showcases the ease in which the standard can be used to create a fungible tokenHtsFungibleTokenSupplyManager.sol
abstract contract that implements creation flow of a new fungible token and allows supply management via mint and burn functionsExampleHtsFungibleToken.sol
that show cases the ease in which the standard can be used to create a fungible tokenExampleHtsFungibleTokenSupplyManager.sol
that showcases the ease in which the standard can be used to create a fungible token with mint and burn supportIHtsNonFungibleToken.sol
that cover HTS token + ERC721 behaviour that can be operated on a token via a deployed contractIHtsNonFungibleTokenProxy.sol
that extendsIHtsNonFungibleToken.sol
behaviour to support interaction with an already existing token that can be operated on a token directly via an EOAnativeFungibleToken.js
with test coverage for initial fungible use casesThese contract aren't meant to satisfy requests from community so that devs don't have to worry about all the innards and amongst other things a dev can easily take a contract and
Related issue(s):
Fixes #1026
Fixes #781
Notes for reviewer:
IHederaTokenService.sol
, these could be added to abstract contracts.Checklist