Adds SafeHTS library #41

stoqnkpL · 2022-08-05T13:25:16Z

Adds SafeHTS.sol providing functionality allowing to execute safe operations on the HTS precompiled contract.
Adds SafeOperations.sol which showcases SafeHTS.sol usage.

Related issue(s):

Fixes #43

Notes for reviewer:

Checklist

Documented (Code comments, README, etc.)
Tested (unit, integration, etc.)

Signed-off-by: Stoyan Panayotov <[email protected]>

Signed-off-by: Miroslav Gatsanoga <[email protected]>

shemnon

All the address->IHederaTokenService changes that came in the force push need to be undone.

shemnon · 2022-08-19T19:57:32Z

safe-hts-precompile/SafeHTS.sol

+        require(responseCode == HederaResponseCodes.SUCCESS, "Safe crypto transfer failed!");
+    }
+
+    function safeMintToken(IHederaTokenService token, uint64 amount, bytes[] memory metadata) internal


These should all go back to address. IHederaTokenService is the interface for the precompile, not the type of the tokens. These contracts are untyped.

After going back to address the example usages of the library in SafeOperations.sol are in the form SafeHTS.safe<MethodName> which stutters. Perhaps I can update the library functions so that they don't start with safe to avoid this stuttering in the usage e.g. resulting in SafeHTS.<methodName>. Any thoughts?

If the goal is to mimic OpenZeppelin's SafeERC20 then the name should stutter. Example - https://github.com/code-423n4/2022-05-alchemix/blob/71abbe683dfd5c0686b7e594fb4f78a14b668d8b/contracts-full/adapters/fuse/FuseTokenAdapterV1.sol#L70

Signed-off-by: Miroslav Gatsanoga <[email protected]>

IvanKavaldzhiev · 2022-08-23T07:51:21Z

safe-hts-precompile/SafeHTS.sol

+    function safeMintToken(address token, uint64 amount, bytes[] memory metadata) internal
+    returns (uint64 newTotalSupply, int64[] memory serialNumbers) {
+        int32 responseCode;
+        (bool success, bytes memory result) = precompileAddress.delegatecall(


Do we deliberately use delegatetcall here?

It was not added in the commits made by me, but looking at the hedera-services PR that precedes this PR it seems to be fixed there.

I don't see an obvious reason to use delegatecall here so I will change it to call.

IvanKavaldzhiev · 2022-08-23T07:57:41Z

safe-hts-precompile/SafeHTS.sol

+        (bool success, bytes memory result) = precompileAddress.call(
+            abi.encodeWithSelector(IHederaTokenService.setApprovalForAll.selector, token, operator, approved));
+        responseCode = success ? abi.decode(result, (int32)) : HederaResponseCodes.UNKNOWN;
+        require(responseCode == HederaResponseCodes.SUCCESS, "Safe set token approval failed!");


The error message could be changed to: "Safe set token approval for all failed!"

IvanKavaldzhiev · 2022-08-23T08:46:05Z

safe-hts-precompile/SafeHTS.sol

+        (bool success, bytes memory result) = precompileAddress.call(
+            abi.encodeWithSelector(IHederaTokenService.isApprovedForAll.selector, token, owner, operator));
+        (responseCode, approved) = success ? abi.decode(result, (int32, bool)) : (HederaResponseCodes.UNKNOWN, false);
+        require(responseCode == HederaResponseCodes.SUCCESS, "Safe query for if approved for all failed!");


Safe is approved for all failed!

IvanKavaldzhiev · 2022-08-23T08:54:10Z

safe-hts-precompile/SafeHTS.sol

+        (bool success, bytes memory result) = precompileAddress.call(
+            abi.encodeWithSelector(IHederaTokenService.updateTokenInfo.selector, token, tokenInfo));
+        responseCode = success ? abi.decode(result, (int32)) : HederaResponseCodes.UNKNOWN;
+        require(responseCode == HederaResponseCodes.SUCCESS, "Safe update token info operation failed!");


Safe update token info failed!

IvanKavaldzhiev · 2022-08-23T08:54:26Z

safe-hts-precompile/SafeHTS.sol

+        (bool success, bytes memory result) = precompileAddress.call(
+            abi.encodeWithSelector(IHederaTokenService.updateTokenExpiryInfo.selector, token, expiryInfo));
+        responseCode = success ? abi.decode(result, (int32)) : HederaResponseCodes.UNKNOWN;
+        require(responseCode == HederaResponseCodes.SUCCESS, "Safe update token expiry info operation failed!");


Safe update token expiry info failed!

IvanKavaldzhiev · 2022-08-23T08:54:39Z

safe-hts-precompile/SafeHTS.sol

+        (bool success, bytes memory result) = precompileAddress.call(
+            abi.encodeWithSelector(IHederaTokenService.updateTokenKeys.selector, token, keys));
+        responseCode = success ? abi.decode(result, (int32)) : HederaResponseCodes.UNKNOWN;
+        require(responseCode == HederaResponseCodes.SUCCESS, "Safe update token keys operation failed!");


Safe update token keys failed!

IvanKavaldzhiev · 2022-08-23T08:56:48Z

safe-hts-precompile/SafeHTS.sol

+        (bool success, bytes memory result) = precompileAddress.call(
+            abi.encodeWithSelector(IHederaTokenService.isToken.selector, token));
+        (responseCode, isToken) = success ? abi.decode(result, (int32, bool)) : (HederaResponseCodes.UNKNOWN, false);
+        require(responseCode == HederaResponseCodes.SUCCESS, "Safe query for if valid token at address failed!");


Safe is token failed!

IvanKavaldzhiev · 2022-08-23T08:57:14Z

safe-hts-precompile/SafeHTS.sol

+        (bool success, bytes memory result) = precompileAddress.call(
+            abi.encodeWithSelector(IHederaTokenService.getTokenType.selector, token));
+        (responseCode, tokenType) = success ? abi.decode(result, (int32, int32)) : (HederaResponseCodes.UNKNOWN, int32(0));
+        require(responseCode == HederaResponseCodes.SUCCESS, "Safe query for token type failed!");


Safe get token type failed!

IvanKavaldzhiev · 2022-08-23T09:04:35Z

safe-hts-precompile/SafeOperations.sol

+
+contract SafeOperations {
+
+    function safeTokenAssociate(address sender, address tokenAddress) external {


You can keep the same names for all methods as they are in the SafeHTS library, e.g. safeAssociateToken

Signed-off-by: Miroslav Gatsanoga <[email protected]>

IvanKavaldzhiev

LGTM

shemnon · 2022-08-25T17:33:14Z

safe-hts-precompile/SafeHTS.sol

@@ -435,7 +435,7 @@ library SafeHTS {
        (bool success, bytes memory result) = precompileAddress.call(
            abi.encodeWithSelector(IHederaTokenService.isToken.selector, token));
        (responseCode, isToken) = success ? abi.decode(result, (int32, bool)) : (HederaResponseCodes.UNKNOWN, false);
-        require(responseCode == HederaResponseCodes.SUCCESS, "Safe query for if valid token at address failed!");
+        require(responseCode == HederaResponseCodes.SUCCESS, "Safe is token failed!");


This reads weird. While all the other variants split out the words I think we should consider keeping this camelCase - Safe isToken failed! or Safe 'is token' failed

Signed-off-by: Miroslav Gatsanoga <[email protected]>

IvanKavaldzhiev

LGTM

* Adds SafeHTS library Signed-off-by: Stoyan Panayotov <[email protected]> * Use IHederaTokenService instead of IHTS Signed-off-by: Miroslav Gatsanoga <[email protected]> * Update solidity pragma in SafeOperations Signed-off-by: Miroslav Gatsanoga <[email protected]> * Add HIP-514 functions to SafeHTS library Signed-off-by: Miroslav Gatsanoga <[email protected]> * Use address instead of IHederaTokenService for token params Signed-off-by: Miroslav Gatsanoga <[email protected]> * Delete unused IHTS.sol Signed-off-by: Miroslav Gatsanoga <[email protected]> * Update SafeOperations.sol Signed-off-by: Miroslav Gatsanoga <[email protected]> * Add HIP-514 functions to SafeOperations.sol Signed-off-by: Miroslav Gatsanoga <[email protected]> * Improve error messages Signed-off-by: Miroslav Gatsanoga <[email protected]> * Use same method names in SafeOperations as in SafeHTS Signed-off-by: Miroslav Gatsanoga <[email protected]> * Use call instead of delegatecall in safeMintToken Signed-off-by: Miroslav Gatsanoga <[email protected]> * Update safeIsToken error message Signed-off-by: Miroslav Gatsanoga <[email protected]> Signed-off-by: Stoyan Panayotov <[email protected]> Signed-off-by: Miroslav Gatsanoga <[email protected]> Co-authored-by: Miroslav Gatsanoga <[email protected]>

Adds SafeHTS library

2b8c66f

Signed-off-by: Stoyan Panayotov <[email protected]>

This was referenced Aug 5, 2022

Add test support for the SafeHTS library #42

Closed

Safe precompile methods (#854) hashgraph/hedera-services#2866

Closed

Nana-EC requested review from lukelee-sl and shemnon August 5, 2022 20:37

Nana-EC assigned stoqnkpL Aug 5, 2022

Nana-EC added HTS Hedera Token Service related P2 labels Aug 5, 2022

MiroslavGatsanoga marked this pull request as draft August 17, 2022 14:18

MiroslavGatsanoga self-assigned this Aug 17, 2022

Use IHederaTokenService instead of IHTS

922aa0d

Signed-off-by: Miroslav Gatsanoga <[email protected]>

MiroslavGatsanoga force-pushed the add-safe-hts-library branch from a51a6b8 to 922aa0d Compare August 18, 2022 12:27

Update solidity pragma in SafeOperations

a8466ab

Signed-off-by: Miroslav Gatsanoga <[email protected]>

shemnon previously approved these changes Aug 18, 2022

View reviewed changes

MiroslavGatsanoga dismissed shemnon’s stale review via bfe3bc0 August 19, 2022 13:42

MiroslavGatsanoga force-pushed the add-safe-hts-library branch from bfe3bc0 to 792c415 Compare August 19, 2022 14:20

shemnon suggested changes Aug 19, 2022

View reviewed changes

Add HIP-514 functions to SafeHTS library

7a5060b

Signed-off-by: Miroslav Gatsanoga <[email protected]>

MiroslavGatsanoga force-pushed the add-safe-hts-library branch from 792c415 to 7a5060b Compare August 22, 2022 12:26

MiroslavGatsanoga added 4 commits August 22, 2022 15:39

Use address instead of IHederaTokenService for token params

5acad17

Signed-off-by: Miroslav Gatsanoga <[email protected]>

Delete unused IHTS.sol

bb3a217

Signed-off-by: Miroslav Gatsanoga <[email protected]>

Update SafeOperations.sol

c008bea

Signed-off-by: Miroslav Gatsanoga <[email protected]>

Add HIP-514 functions to SafeOperations.sol

84e1199

Signed-off-by: Miroslav Gatsanoga <[email protected]>

MiroslavGatsanoga marked this pull request as ready for review August 22, 2022 13:48

MiroslavGatsanoga requested review from shemnon and IvanKavaldzhiev August 22, 2022 13:48

shemnon previously approved these changes Aug 23, 2022

View reviewed changes

IvanKavaldzhiev reviewed Aug 23, 2022

View reviewed changes

Improve error messages

5350021

Signed-off-by: Miroslav Gatsanoga <[email protected]>

MiroslavGatsanoga dismissed shemnon’s stale review via 5350021 August 23, 2022 10:51

MiroslavGatsanoga added 2 commits August 23, 2022 14:00

Use same method names in SafeOperations as in SafeHTS

93e3b04

Signed-off-by: Miroslav Gatsanoga <[email protected]>

Use call instead of delegatecall in safeMintToken

c881098

Signed-off-by: Miroslav Gatsanoga <[email protected]>

MiroslavGatsanoga requested a review from IvanKavaldzhiev August 23, 2022 11:12

IvanKavaldzhiev previously approved these changes Aug 23, 2022

View reviewed changes

MiroslavGatsanoga requested review from shemnon and Nana-EC August 23, 2022 13:28

shemnon previously approved these changes Aug 25, 2022

View reviewed changes

Nana-EC previously approved these changes Aug 25, 2022

View reviewed changes

Update safeIsToken error message

56f994d

Signed-off-by: Miroslav Gatsanoga <[email protected]>

MiroslavGatsanoga dismissed stale reviews from Nana-EC, shemnon, and IvanKavaldzhiev via 56f994d August 26, 2022 07:00

MiroslavGatsanoga requested review from IvanKavaldzhiev, shemnon and Nana-EC August 26, 2022 07:01

IvanKavaldzhiev approved these changes Aug 26, 2022

View reviewed changes

shemnon approved these changes Aug 26, 2022

View reviewed changes

MiroslavGatsanoga merged commit 2955081 into hashgraph:main Aug 29, 2022

MiroslavGatsanoga deleted the add-safe-hts-library branch August 29, 2022 07:27

Nana-EC added this to the 0.2.0 milestone Oct 11, 2022

Nana-EC added the enhancement New feature or request label Oct 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds SafeHTS library #41

Adds SafeHTS library #41

stoqnkpL commented Aug 5, 2022 •

edited by MiroslavGatsanoga

Loading

shemnon left a comment

shemnon Aug 19, 2022 •

edited

Loading

MiroslavGatsanoga Aug 22, 2022

MiroslavGatsanoga Aug 22, 2022

shemnon Aug 23, 2022

IvanKavaldzhiev Aug 23, 2022

MiroslavGatsanoga Aug 23, 2022

IvanKavaldzhiev Aug 23, 2022

IvanKavaldzhiev Aug 23, 2022

IvanKavaldzhiev Aug 23, 2022

IvanKavaldzhiev Aug 23, 2022

IvanKavaldzhiev Aug 23, 2022

IvanKavaldzhiev Aug 23, 2022

IvanKavaldzhiev Aug 23, 2022 •

edited

Loading

IvanKavaldzhiev Aug 23, 2022

IvanKavaldzhiev left a comment

shemnon Aug 25, 2022

MiroslavGatsanoga Aug 26, 2022

IvanKavaldzhiev left a comment


		contract SafeOperations {

		function safeTokenAssociate(address sender, address tokenAddress) external {

Adds SafeHTS library #41

Adds SafeHTS library #41

Conversation

stoqnkpL commented Aug 5, 2022 • edited by MiroslavGatsanoga Loading

shemnon left a comment

Choose a reason for hiding this comment

shemnon Aug 19, 2022 • edited Loading

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

IvanKavaldzhiev Aug 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IvanKavaldzhiev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IvanKavaldzhiev left a comment

Choose a reason for hiding this comment

stoqnkpL commented Aug 5, 2022 •

edited by MiroslavGatsanoga

Loading

shemnon Aug 19, 2022 •

edited

Loading

IvanKavaldzhiev Aug 23, 2022 •

edited

Loading