Fail invalid consensus outcomes per request instead of signing them - #748
Fail invalid consensus outcomes per request instead of signing them#748ilija42 wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bad5f0260
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // Every observation contributes to the median timestamp, so an outcome that falls back to the caller's default | ||
| // after f+1 errors is still stamped with when the DON observed it. | ||
| timestamps = append(timestamps, obs.ReceivedAt) |
There was a problem hiding this comment.
During a rolling upgrade, this unconditional change makes upgraded nodes include error observations in the median timestamp while nodes running the parent revision include only value observations. For requests containing errors—especially f+1 errors followed by a default—the same attributed observations therefore produce different serialized outcomes, and the versions can also disagree later between an epoch-zero success and an invalid-outcome failure report. This can prevent enough matching OCR signatures from being collected or allow the old invalid behavior depending on the participating cohort; negotiate this semantic change through an observation flag, as the adjacent error-handling migrations do.
Useful? React with 👍 / 👎.
0bad5f0 to
640ba85
Compare
|
1 participant
The
Reportsphase signed outcomes that had an empty payload, an unknown request type, or a zero timestamp. One invalid outcome also stopped the full round and dropped all other requests in the batch.This change checks each outcome before the plugin signs it. If the check fails, the plugin makes a failure report with the new
INVALID_OUTCOMEcode for that request only. The report count limit now includes oversized reports.Error observations now add their
received_attime to the median timestamp. A new flag onRequestObservationcontrols this. The plugin uses the new behavior only when all observations for the request set the flag, so a mixed set of nodes makes the same outcome.SimpleandReportreject a report that has no payload after the metadata prefix.