Skip to content

Commit

Permalink
Avoid wasting gas on vault reentrancy check (#2467)
Browse files Browse the repository at this point in the history
  • Loading branch information
ylv-io committed Jun 5, 2023
1 parent db24e83 commit 7b2ab0a
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 6 deletions.
14 changes: 8 additions & 6 deletions pkg/pool-utils/contracts/lib/VaultReentrancyLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ library VaultReentrancyLib {
// view.
//
// This staticcall always reverts, but we need to make sure it doesn't fail due to a re-entrancy attack.
// Staticcalls consume all gas forwarded to them on a revert. By default, almost the entire available gas
// is forwarded to the staticcall, causing the entire call to revert with an 'out of gas' error.
// Staticcalls consume all gas forwarded to them on a revert caused by storage modification.
// By default, almost the entire available gas is forwarded to the staticcall,
// causing the entire call to revert with an 'out of gas' error.
//
// We set the gas limit to 100k, but the exact number doesn't matter because view calls are free, and non-view
// calls won't waste the entire gas limit on a revert. `manageUserBalance` is a non-reentrant function in the
// Vault, so calling it invokes `_enterNonReentrant` in the `ReentrancyGuard` contract, reproduced here:
// We set the gas limit to 10k for the staticcall to
// avoid wasting gas when it reverts due to storage modification.
// `manageUserBalance` is a non-reentrant function in the Vault, so calling it invokes `_enterNonReentrant`
// in the `ReentrancyGuard` contract, reproduced here:
//
// function _enterNonReentrant() private {
// // If the Vault is actually being reentered, it will revert in the first line, at the `_require` that
Expand All @@ -73,7 +75,7 @@ library VaultReentrancyLib {
// `manageUserBalance` even gets called), any other error would generate non-zero revertData, so checking for
// empty data guards against this case too.

(, bytes memory revertData) = address(vault).staticcall{ gas: 100_000 }(
(, bytes memory revertData) = address(vault).staticcall{ gas: 10_000 }(
abi.encodeWithSelector(vault.manageUserBalance.selector, 0)
);

Expand Down
8 changes: 8 additions & 0 deletions pkg/pool-utils/test/VaultReentrancyLib.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,14 @@ describe('VaultReentrancyLib', function () {

expectEvent.inReceipt(receipt, 'ProtectedFunctionCalled');
});

it('can call a protected view function outside the Vault context', async () => {
await expect(pool.protectedViewFunction()).to.not.be.reverted;
});

it('do not waste gas', async () => {
await expect(pool.protectedFunction({ gasLimit: 40000 })).to.not.be.reverted;
});
});
});

Expand Down

0 comments on commit 7b2ab0a

Please sign in to comment.