search

MrToph / goostew

MIT License
49 stars 4 forks source link

Loss possible for sufficiently small inputs? #2

Closed Minh-Trng closed 2 years ago

Minh-Trng commented 2 years ago

Hey,

I have been playing around with the test cases a bit and found that if you extend the no loss test by having the other 2 users redeem as well, you can end up with one user getting quite a bit less than they should have. Since the code has gotten a bit more verbose than it should be for a test, I have also added a shorter isolated test with fixed numbers, to demonstrate the issue. However, I found that the problem only persists for small amounts of GOO input and short delays, so for practical purposes this might be a non-issue after all.

Full GooStew.t.sol

Output of testNoLossIsolatedScenario():

Logs:                                                                       
  user1 redeemed goo amount                                                 
  195562859452                                                              
  user0 redeemed goo amount                                                 
  195562859452                                                              
  Error: a >= b not satisfied [uint]                                        
    Value a: 195562859452                                                   
    Value b: 1455765164122
MrToph commented 2 years ago

Hey, I'm pretty sure this is because we send MIN_GOO_SHARES_INITIAL_MINT of the first deposit to a burn address. (To ensure that sharesPrice never resets to 1e18 if everyone would redeem their goo shares. We want the shares price to never be decreasing. This is an important invariant for integrations.)

In your example. the first depositor will not receive any goo shares themselves as they only deposit MIN_GOO_SHARES_INITIAL_MINT, therefore it's a loss to them. However, it shouldn't be an issue in practice. It's only done on the first deposit and we can bootstrap the contract by depositing this goo amount ourselves, it's just 1e12 = 0.000001 GOO.

What you can try is setting MIN_GOO_SHARES_INITIAL_MINT to zero in your example, and it should pass.

MrToph commented 2 years ago

Implemented your test and they pass after doing the initial deposit. There are indeed some rounding issues with tiny goo amounts, so if a depositor deposits goo, they need to provide at least 0.000001 goo at the moment.