search

Loopring / protocols

A zkRollup DEX & Payment Protocol
https://loopring.org
331 stars 122 forks source link

Make quota calculation more robust #2581

Closed dong77 closed 2 years ago

dong77 commented 2 years ago

With the current implementation, when the amount is ~uint(0), the calculation in this price oracle will throw and cause the whole transaction to fail.

kongliangzhong commented 2 years ago

please merge to release_loopring_3.6.3

Brechtpd commented 2 years ago

I don't think returning 0 is the correct behavior, that would mean that the quota wouldn't have any impact on infinite approvals, which means the quota is basically rendered useless (just always approve infinite and the wallet owner can do whatever he wants with all the wallet funds).

Failing here with an overflow seems reasonable enough to me, because the tx should fail regardless so it would just happen somewhere else. If the quota is disabled the price oracle isn't called as far as I can see so the current behavior looks okay to me. If users want to do infinite approvals they have to disable the daily quota, I don't think there's any way around that.

dong77 commented 2 years ago

I don't think returning 0 is the correct behavior, that would mean that the quota wouldn't have any impact on infinite approvals, which means the quota is basically rendered useless (just always approve infinite and the wallet owner can do whatever he wants with all the wallet funds).

Failing here with an overflow seems reasonable enough to me, because the tx should fail regardless so it would just happen somewhere else. If the quota is disabled the price oracle isn't called as far as I can see so the current behavior looks okay to me. If users want to do infinite approvals they have to disable the daily quota, I don't think there's any way around that.

I agree.