Skip to content
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

Split maxSettlementFee into maxSyncFee and maxAsyncFee #508

Merged
merged 9 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
3 changes: 2 additions & 1 deletion packages/core/test/integration/helpers/setupHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,8 @@ export async function deployProtocol(chainlinkContext?: ChainlinkContext): Promi
await oracleFactory.connect(owner).register(chainlink.oracleFactory.address)
await oracleFactory.connect(owner).updateParameter({
maxGranularity: 10000,
maxSettlementFee: parse6decimal('1000'),
maxAsyncFee: parse6decimal('500'),
maxSyncFee: parse6decimal('500'),
maxOracleFee: parse6decimal('0.5'),
})
const oracle = IOracle__factory.connect(
Expand Down
2 changes: 1 addition & 1 deletion packages/oracle/contracts/OracleFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ contract OracleFactory is IOracleFactory, Factory {
// Re-initialize if owner is unset
if (owner() == address(0)) __Factory__initialize();

_parameter.store(OracleParameter(1, UFixed6Lib.ZERO, UFixed6Lib.ZERO));
_parameter.store(OracleParameter(1, UFixed6Lib.ZERO, UFixed6Lib.ZERO, UFixed6Lib.ZERO));
}

/// @notice Returns the global oracle parameter
Expand Down
3 changes: 2 additions & 1 deletion packages/oracle/contracts/keeper/KeeperOracle.sol
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ contract KeeperOracle is IKeeperOracle, Instance {
priceResponse.asyncFee = UFixed6Lib.from(factory.settlementGasOracle().cost(0), true);
priceResponse.oracleFee = keeperOracleParameter.oracleFee;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually now that applyFeeMaximum is so simple, maybe we can just apply the .min(oracleParameter....) directly on these above and delete the helper function altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed helper method in aac4ffe

priceResponse.applyFeeMaximum(
oracleParameter.maxSettlementFee,
oracleParameter.maxSyncFee,
oracleParameter.maxAsyncFee,
_localCallbacks[oracleVersion.timestamp].length()
);

Expand Down
18 changes: 10 additions & 8 deletions packages/oracle/contracts/keeper/types/PriceResponse.sol
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,17 @@ library PriceResponseLib {
return self.syncFee.add(self.asyncFee.mul(UFixed6Lib.from(callbacks)));
}

/// @notice Scales down sync and async fees if they exceed the maximum settlement fee
/// @notice Scales down sync and async fees if they exceed the maximums
/// @param self The price response object
/// @param maxSettlementFee The maximum settlement fee
function applyFeeMaximum(PriceResponse memory self, UFixed6 maxSettlementFee, uint256 callbacks) internal pure {
UFixed6 totalSettlementFee = settlementFee(self, callbacks);
if (totalSettlementFee.gt(maxSettlementFee)) {
self.syncFee = self.syncFee.muldiv(maxSettlementFee, totalSettlementFee);
self.asyncFee = self.asyncFee.muldiv(maxSettlementFee, totalSettlementFee);
}
/// @param maxSyncFee The max sync fee
/// @param maxAsyncFee The max async fee
/// @param callbacks The number of settlement callbacks to be made
function applyFeeMaximum(PriceResponse memory self, UFixed6 maxSyncFee, UFixed6 maxAsyncFee, uint256 callbacks) internal pure {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of

if (self.syncFee.gt(maxSyncFee))
    self.syncFee = maxSyncFee;

it's a bit cleaner to write these caps as:

self.syncFee = self.syncFee.min(maxSyncFee);

if (self.syncFee.gt(maxSyncFee))
self.syncFee = maxSyncFee;
UFixed6 asyncFee = self.asyncFee.mul(UFixed6Lib.from(callbacks));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The async fee maximum will cap the individual async fees charges (so that if we have tons of orders in a single version we're not capping the total amount charged).

So no need to multiply this by the callback amount, that won't be needed anymore.

if (asyncFee.gt(maxAsyncFee))
self.asyncFee = maxAsyncFee.div(UFixed6Lib.from(callbacks));
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/oracle/contracts/test/PriceResponseTester.sol
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ contract PriceResponseTester {
return PriceResponseLib.settlementFee(self, callbacks);
}

function applyFeeMaximum(UFixed6 maxSettlementFee, uint256 callbacks) external {
function applyFeeMaximum(UFixed6 maxSyncFee, UFixed6 maxAsyncFee, uint256 callbacks) external {
PriceResponse memory newPriceResponse = read();
newPriceResponse.applyFeeMaximum(maxSettlementFee, callbacks);
newPriceResponse.applyFeeMaximum(maxSyncFee, maxAsyncFee, callbacks);
store(newPriceResponse);
}
}
19 changes: 13 additions & 6 deletions packages/oracle/contracts/types/OracleParameter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,20 @@ struct OracleParameter {
/// @dev The cap for the granularity setting in seconds
uint256 maxGranularity;

/// @dev the cap for the settle fee in absolute terms
UFixed6 maxSettlementFee;
/// @dev the cap for the sync fee in absolute terms
UFixed6 maxSyncFee;

/// @dev the cap for the async fee in absolute terms
UFixed6 maxAsyncFee;

/// @dev The cap for the oracle fee in relative terms
UFixed6 maxOracleFee;
}
struct StoredOracleParameter {
/* slot 0 */
uint16 maxGranularity; // <= 65k
uint48 maxSettlementFee; // <= 281m
uint48 maxSyncFee; // <= 281m
uint48 maxAsyncFee; // <= 281m
uint24 maxOracleFee; // <= 100%
}
struct OracleParameterStorage { StoredOracleParameter value; }
Expand All @@ -31,7 +35,8 @@ library OracleParameterStorageLib {
StoredOracleParameter memory storedValue = self.value;
return OracleParameter(
uint256(storedValue.maxGranularity),
UFixed6.wrap(uint256(storedValue.maxSettlementFee)),
UFixed6.wrap(uint256(storedValue.maxSyncFee)),
UFixed6.wrap(uint256(storedValue.maxAsyncFee)),
UFixed6.wrap(uint256(storedValue.maxOracleFee))
);
}
Expand All @@ -45,12 +50,14 @@ library OracleParameterStorageLib {
validate(newValue);

if (newValue.maxGranularity > type(uint16).max) revert OracleParameterStorageInvalidError();
if (newValue.maxSettlementFee.gt(UFixed6.wrap(type(uint48).max))) revert OracleParameterStorageInvalidError();
if (newValue.maxSyncFee.gt(UFixed6.wrap(type(uint48).max))) revert OracleParameterStorageInvalidError();
if (newValue.maxAsyncFee.gt(UFixed6.wrap(type(uint48).max))) revert OracleParameterStorageInvalidError();
if (newValue.maxOracleFee.gt(UFixed6.wrap(type(uint24).max))) revert OracleParameterStorageInvalidError();

self.value = StoredOracleParameter(
uint16(newValue.maxGranularity),
uint48(UFixed6.unwrap(newValue.maxSettlementFee)),
uint48(UFixed6.unwrap(newValue.maxSyncFee)),
uint48(UFixed6.unwrap(newValue.maxAsyncFee)),
uint24(UFixed6.unwrap(newValue.maxOracleFee))
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,8 @@ testOracles.forEach(testOracle => {
await oracleFactory.initialize()
await oracleFactory.connect(owner).updateParameter({
maxGranularity: 10000,
maxSettlementFee: parse6decimal('1000'),
maxSyncFee: parse6decimal('500'),
maxAsyncFee: parse6decimal('500'),
maxOracleFee: parse6decimal('0.5'),
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ testOracles.forEach(testOracle => {
await oracleFactory.initialize()
await oracleFactory.connect(owner).updateParameter({
maxGranularity: 10000,
maxSettlementFee: parse6decimal('1000'),
maxSyncFee: parse6decimal('500'),
maxAsyncFee: parse6decimal('500'),
maxOracleFee: parse6decimal('0.5'),
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ testOracles.forEach(testOracle => {
await oracleFactory.initialize()
await oracleFactory.connect(owner).updateParameter({
maxGranularity: 10000,
maxSettlementFee: parse6decimal('1000'),
maxSyncFee: parse6decimal('500'),
maxAsyncFee: parse6decimal('500'),
maxOracleFee: parse6decimal('0.5'),
})

Expand Down
34 changes: 28 additions & 6 deletions packages/oracle/test/unit/types/OracleParameter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const { ethers } = HRE

const DEFAULT_ORACLE_PARAMETER: OracleParameterStruct = {
maxGranularity: 1,
maxSettlementFee: 0,
maxSyncFee: 0,
maxAsyncFee: 0,
maxOracleFee: 0,
}

Expand Down Expand Up @@ -65,22 +66,43 @@ describe('OracleParameter', () => {
})
})

context('.maxSettlementFee', async () => {
context('.maxSyncFee', async () => {
const STORAGE_SIZE = 48
it('saves if in range', async () => {
await oracleParameter.store({
...DEFAULT_ORACLE_PARAMETER,
maxSettlementFee: BigNumber.from(2).pow(STORAGE_SIZE).sub(1),
maxSyncFee: BigNumber.from(2).pow(STORAGE_SIZE).sub(1),
})
const value = await oracleParameter.read()
expect(value.maxSettlementFee).to.equal(BigNumber.from(2).pow(STORAGE_SIZE).sub(1))
expect(value.maxSyncFee).to.equal(BigNumber.from(2).pow(STORAGE_SIZE).sub(1))
})

it('reverts if maxSettlementFee out of range', async () => {
it('reverts if maxSyncFee out of range', async () => {
await expect(
oracleParameter.store({
...DEFAULT_ORACLE_PARAMETER,
maxSettlementFee: BigNumber.from(2).pow(STORAGE_SIZE),
maxSyncFee: BigNumber.from(2).pow(STORAGE_SIZE),
}),
).to.be.revertedWithCustomError(oracleParameter, 'OracleParameterStorageInvalidError')
})
})

context('.maxAsyncFee', async () => {
const STORAGE_SIZE = 48
it('saves if in range', async () => {
await oracleParameter.store({
...DEFAULT_ORACLE_PARAMETER,
maxAsyncFee: BigNumber.from(2).pow(STORAGE_SIZE).sub(1),
})
const value = await oracleParameter.read()
expect(value.maxAsyncFee).to.equal(BigNumber.from(2).pow(STORAGE_SIZE).sub(1))
})

it('reverts if maxAsyncFee out of range', async () => {
await expect(
oracleParameter.store({
...DEFAULT_ORACLE_PARAMETER,
maxAsyncFee: BigNumber.from(2).pow(STORAGE_SIZE),
}),
).to.be.revertedWithCustomError(oracleParameter, 'OracleParameterStorageInvalidError')
})
Expand Down
56 changes: 29 additions & 27 deletions packages/oracle/test/unit/types/PriceResponse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,94 +300,96 @@ describe('PriceResponse', () => {
})

describe('#applyFeeMaximum', () => {
const maxSyncFee = parse6decimal('1')
const maxAsyncFee = parse6decimal('0.2')

it('calculates correct fee w/ non-zero sync, zero async', async () => {
await priceResponse.store({
...DEFAULT_PRICE_RESPONSE,
syncFee: parse6decimal('1'),
asyncFee: parse6decimal('0.2'),
syncFee: parse6decimal('2'),
asyncFee: parse6decimal('0.4'),
})

await priceResponse.applyFeeMaximum(parse6decimal('0.5'), 0)
await priceResponse.applyFeeMaximum(maxSyncFee, maxAsyncFee, 0)

const value = await priceResponse.read()

expect(value.syncFee).to.equal(parse6decimal('0.5'))
expect(value.asyncFee).to.equal(parse6decimal('0.1'))
expect(value.syncFee).to.equal(maxSyncFee)
expect(value.asyncFee).to.equal(parse6decimal('0.4'))
})

it('calculates correct fee w/ non-zero sync, single async', async () => {
await priceResponse.store({
...DEFAULT_PRICE_RESPONSE,
syncFee: parse6decimal('1'),
asyncFee: parse6decimal('0.2'),
syncFee: parse6decimal('2'),
asyncFee: parse6decimal('0.4'),
})

await priceResponse.applyFeeMaximum(parse6decimal('0.6'), 1)
await priceResponse.applyFeeMaximum(maxSyncFee, maxAsyncFee, 1)

const value = await priceResponse.read()

expect(value.syncFee).to.equal(parse6decimal('0.5'))
expect(value.asyncFee).to.equal(parse6decimal('0.1'))
expect(value.syncFee).to.equal(maxSyncFee)
expect(value.asyncFee).to.equal(maxAsyncFee)
})

it('calculates correct fee w/ non-zero sync, multiple async', async () => {
await priceResponse.store({
...DEFAULT_PRICE_RESPONSE,
syncFee: parse6decimal('1'),
asyncFee: parse6decimal('0.2'),
syncFee: parse6decimal('2'),
asyncFee: parse6decimal('0.4'),
})

await priceResponse.applyFeeMaximum(parse6decimal('1.0'), 5)
await priceResponse.applyFeeMaximum(maxSyncFee, maxAsyncFee, 5)

const value = await priceResponse.read()

expect(value.syncFee).to.equal(parse6decimal('0.5'))
expect(value.asyncFee).to.equal(parse6decimal('0.1'))
expect(value.syncFee).to.equal(maxSyncFee)
expect(value.asyncFee).to.equal(parse6decimal('0.04'))
})

it('calculates correct fee w/ zero sync, zero async', async () => {
await priceResponse.store({
...DEFAULT_PRICE_RESPONSE,
syncFee: parse6decimal('0'),
asyncFee: parse6decimal('0.2'),
asyncFee: parse6decimal('0'),
})

await priceResponse.applyFeeMaximum(parse6decimal('0.0'), 0)
await priceResponse.applyFeeMaximum(maxSyncFee, maxAsyncFee, 0)

const value = await priceResponse.read()

expect(value.syncFee).to.equal(parse6decimal('0'))
expect(value.asyncFee).to.equal(parse6decimal('0.2'))
expect(value.asyncFee).to.equal(parse6decimal('0'))
})

it('calculates correct fee w/ zero sync, single async', async () => {
await priceResponse.store({
...DEFAULT_PRICE_RESPONSE,
syncFee: parse6decimal('0'),
asyncFee: parse6decimal('0.2'),
asyncFee: parse6decimal('0.4'),
})

await priceResponse.applyFeeMaximum(parse6decimal('0.1'), 1)
await priceResponse.applyFeeMaximum(maxSyncFee, maxAsyncFee, 1)

const value = await priceResponse.read()

expect(value.syncFee).to.equal(parse6decimal('0.0'))
expect(value.asyncFee).to.equal(parse6decimal('0.1'))
expect(value.syncFee).to.equal(parse6decimal('0'))
expect(value.asyncFee).to.equal(maxAsyncFee)
})

it('calculates correct fee w/ zero sync, multiple async', async () => {
await priceResponse.store({
...DEFAULT_PRICE_RESPONSE,
syncFee: parse6decimal('0'),
asyncFee: parse6decimal('0.2'),
asyncFee: parse6decimal('0.4'),
})

await priceResponse.applyFeeMaximum(parse6decimal('0.5'), 5)
await priceResponse.applyFeeMaximum(maxSyncFee, maxAsyncFee, 5)

const value = await priceResponse.read()

expect(value.syncFee).to.equal(parse6decimal('0.0'))
expect(value.asyncFee).to.equal(parse6decimal('0.1'))
expect(value.syncFee).to.equal(parse6decimal('0'))
expect(value.asyncFee).to.equal(parse6decimal('0.04'))
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ export function RunPythOracleTests(

await oracleFactory.updateParameter({
maxGranularity: 1,
maxSettlementFee: parse6decimal('1.5'),
maxSyncFee: parse6decimal('1'),
maxAsyncFee: parse6decimal('1'),
maxOracleFee: parse6decimal('0.5'),
})
;[pythOracleFactory, keeperOracle] = await getKeeperOracle()
Expand Down
Loading