mev-boost-relay icon indicating copy to clipboard operation
mev-boost-relay copied to clipboard

Only commit to saving payload once the response has been found

Open austonst opened this issue 2 years ago • 3 comments

📝 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 (via defer 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
  • [✅] I have seen and agree to CONTRIBUTING.md

austonst avatar Sep 18 '23 22:09 austonst

cc @michaelneuder

JustinDrake avatar Sep 26 '23 09:09 JustinDrake

nice catch! LGTM

michaelneuder avatar Oct 03 '23 20:10 michaelneuder

Same deal, just the code in the moved block has changed slightly, so needed to rebase.

austonst avatar Feb 07 '24 18:02 austonst