Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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/quick-pianos-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'openzeppelin-solidity': minor
---

`ReentrancyGuard` and `ReentrancyGuardTransient`: Add `nonReentrantView`, a read-only version of the `nonReentrant` modifier.
5 changes: 5 additions & 0 deletions contracts/mocks/ReentrancyAttack.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,9 @@ contract ReentrancyAttack is Context {
(bool success, ) = _msgSender().call(data);
require(success, "ReentrancyAttack: failed call");
}

function staticcallSender(bytes calldata data) public view {
(bool success, ) = _msgSender().staticcall(data);
require(success, "ReentrancyAttack: failed call");
}
}
9 changes: 9 additions & 0 deletions contracts/mocks/ReentrancyMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ contract ReentrancyMock is ReentrancyGuard {
_count();
}

function viewCallback() external view nonReentrantView returns (uint256) {
return counter;
}

function countLocalRecursive(uint256 n) public nonReentrant {
if (n > 0) {
_count();
Expand All @@ -36,6 +40,11 @@ contract ReentrancyMock is ReentrancyGuard {
attacker.callSender(abi.encodeCall(this.callback, ()));
}

function countAndCallView(ReentrancyAttack attacker) public nonReentrant {
_count();
attacker.staticcallSender(abi.encodeCall(this.viewCallback, ()));
}

function _count() private {
counter += 1;
}
Expand Down
9 changes: 9 additions & 0 deletions contracts/mocks/ReentrancyTransientMock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ contract ReentrancyTransientMock is ReentrancyGuardTransient {
_count();
}

function viewCallback() external view nonReentrantView returns (uint256) {
return counter;
}

function countLocalRecursive(uint256 n) public nonReentrant {
if (n > 0) {
_count();
Expand All @@ -36,6 +40,11 @@ contract ReentrancyTransientMock is ReentrancyGuardTransient {
attacker.callSender(abi.encodeCall(this.callback, ()));
}

function countAndCallView(ReentrancyAttack attacker) public nonReentrant {
_count();
attacker.staticcallSender(abi.encodeCall(this.viewCallback, ()));
}

function _count() private {
counter += 1;
}
Expand Down
21 changes: 19 additions & 2 deletions contracts/utils/ReentrancyGuard.sol
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,28 @@ abstract contract ReentrancyGuard {
_nonReentrantAfter();
}

function _nonReentrantBefore() private {
// On the first call to nonReentrant, _status will be NOT_ENTERED
/**
* @dev A `view` only version of {nonReentrant}. Use to block view functions
* from being called, preventing reading from inconsistent contract state.
*
* CAUTION: This is a "view" modifier and does not change the reentrancy
* status. Use it only on view functions. For payable or non-payable functions,
* use the standard {nonReentrant} modifier instead.
*/
modifier nonReentrantView() {
_nonReentrantBeforeView();
_;
}

function _nonReentrantBeforeView() private view {
if (_status == ENTERED) {
revert ReentrancyGuardReentrantCall();
}
}

function _nonReentrantBefore() private {
// On the first call to nonReentrant, _status will be NOT_ENTERED
_nonReentrantBeforeView();

// Any calls to nonReentrant after this point will fail
_status = ENTERED;
Expand Down
21 changes: 19 additions & 2 deletions contracts/utils/ReentrancyGuardTransient.sol
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,28 @@ abstract contract ReentrancyGuardTransient {
_nonReentrantAfter();
}

function _nonReentrantBefore() private {
// On the first call to nonReentrant, REENTRANCY_GUARD_STORAGE.asBoolean().tload() will be false
/**
* @dev A `view` only version of {nonReentrant}. Use to block view functions
* from being called, preventing reading from inconsistent contract state.
*
* CAUTION: This is a "view" modifier and does not change the reentrancy
* status. Use it only on view functions. For payable or non-payable functions,
* use the standard {nonReentrant} modifier instead.
*/
modifier nonReentrantView() {
_nonReentrantBeforeView();
_;
}

function _nonReentrantBeforeView() private view {
if (_reentrancyGuardEntered()) {
revert ReentrancyGuardReentrantCall();
}
}

function _nonReentrantBefore() private {
// On the first call to nonReentrant, REENTRANCY_GUARD_STORAGE.asBoolean().tload() will be false
_nonReentrantBeforeView();

// Any calls to nonReentrant after this point will fail
REENTRANCY_GUARD_STORAGE.asBoolean().tstore(true);
Expand Down
16 changes: 12 additions & 4 deletions test/utils/ReentrancyGuard.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ for (const variant of ['', 'Transient']) {
async function fixture() {
const name = `Reentrancy${variant}Mock`;
const mock = await ethers.deployContract(name);
return { name, mock };
const attacker = await ethers.deployContract('ReentrancyAttack');
return { name, mock, attacker };
}

beforeEach(async function () {
Expand All @@ -20,9 +21,16 @@ for (const variant of ['', 'Transient']) {
expect(await this.mock.counter()).to.equal(1n);
});

it('does not allow remote callback', async function () {
const attacker = await ethers.deployContract('ReentrancyAttack');
await expect(this.mock.countAndCall(attacker)).to.be.revertedWith('ReentrancyAttack: failed call');
it('nonReentrantView function can be called', async function () {
await this.mock.viewCallback();
});

it('does not allow remote callback to nonReentrant function', async function () {
await expect(this.mock.countAndCall(this.attacker)).to.be.revertedWith('ReentrancyAttack: failed call');
});

it('does not allow remote callback to nonReentrantView function', async function () {
await expect(this.mock.countAndCallView(this.attacker)).to.be.revertedWith('ReentrancyAttack: failed call');
});

it('_reentrancyGuardEntered should be true when guarded', async function () {
Expand Down