vlzhr / puzzleswap-contracts

7 stars 10 forks source link

Brief audit of `puzzle-custompool.ride` #1

Closed deemru closed 2 years ago

deemru commented 2 years ago

Only watched swap() function as the most important one:

✅ LOW: No check for payments size == 1. https://github.com/vlzhr/puzzleswap-contracts/blob/6297d0555840b8ec6a08acd847571a9c232aba4a/puzzle-custompool.ride#L434

✅ HIGH: Logic flaw. (see below) https://github.com/vlzhr/puzzleswap-contracts/blob/6297d0555840b8ec6a08acd847571a9c232aba4a/puzzle-custompool.ride#L173

✅ CRITICAL: Unexpected key-value overwrites (same assetId possible as stated above) https://github.com/vlzhr/puzzleswap-contracts/blob/6297d0555840b8ec6a08acd847571a9c232aba4a/puzzle-custompool.ride#L483-L484

✅ MEDIUM: Performance. You should throw() as soon as possible to bypass failed transaction status for a user. Not after your service invoke. The best place is just after let AmountOut = .... https://github.com/vlzhr/puzzleswap-contracts/blob/6297d0555840b8ec6a08acd847571a9c232aba4a/puzzle-custompool.ride#L449-L462

✅ MEDIUM: Performance. Same as above. https://github.com/vlzhr/puzzleswap-contracts/blob/6297d0555840b8ec6a08acd847571a9c232aba4a/puzzle-custompool.ride#L466-L467

✅ TRIVIAL: Unused variables. https://github.com/vlzhr/puzzleswap-contracts/blob/6297d0555840b8ec6a08acd847571a9c232aba4a/puzzle-custompool.ride#L6 https://github.com/vlzhr/puzzleswap-contracts/blob/6297d0555840b8ec6a08acd847571a9c232aba4a/puzzle-custompool.ride#L438

✅ HIGH: Wrong calculations. AssetInBalance and feeAssetOutBalance could be changed after first calculateOutAmount(virtual swap happened) https://github.com/vlzhr/puzzleswap-contracts/blob/6297d0555840b8ec6a08acd847571a9c232aba4a/puzzle-custompool.ride#L449-L450

✅ HIGH: Wrong calculations. If feeAssetOut equals AssetIn or AssetOut (basically one of the assets is USDN) its still MUST be accounted or it will lead to wrong higher interest rates (for USDN). https://github.com/vlzhr/puzzleswap-contracts/blob/6297d0555840b8ec6a08acd847571a9c232aba4a/puzzle-custompool.ride#L473

vlzhr commented 2 years ago

@deemru thanks for your help. very appreciate it! fixing the points you mentioned would love your help on a brief overview of other functions (generateIndex, redeemIndex and claimIndexRewards)

deemru commented 2 years ago

Lets finish with swap() first:

deemru commented 2 years ago

As the CRITICAL is still here, you should at least disable swap() that AssetIn == AssetOut after: https://github.com/vlzhr/puzzleswap-contracts/blob/6ceffd5ab6d236ff2fa6baf97c8e3a2d3f842385/puzzle-custompool.ride#L437