Skip to content

Commit

Permalink
fix: make sure ETH transfer call never reverts
Browse files Browse the repository at this point in the history
  • Loading branch information
xenide committed Jul 7, 2024
1 parent 8f60414 commit 2a020c9
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 10 deletions.
14 changes: 4 additions & 10 deletions src/ReservoirPriceOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@ import { IERC20 } from "forge-std/interfaces/IERC20.sol";
import { IERC4626 } from "forge-std/interfaces/IERC4626.sol";

import { OracleErrors } from "src/libraries/OracleErrors.sol";
import {
IReservoirPriceOracle,
OracleAverageQuery,
OracleLatestQuery
} from "src/interfaces/IReservoirPriceOracle.sol";
import { IReservoirPriceOracle, OracleAverageQuery, OracleLatestQuery } from "src/interfaces/IReservoirPriceOracle.sol";
import { IPriceOracle } from "src/interfaces/IPriceOracle.sol";
import { QueryProcessor, ReservoirPair, PriceType } from "src/libraries/QueryProcessor.sol";
import { Utils } from "src/libraries/Utils.sol";
Expand Down Expand Up @@ -241,12 +237,10 @@ contract ReservoirPriceOracle is IPriceOracle, IReservoirPriceOracle, Owned(msg.
lPayoutAmt = block.basefee * rewardGasAmount;
}

if (lPayoutAmt <= address(this).balance) {
// REVIEW: This can still revert, rather than balance checking we
// could just try/catch or use Yul to ignore reverts?
payable(aRecipient).transfer(lPayoutAmt);
// does not revert even if transfer fails
assembly ("memory-safe") {
let result := call(gas(), aRecipient, lPayoutAmt, codesize(), 0x00, codesize(), 0x00)

Check warning on line 242 in src/ReservoirPriceOracle.sol

View workflow job for this annotation

GitHub Actions / lint

Variable "result" is unused
}
// do nothing if lPayoutAmt is greater than the balance
}

/// @return rRoute The route to determine the price between aToken0 and aToken1
Expand Down
11 changes: 11 additions & 0 deletions test/mock/GasBuster.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

contract GasBuster {
// Allow the contract to receive ETH
receive() external payable {
while(true) {

Check warning on line 7 in test/mock/GasBuster.sol

View workflow job for this annotation

GitHub Actions / lint

Code contains empty blocks
// This loop will continue until all gas is consumed
}
}
}
26 changes: 26 additions & 0 deletions test/unit/ReservoirPriceOracle.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { EnumerableSetLib } from "lib/solady/src/utils/EnumerableSetLib.sol";
import { Constants } from "src/libraries/Constants.sol";
import { MockFallbackOracle } from "test/mock/MockFallbackOracle.sol";
import { StubERC4626 } from "test/mock/StubERC4626.sol";
import { GasBuster } from "test/mock/GasBuster.sol";

contract ReservoirPriceOracleTest is BaseTest {
using Utils for *;
Expand Down Expand Up @@ -621,6 +622,31 @@ contract ReservoirPriceOracleTest is BaseTest {
assertEq(lPriceAB, 0); // composite price is not stored in the cache
}

function testUpdatePrice_RecipientOutOfGas() external {
// arrange
GasBuster lGasBuster = new GasBuster();

_writePriceCache(address(_tokenA), address(_tokenB), 5e18);
deal(address(_oracle), 1 ether);

skip(1);
_pair.sync();
skip(_oracle.twapPeriod() * 2);
_tokenA.mint(address(_pair), 2e18);
_pair.swap(2e18, true, address(this), "");

// act
_oracle.updatePrice(address(_tokenB), address(_tokenA), address(lGasBuster));

// assert
(uint256 lPrice,) = _oracle.priceCache(address(_tokenA), address(_tokenB));
assertEq(lPrice, 98_918_868_099_219_913_512);
(lPrice,) = _oracle.priceCache(address(_tokenB), address(_tokenA));
assertEq(lPrice, 0);
assertEq(address(this).balance, block.basefee * _oracle.rewardGasAmount());
assertEq(address(_oracle).balance, 1 ether - block.basefee * _oracle.rewardGasAmount());
}

function testSetRoute() public {
// arrange
address lToken0 = address(_tokenB);
Expand Down

0 comments on commit 2a020c9

Please sign in to comment.