[NO-2648] [L-2 Jan Audit] Check return value of transferFrom
#503
Conversation
|
✅ Linked to Task NO-2648 · [L-2] purchasingToken.transferFrom() return value is not checked |
contracts/Market.sol
Outdated
| @@ -1119,6 +1119,7 @@ contract Market is | |||
| from: 0, | |||
| to: countOfRemovalsAllocated | |||
| }); | |||
| bool successfulTransfer; | |||
There was a problem hiding this comment.
nit - booleans best prefixed with affirmative phrase (is, can, has)
https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/names-of-type-members#names-of-properties
https://www.comp.nus.edu.sg/~cs2103/AY1617S1/contents/coding-standards-java.html#:~:text=Boolean%20variables%20should%20be%20prefixed%20with%20'is'&text=This%20is%20the%20naming%20convention,names%20like%20status%20or%20flag.
There was a problem hiding this comment.
IE - isTransferSuccessful
There was a problem hiding this comment.
haha I actually thought about this while I was naming the variable, but apparently not that hard. supported nit
See Jira ticket, which also links to audit report, for more details.
Checks the return value of
transferFromsince unsafe implementations of ERC20 sometimes returnfalseinstead of reverting if a transfer fails. In this case, we should revert.