delvtech / hyperdrive

An automated market maker for fixed and variable yield with on-demand terms.
Apache License 2.0
33 stars 4 forks source link

`test_nonstandard_decimals_lp` failure #630

Closed jalextowle closed 1 year ago

jalextowle commented 1 year ago

https://github.com/delvtech/hyperdrive/actions/runs/6595763873/job/17921109535?pr=626

jrhea commented 1 year ago

This seems like an issue we need to look into. Running this test with printouts gives us:

  minimumTransactionAmount 1.000000
  config.minimumTransactionAmount 1.000000
  Alice initializes pool with liquidity: 500,000,000.000000
  Alice receives LP Shares: 499,999,998.000000
  Bob adds liquidity: 500,000,000.000000
  Bob receives LP Shares: 500,000,000.000000
  Bob opens a long with 60,348,175.507674 base for 61,419,763.550123 bonds
  Bob opens a short with 82,174,808.543328 base for 1,142,522,704.554135 bonds
  Alice removes liquidity. baseProceeds: 139.248433 withdrawalShares: 539,023,254.224824
  Celine adds liquidity:  500,000,000.000000
  Celine receives LP Shares: 0.923888
  Bob closes 61,419,763.550123 bond long and gets 30,030,312.930023 base
  Bob closes 1,142,522,704.554135 bond short and gets 573,469,272.582876 base
  Alice redeems withdrawal shares. proceeds: 539,023,255.335508
  Bob removes liquidity. baseProceeds: 500,000,001.030273 withdrawalShares: 0.000000

So the first thing to notice is that when Celine adds 500_000_000 of liquidity, she only receieves 0.923888 LP Shares. This is already a problem bc with the minimumTransactionAmount = 1, she can't redeem. The other thing to consider is how bad of a deal she is getting here.

jrhea commented 1 year ago

The fix here is to enforce min/max apr guards in the test. Additionally, need to add a check to addLiquidity s.t. if the number of lpShares to be minted is < minTransaction amount, it should revert