From 00698638c22072e8f7ce5be471e0835ef07c0c98 Mon Sep 17 00:00:00 2001 From: C4 <81770958+code423n4@users.noreply.github.com> Date: Fri, 25 Nov 2022 17:52:42 +0100 Subject: [PATCH] Report for issue #129 updated by simon135 --- data/simon135-Q.md | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/data/simon135-Q.md b/data/simon135-Q.md index fad8b52..c6aad2f 100644 --- a/data/simon135-Q.md +++ b/data/simon135-Q.md @@ -1,5 +1,5 @@ ##### make these checks into a single modifier to make the code more readable: -``` +```js if (_pxGmx == address(0)) revert ZeroAddress(); if (_pxGlp == address(0)) revert ZeroAddress(); if (_pirexFees == address(0)) revert ZeroAddress(); @@ -12,13 +12,13 @@ if (_stakedGlp == address(0)) revert ZeroAddress(); ``` instead, make it into a modifier -``` +```js modifer nonzero(adddress t) { if(t== address(0)) revert ZeroAddress(); } ``` ##### rename the `assets` var to `amount` for readability -``` +```js function _computeAssetAmounts(Fees f, uint256 assets) ``` make into `uint256 amount` @@ -28,9 +28,37 @@ it's very confusing because the `postFeeAmount` is the real fee amount and `fee instead: `feeAmount`= the fee `postFeeAmount` the `amount-fee` -#### add parameters to `beforeDeposit` to stop huge slippage loss and to make it more useable -``` +##### add parameters to `beforeDeposit` to stop huge slippage loss and to make it more useable +```js if (totalAssets() != 0) beforeDeposit(address(0), 0, 0); ``` -https://github.com/code-423n4/2022-11-redactedcartel/blob/684627b7889e34ba7799e50074d138361f0f532b/src/vaults/AutoPxGmx.sol#L383 \ No newline at end of file +https://github.com/code-423n4/2022-11-redactedcartel/blob/684627b7889e34ba7799e50074d138361f0f532b/src/vaults/AutoPxGmx.sol#L383 + +##### if `distrbutorbalance= 0` then rewards won't be processed even though there are rewards that need to get processed + +```js + + uint256 blockReward = pendingRewards > distributorBalance + ? distributorBalance + : pendingRewards; + uint256 precision = r.PRECISION(); + uint256 cumulativeRewardPerToken = r.cumulativeRewardPerToken() + + ((blockReward * precision) / r.totalSupply()); +``` +so if `pendingRewards > distributorBalance` The amount is `distributorBalance` instead of the actual pending amounts +ex: + +`pendingRewards= 1ether` +`distributorBalance=0` +The rewards will be `0` and no rewards will be distributed because of the ternary operator wrong use +instead, make the calculation into + +```js +blockReward=pendingRewards; + uint256 cumulativeRewardPerToken = r.cumulativeRewardPerToken() + + ((blockReward * precision) / r.totalSupply()); + +``` + +