opt-in hasher - #23480
Conversation
CORA - Pending ReviewersAll codeowners have approved! ✅ Legend: ✅ Approved | ❌ Changes Requested | 💬 Commented | 🚫 Dismissed | ⏳ Pending | ❓ Unknown For more details, see the full review summary. |
|
I see you updated files related to
|
|
✅ No conflicts with other open PRs targeting |
| WorkflowDonConfigVersion: md.WorkflowDonConfigVersion, | ||
| ReferenceID: md.ReferenceID, | ||
| DecodedWorkflowName: md.DecodedWorkflowName, | ||
| SpendLimits: md.SpendLimits, |
There was a problem hiding this comment.
SpendLimits are already excluded in the default hasher. We should remove that field from base.
|
|
||
| // optInWriteReportExcludeSignaturesHasher combines the metadata opt-in allowlist | ||
| // with WriteReport-specific signature exclusion, mirroring writeReportExcludeSignaturesHasher. | ||
| type optInWriteReportExcludeSignaturesHasher struct { |
There was a problem hiding this comment.
Could we keep existing hasher objects instead of creating new ones?
We could make them identical to current prod state by excluding workflowTag from the base struct but having it under a per-field flag that is ON by default.
That way you avoid the opt-in global ON/OFF flag altogether. The behavior switches to opt-in seamlessly and then after rollout be can exclude workflowTag.
| DecodedWorkflowName: md.DecodedWorkflowName, | ||
| WorkflowRegistryChainSelector: md.WorkflowRegistryChainSelector, | ||
| WorkflowRegistryAddress: md.WorkflowRegistryAddress, | ||
| EngineVersion: md.EngineVersion, |
There was a problem hiding this comment.
nit: you could list all remaining fields in comments to indicate that they are explicitly excluded
|
4 participants
Changing workflow tags persistence in the DB resulted in non-identical hashes in the remote executable server. This change gives us an opt-in hasher.
The previous hashing model was opt-out. We selected fields that we allow to differ (i.e. do not require BFT quorum guarantees). IMO this should have been opt-in from the start. Protos are backwards compatible, hashing a full proto payload is not.