BigNumber allowance amount doesn't work for AccountAllowanceApproveTransaction

[iron4548]

Description

I have this code

async function approveTokenAllowanceWithChecks(client:Client, ownerId:AccountId, spenderId:ContractId, tokenId:TokenId, tokenAllowanceTiny:BigNumber) {

  //Docs: https://mainnet-public.mirrornode.hedera.com/api/v1/docs/#/accounts/listTokenAllowancesByAccountId

  const params = `spender.id=${spenderId}&token.id=${tokenId}`;
  const url = `${config.mirrorNodeBaseUrl}/api/v1/accounts/${ownerId}/allowances/tokens?${params}`;

  const response = await axios.get(url);

  for (const allowance of response.data.allowances) {
    if (allowance.amount >= tokenAllowanceTiny ) {
      return; //required allowance is already given
    }
  }

  console.info(`Approving spender (${spenderId}) allowance for token ${tokenId}..`);

  const result = await new AccountAllowanceApproveTransaction()
  .approveTokenAllowance(tokenId, ownerId, spenderId, tokenAllowanceTiny)
  .execute(client);

  const receipt = await result.getReceipt(client);
  console.info(`Token allowance result: ${receipt.status}`);
}

I get a 'SUCCESS' status each time I execute this but the allowance isn't given (according to hashscan.io). However, if I add tokenAllowanceTiny.toNumber() then it will work. I had thought i'd be able to pass a BigNumber since the allowance amount is the smallest unit of the token (tiny value), and the number type for Javascript/Typescript may not be large enough.

Can support be added to allow passing a BigNumber for the amount parameter please? Currently the parameter syntax is amount : any which doesn't make it clear to the coder / reader. Changing the syntax to be amount: number | BigNumber would make more sense and is used elsewhere such as ContractFunctionParameters's addUint256()

Steps to reproduce

Pass a BigNumber value for the amount parameter for AccountAllowanceApproveTransaction's approveTokenAllowance() method and observe that no actual allowance is given after executing it, and getting a SUCCESS status.

Additional context

No response

Hedera network

testnet

Version

2.34.1

Operating system

None

[svetoslav-nikol0v]

Hi iron4548,
Currently, the type of the amount parameter of approveTokenAllowance() of AccountAllowanceApproveTransaction is Long | number not any.
Also, there is a convertToBigNumber() method that you can use to convert string or number or Long to BigNumber.
Can you use it in your implementation and see if it works for you, please?

[ochikov]

@iron4548 any news here? We are going to close the issue if there is no update.

[iron4548]

Thanks for the bump

I updated the sdk to 2.35.0 from 2.34.1, and it was still showing 'any' for the popup tooltip in Visual Code. But if I navigate to the method it does show Long | number. It must be caching the old one somewhere.

I tried these:

1. Deleted Visual Code Cache and CacheData folder in roaming and relaunch
2. Created a brand new project
3. Restarted TS service

And it still shows 'any'. I'm just having a bad dev day.. ;-)

Anyhow, I change the value back to BigNumber from number, and no compile-time error was thrown and I was still able to run the code.

/**
 * Approve token allowance for spender if needed
 * @param client client instance
 * @param owner account id of the owner containing the token balance
 * @param spender account id of the spender to spend the token balance
 * @param tokenId the token id in question
 * @param tokenAllowanceTiny the smallest unit of the token balance to approve
 */
async function approveTokenAllowanceWithChecks(client:Client, ownerId:AccountId, spenderId:ContractId|AccountId, tokenId:TokenId, tokenAllowanceTiny:BigNumber) {
  //Docs: https://mainnet-public.mirrornode.hedera.com/api/v1/docs/#/accounts/listTokenAllowancesByAccountId
  const params = `spender.id=${spenderId}&token.id=${tokenId}`;
  const url = `${config.mirrorNodeBaseUrl}/api/v1/accounts/${ownerId}/allowances/tokens?${params}`;
  const response = await axios.get(url);

  for (const allowance of response.data.allowances) {
    if (tokenAllowanceTiny.comparedTo(new BigNumber(allowance.amount)) >= 0 ) {
      return; //required allowance is already given
    }
  }

  console.log(`Approving spender (${spenderId}) allowance for token ${tokenId}..`);

  const result = await new AccountAllowanceApproveTransaction()
  .approveTokenAllowance(tokenId, ownerId, spenderId, tokenAllowanceTiny)
  .execute(client);

  const receipt = await result.getReceipt(client);
  console.log(`Token allowance result: ${receipt.status}`);
}

I got the misleading "Token allowance result: SUCCESS" so it lead me to think that it worked without issue. But then later down the track I ran into CONTRACT_REVERT_EXECUTED with a 'Safe token transfer failed!'. Then eventually after some time I found out that I had only given '0' spender allowance to the router where it should have been 10000.

Maybe the reason why I'm still getting this issue is Visual Code is somehow thinking it's still 'any'. I'll continue to try and figure it out..

Anyhow, why not just add support for BigNumber.js to avoid other devs running into this headache. BigNumber as parameter is used in multiple places where the token's smallest unit (TinyValue / TinyBar) is expected.

[svetoslav-nikol0v]

Hi iron4548,
Hedera doesn't support BigNumber as a type for the amount parameter for AccountAllowanceApproveTransaction.

[iron4548]

Hi iron4548, Hedera doesn't support BigNumber as a type for the amount parameter for AccountAllowanceApproveTransaction.

Yes, I know. However I think it can be an easy mistake for devs to make, so why not add support for BigNumber.js ? If it's easy enough to do.

[rokn]

Hey @iron4548, that wouldn't make much sense for the SDK as it can introduce some weird behaviours. What would we do with numbers that are bigger than int64? Overall I think that with the typings set correctly as Long | number it should be sufficient for devs to know what is expected of the function.

[iron4548]

"Long | number it should be sufficient for devs to know what is expected of the function"

For me, it shows it as 'any' for some reason (even with the latest sdk version) so it doesn't help. Not sure what others experience in their editor. I'm using Visual Code. That threw me off in the first place.

"What would we do with numbers that are bigger than int64"

Throw an error if it's bigger than int64? Otherwise safely convert to Long or whatever behind the scene?

For some context, in my code I typically use BigNumber.js to deal with tiny values as I interact with SaucerSwap smart contracts (Uniswap fork) and different tokens a lot. For event emits and contract call results, the values are all tiny values so I use BigNumber.js to deal with them. It was just natural to me to pass a BigNumber value to AccountAllowanceApproveTransaction because it expects the token's smallest unit. And I'm passing around BigNumber.js in many places where the token's smallest unit is expected.

There is no direct conversion from BigNumber to Long (I'll have to install the 'long' module and then to do something like let longValue = Long.fromString(bigNumber.toString()). There's a ToNumber() method for the BigNumber class. But that will cause the precision to be lost when working with a token with a large supply and max decimal places.

The overall goal is to achieve a consistent developer experience throughout the SDK.

For example, ContractFunctionParameters().addUint256() and similar doesn't take in a Long variable, but it takes in BigNumber variable. It should offer Long here too.

Furthermore, these methods can take in a BigNumber as a parameter. The Hbar class behind the scene handles it and store it as a tinybar.

• AccountCreateTransaction().initialBalance()
• ContractCreateFlow().setInitialBalance()
• ContractExecuteTransaction().setPayableAmount()
• EthereumFlow().setMaxGasAllowanceHbar()
• EthereumTransaction().setMaxGasAllowance()
• AccountAllowanceApproveTransaction().addHbarAllowance()

Anyway, if you insist that you won't be adding a BigNumber as a parameter option for AccountAllowanceApproveTransaction please go ahead and close this. I'll deal with it using this workaround: let longValue = Long.fromString(bigNumber.toString());

[iron4548]

Just thought I'd post this.

After running npm i long to install the Long module on my Typescript project, the parameter now shows as 'Long | Number' from 'any' in Visual Code. And now I'm getting errors like 'BigNumber' is not assignable to parameter of type 'number | Long'

Very strange!

[svetoslav-nikol0v]

Hey iron4548,
Thank you for your input and suggestions. But this change would requires a lot of modifications as well as deep testing.
We’ll discuss this in more detail and how to implement it in one of the upcoming versions of SDK. We are going to keep you updated once the changes are planned and scheduled.