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.

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

set roundId to 1 & write to vstorage at start #10291

Merged
merged 4 commits into from
Oct 18, 2024
Merged

set roundId to 1 & write to vstorage at start #10291

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
84d09c7
refactor: set roundId to 1 & write to vstorage at start
Chris-Hibbert Oct 17, 2024
c302226
test: repair a test that needs more time for voting.
Chris-Hibbert Oct 17, 2024
018cf59
test: add a check to show that the roundId has been published
Chris-Hibbert Oct 17, 2024
1bc091a
Merge branch 'master' into 9886-resetRoundID
mergify[bot] Oct 17, 2024
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
17 changes: 16 additions & 1 deletion packages/boot/test/bootstrapTests/price-feed-replace.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,23 @@ test.serial('setupVaults; run updatePriceFeeds proposals', async t => {
refreshAgoricNamesRemotes,
setupVaults,
governanceDriver: gd,
readLatest,
} = t.context;

await setupVaults(collateralBrandKey, managerIndex, setup);

const instancePre = agoricNamesRemotes.instance['ATOM-USD price feed'];

t.like(readLatest('published.priceFeed.ATOM-USD_price_feed.latestRound'), {
roundId: 1n,
});

await priceFeedDrivers[collateralBrandKey].setPrice(15.99);

t.like(readLatest('published.priceFeed.ATOM-USD_price_feed.latestRound'), {
Copy link
Member

Choose a reason for hiding this comment

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

We should test before setPrice (and after the core-eval) to match the problem we saw in practice, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a test before that shows that roundID is 1.

The test on line 148 (it'll move, so early in '2. trigger liquidation by changing price') checks after the coreEval and before setting prices again. Do you want another check in this test ('setupVaults')?

Copy link
Member

Choose a reason for hiding this comment

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

hm. I think I wasn't reading this quite right. no, I don't see any more tests that I want.

roundId: 2n,
});

const priceFeedBuilder =
'@agoric/builders/scripts/inter-protocol/updatePriceFeeds.js';
t.log('building', priceFeedBuilder);
Expand Down Expand Up @@ -132,12 +143,16 @@ test.serial('2. trigger liquidation by changing price', async t => {

await priceFeedDrivers[collateralBrandKey].setPrice(9.99);

t.log(readLatest('published.priceFeed.ATOM-USD_price_feed'), {
t.like(readLatest('published.priceFeed.ATOM-USD_price_feed'), {
// aka 9.99
amountIn: { value: 1000000n },
amountOut: { value: 9990000n },
});

t.like(readLatest('published.priceFeed.ATOM-USD_price_feed.latestRound'), {
roundId: 1n,
});

// check nothing liquidating yet
const liveSchedule: ScheduleNotification = readLatest(
'published.auction.schedule',
Expand Down
57 changes: 53 additions & 4 deletions packages/inter-protocol/src/price/roundsManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ const V3_NO_DATA_ERROR = 'No data present';
/** @type {bigint} */
export const ROUND_MAX = BigInt(2 ** 32 - 1);

const trace = makeTracer('RoundsM', false);
const trace = makeTracer('RoundsM', true);
Copy link
Member

Choose a reason for hiding this comment

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

Do you mean to turn that on in production?
(I don't object. I'm just checking that you didn't mean to turn it back off.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I went back and forth. I settled on yes, because we're diagnosing issues here, and it would give some added visibility.


/** @param {bigint} roundId */
const validRoundId = roundId => {
Expand Down Expand Up @@ -172,10 +172,13 @@ export const prepareRoundsManagerKit = baggage =>
rounds,
unitIn,
};

const roundId = 0n;

return {
...immutable,
lastValueOutForUnitIn: null,
reportingRoundId: 0n,
reportingRoundId: roundId,
};
},
{
Expand Down Expand Up @@ -600,8 +603,8 @@ export const prepareRoundsManagerKit = baggage =>

/**
* a method to provide all current info oracleStatuses need. Intended
* only only to be callable by oracleStatuses. Not for use by contracts
* to read state.
* only to be callable by oracleStatuses. Not for use by contracts to
* read state.
*
* @param {OracleStatus} status
* @param {Timestamp} blockTimestamp
Expand Down Expand Up @@ -728,4 +731,50 @@ export const prepareRoundsManagerKit = baggage =>
},
},
},
{
finish: ({ state }) => {
const { details, rounds, timerPresence } = state;
// Zero is treated as special as roundId and in times. It's hard to
// avoid on restart and in tests, so make 1 the minimum

const firstRound = 1n;
state.reportingRoundId = firstRound;
details.init(
firstRound,
harden({
submissions: [],
maxSubmissions: state.maxSubmissionCount,
minSubmissions: state.minSubmissionCount,
roundTimeout: state.timeout,
}),
);

// Cannot await in first crank. Fail if no timestamp available
void E.when(
E(timerPresence).getCurrentTimestamp(),
nowMaybe => {
const now =
TimeMath.compareAbs(nowMaybe, 1n) < 0
? TimeMath.coerceTimestampRecord(1n, nowMaybe.timerBrand)
: nowMaybe;

const round = harden({
answer: 0n,
startedAt: now,
updatedAt: 0n,
answeredInRound: 0n,
});
rounds.init(firstRound, round);

// In case this is a replacement priceFeed, set roundId in vstorage.
void state.latestRoundPublisher.write({
roundId: firstRound,
startedAt: round.startedAt,
startedBy: 'uninitialized',
});
},
reason => Fail`need a timestamp to start roundsManager ${reason}`,
);
},
},
);
126 changes: 86 additions & 40 deletions packages/inter-protocol/test/price/fluxAggregatorKit.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,46 +94,53 @@ test('basic, with snapshot', async t => {

t.log('----- round 1: basic consensus');
await oracleTimer.tick();
await E(oracleA).pushPrice({ roundId: 1, unitPrice: 100n });
await E(oracleB).pushPrice({ roundId: 1, unitPrice: 200n });
await E(oracleC).pushPrice({ roundId: 1, unitPrice: 300n });
await E(oracleC).pushPrice({ roundId: 1, unitPrice: 130n });
await E(oracleA).pushPrice({ roundId: 1, unitPrice: 110n });
await E(oracleB).pushPrice({ roundId: 1, unitPrice: 120n });
await oracleTimer.tick();

t.log('----- round 2: more prices');
await oracleTimer.tick();
await E(oracleA).pushPrice({ roundId: 2, unitPrice: 100n });
await E(oracleC).pushPrice({ roundId: 2, unitPrice: 300n });
await E(oracleB).pushPrice({ roundId: 2, unitPrice: 200n });
await oracleTimer.tick();

const round1Attempt1 = await E(aggregator.creator).getRoundData(1);
t.is(round1Attempt1.roundId, 1n);
t.is(round1Attempt1.answer, 200n);
t.is(round1Attempt1.answer, 120n);

t.log('----- round 2: check restartDelay implementation');
// since oracle A initialized the last round, it CANNOT start another round before
// the restartDelay, which means its submission will be IGNORED. this means the median
// should ONLY be between the OracleB and C values, which is why it is 25000
await oracleTimer.tick();
await t.throwsAsync(E(oracleA).pushPrice({ roundId: 2, unitPrice: 1000n }), {
await t.throwsAsync(E(oracleA).pushPrice({ roundId: 3, unitPrice: 1000n }), {
message:
'round 2 not accepting submissions from oracle "agorice1priceOracleA"',
'round 3 not accepting submissions from oracle "agorice1priceOracleA"',
});
await E(oracleB).pushPrice({ roundId: 2, unitPrice: 2000n });
await E(oracleC).pushPrice({ roundId: 2, unitPrice: 3000n });
await E(oracleB).pushPrice({ roundId: 3, unitPrice: 2000n });
await E(oracleC).pushPrice({ roundId: 3, unitPrice: 3000n });
await oracleTimer.tick();

const round1Attempt2 = await E(aggregator.creator).getRoundData(1);
t.is(round1Attempt2.answer, 200n);
const round2Attempt1 = await E(aggregator.creator).getRoundData(2);
t.is(round2Attempt1.answer, 2500n);
t.is(round2Attempt1.answer, 200n);
const round3Attempt1 = await E(aggregator.creator).getRoundData(3);
t.is(round3Attempt1.answer, 2500n);

t.log('----- round 3: check oracle submission order');
t.log('----- round 4: check oracle submission order');
// unlike the previous test, if C initializes, all submissions should be recorded,
// which means the median will be the expected 5000 here
await oracleTimer.tick();
await E(oracleC).pushPrice({ roundId: 3, unitPrice: 5000n });
await E(oracleA).pushPrice({ roundId: 3, unitPrice: 4000n });
await E(oracleB).pushPrice({ roundId: 3, unitPrice: 6000n });
await E(oracleC).pushPrice({ roundId: 4, unitPrice: 5000n });
await E(oracleA).pushPrice({ roundId: 4, unitPrice: 4000n });
await E(oracleB).pushPrice({ roundId: 4, unitPrice: 6000n });
await oracleTimer.tick();

const round1Attempt3 = await E(aggregator.creator).getRoundData(1);
t.is(round1Attempt3.answer, 200n);
const round3Attempt1 = await E(aggregator.creator).getRoundData(3);
t.is(round3Attempt1.answer, 5000n);
const round2Attempt2 = await E(aggregator.creator).getRoundData(2);
t.is(round2Attempt2.answer, 200n);
const round4Attempt1 = await E(aggregator.creator).getRoundData(4);
t.is(round4Attempt1.answer, 5000n);

const doc = {
node: 'priceAggregator',
Expand Down Expand Up @@ -526,6 +533,18 @@ test('suggest', async t => {
t.is(round1Attempt1.roundId, 1n);
t.is(round1Attempt1.answer, 200n);

t.deepEqual(
await E(aggregator.creator).oracleRoundState('agorice1priceOracleC', 1n),
{
eligibleForSpecificRound: false,
oracleCount: 3,
latestSubmission: 300n,
queriedRoundId: 1n,
roundTimeout: 5,
startedAt: toTS(1n),
},
);

// ----- round 2: add a new oracle and confirm the suggested round is correct
await oracleTimer.tick();
await E(oracleB).pushPrice({ roundId: 2, unitPrice: 1000n });
Expand Down Expand Up @@ -607,22 +626,41 @@ test('notifications', async t => {
const { oracle: oracleB } = await E(aggregator.creator).initOracle(
'agorice1priceOracleB',
);
const { oracle: oracleC } = await E(aggregator.creator).initOracle(
'agorice1priceOracleC',
);

// Rounds 0 and 1 are weird. Let's jump to round 2
await oracleTimer.tick();
await E(oracleA).pushPrice({ roundId: 1, unitPrice: 350n });
await E(oracleB).pushPrice({ roundId: 1, unitPrice: 250n });
await E(oracleC).pushPrice({ roundId: 1, unitPrice: 150n });
await oracleTimer.tick();
await eventLoopIteration();

await E(oracleB).pushPrice({ roundId: 2, unitPrice: 450n });
await E(oracleC).pushPrice({ roundId: 2, unitPrice: 650n });
await E(oracleA).pushPrice({ roundId: 2, unitPrice: 550n });
await oracleTimer.tick();
await eventLoopIteration();

const publicTopics = await E(aggregator.public).getPublicTopics();
const eachLatestRound = subscribeEach(publicTopics.latestRound.subscriber)[
Symbol.asyncIterator
]();

await oracleTimer.tick();
await E(oracleA).pushPrice({ roundId: 1, unitPrice: 100n });
t.deepEqual((await eachLatestRound.next()).value, {
roundId: 1n,
startedAt: toTS(1n),
startedBy: 'agorice1priceOracleA',
roundId: 2n,
startedAt: toTS(2n),
startedBy: 'agorice1priceOracleB',
});
await E(oracleB).pushPrice({ roundId: 1, unitPrice: 200n });

await oracleTimer.tick();
await E(oracleA).pushPrice({ roundId: 3, unitPrice: 100n });

await oracleTimer.tick();
await E(oracleB).pushPrice({ roundId: 3, unitPrice: 200n });
await eventLoopIteration();

t.deepEqual(
aggregator.mockStorageRoot.getBody(
'mockChainStorageRoot.priceAggregator.LINK-USD_price_feed',
Expand All @@ -634,40 +672,48 @@ test('notifications', async t => {
value: 150n, // AVG(100, 200)
},
timer: Far('ManualTimer'),
timestamp: toMockTS(1n),
timestamp: toMockTS(5n),
},
);

await t.throwsAsync(E(oracleA).pushPrice({ roundId: 2, unitPrice: 1000n }), {
await t.throwsAsync(E(oracleA).pushPrice({ roundId: 4, unitPrice: 1000n }), {
message:
'round 2 not accepting submissions from oracle "agorice1priceOracleA"',
'round 4 not accepting submissions from oracle "agorice1priceOracleA"',
});
// A started last round so fails to start next round
t.deepEqual(
// subscribe fresh because the iterator won't advance yet
(await publicTopics.latestRound.subscriber.subscribeAfter()).head.value,
{
roundId: 1n,
startedAt: toTS(1n),
roundId: 3n,
startedAt: toTS(4n),
startedBy: 'agorice1priceOracleA',
},
);

t.deepEqual((await eachLatestRound.next()).value, {
roundId: 3n,
startedAt: toTS(4n),
startedBy: 'agorice1priceOracleA',
});

// B gets to start it
await E(oracleB).pushPrice({ roundId: 2, unitPrice: 1000n });
await E(oracleB).pushPrice({ roundId: 4, unitPrice: 1000n });
t.deepEqual((await eachLatestRound.next()).value, {
roundId: 2n,
startedAt: toTS(1n),
roundId: 4n,
startedAt: toTS(5n),
startedBy: 'agorice1priceOracleB',
});
// A joins in
await E(oracleA).pushPrice({ roundId: 2, unitPrice: 1000n });
await E(oracleA).pushPrice({ roundId: 4, unitPrice: 1000n });
// writes to storage
t.deepEqual(
aggregator.mockStorageRoot.getBody(
'mockChainStorageRoot.priceAggregator.LINK-USD_price_feed.latestRound',
),
{ roundId: 2n, startedAt: toMockTS(1n), startedBy: 'agorice1priceOracleB' },
{ roundId: 4n, startedAt: toMockTS(5n), startedBy: 'agorice1priceOracleB' },
);
await oracleTimer.tick();

await eventLoopIteration();
t.deepEqual(
Expand All @@ -681,15 +727,15 @@ test('notifications', async t => {
value: 1000n, // AVG(1000, 1000)
},
timer: Far('ManualTimer'),
timestamp: toMockTS(1n),
timestamp: toMockTS(6n),
},
);

// A can start again
await E(oracleA).pushPrice({ roundId: 3, unitPrice: 1000n });
await E(oracleA).pushPrice({ roundId: 5, unitPrice: 1000n });
t.deepEqual((await eachLatestRound.next()).value, {
roundId: 3n,
startedAt: toTS(1n),
roundId: 5n,
startedAt: toTS(6n),
startedBy: 'agorice1priceOracleA',
});
// no new price yet publishable
Expand Down
6 changes: 3 additions & 3 deletions packages/inter-protocol/test/price/snapshots/fluxAggregatorKit.test.js.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,17 @@ Generated by [AVA](https://avajs.dev).
},
timer: Object @Alleged: ManualTimer {},
timestamp: {
absValue: 6n,
absValue: 8n,
timerBrand: Object @Alleged: timerBrand {},
},
},
],
[
'published.priceAggregator.LINK-USD_price_feed.latestRound',
{
roundId: 3n,
roundId: 4n,
startedAt: {
absValue: 5n,
absValue: 7n,
timerBrand: Object @Alleged: timerBrand {},
},
startedBy: 'agorice1priceOracleC',
Expand Down
Binary file modified packages/inter-protocol/test/price/snapshots/fluxAggregatorKit.test.js.snap
View file
Open in desktop
Binary file not shown.
Loading
Loading