hashgraph / hedera-services

Crypto, token, consensus, file, and smart contract services for the Hedera public ledger
Apache License 2.0
314 stars 138 forks source link

spender allowance does not decrease as allowance is spent #6262

Open mshakeg opened 1 year ago

mshakeg commented 1 year ago

Description

I initially created hashgraph/hedera-mirror-node-explorer/issues/567 , however, @ericleponner indicated that this is an issue at the service layer.

I assumed it was simply the mirror node returning a stale allowance(i.e. the allowance that was initially approved), but it seems it's not being updated at the service layer.

I'd also recommend interpreting type(int64).max as an infinite allowance(i.e. does not need to be decreased as spender spends) which is similar to ERC20 tokens where an allowance of type(uint256).max is interpreted as an infinite approval/allowance.

Steps to reproduce

I'm not sure how to get both fungible and nonfungible token allowances using the SDK, I tried searching for a way to do so in the docs to no avail.

If there is a way then on mainnet simply get the allowance for spender 0.0.2178425 for token 0.0.2178426 approved by account 0.0.1055570, if the allowance is still 100 WHBAR then this confirms the issue.

Additional context

No response

Hedera network

mainnet

Version

v0.36.4

Operating system

None

tinker-michaelj commented 1 year ago

Hi @mshakeg, I can confirm the allowances are being decreased, but the TransactionRecord exported to mirror node is missing the information that this tokenTransfer() used up the allowances.

I am fixing the TransactionRecord now; and will consult with mirror node operators to see the best way to export the missing allowance information for this transaction (and any others affected).

Tyvm.

tinker-michaelj commented 1 year ago

Hi again @mshakeg, I have opened a PR to fix consensus nodes to export the missing is_approval = true flag in your scenario.

However, I didn't realize when I replied above that there is a pending HIP-336 to make the public mirror node start using these is_approval flags to "spend down" allowances in real time.

So even when my fix is deployed, not until HIP-336 is implemented will you be able to use mirror node for real-time approvals and allowances.

How urgent is it for you to have access to this data through consensus or mirror nodes? (As opposed to tracking approvals and allowances in your app.)

mshakeg commented 1 year ago

Hi @tinker-michaelj

So even when my fix is deployed, not until HIP-336 is implemented will you be able to use mirror node for real-time approvals and allowances.

Fantastic!


How urgent is it for you to have access to this data through consensus or mirror nodes?

Not that urgent given that the app will (ideally) request an infinite allowance which is not currently possible on Hedera as I understand, however, it'll be more useful to other apps that specify/require a finite allowance.

tinker-michaelj commented 1 year ago

Not that urgent given that the app will (ideally) request an infinite allowance which is not currently possible on Hedera as I understand, however, it'll be more useful to other apps that specify/require a finite allowance.

Got it, tyvm for this feedback! Will raise the idea of treating type(int64).max as infinite allowance; that makes a ton of sense to me.

mshakeg commented 1 year ago

Hey @tinker-michaelj I just wanted to check back with you on whether infinite allowances have been considered?

mshakeg commented 1 year ago

@tinker-michaelj regarding infinite allowances I'd suggest any value >= type(int64).max && <= type(uint256).max

web3-nomad commented 3 months ago

@mshakeg is reporting that this issue is not resolved

mshakeg commented 3 months ago

Building on this users should be able to grant allowances that exceed the max total supply.

https://hashscan.io/mainnet/transaction/1723153165.174759004