From cd9e4c6024f52f67aa48a27c6c4faef62841bf9a Mon Sep 17 00:00:00 2001 From: Mariusz Jasuwienas Date: Tue, 18 Feb 2025 13:37:35 +0100 Subject: [PATCH] fix: hts isassociated will revert when account does not exist (#242) Signed-off-by: Mariusz Jasuwienas --- contracts/HtsSystemContract.sol | 7 +++--- contracts/HtsSystemContractJson.sol | 16 ++++++------ ...SystemContract.sol => IHederaAccounts.sol} | 2 +- contracts/MirrorNode.sol | 14 +++++------ test/HTS.t.sol | 10 ++++++++ test/IHRC719.t.sol | 25 +++++++++++-------- 6 files changed, 43 insertions(+), 31 deletions(-) rename contracts/{IHASSystemContract.sol => IHederaAccounts.sol} (92%) diff --git a/contracts/HtsSystemContract.sol b/contracts/HtsSystemContract.sol index fba0bd4..dcd48e7 100644 --- a/contracts/HtsSystemContract.sol +++ b/contracts/HtsSystemContract.sol @@ -4,7 +4,7 @@ pragma solidity ^0.8.0; import {IERC20} from "./IERC20.sol"; import {IERC721} from "./IERC721.sol"; import {IHRC719} from "./IHRC719.sol"; -import {IHASSystemContract} from "./IHASSystemContract.sol"; +import {IHederaAccounts} from "./IHederaAccounts.sol"; import {IHederaTokenService} from "./IHederaTokenService.sol"; import {HederaResponseCodes} from "./HederaResponseCodes.sol"; import {KeyLib} from "./KeyLib.sol"; @@ -510,7 +510,7 @@ contract HtsSystemContract is IHederaTokenService { // 28: (bytes args for HTS method call, if any) require(msg.data.length >= 28, "fallback: not enough calldata"); - if (address(this) == HTS_ADDRESS && bytes4(msg.data[0:4]) == IHASSystemContract.accountExists.selector) { + if (address(this) == HTS_ADDRESS && bytes4(msg.data[0:4]) == IHederaAccounts.accountExists.selector) { return abi.encode(__accountExists(address(bytes20(msg.data[16:36])))); } @@ -777,8 +777,7 @@ contract HtsSystemContract is IHederaTokenService { return abi.encode(true); } if (selector == IHRC719.isAssociated.selector) { - require(IHASSystemContract(HTS_ADDRESS).accountExists(msg.sender)); - + require(IHederaAccounts(HTS_ADDRESS).accountExists(msg.sender)); bytes32 slot = _isAssociatedSlot(msg.sender); bool res; assembly { res := sload(slot) } diff --git a/contracts/HtsSystemContractJson.sol b/contracts/HtsSystemContractJson.sol index 732d934..251a37a 100644 --- a/contracts/HtsSystemContractJson.sol +++ b/contracts/HtsSystemContractJson.sol @@ -502,20 +502,20 @@ contract HtsSystemContractJson is HtsSystemContract { return slot; } - function _accountExistsSlot(address account) internal override returns (bytes32) { - bytes32 slot = super._accountExistsSlot(account); + function _isApprovedForAllSlot(address owner, address operator) internal override virtual returns (bytes32) { + bytes32 slot = super._isApprovedForAllSlot(owner, operator); if (_shouldFetch(slot)) { - bool exists = mirrorNode().doAccountExist(account); - _setValue(slot, bytes32(uint256(exists ? 1 : 0))); + bool approved = mirrorNode().isApprovedForAll(address(this), owner, operator); + _setValue(slot, bytes32(uint256(approved ? 1 : 0))); } return slot; } - function _isApprovedForAllSlot(address owner, address operator) internal override virtual returns (bytes32) { - bytes32 slot = super._isApprovedForAllSlot(owner, operator); + function _accountExistsSlot(address account) internal override returns (bytes32) { + bytes32 slot = super._accountExistsSlot(account); if (_shouldFetch(slot)) { - bool approved = mirrorNode().isApprovedForAll(address(this), owner, operator); - _setValue(slot, bytes32(uint256(approved ? 1 : 0))); + bool exists = mirrorNode().accountExist(account); + _setValue(slot, bytes32(uint256(exists ? 1 : 0))); } return slot; } diff --git a/contracts/IHASSystemContract.sol b/contracts/IHederaAccounts.sol similarity index 92% rename from contracts/IHASSystemContract.sol rename to contracts/IHederaAccounts.sol index aa3c4bd..3ae8b71 100644 --- a/contracts/IHASSystemContract.sol +++ b/contracts/IHederaAccounts.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity >=0.4.9 <0.9.0; -interface IHASSystemContract { +interface IHederaAccounts { /// @notice Checks if the account is created on the hedera network /// @dev This function returns the existence status of the queried account /// @return exists True if the account exists, false otherwise diff --git a/contracts/MirrorNode.sol b/contracts/MirrorNode.sol index 3556ab6..2a71fa5 100644 --- a/contracts/MirrorNode.sol +++ b/contracts/MirrorNode.sol @@ -101,13 +101,6 @@ abstract contract MirrorNode { return false; } - function doAccountExist(address account) external returns (bool) { - try this.fetchAccount(vm.toString(account)) returns (string memory json) { - return vm.keyExistsJson(json, ".evm_address"); - } catch {} - return false; - } - function getAccountAddress(string memory accountId) public returns (address) { if (bytes(accountId).length == 0 || keccak256(bytes(accountId)) == keccak256(bytes("null")) @@ -131,6 +124,13 @@ abstract contract MirrorNode { return address(uint160(accountNum)); } + function accountExist(address account) external returns (bool) { + try this.fetchAccount(vm.toString(account)) returns (string memory json) { + return vm.keyExistsJson(json, ".evm_address"); + } catch {} + return false; + } + // Lets store the response somewhere in order to prevent multiple calls for the same account id function _getAccountNum(address account) private returns (uint32) { if ((uint160(account) >> 32) == 0) { diff --git a/test/HTS.t.sol b/test/HTS.t.sol index ff80170..c6f8e16 100644 --- a/test/HTS.t.sol +++ b/test/HTS.t.sol @@ -4,6 +4,7 @@ pragma solidity ^0.8.0; import {Test} from "forge-std/Test.sol"; import {HederaResponseCodes} from "../contracts/HederaResponseCodes.sol"; import {HtsSystemContract, HTS_ADDRESS} from "../contracts/HtsSystemContract.sol"; +import {IHederaAccounts} from "../contracts/IHederaAccounts.sol"; import {IHederaTokenService} from "../contracts/IHederaTokenService.sol"; import {IERC20} from "../contracts/IERC20.sol"; import {IERC721} from "../contracts/IERC721.sol"; @@ -901,4 +902,13 @@ contract HTSTest is Test, TestSetup { assertEq(setApprovalForAllResponseCode, HederaResponseCodes.SUCCESS); assertTrue(IERC721(CFNFTFF).isApprovedForAll(CFNFTFF_TREASURY, operator)); } + + function test_HTS_accountExists_should_return_true_for_existing_account() view external { + assertTrue(IHederaAccounts(HTS_ADDRESS).accountExists(CFNFTFF_TREASURY)); + } + + function test_HTS_accountExists_should_return_false_for_non_existing_account() external { + address alice = makeAddr("alice"); + assertFalse(IHederaAccounts(HTS_ADDRESS).accountExists(alice)); + } } diff --git a/test/IHRC719.t.sol b/test/IHRC719.t.sol index 4ddd7c2..e8b5d43 100644 --- a/test/IHRC719.t.sol +++ b/test/IHRC719.t.sol @@ -6,33 +6,31 @@ import {TestSetup} from "./lib/TestSetup.sol"; import {IHRC719} from "../contracts/IHRC719.sol"; contract IHRC719TokenAssociationTest is Test, TestSetup { - address alice = 0xa3612A87022a4706FC9452C50abd2703ac4Fd7d9; - function setUp() external { setUpMockStorageForNonFork(); } function test_IHRC719_isAssociated() external { - vm.prank(alice); + vm.prank(MFCT_TREASURY); assertEq(IHRC719(USDC).isAssociated(), false); } function test_IHRC719_associate() external { - vm.startPrank(alice); + vm.startPrank(MFCT_TREASURY); assertEq(IHRC719(USDC).associate(), 1); assertEq(IHRC719(USDC).isAssociated(), true); vm.stopPrank(); } function test_IHRC719_dissociate() external { - vm.startPrank(alice); + vm.startPrank(MFCT_TREASURY); assertEq(IHRC719(USDC).dissociate(), 1); assertEq(IHRC719(USDC).isAssociated(), false); vm.stopPrank(); } function test_IHRC719_associate_then_dissociate() external { - vm.startPrank(alice); + vm.startPrank(MFCT_TREASURY); assertEq(IHRC719(USDC).associate(), 1); assertEq(IHRC719(USDC).isAssociated(), true); assertEq(IHRC719(USDC).dissociate(), 1); @@ -41,18 +39,16 @@ contract IHRC719TokenAssociationTest is Test, TestSetup { } function test_IHRC719_different_accounts() external { - address bob = 0x435d7D41D4f69F958bda7A8D9f549a0dD9B64c86; - - vm.startPrank(alice); + vm.startPrank(MFCT_TREASURY); assertEq(IHRC719(USDC).associate(), 1); assertEq(IHRC719(USDC).isAssociated(), true); - vm.startPrank(bob); + vm.startPrank(CFNFTFF_TREASURY); assertEq(IHRC719(USDC).isAssociated(), false); assertEq(IHRC719(USDC).associate(), 1); assertEq(IHRC719(USDC).isAssociated(), true); - vm.startPrank(alice); + vm.startPrank(MFCT_TREASURY); assertEq(IHRC719(USDC).dissociate(), 1); assertEq(IHRC719(USDC).isAssociated(), false); @@ -72,4 +68,11 @@ contract IHRC719TokenAssociationTest is Test, TestSetup { vm.stopPrank(); } + + function test_IHRC719_with_non_existing_account() external { + vm.expectRevert(); + address alice = makeAddr('alice'); + vm.prank(alice); + IHRC719(USDC).isAssociated(); + } }