-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add extra oracle tests #734
Conversation
contracts/test/OracleMainnet.t.sol
Outdated
// Etch the mock code to the RETH token address | ||
vm.etch(address(rethToken), _mockTokenCode); | ||
// // Wrap so we can use the mock's functions | ||
// GasGuzzlerToken mockReth = GasGuzzlerToken(address(rethToken)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a leftover?
contracts/test/OracleMainnet.t.sol
Outdated
// After etching the gas guzzler to the LST, confirm the same call with 500k gas now reverts due to OOG | ||
vm.expectRevert(MainnetPriceFeedBase.InsufficientGasForExternalCall.selector); | ||
// just catch return val to suppress warning | ||
(success,) = address(wstethPriceFeed).call{gas: 10000}(abi.encodeWithSignature("fetchPrice()")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says “the same call with 500k”, but we are sending only 10k?
contracts/test/OracleMainnet.t.sol
Outdated
// After etching the gas guzzler to the LST, confirm the same call with 500k gas now reverts due to OOG | ||
vm.expectRevert(MainnetPriceFeedBase.InsufficientGasForExternalCall.selector); | ||
// just catch return val to suppress warning | ||
(success,) = address(rethPriceFeed).call{gas: 10000}(abi.encodeWithSignature("fetchPrice()")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
contracts/test/OracleMainnet.t.sol
Outdated
|
||
function testRevertLowGasRETHToken() public { | ||
// Confirm call to the real external contracts succeeds with sufficient gas i.e. 500k | ||
(bool success,) = address(wstethPriceFeed).call{gas: 500000}(abi.encodeWithSignature("fetchPrice()")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn’t this be rethPrice
?
} | ||
|
||
// WSTETH exchange rate getter | ||
function stEthPerToken() external view returns (uint256) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t we need one for RETH?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
(bool success,) = address(wstethPriceFeed).call{gas: 10000}(abi.encodeWithSignature("fetchPrice()")); | ||
assertFalse(success); | ||
(bool revertAsExpected,) = address(wstethPriceFeed).call{gas: 500000}(abi.encodeWithSignature("fetchPrice()")); | ||
assertTrue(revertAsExpected); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, so when the failure is catched by vm.expecRevert
then bool
return value is true?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, strange design choice in Foundry IMO. See the "Gotcha: low level calls" section here:
https://book.getfoundry.sh/cheatcodes/expect-revert
No description provided.