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

Jump to bottom

Add PrimaryFeedUnlocked event to DualAggregator #15381

Merged
merged 2 commits into from
Nov 22, 2024
+58 −28
Merged

Add PrimaryFeedUnlocked event to DualAggregator #15381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7a1008e
Add new PrimaryFeedUnlocked event
alejoberardino Nov 22, 2024
fe9924a
Add test for event
Oozyx Nov 22, 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
49 changes: 25 additions & 24 deletions contracts/gas-snapshots/feeds.gas-snapshot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,10 @@ LatestTimestamp:test_ReturnsSyncedTimestamp() (gas: 170861)
LatestTransmissionDetails:test_ReturnsLatestTransmissionDetails() (gas: 19740)
LatestTransmissionDetails:test_RevertIf_NotEOA() (gas: 8558)
LinkAvailableForPayment:test_ReturnsBalanceWhenNothingDue() (gas: 189995)
LinkAvailableForPayment:test_ReturnsRemainingBalanceWhenHasDues() (gas: 369810)
OracleObservationCount:test_ReturnsCorrectObservationCount() (gas: 223298)
LinkAvailableForPayment:test_ReturnsRemainingBalanceWhenHasDues() (gas: 369822)
OracleObservationCount:test_ReturnsCorrectObservationCount() (gas: 223310)
OracleObservationCount:test_ReturnsZeroWhenNoObservations() (gas: 20050)
OwedPayment:test_ReturnOwedAmount() (gas: 157902)
OwedPayment:test_ReturnOwedAmount() (gas: 157914)
OwedPayment:test_ReturnZeroIfTransmitterNotActive() (gas: 11291)
RequestNewRound:test_RevertIf_NotRequester() (gas: 15094)
RequestNewRound:test_ShouldRequestNewRound() (gas: 32118)
Expand All @@ -85,40 +85,41 @@ SetValidatorConfig:test_EmitsValidatorConfigSet() (gas: 37293)
TransferPayeeship:test_EmitsPayeeshipTransferredRequested() (gas: 46313)
TransferPayeeship:test_RevertIf_SenderIsProposed() (gas: 19978)
TransferPayeeship:test_RevertIf_SenderNotCurrentPayee() (gas: 10713)
Transmit:test_BothFeedsStalledIncomingPrimaryReportIsFromBeforeCutoffTime() (gas: 417229)
Transmit:test_BothFeedsStalledIncomingReportIsFromAfterCutoffTime() (gas: 416765)
Transmit:test_BothFeedsStalledIncomingSecondaryReportIsFromBeforeCutoffTime() (gas: 457955)
Transmit:test_IncomingSecondaryReportHasNotBeenRecordedOlderThanLatestReportNewerThanCutoffTime() (gas: 490673)
Transmit:test_IncomingSecondaryReportHasNotBeenRecordedOlderThanLatestReportOlderThanCutoffTime() (gas: 507994)
Transmit:test_IncomingSecondaryReportRevertsDueToMaxIterations() (gas: 790831)
Transmit:test_OutOfSyncFeedsPrimaryIsSourcedFromSecondaryWithLockDelay() (gas: 414120)
Transmit:test_OutOfSyncFeedsSecondaryFeedFallbackToStandardFeed() (gas: 785444)
Transmit:test_BothFeedsStalledIncomingPrimaryReportIsFromBeforeCutoffTime() (gas: 417253)
Transmit:test_BothFeedsStalledIncomingReportIsFromAfterCutoffTime() (gas: 416789)
Transmit:test_BothFeedsStalledIncomingSecondaryReportIsFromBeforeCutoffTime() (gas: 457979)
Transmit:test_EmitsPrimaryFeedUnlockedWhenPrimaryFeedIsUnlocked() (gas: 239616)
Transmit:test_IncomingSecondaryReportHasNotBeenRecordedOlderThanLatestReportNewerThanCutoffTime() (gas: 491369)
Transmit:test_IncomingSecondaryReportHasNotBeenRecordedOlderThanLatestReportOlderThanCutoffTime() (gas: 508018)
Transmit:test_IncomingSecondaryReportRevertsDueToMaxIterations() (gas: 790891)
Transmit:test_OutOfSyncFeedsPrimaryIsSourcedFromSecondaryWithLockDelay() (gas: 414189)
Transmit:test_OutOfSyncFeedsSecondaryFeedFallbackToStandardFeed() (gas: 785492)
Transmit:test_ReadExpectedInitialState() (gas: 146325)
Transmit:test_RevertIf_CalldataLengthMismatch() (gas: 23373)
Transmit:test_RevertIf_CalldataLengthMismatch() (gas: 23351)
Transmit:test_RevertIf_ConfigDigestMismatch() (gas: 20923)
Transmit:test_RevertIf_DuplicateSigner() (gas: 89805)
Transmit:test_RevertIf_MedianIsOutOfMinMaxRange() (gas: 155167)
Transmit:test_RevertIf_NumObservationsOutOfBounds() (gas: 95658)
Transmit:test_RevertIf_NumObservationsOutOfBounds() (gas: 95636)
Transmit:test_RevertIf_ReportLengthMismatch() (gas: 78027)
Transmit:test_RevertIf_SendTheSameReportAlternateFirstPrimaryFeed() (gas: 337063)
Transmit:test_RevertIf_SendTheSameReportAlternateFirstSecondaryFeed() (gas: 334495)
Transmit:test_RevertIf_SendTheSameReportTwiceToThePrimaryFeed() (gas: 265425)
Transmit:test_RevertIf_SendTheSameReportTwiceToTheSecondaryFeed() (gas: 266117)
Transmit:test_RevertIf_SendTheSameReportAlternateFirstPrimaryFeed() (gas: 337053)
Transmit:test_RevertIf_SendTheSameReportAlternateFirstSecondaryFeed() (gas: 335777)
Transmit:test_RevertIf_SendTheSameReportTwiceToThePrimaryFeed() (gas: 265437)
Transmit:test_RevertIf_SendTheSameReportTwiceToTheSecondaryFeed() (gas: 266129)
Transmit:test_RevertIf_SignatureError() (gas: 86518)
Transmit:test_RevertIf_SignaturesOutOfRegistration() (gas: 25139)
Transmit:test_RevertIf_TooFewValuesToTrustMedian() (gas: 69820)
Transmit:test_RevertIf_UnauthorizedTransmitter() (gas: 14171)
Transmit:test_RevertIf_WrongNumberOfSignatures() (gas: 25000)
Transmit:test_SyncFeedsTransmitSecondaryFirstAlwaysSameBlock() (gas: 465213)
Transmit:test_SyncFeedsTransmitSecondaryFirstNeverSameBlock() (gas: 466181)
Transmit:test_SyncFeedsTransmitStandardFirstAlwaysSameBlock() (gas: 466664)
Transmit:test_SyncFeedsTransmitStandardFirstNeverSameBlock() (gas: 508501)
Transmit:test_SyncFeedsTransmitSecondaryFirstAlwaysSameBlock() (gas: 467777)
Transmit:test_SyncFeedsTransmitSecondaryFirstNeverSameBlock() (gas: 468745)
Transmit:test_SyncFeedsTransmitStandardFirstAlwaysSameBlock() (gas: 466688)
Transmit:test_SyncFeedsTransmitStandardFirstNeverSameBlock() (gas: 508525)
TypeAndVersion:test_IsCorrect() (gas: 10130)
ValidateAnswer:test_RevertIf_InsufficientGas() (gas: 329840)
ValidateAnswer:test_ValidateWithContractAddress() (gas: 374085)
ValidateAnswer:test_ValidateWithZeroAddressContract() (gas: 244497)
ValidateAnswer:test_ValidateWithContractAddress() (gas: 374097)
ValidateAnswer:test_ValidateWithZeroAddressContract() (gas: 244509)
Version:test_ReturnsTheCorrectVersion() (gas: 8703)
WithdrawFunds:test_RevertIf_InsufficientBalance() (gas: 337700)
WithdrawFunds:test_RevertIf_InsufficientBalance() (gas: 337712)
WithdrawFunds:test_RevertIf_InsufficientFunds() (gas: 191296)
WithdrawFunds:test_RevertIf_NotOwner() (gas: 14903)
WithdrawPayment:test_PaysOracles() (gas: 234993)
Expand Down
8 changes: 8 additions & 0 deletions contracts/src/v0.8/feeds/DualAggregator.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,10 @@ contract DualAggregator is OCR2Abstract, Ownable2StepMsgSender, AggregatorV2V3In
/// @param secondaryRoundId the new secondary round id.
event SecondaryRoundIdUpdated(uint32 indexed secondaryRoundId);

/// @notice indicates that a new report arrived from the primary feed and the report had already been stored .
/// @param primaryRoundId the new primary round id (if we're at the next block since the report it should be the same).
event PrimaryFeedUnlocked(uint32 indexed primaryRoundId);
Copy link

Choose a reason for hiding this comment

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

Would it make more sense to emit an event when the lock status changes, instead of only when it unlocks?

I was thinking we could call it: PrimaryFeedLockStatusUpdated(bool isLocked, uint32 roundId)

Copy link
Author

@alejoberardino alejoberardino Nov 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The issue with emitting that way is that it wouldn't really give us new information, as cases where the new report comes in through the secondary feed are covered by the new tx + secondary event, and the way it's implemented currently this boolean is always set (for simplicity, which makes sense to me).

The addition of this event makes it so we keep it very simple on the metrics side and basically simply compare round Ids. This new one covers the case where the report came in through the secondary feed and in the same block the update through the primary, which is very unlikely but seemed easy to cover. The one case that cannot be covered by events alone are fallbacks as we have discussed previously, but that will be a separate effort to monitor.


/// @notice emitted when a new cutoff time is set.
/// @param cutoffTime the new defined cutoff time.
event CutoffTimeSet(uint32 cutoffTime);
Expand Down Expand Up @@ -776,6 +780,10 @@ contract DualAggregator is OCR2Abstract, Ownable2StepMsgSender, AggregatorV2V3In
_verifySignatures(reportContext, report, rs, ss, rawVs);

_report(hotVars, reportContext[0], epochAndRound, report_, isSecondary);
} else {
// If the report is the same and the latest sender was the secondary feed,
// we're effectively unlocking the primary feed with this
emit PrimaryFeedUnlocked(s_hotVars.latestAggregatorRoundId);
}

// Store if the latest report was secondary or not.
Expand Down
29 changes: 25 additions & 4 deletions contracts/src/v0.8/feeds/test/DualAggregator.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,6 +602,8 @@ contract Transmit is ConfiguredDualAggregatorBaseTest {
uint32 timestamp;
}

event PrimaryFeedUnlocked(uint32 indexed primaryRoundId);

function setUp() public override {
super.setUp();

Expand Down Expand Up @@ -1461,6 +1463,25 @@ contract Transmit is ConfiguredDualAggregatorBaseTest {
);
}

function test_EmitsPrimaryFeedUnlockedWhenPrimaryFeedIsUnlocked() public {
_changePrank(s_aggregator.getTransmitters()[0]);

Report memory report1 = Report({price: 1, timestamp: uint32(block.timestamp)});
ReportGenerator.SignedReport memory signedReport1 =
s_reportGenerator.generateSignedReport(report1.price, report1.timestamp);

// transmit to secondary
s_aggregator.transmitSecondary(
signedReport1.reportContext, signedReport1.report, signedReport1.rs, signedReport1.ss, signedReport1.rawVs
);

vm.expectEmit();
emit PrimaryFeedUnlocked(1);
s_aggregator.transmit(
signedReport1.reportContext, signedReport1.report, signedReport1.rs, signedReport1.ss, signedReport1.rawVs
);
}

function _transmitAndCheck(
ReportGenerator.SignedReport memory signedReport,
int256 expectedStandardFeedAnswer,
Expand All @@ -1470,14 +1491,14 @@ contract Transmit is ConfiguredDualAggregatorBaseTest {
) internal {
_changePrank(s_aggregator.getTransmitters()[0]);

if (transmitSecondary) {
s_aggregator.transmitSecondary(
if (transmitPrimary) {
s_aggregator.transmit(
signedReport.reportContext, signedReport.report, signedReport.rs, signedReport.ss, signedReport.rawVs
);
}

if (transmitPrimary) {
s_aggregator.transmit(
if (transmitSecondary) {
s_aggregator.transmitSecondary(
signedReport.reportContext, signedReport.report, signedReport.rs, signedReport.ss, signedReport.rawVs
);
}
Expand Down
Loading