Bug in NFT and FT allowances in the precompile

[ed-marquez]

Description

When approving allowances for FTs and NFTs via the precompiles, the process does not complete as expected because there's a bug where the precompile doesn't seem to understand that a token and an account are already associated.

This same process works OK and as expected when done through the SDKs.

In the FT/NFT example:

• 5 entities are involved: Operator, Treasury, Alice, Bob, and the token.
• The Treasury of the token approves an allowance for Alice of 50 FT units / 1 NFT
• Alice should send bob 10 FT / 1 NFT units from the allowance
• Again, this same process was successfully completed using the JS SDK for both FT and NFT

The Issue:
The allowance transfer throws the error: SPENDER_DOES_NOT_HAVE_ALLOWANCE. See the error file for console output and additional details. Approval-Allowance bug - error message.txt

The crypto transfer tx in Alice's tx history in HashScan correctly shows that she doesn't have the allowance.

The operator tx history, specifically the failed contract call for approving the allowance FALSELY shows: TOKEN_NOT_ASSOCIATED_TO_ACCOUNT.

The console output (and tx history) shows that both Alice and Bob are associated with the token. As a test, the treasury already sent some tokens to Alice using the SDK before calling the contract.

So even though Alice already owns tokens, the precompile seems to think that Alice is not associated to the token => thus the allowance approval via precompile fails => thus the approved token transfer fails.

Tagging @Nana-EC @tinker-michaelj for visibility. Thank you in advanced team : )

Steps to reproduce

1. Open the following GitPod: https://gitpod.io/#https://github.com/ed-marquez/quick-contract-examples
2. Add a .env file with your own credentials
3. Try running the files: 0_test_precompile_FT.js and 0_test_precompile_NFT.js
4. Monitor the operator account activity in HashScan for additional details

Additional context

No response

Hedera network

mainnet, testnet

Version

v2.18

Operating system

Windows

[a-ridley]

This bug is also affecting granting an allowance to an account using ERC20 standard calls. Currently, users cannot grant an allowance to an account and complete a token transfer.



Please see the link below for the repo, an explanation of what is being done in the repo, and steps to reproduce:



This below code example does the following:

1. creates 3 accounts: Treasury, Alice, and Bob.
2. creates an FT of token symbol POP
3. executes a tokenAssocations for Alice and Bob to POP
4. executes a token transfer of 3 POP from Treasury to Alice
5. deploys a smart contract
6. executes a ContractExecuteTransaction for the ERC20 approve function
7. executes an approved token transfer


Steps to reproduce: https://gitpod.io/#snapshot/7f7e3241-10f9-443b-88d0-cae4d43222c8

1. Open the following GitPod:
2. Add a .env file with your own credentials
3. Run node index.js in the gitpod terminal
4. Monitor account activity in HashScan for additional details

[Burstall]

To add to this discussion:

HederaTokenService.sol approveNFT method takes a uint256 input for the serial (per the gitpod example) however it dynamically binds to send to the precompile using the IHederaTokenService which expects an int64. If I create my own method making the low-level call dynamically to precompile via IHederaTokenService passing an int64 then I get it to work. This is not the only case of inconsistency, unfortunately.

function approveNFT(address token, int64 serialNumber) external returns (int responseCode) {
		// HederaTokenSerive.sol implments the method as unit256 serialNumber
		// however it is called via the interface (IHederaTokenService) where serialNumber is int64
		(bool success, bytes memory result) = precompileAddress.call(
            abi.encodeWithSelector(IHederaTokenService.approveNFT.selector,
            token, address(this), serialNumber));
        responseCode = success ? abi.decode(result, (int32)) : HederaResponseCodes.UNKNOWN;
	}

[Nana-EC]

HI @Burstall thanks for highlighting this.
We are aware of it and are carefully considering the right approach with minimal impact.
This was captured in hashgraph/hedera-services#3916

@ed-marquez and @a-ridley thanks for the original issue.
We're aware and are investigating how best to approach this.
See #69 and hashgraph/hedera-services#3758

[Ivo-Yankov]

The code from gitpod completes succesfully without any errors and the CRYPTO_APPROVE_ALLOWANCE transaction doesn't fail. @ed-marquez @a-ridley Could you please confirm if the issue is resolved?

[a-ridley]

The code from gitpod completes succesfully without any errors and the CRYPTO_APPROVE_ALLOWANCE transaction doesn't fail. @ed-marquez @a-ridley Could you please confirm if the issue is resolved?

The issue is resolved. I had to make the following change to the smart contract: use delegatecall instead of call. The CRYPTO_APPROVE_ALLOWANCE transaction completes successfully.

[Nana-EC]

Thanks closing this.

@Burstall where are you seeing an int64 serialNumber in IHederaTokenService?
The current and previous data types were all uint256.
Notably this will change with #99 so feel free to comment there regarding your issue.

[Burstall]

looks good now on the latest set I saw. I see things moving around a little and am trying to keep up to date accordingly. thanks.