Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgraded Q -> 2 from #17 [1680620718364] #47

Closed
c4-judge opened this issue Apr 4, 2023 · 2 comments
Closed

Upgraded Q -> 2 from #17 [1680620718364] #47

c4-judge opened this issue Apr 4, 2023 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value duplicate-23 satisfactory satisfies C4 submission criteria; eligible for awards

Comments

@c4-judge
Copy link
Contributor

c4-judge commented Apr 4, 2023

Judge has assessed an item in Issue #17 as 2 risk. The relevant finding follows:

[L-05] Check that staking cannot occur when endTime is reached
The MuteAmplifier.stake function should require that the current timestamp is smaller than endTime even when the call to stake is the first that ever occurred.
Currently the check is only made in the case that the call to stake is not the first.
The check should be made in both cases.
This is because when staking occurs when block.timestamp >= endTime, no rewards will be paid out. Additionally the user needs to pay the management fee on his LP token stake. So there is really no point in allowing users to do it because it only hurts them.

Fix:

diff --git a/contracts/amplifier/MuteAmplifier.sol b/contracts/amplifier/MuteAmplifier.sol
index 9c6fcb5..460c408 100644
--- a/contracts/amplifier/MuteAmplifier.sol
+++ b/contracts/amplifier/MuteAmplifier.sol
@@ -202,13 +202,12 @@ contract MuteAmplifier is Ownable{
*/
function stake(uint256 lpTokenIn) external virtual update nonReentrant {
require(lpTokenIn > 0, "MuteAmplifier::stake: missing stake");

  •    require(block.timestamp < endTime, "MuteAmplifier::stake: staking is over");
       require(block.timestamp >= startTime && startTime !=0, "MuteAmplifier::stake: not live yet");
       require(IERC20(muteToken).balanceOf(address(this)) > 0, "MuteAmplifier::stake: no reward balance");
    
       if (firstStakeTime == 0) {
           firstStakeTime = block.timestamp;
    
  •    } else {
    
  •        require(block.timestamp < endTime, "MuteAmplifier::stake: staking is over");
       }
    
       lpToken.safeTransferFrom(msg.sender, address(this), lpTokenIn);
    
@c4-judge c4-judge added the 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value label Apr 4, 2023
c4-judge added a commit that referenced this issue Apr 4, 2023
@c4-judge c4-judge closed this as completed Apr 4, 2023
@c4-judge
Copy link
Contributor Author

c4-judge commented Apr 4, 2023

Picodes marked the issue as duplicate of #23

@c4-judge c4-judge added duplicate-23 satisfactory satisfies C4 submission criteria; eligible for awards labels Apr 4, 2023
@c4-judge
Copy link
Contributor Author

Picodes marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value duplicate-23 satisfactory satisfies C4 submission criteria; eligible for awards
Projects
None yet
Development

No branches or pull requests

1 participant