Introduce ReentrancyGuardLib allowing more agile function accessibility control #3846
Conversation
k06a
force-pushed
the
feature/reentrancy-guard-lib
branch
from
a6eb04f
to
bb9cae6
Compare
|
@frangio what do you think? |
|
@Amxx please check out the PR, I believe it is good improvement because it can bring more granular support to this feature. |
There was a problem hiding this comment.
How ironic that @k06a asks me to review that when I'm probably the one person in the entire ethereum space that is fundamentally against ReentrancyGuard.
Still, some devs want to use them because they think its a good solution to they improper design. /s
This could be interesting, I'm not really convinced by the data structure.
| struct Data { | ||
| uint256 _status; | ||
| } |
There was a problem hiding this comment.
I get that the structure is usefull to pass storage pointers to the methods. But it also means that is will use a full slot.
If devs need multiple independant guards, being able to put them in a single slot would be a big plus!
I would do that using a UDVT that is a uint8 ... but then you lose the storage pointer design.
There was a problem hiding this comment.
Bits guardians makes a lot of sense, will consider if it’s possible to have handy API
| } | ||
|
|
||
| function init(Data storage self) internal { | ||
| require(self._status == _NOT_INITIALIZED, "ReentrancyGuard: init already called"); |
There was a problem hiding this comment.
If we ever do that, we'll do custom errors (we don't do revert strings anymore)
There was a problem hiding this comment.
Great, this PR is half-year old, BTW :)
|
We used this guardian in one of ERC20 extensions, it’s obvious not a good idea to use singleton guardian for multiple different extensions, so using separate struct makes a lot of sense. |
Consider having this kind of library, this would allow to have reentrancy guard in different smart contracts and will make it more agile in case of inheriting from multiple contract who use it for different purposes.
PR Checklist
To be done...