feat: adds execution enforcement to preconfirmations #161
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
Relates to the Bid Expression discussion occurring here. |
ckartik
force-pushed
the
ckartik/add-revertibility-check
branch
from
9000c96 to
1ad9a07
Compare
mrekucci
reviewed
Jun 25, 2024
shaspitz
reviewed
Jun 25, 2024
aloknerurkar
reviewed
Jul 1, 2024
mrekucci
reviewed
Jul 2, 2024
mrekucci
reviewed
Jul 2, 2024
aloknerurkar
reviewed
Jul 3, 2024
aloknerurkar
reviewed
Jul 3, 2024
| type EthClient interface { | ||
| BlockNumber(ctx context.Context) (uint64, error) | ||
| HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error) | ||
| BlockByNumber(ctx context.Context, number *big.Int) (*types.Block, error) |
Collaborator
There was a problem hiding this comment.
Why is this required? This interface is a subset of the methods of the EthClient. We dont use the BlockByNumber in this pkg. You can still pass your retry client to this pkg even after removing this.
aloknerurkar
approved these changes
Jul 3, 2024
Collaborator
aloknerurkar
left a comment
aloknerurkar
left a comment
There was a problem hiding this comment.
Apart from the nitpicks, things LGTM! Before checking in just fix the 2 comments.
- Remove the additional method from the EthClient interface
- Add check for errorCode == 429 everywhere in the retry logic.
ckartik
force-pushed
the
ckartik/add-revertibility-check
branch
4 times, most recently
from
fb1c4ae to
30b909c
Compare
mrekucci
reviewed
Jul 5, 2024
ckartik
force-pushed
the
ckartik/add-revertibility-check
branch
from
011fb9d to
a220daa
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Motivation
Preconfirmations can trivially be satisfied by most block builders by placing the transactions at the bottom of the block. This is known as an inclusion preconfirmation. However, most preconfirmations are meant for contested state, like doing cex-dex arbitrage to bring the price an on-chain DEX or AMM into parity with the external world. In such cases, it's important to ensure the provider doesn't simply include the preconfirmation, but also ensures its successful execution.
Implementation Details
We alter the data stored in our LRU cache to also keep track of the status of the transaction (success v.s failure). We subsequently continue our preconfirmation validation as normal, with the additional check of successful execution before the disbursement of rewards.
Roadmap and relevance
This is not a required check, as builders generally provide revert protection. This one of two PRs in this theme, that will introduce a bid expression where parties to can dictate which transactions are required to succeed and which ones can revert. The subsequent PR will deal with adding support for bid expressions, along with the first bid expression being the ability to specify which transactions in a bundle are ok to revert.