Add PrimaryFeedUnlocked event to DualAggregator #15381
Conversation
|
I see you updated files related to |
AER Report: Operator UI CIaer_workflow , commit , Breaking Changes GQL Check 1. Workflow failed to complete successfully:[convictional/trigger-workflow-and-wait@f69fa9e]Source of Error:Why: The triggered workflow did not complete successfully. The status was marked as "completed" but the conclusion was "failure," which caused the upstream job to propagate the failure. Suggested fix: Investigate the logs of the triggered workflow (ID: 11978688320) to identify the specific reason for the failure. Address the root cause in the downstream workflow to ensure it completes successfully. AER Report: CI Core ran successfully ✅ |
| @@ -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); | |||
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.
|
Adds an event to cover a missing case in monitoring.