Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/three-parents-argue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': patch
---

`ReentrancyGuard` and `ReentrancyGuardTransient`: Add `_reentrancyGuardStorageSlot()`, a `pure virtual` function, that can be overriden to allow slot customization.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Make the hook view-virtual (to enable address-derived slots) and fix wording.

Declaring the hook as pure prevents overrides from using address(this) (required for per-module, diamond-safe slots). Also fix typo and comma splice.

-`ReentrancyGuard` and `ReentrancyGuardTransient`: Add `_reentrancyGuardStorageSlot()`, a `pure virtual` function, that can be overriden to allow slot customization.
+`ReentrancyGuard` and `ReentrancyGuardTransient`: Add `_reentrancyGuardStorageSlot()`, an `internal view virtual` function that can be overridden to allow slot customization.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
`ReentrancyGuard` and `ReentrancyGuardTransient`: Add `_reentrancyGuardStorageSlot()`, a `pure virtual` function, that can be overriden to allow slot customization.
`ReentrancyGuard` and `ReentrancyGuardTransient`: Add `_reentrancyGuardStorageSlot()`, an `internal view virtual` function that can be overridden to allow slot customization.
🧰 Tools
🪛 LanguageTool

[grammar] ~5-~5: Ensure spelling is correct
Context: ... a pure virtual function, that can be overriden to allow slot customization.

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)


[grammar] ~5-~5: There might be a mistake here.
Context: ...e overriden to allow slot customization.

(QB_NEW_EN)

🤖 Prompt for AI Agents
.changeset/three-parents-argue.md around line 5: the release note currently
states the hook as a `pure virtual` function which prevents overrides from
referencing `address(this)` (needed for per-module, diamond-safe,
address-derived storage slots) and contains a wording/typo/comma-splice; change
the description to say the hook is a `view virtual` function (so overrides can
use `address(this)`), and rewrite the sentence to fix the typo and comma splice
and clarify that the function can be overridden to customize the storage slot
(e.g., for per-module, diamond-safe slots derived from addresses).

31 changes: 23 additions & 8 deletions contracts/utils/ReentrancyGuard.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

pragma solidity ^0.8.20;

import {StorageSlot} from "./StorageSlot.sol";

/**
* @dev Contract module that helps prevent reentrant calls to a function.
*
Expand All @@ -21,8 +23,19 @@ pragma solidity ^0.8.20;
* TIP: If you would like to learn more about reentrancy and alternative ways
* to protect against it, check out our blog post
* https://blog.openzeppelin.com/reentrancy-after-istanbul/[Reentrancy After Istanbul].
*
* As of v5.5, this storage-based version of the reentrancy guard is DEPRECATED. In the
* next major release (v6.0), {ReentrancyGuardTransient} will be removed, and its logic
* moved to this contract, making transient storage the standard storage option for
* reentrancy guards.
*/
abstract contract ReentrancyGuard {
using StorageSlot for bytes32;

// keccak256(abi.encode(uint256(keccak256("openzeppelin.storage.ReentrancyGuard")) - 1)) & ~bytes32(uint256(0xff))
bytes32 private constant REENTRANCY_GUARD_STORAGE =
0x9b779b17422d0df92223018b32b4d1fa46e071723d6817e2486d003becc55f00;

// Booleans are more expensive than uint256 or any type that takes up a full
// word because each write operation emits an extra SLOAD to first read the
// slot's contents, replace the bits taken up by the boolean, and then write
Expand All @@ -37,15 +50,13 @@ abstract contract ReentrancyGuard {
uint256 private constant NOT_ENTERED = 1;
uint256 private constant ENTERED = 2;

uint256 private _status;

/**
* @dev Unauthorized reentrant call.
*/
error ReentrancyGuardReentrantCall();

constructor() {
_status = NOT_ENTERED;
_reentrancyGuardStorageSlot().getUint256Slot().value = NOT_ENTERED;
}

/**
Expand All @@ -62,26 +73,30 @@ abstract contract ReentrancyGuard {
}

function _nonReentrantBefore() private {
// On the first call to nonReentrant, _status will be NOT_ENTERED
if (_status == ENTERED) {
// On the first call to nonReentrant, REENTRANCY_GUARD_STORAGE.getUint256Slot().value will be NOT_ENTERED
if (_reentrancyGuardEntered()) {
revert ReentrancyGuardReentrantCall();
}

// Any calls to nonReentrant after this point will fail
_status = ENTERED;
_reentrancyGuardStorageSlot().getUint256Slot().value = ENTERED;
}

function _nonReentrantAfter() private {
// By storing the original value once again, a refund is triggered (see
// https://eips.ethereum.org/EIPS/eip-2200)
_status = NOT_ENTERED;
_reentrancyGuardStorageSlot().getUint256Slot().value = NOT_ENTERED;
}

/**
* @dev Returns true if the reentrancy guard is currently set to "entered", which indicates there is a
* `nonReentrant` function in the call stack.
*/
function _reentrancyGuardEntered() internal view returns (bool) {
return _status == ENTERED;
return _reentrancyGuardStorageSlot().getUint256Slot().value == ENTERED;
}

function _reentrancyGuardStorageSlot() internal pure virtual returns (bytes32) {
return REENTRANCY_GUARD_STORAGE;
}
}
10 changes: 7 additions & 3 deletions contracts/utils/ReentrancyGuardTransient.sol
Original file line number Diff line number Diff line change
Expand Up @@ -44,18 +44,22 @@ abstract contract ReentrancyGuardTransient {
}

// Any calls to nonReentrant after this point will fail
REENTRANCY_GUARD_STORAGE.asBoolean().tstore(true);
_reentrancyGuardStorageSlot().asBoolean().tstore(true);
}

function _nonReentrantAfter() private {
REENTRANCY_GUARD_STORAGE.asBoolean().tstore(false);
_reentrancyGuardStorageSlot().asBoolean().tstore(false);
}

/**
* @dev Returns true if the reentrancy guard is currently set to "entered", which indicates there is a
* `nonReentrant` function in the call stack.
*/
function _reentrancyGuardEntered() internal view returns (bool) {
return REENTRANCY_GUARD_STORAGE.asBoolean().tload();
return _reentrancyGuardStorageSlot().asBoolean().tload();
}

function _reentrancyGuardStorageSlot() internal pure virtual returns (bytes32) {
return REENTRANCY_GUARD_STORAGE;
}
}