Skip to content

fix(oracle): reject silent price update failures #316

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.

Sign up for GitHub

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

Open
0xCardiE wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(oracle): reject silent price update failures #316

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 76 additions & 77 deletions src/PriceOracle.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ contract PriceOracle is AccessControl {
*@dev Emitted on every price update.
*/
event PriceUpdate(uint256 price);
event StampPriceUpdateFailed(uint256 attemptedPrice);

// ----------------------------- Custom Errors ------------------------------
error CallerNotAdmin(); // Caller is not the admin
error CallerNotPriceUpdater(); // Caller is not a price updater
error PriceAlreadyAdjusted(); // Price already adjusted in this round
error UnexpectedZero(); // Redundancy needs to be higher then 0
error OraclePaused(); // Oracle is paused and cannot adjust price

// ----------------------------- CONSTRUCTOR ------------------------------

Expand All @@ -71,97 +71,57 @@ contract PriceOracle is AccessControl {

/**
* @notice Manually set the price.
* @dev Can only be called by the admin role.
* @dev Can only be called by the admin role. Reverts if PostageStamp rejects the update.
* @param _price The new price.
*/ function setPrice(uint32 _price) external returns (bool) {
*/
function setPrice(uint32 _price) external returns (bool) {
if (!hasRole(DEFAULT_ADMIN_ROLE, msg.sender)) {
revert CallerNotAdmin();
}

uint64 _currentPriceUpScaled = _price << 10;
uint64 _minimumPriceUpscaled = minimumPriceUpscaled;

// Enforce minimum price
if (_currentPriceUpScaled < _minimumPriceUpscaled) {
_currentPriceUpScaled = _minimumPriceUpscaled;
}
currentPriceUpScaled = _currentPriceUpScaled;

// Check if the setting of price in postagestamp succeded
(bool success, ) = address(postageStamp).call(
abi.encodeWithSignature("setPrice(uint256)", uint256(currentPrice()))
);
if (!success) {
emit StampPriceUpdateFailed(currentPrice());
return false;
uint64 newPriceUpScaled = uint64(_price) << 10;
if (newPriceUpScaled < minimumPriceUpscaled) {
newPriceUpScaled = minimumPriceUpscaled;
}
emit PriceUpdate(currentPrice());

_commitPrice(newPriceUpScaled);
return true;
}

/**
* @notice Adjust the price based on observed redundancy.
* @dev Reverts when paused, already adjusted this round, or PostageStamp rejects the update.
*/
function adjustPrice(uint16 redundancy) external returns (bool) {
if (isPaused == false) {
if (!hasRole(PRICE_UPDATER_ROLE, msg.sender)) {
revert CallerNotPriceUpdater();
}

uint16 usedRedundancy = redundancy;
uint64 currentRoundNumber = currentRound();

// Price can only be adjusted once per round
if (currentRoundNumber <= lastAdjustedRound) {
revert PriceAlreadyAdjusted();
}
// Redundancy may not be zero
if (redundancy == 0) {
revert UnexpectedZero();
}

// Enforce maximum considered extra redundancy
uint16 maxConsideredRedundancy = targetRedundancy + maxConsideredExtraRedundancy;
if (redundancy > maxConsideredRedundancy) {
usedRedundancy = maxConsideredRedundancy;
}

uint64 _currentPriceUpScaled = currentPriceUpScaled;
uint64 _minimumPriceUpscaled = minimumPriceUpscaled;
uint32 _priceBase = priceBase;

// Set the number of rounds that were skipped, we substract 1 as lastAdjustedRound is set below and default result is 1
uint64 skippedRounds = currentRoundNumber - lastAdjustedRound - 1;
if (isPaused) {
revert OraclePaused();
}
if (!hasRole(PRICE_UPDATER_ROLE, msg.sender)) {
revert CallerNotPriceUpdater();
}

// We first apply the increase/decrease rate for the current round
uint32 _changeRate = changeRate[usedRedundancy];
_currentPriceUpScaled = (_changeRate * _currentPriceUpScaled) / _priceBase;
uint64 currentRoundNumber = currentRound();

// If previous rounds were skipped, use MAX price increase for the previous rounds
if (skippedRounds > 0) {
_changeRate = changeRate[0];
for (uint64 i = 0; i < skippedRounds; i++) {
_currentPriceUpScaled = (_changeRate * _currentPriceUpScaled) / _priceBase;
}
}
// Price can only be adjusted once per round
if (currentRoundNumber <= lastAdjustedRound) {
revert PriceAlreadyAdjusted();
}
// Redundancy may not be zero
if (redundancy == 0) {
revert UnexpectedZero();
}

// Enforce minimum price
if (_currentPriceUpScaled < _minimumPriceUpscaled) {
_currentPriceUpScaled = _minimumPriceUpscaled;
}
uint16 usedRedundancy = redundancy;
uint16 maxConsideredRedundancy = targetRedundancy + maxConsideredExtraRedundancy;
if (redundancy > maxConsideredRedundancy) {
usedRedundancy = maxConsideredRedundancy;
}

currentPriceUpScaled = _currentPriceUpScaled;
lastAdjustedRound = currentRoundNumber;
uint64 newPriceUpScaled = _computeAdjustedPriceUpScaled(usedRedundancy, currentRoundNumber);

// Check if the price set in postagestamp succeded
(bool success, ) = address(postageStamp).call(
abi.encodeWithSignature("setPrice(uint256)", uint256(currentPrice()))
);
if (!success) {
emit StampPriceUpdateFailed(currentPrice());
return false;
}
emit PriceUpdate(currentPrice());
return true;
}
return false;
_commitPrice(newPriceUpScaled);
lastAdjustedRound = currentRoundNumber;
return true;
}

function pause() external {
Expand Down Expand Up @@ -207,4 +167,43 @@ contract PriceOracle is AccessControl {
// We downcasted to uint32 and bitshift it by 2^10
return uint32((minimumPriceUpscaled) >> 10);
}

/**
* @dev Push the price to PostageStamp first; only update local state after success.
*/
function _commitPrice(uint64 newPriceUpScaled) internal {
uint32 newPrice = uint32(newPriceUpScaled >> 10);
postageStamp.setPrice(uint256(newPrice));
currentPriceUpScaled = newPriceUpScaled;
emit PriceUpdate(newPrice);
}

function _computeAdjustedPriceUpScaled(
uint16 usedRedundancy,
uint64 currentRoundNumber
) internal view returns (uint64) {
uint64 newPriceUpScaled = currentPriceUpScaled;
uint32 _priceBase = priceBase;

// Set the number of rounds that were skipped, we substract 1 as lastAdjustedRound is set below and default result is 1
uint64 skippedRounds = currentRoundNumber - lastAdjustedRound - 1;

// We first apply the increase/decrease rate for the current round
uint32 _changeRate = changeRate[usedRedundancy];
newPriceUpScaled = (_changeRate * newPriceUpScaled) / _priceBase;

// If previous rounds were skipped, use MAX price increase for the previous rounds
if (skippedRounds > 0) {
_changeRate = changeRate[0];
for (uint64 i = 0; i < skippedRounds; i++) {
newPriceUpScaled = (_changeRate * newPriceUpScaled) / _priceBase;
}
}

if (newPriceUpScaled < minimumPriceUpscaled) {
newPriceUpScaled = minimumPriceUpscaled;
}

return newPriceUpScaled;
}
}
6 changes: 5 additions & 1 deletion src/Redistribution.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,11 @@ contract Redistribution is AccessControl, Pausable {
}
}

bool success = OracleContract.adjustPrice(uint16(redundancyCount));
// Use a low-level call so claim finalization is not blocked when price adjustment
// cannot proceed (e.g. oracle paused). Oracle still reverts on direct calls.
(bool success, ) = address(OracleContract).call(
abi.encodeWithSelector(IPriceOracle.adjustPrice.selector, uint16(redundancyCount))
);
if (!success) {
emit PriceAdjustmentSkipped(uint16(redundancyCount));
}
Expand Down
22 changes: 21 additions & 1 deletion test/PriceOracle.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@
},
auto: {
notZero: 'UnexpectedZero()',
paused: 'OraclePaused()',
},
};

describe('PriceOracle', function () {
let minimumPrice: number;

Check warning on line 33 in test/PriceOracle.test.ts

View workflow job for this annotation

GitHub Actions / Check Code Quality & Run Tests 

'minimumPrice' is assigned a value but never used

describe('when deploying contract', function () {
beforeEach(async function () {
Expand Down Expand Up @@ -179,7 +180,7 @@
describe('automatic update', function () {
let minPriceString: string;
let priceOracle: Contract, postageStamp: Contract;
let priceBaseString: string;

Check warning on line 183 in test/PriceOracle.test.ts

View workflow job for this annotation

GitHub Actions / Check Code Quality & Run Tests 

'priceBaseString' is assigned a value but never used
let priceBase: number;

beforeEach(async function () {
Expand Down Expand Up @@ -246,12 +247,31 @@
expect(await priceOracle.currentPrice()).to.be.eq(newPrice1);
expect(await postageStamp.lastPrice()).to.be.eq(newPrice1);

await priceOracleU.adjustPrice(1);
await expect(priceOracleU.adjustPrice(1)).to.be.revertedWith(errors.auto.paused);

expect(await priceOracle.currentPrice()).to.be.eq(newPrice1);
expect(await postageStamp.lastPrice()).to.be.eq(newPrice1);
});

it('reverts and keeps state when postage rejects the price update', async function () {
const priceOracleU = await ethers.getContract('PriceOracle', updater);
const priceOracleRole = await postageStamp.PRICE_ORACLE_ROLE();

await mineNBlocks(roundLength);

const lastAdjustedBefore = await priceOracle.lastAdjustedRound();
const priceUpScaledBefore = await priceOracle.currentPriceUpScaled();
const stampPriceBefore = await postageStamp.lastPrice();

await postageStamp.revokeRole(priceOracleRole, priceOracle.address);

await expect(priceOracleU.adjustPrice(1)).to.be.revertedWith('PriceOracleOnly()');

expect(await priceOracle.lastAdjustedRound()).to.be.eq(lastAdjustedBefore);
expect(await priceOracle.currentPriceUpScaled()).to.be.eq(priceUpScaledBefore);
expect(await postageStamp.lastPrice()).to.be.eq(stampPriceBefore);
});

it('if redundany factor modulates', async function () {
const priceOracleU = await ethers.getContract('PriceOracle', updater);

Expand Down
Loading