Skip to content
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

Feature Request: Add ReentrancyGuard #192

Closed
Raiden1411 opened this issue Oct 28, 2022 · 3 comments
Closed

Feature Request: Add ReentrancyGuard #192

Raiden1411 opened this issue Oct 28, 2022 · 3 comments

Comments

@Raiden1411
Copy link
Contributor

Raiden1411 commented Oct 28, 2022

This is a long shot but i think it may be worth a try.

So the idea here is to have solady with all the "standard" contracts much like oz and solmate have.

This way when working with solady as your library of choice you can have all the tools at your disposal instead of having to import from different libraries (unless its very specific). This would make the project have less "bloat" and be cleaner overall.

The code bellow passes on all of solmate's tests as well as saving 100 gas on protected calls.

abstract contract ReentrancyGuard {

    uint256 private constant _UNLOCK = 1;
    uint256 private constant _LOCKED = 2;
    uint256 private constant _REENTRANCY_SELECTOR = 0xab143c06;

    uint256 private _locked = _UNLOCK;
    
    error Reentrancy();

    modifier nonReentrant() virtual {
    
        /// @solidity memory-safe-assembly
        assembly{
            if iszero(eq(sload(_locked.slot), _UNLOCK)) {
                mstore(0x00, _REENTRANCY_SELECTOR)
                revert(0x1c, 0x04)
            }

            sstore(_locked.slot, _LOCKED)
        }

        _;
        
        /// @solidity memory-safe-assembly
        assembly{
            sstore(_locked.slot, _UNLOCK)
        }
    }
}
@Raiden1411 Raiden1411 changed the title Request Feature: Add ReentrancyGuard Feature Request: Add ReentrancyGuard Oct 28, 2022
@Vectorized
Copy link
Owner

Vectorized commented Oct 29, 2022

I'd actually encourage devs to DIY their own reentrancy guard (packed with other variables), so that they save on a cold SLOAD.

Hmm...

@Raiden1411
Copy link
Contributor Author

I agree with that statement, much like you did with ERC721A.

However not all devs will do it and this would be a option for them to use in case they need it.
If you think this goes against the philosophy defined in (#175) feel free to close this issue. But since solady tries to have optimized gas snippets on all areas I though this could belong here.

@z0r0z
Copy link
Collaborator

z0r0z commented Oct 2, 2023

I think we might reopen this just to entertain the doubts of contract developers, particularly if an upgrade path (so v0 might have ReentrancyGuard, while v1 bakes). Whether we like it or not, RG is sticky af and often just tells external parties what cannot happen even if wasteful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants