near / near-indexer-events

7 stars 1 forks source link

FT Contract `baby.tkn.near` overflows DB max amount value #14

Closed telezhnaya closed 2 years ago

telezhnaya commented 2 years ago
ft_balance_of contract baby.tkn.near, block 6t8yYevd1HSXbkuCsfdu1KunTnWp8ctJZKsqoWMLgfud, account_id babyshiba.near
340171165809803503152000000000000000000
u128::max
340282366920938463463374607431768211456

In Rust applications, we use u128 for balance values. In the DB, we use numeric(38, 0) (it's i128 in Rust terminology), because of the limitations of Redshift and AlloyDB. They do not support numeric types bigger than numeric(38, 0).

340171165809803503152000000000000000000 does not fit into that.

It actually raises several questions:

I see several options on the DB side:


FYI: it makes sense to compare these numbers with NEAR total supply. It's 1104019057751524304514351326173247 which fits into numeric(34, 0), so the difference is 10^5.

telezhnaya commented 2 years ago

Temporary fix https://github.com/near/near-indexer-events/commit/9b4bb8dcfe2aad112ba55a7079e7911cf903b549 We need to make the better decision here

frol commented 2 years ago

Well, NEP-141 defines the constraints:

All amounts, balances and allowance are limited by U128 (max value 2**128 - 1).

Thus, we can ban the contracts that overflow that (so we at least are not in that bad state). However, it is not fair to ban those that overflow I128 (I believe we could have limited the amounts to I128 range, but we did not 😢).

I feel numeric(39, 0) is the right way to go, and when we face the problem with the specific database, we will address it there (unless we already use one that fails to deal with numeric(39, 0))

telezhnaya commented 2 years ago

I'm just testing the solution on the random periods of blocks trying to validate the code. I haven't seen the previous history for this contract and account.

@frol my guess here: there was an overflow inside the contract, some blocks before (I don't have proofs for that, just a guess). I banned the contract for now (I agree, it's not honest) I'll investigate this case deeper when I run the solution from genesis.

I feel numeric(39, 0) gives us the constraint on the DB that we want to avoid :(

telezhnaya commented 2 years ago

I finally changed the type to numeric(40, 0) (could be numeric(39, 0)) because otherwise, we have to ban wrap.near :(