Only commit to saving payload once the response has been found #532
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 Summary
In #512
handleGetPayload
was changed to save execution payloads even if the publication was unsuccessful. However, the function commits to saving the payload before it has performed its check to see if the payload in question is available. This PR swaps the order of these operations, so that it does not try to save a payload that cannot be found.⛱ Motivation and Context
Currently,
handleGetPayload
commits to saving the requested execution payload (viadefer func()
) immediately after the header signature is verified, in order to ensure that data is stored long-term. Next, the function checks if the payload response is available in the datastore. If it is not available (the relay had never seen the payload, and the proposer is querying all relays anyway), that situation will be caught here and the function will exit relatively gracefully with a descriptive warning.After that, the deferred code is run, regardless of whether the response was found. If the corresponding bid was never received, the code will fail its first check, producing an error that appears to be serious at first glance:
level=error msg="failed to get bidTrace for delivered payload from redis"
This PR changes the order of operations: the function only attempts to save the execution payload if the payload is actually available to be saved. The main benefit is in removal of confusing false-positive errors, though it additionally removes one redis read from the case where the payload is unavailable. This does not affect the desired behavior in #512: the payload will be saved regardless of whether publication was successful.
✅ I have run these commands
make lint
make test-race
go mod tidy
CONTRIBUTING.md