function _commitPrice(
address oracleProvider,
uint256 value,
uint256 index,
uint256 version,
bytes memory data,
bool revertOnFailure
) internal {
UFixed18 balanceBefore = DSU.balanceOf();
if (revertOnFailure) {
IPythOracle(oracleProvider).commit{value: value}(index, version, data);
} else {
try IPythOracle(oracleProvider).commit{value: value}(index, version, data) { }
catch { }
}
// Return through keeper reward if any
DSU.push(msg.sender, DSU.balanceOf().sub(balanceBefore));
}
While we cannot see a way to exploit this Reentrancy to unauthenticated contracts, the fix did prevent calls to unauthentic vaults and markets.
However, the COMMIT_PRICE action can still be used to call an unauthenticated oracleProvider.
Fixed.
Fixed!
This fixes #139 as well.
Fixed.
Fixed.
Fixed.
Fixed.
Fixed.
With the updated code, when there are multiple new orders within the same hour, while the keeper only gets 1 unit of the keeper fee, each order will pay for 1 unit of keeper fee.
A more fair approach would be either:
function _loadPendingPositions(
Context memory context,
address account
) private view returns (
Position[] memory pendingLocalPositions,
Fixed6 collateralAfterFees,
UFixed6 closableAmount
) {
// load latest position information
collateralAfterFees = context.local.collateral;
closableAmount = context.latestPosition.local.magnitude();
pendingLocalPositions = new Position[](
context.local.currentId - Math.min(context.local.latestId, context.local.currentId)
);
UFixed6 previousMagnitude = closableAmount;
// load pending position information
for (uint256 i; i < pendingLocalPositions.length - 1; i++) {
pendingLocalPositions[i] = _pendingPositions[account][context.local.latestId + 1 + i].read();
pendingLocalPositions[i].adjust(context.latestPosition.local);
}
pendingLocalPositions[pendingLocalPositions.length - 1] = context.currentPosition.local; // current positions hasn't been stored yet
for (uint256 i; i < pendingLocalPositions.length; i++) {
collateralAfterFees = collateralAfterFees
.sub(Fixed6Lib.from(pendingLocalPositions[i].fee))
.sub(Fixed6Lib.from(pendingLocalPositions[i].keeper));
closableAmount = closableAmount.sub(
previousMagnitude.sub(pendingLocalPositions[i].magnitude().min(previousMagnitude))
);
previousMagnitude = pendingLocalPositions[i].magnitude();
}
}
L590 will revert due to underflow in the following case:
context.latestPosition.local.magnitude() is equal to 0pendingLocalPositions[0].magnitude() is equal to 10pendingLocalPositions[1].magnitude() is equal to 5The initial value of previousMagnitude is 0.
After the first run (i: 0):
At the second run (i: 1), at L590, 0 - 10 will revert.
modifier keep(UFixed18 multiplier, uint256 buffer, bytes memory dynamicCalldata, bytes memory data) {
uint256 startGas = gasleft();
_;
uint256 gasUsed = startGas - gasleft();
UFixed18 keeperFee = UFixed18Lib.from(gasUsed)
.mul(multiplier)
.add(UFixed18Lib.from(buffer))
.mul(UFixed18.wrap(block.basefee))
.add(_calculateDynamicFee(dynamicCalldata))
.mul(_etherPrice());
_raiseKeeperFee(keeperFee, data);
keeperToken().push(msg.sender, keeperFee);
emit KeeperCall(msg.sender, gasUsed, multiplier, buffer, keeperFee);
}
function commitRequested(uint256 versionIndex, bytes calldata updateData)
public
payable
keep(KEEPER_REWARD_PREMIUM, KEEPER_BUFFER, updateData, "")
{
The dynamicCalldata passed to the keep modifier is not the entire calldata. This will result in a wrong result of _calculateDynamicFee().
Consider calculating the length of the calldata before calling keep and change bytes memory dynamicCalldata to uint256 calldataLength.
No PR attached.
Fixed alongside with #180.
Fixed.
Fixed.
Ack.
Ack.
Fixed.
The updated version may lower the precision from a higher value (e.g. 10) to the fixed precision of 6.
For example, the price of BTT/USD would be rounding down to 0 with the updated version.
Fixed.
/// @notice Returns the latest synced oracle version and the current oracle version
/// @return The latest synced oracle version
/// @return The current oracle version collecting new orders
function status() external view returns (OracleVersion memory, uint256) {
return (latest(), current());
}
/// @notice Returns the latest synced oracle version
/// @return latestVersion Latest oracle version
function latest() public view returns (OracleVersion memory latestVersion) {
if (_latestVersion == 0) return latestVersion;
return latestVersion = OracleVersion(_latestVersion, _prices[_latestVersion], true);
}
/// @notice Commits the price represented by `updateData` to the next version that needs to be committed
/// @dev Will revert if there is an earlier versionIndex that could be committed with `updateData`
/// @param versionIndex The index of the version to commit
/// @param updateData The update data to commit
function commitRequested(uint256 versionIndex, bytes calldata updateData)
public
payable
keep(KEEPER_REWARD_PREMIUM, KEEPER_BUFFER, updateData, "")
{
// This check isn't necessary since the caller would not be able to produce a valid updateData
// with an update time corresponding to a null version, but reverting with a specific error is
// clearer.
if (nextVersionIndexToCommit >= versionList.length) revert PythOracleNoNewVersionToCommitError();
if (versionIndex < nextVersionIndexToCommit) revert PythOracleVersionIndexTooLowError();
uint256 versionToCommit = versionList[versionIndex];
PythStructs.Price memory pythPrice = _validateAndGetPrice(versionToCommit, updateData);
// Price must be more recent than that of the most recently committed version
if (pythPrice.publishTime <= lastCommittedPublishTime) revert PythOracleNonIncreasingPublishTimes();
lastCommittedPublishTime = pythPrice.publishTime;
// Ensure that the keeper is committing the earliest possible version
if (versionIndex > nextVersionIndexToCommit) {
uint256 previousVersion = versionList[versionIndex - 1];
// We can only skip the previous version if the grace period has expired
if (block.timestamp <= previousVersion + GRACE_PERIOD) revert PythOracleGracePeriodHasNotExpiredError();
// If the update is valid for the previous version, we can't skip the previous version
if (
pythPrice.publishTime >= previousVersion + MIN_VALID_TIME_AFTER_VERSION &&
pythPrice.publishTime <= previousVersion + MAX_VALID_TIME_AFTER_VERSION
) revert PythOracleUpdateValidForPreviousVersionError();
}
_recordPrice(versionToCommit, pythPrice);
nextVersionIndexToCommit = versionIndex + 1;
_latestVersion = versionToCommit;
emit OracleProviderVersionFulfilled(versionToCommit);
}
/// @notice Commits the price to a non-requested version
/// @dev This commit function may pay out a keeper reward if the committed version is valid
/// for the next requested version to commit. A proper `versionIndex` must be supplied in case we are
/// ahead of an invalidated requested version and need to verify that the provided version is valid.
/// @param versionIndex The next committable index, taking into account any passed invalid requested versions
/// @param oracleVersion The oracle version to commit
/// @param updateData The update data to commit
function commit(uint256 versionIndex, uint256 oracleVersion, bytes calldata updateData) external payable {
// Must be before the next requested version to commit, if it exists
// Otherwise, try to commit it as the next request version to commit
if (
versionList.length > versionIndex && // must be a requested version
versionIndex >= nextVersionIndexToCommit && // must be the next (or later) requested version
oracleVersion == versionList[versionIndex] // must be the corresponding timestamp
) {
commitRequested(versionIndex, updateData);
return;
}
PythStructs.Price memory pythPrice = _validateAndGetPrice(oracleVersion, updateData);
// Price must be more recent than that of the most recently committed version
if (pythPrice.publishTime <= lastCommittedPublishTime) revert PythOracleNonIncreasingPublishTimes();
lastCommittedPublishTime = pythPrice.publishTime;
// Oracle version must be more recent than that of the most recently committed version
uint256 minVersion = _latestVersion;
uint256 maxVersion = versionList.length > versionIndex ? versionList[versionIndex] : current();
if (versionIndex < nextVersionIndexToCommit) revert PythOracleVersionIndexTooLowError();
if (versionIndex > nextVersionIndexToCommit && block.timestamp <= versionList[versionIndex - 1] + GRACE_PERIOD)
revert PythOracleGracePeriodHasNotExpiredError();
if (oracleVersion <= minVersion || oracleVersion >= maxVersion) revert PythOracleVersionOutsideRangeError();
_recordPrice(oracleVersion, pythPrice);
_latestVersion = oracleVersion;
}
latest() may unexpectedly go back in time.
Given:
When:
commit({versionIndex: 19, oracleVersion: 18:59:59, updateData: (pyth data of 19:00:03)})
oracleVersion == versionList[versionIndex] i.e., 18:59:59 == 19:00:00 is false, so it won't enter commitRequested()commit() does not have a check like commitRequested() L153-L157, so it can skip versionIndex: 18, even if updateData is a valid price for versionIndex: 18latest() returns OracleVersion(timestamp: 18:59:59, ...)commitRequested({versionIndex: 18, updateData: (pyth data of 19:00:04)})
commitRequested() does not have a check like commit() L199's newlatestVersion > oldlatestVersion (i.e., versionToCommit > _latestVersion)latest() returns OracleVersion(timestamp: 18:59:58, ...)In addition to the invariant of newPrice.publishTime > lastCommittedPublishTime, make sure newLatestVersion > oldLatestVersion.
If we assume it's possible for the previous Python feed to experience a more severe issue: instead of not having an eligible price for the requested oracleVersion, the feed completely stopped working after the requested time, making it impossible to find ANY valid price at a later time than the last requested time, this issue would still exist.
Consider adding a method on Oracle to cancel a requested version from the previous OracleProvider.
Fixed.
Ack.