[CRE-2087] Propagate execution timestamps for feature flags#1935
[CRE-2087] Propagate execution timestamps for feature flags#1935
Conversation
✅ API Diff Results -
|
| } | ||
| } | ||
|
|
||
| func CapabilityRequestFromProto(pr *CapabilityRequest) (capabilities.CapabilityRequest, error) { |
There was a problem hiding this comment.
Who is supposed to set the right timestamp on this capabilityRequest?
Asking because for making a call from Workflow DON -> Vault DON, I see the capability request is created here, and till now no timestamp there:
https://github.com/smartcontractkit/chainlink/blob/af80ea0f7d313af2b164a509e48b4ad5a77b6cbd/core/services/workflows/v2/secrets.go#L219-L225
There was a problem hiding this comment.
I guess this is an open question separate to this PR.
So approving this PR.
There was a problem hiding this comment.
Engine will set it - core PR coming.
| string workflow_registry_chain_selector = 12; | ||
| string workflow_registry_address = 13; | ||
| string engine_version = 14; | ||
| google.protobuf.Timestamp execution_timestamp = 15; |
There was a problem hiding this comment.
Wait, does this new field also become part of the payload hash on server here:
https://github.com/smartcontractkit/chainlink/blob/41586b4eec7437b564ae77614e8d7333d47a3796/core/capabilities/remote/executable/server.go#L269-L271
If yes, then isn't there a risk that when dontime fails, and we fallback to local node time, then all nodes will use a different timestamp, thus getting a different hash above, causing F+1 failures?
Is that an acceptable situation?
There was a problem hiding this comment.
This field will be excluded in the hasher similarly to other existing fields with the same problem - core PR coming.
2 participants
No description provided.