The challenge-1-decentralized-staking's discuss from umair-mirza

umair-mirza / challenge-1-decentralized-staking Goto Github PK

View Code? Open in Web Editor NEW

A Decentralized Staking app for the Challenge-1 of Scaffold-Eth

License: MIT License

JavaScript 13.31% Solidity 0.19% Shell 0.63% CSS 85.44% HTML 0.13% Less 0.06% Dockerfile 0.17% TypeScript 0.07%

challenge-1-decentralized-staking's Issues

Remarks

Please give names to the parameters in your Stake event. It makes it easy to understand your code, its purpose, and will be helpful for indexing (like creating a subgraph).
Good practice to always set initializing values even when opting for default ones (openForWithdraw state variable)
Your execute function can be more optimized if you check for openForWithdraw. As we know that execute has been already performed if the variable is true.

function execute() public notCompleted {
    //if excuted was performed succesfully before, show an error to reflect this//
    require(!openForWithdraw, "Actions has been already performed!");
    // if deadline is not passed, show error to refelct it//
    require(block.timestamp > deadline, "Deadline is not completed yet!");
    // if is used instead of require becasu we have a counter action to set "openForWithdraw" to true//�
    if(address(this).balance >= threshold) {
      exampleExternalContract.complete{value: address(this).balance}();
    } else {
      // If the `threshold` was not met, allow everyone to call a `withdraw()` function
      openForWithdraw = true;
    }
  }

Bug in withdraw function. You are not updating the balances which will allow user to withdraw multiple times.
Your modifiers must be above functions to follow best practices and conventions.
Good job on deploying and verifying the contract. Please try to link them in readme.
Lack of comments. Please add comments with @notice, @params, etc to make your code easy to understand

Recommend Projects