Skip to content

Commit 6ad1f01

Browse files
committed
Add nonReentrantView modifier
1 parent 101bbaf commit 6ad1f01

File tree

6 files changed

+69
-4
lines changed

6 files changed

+69
-4
lines changed

contracts/mocks/ReentrancyAttack.sol

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,9 @@ contract ReentrancyAttack is Context {
99
(bool success, ) = _msgSender().call(data);
1010
require(success, "ReentrancyAttack: failed call");
1111
}
12+
13+
function staticcallSender(bytes calldata data) public view {
14+
(bool success, ) = _msgSender().staticcall(data);
15+
require(success, "ReentrancyAttack: failed call");
16+
}
1217
}

contracts/mocks/ReentrancyMock.sol

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ contract ReentrancyMock is ReentrancyGuard {
1616
_count();
1717
}
1818

19+
function viewCallback() external view nonReentrantView returns (uint256) {
20+
return counter;
21+
}
22+
1923
function countLocalRecursive(uint256 n) public nonReentrant {
2024
if (n > 0) {
2125
_count();
@@ -36,6 +40,11 @@ contract ReentrancyMock is ReentrancyGuard {
3640
attacker.callSender(abi.encodeCall(this.callback, ()));
3741
}
3842

43+
function countAndCallView(ReentrancyAttack attacker) public nonReentrant {
44+
_count();
45+
attacker.staticcallSender(abi.encodeCall(this.viewCallback, ()));
46+
}
47+
3948
function _count() private {
4049
counter += 1;
4150
}

contracts/mocks/ReentrancyTransientMock.sol

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ contract ReentrancyTransientMock is ReentrancyGuardTransient {
1616
_count();
1717
}
1818

19+
function viewCallback() external view nonReentrantView {}
20+
1921
function countLocalRecursive(uint256 n) public nonReentrant {
2022
if (n > 0) {
2123
_count();
@@ -36,6 +38,11 @@ contract ReentrancyTransientMock is ReentrancyGuardTransient {
3638
attacker.callSender(abi.encodeCall(this.callback, ()));
3739
}
3840

41+
function countAndCallView(ReentrancyAttack attacker) public nonReentrant {
42+
_count();
43+
attacker.staticcallSender(abi.encodeCall(this.viewCallback, ()));
44+
}
45+
3946
function _count() private {
4047
counter += 1;
4148
}

contracts/utils/ReentrancyGuard.sol

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,24 @@ abstract contract ReentrancyGuard {
6161
_nonReentrantAfter();
6262
}
6363

64+
/**
65+
* @dev View variant of the `nonReentrant` modifier. Can be used to prevent view functions from being called
66+
* while the internal state of the contract is inconsistent, and invariants do not hold.
67+
*
68+
* This being a "view" version of the modifier, it will not set the reentrancy status. This modifier should only
69+
* be used in view functions. Payable and non-payable function should use the standard "nonReentrant" modifier.
70+
*/
71+
modifier nonReentrantView() {
72+
_nonReentrantBeforeView();
73+
_;
74+
}
75+
76+
function _nonReentrantBeforeView() private view {
77+
if (_status == ENTERED) {
78+
revert ReentrancyGuardReentrantCall();
79+
}
80+
}
81+
6482
function _nonReentrantBefore() private {
6583
// On the first call to nonReentrant, _status will be NOT_ENTERED
6684
if (_status == ENTERED) {

contracts/utils/ReentrancyGuardTransient.sol

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,24 @@ abstract contract ReentrancyGuardTransient {
3737
_nonReentrantAfter();
3838
}
3939

40+
/**
41+
* @dev View variant of the `nonReentrant` modifier. Can be used to prevent view functions from being called
42+
* while the internal state of the contract is inconsistent, and invariants do not hold.
43+
*
44+
* This being a "view" version of the modifier, it will not set the reentrancy status. This modifier should only
45+
* be used in view functions. Payable and non-payable function should use the standard "nonReentrant" modifier.
46+
*/
47+
modifier nonReentrantView() {
48+
_nonReentrantBeforeView();
49+
_;
50+
}
51+
52+
function _nonReentrantBeforeView() private view {
53+
if (_reentrancyGuardEntered()) {
54+
revert ReentrancyGuardReentrantCall();
55+
}
56+
}
57+
4058
function _nonReentrantBefore() private {
4159
// On the first call to nonReentrant, REENTRANCY_GUARD_STORAGE.asBoolean().tload() will be false
4260
if (_reentrancyGuardEntered()) {

test/utils/ReentrancyGuard.test.js

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ for (const variant of ['', 'Transient']) {
77
async function fixture() {
88
const name = `Reentrancy${variant}Mock`;
99
const mock = await ethers.deployContract(name);
10-
return { name, mock };
10+
const attacker = await ethers.deployContract('ReentrancyAttack');
11+
return { name, mock, attacker };
1112
}
1213

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

23-
it('does not allow remote callback', async function () {
24-
const attacker = await ethers.deployContract('ReentrancyAttack');
25-
await expect(this.mock.countAndCall(attacker)).to.be.revertedWith('ReentrancyAttack: failed call');
24+
it('nonReentrantView function can be called', async function () {
25+
await this.mock.viewCallback();
26+
});
27+
28+
it('does not allow remote callback to nonReentrant function', async function () {
29+
await expect(this.mock.countAndCall(this.attacker)).to.be.revertedWith('ReentrancyAttack: failed call');
30+
});
31+
32+
it('does not allow remote callback to nonReentrantView function', async function () {
33+
await expect(this.mock.countAndCallView(this.attacker)).to.be.revertedWith('ReentrancyAttack: failed call');
2634
});
2735

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

0 commit comments

Comments
 (0)