QA Report #82
Labels
bug
Something isn't working
QA (Quality Assurance)
Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax
Context wants _msgSender() as a replacement for msg.sender. For regular transactions it returns msg.sender and for meta transactions it can be used to return the end user rather than the relayer. So I think LensNFTBase and all the contracts that inherit from it should use _msgSender(), not msg.sender, e.g.:
LensNFTBase
CollectNFT
FollowNFT
And there are many usages of msg.sender in LensHub.
If you really need Context there, consider replacing msg.sender with _msgSender().
FeeModuleBase
ModuleGlobals
However, whenPublishingEnabled modifier is applied not only to post functions but also comment and mirror. But maybe it's just me misinterpreting the situation and comment/mirror are also considered as publication creation functions.
More details: https://github.com/miguelmota/solidity-idiosyncrasies
However, I do not see any serious issue with your implementation, because you allow only a whitelisted set of characters. so just added this FYI.
but again, maybe you are reading it differently.
contract LensHubStorage should be abstract cuz it doesn't make sense on its own.
Fee modules have this comment, but there is no such thing as FeeCollectModuleBase:
I think it should be FeeModuleBase.
Handle registrations can be frontrunned, but I am not sure if that's a big issue because onlyWhitelistedProfileCreator can do that, and the prevention would introduce unnecessary complexity.
The same sigNonces mapping is used for all the actions. To prevent replay attacks, upon successful execution sigNonces is incremented. This means that to sign unrelated actions a signer needs to wait for one by one to execute it on-chain, it can't happen in parallel. As an active user I want to sign txs to create a post, follow, delegate, change dispatcher, etc independently without waiting for one to finish before signing another.
Consider explicitly marking all the main user functions as non-reentrant, especially those that interact with modules, because many more modules can be whitelisted in the future and there is no guarantee that they will follow the Checks-Effects-Interactions pattern.
The text was updated successfully, but these errors were encountered: