-
Notifications
You must be signed in to change notification settings - Fork 31
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
Resolve Certik v3 Reports #437
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
5 Ignored Deployments
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Modifiers comments (optional)
Besides @dev tag this looks good to me. |
* fix unnecessary math utils, and add comments to bulkPayout function * update doc and add missing require for amount in bulkpayout * use modifier * fix test
@@ -182,8 +180,21 @@ contract Escrow is IEscrow, ReentrancyGuard { | |||
} | |||
|
|||
/** | |||
* @dev Bulk payout workers | |||
* Should fail if any of the transaction is failing. | |||
* @dev Performs bulk payout to multiple workers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should stay @notice : Explain to an end user what this doe
@dev is the tag to : Explain to a developer any extra details
@notice : Performs bulk payout to multiple workers
@dev Escrow needs to be completed ...
I think certik wanted to have a clear comments :
- solc --devdoc --pretty-json Escrow.sol will return details with comments.
to check [https://coinsbench.com/natspec-the-right-way-to-comment-ethereum-smart-contracts-6762082252d4]
Description
Certik v3 report is asking to write more comprehensive documentation about
bulkPayout
function for clarity.Summary of changes
SafeMath
utils inHMToken.sol
.bulkPayout
function ofEscrow.sol
.How test the changes
Related issues
Keywords for linking issues
Closes #438
Operational checklist