mev: no interrupt if it is too later#2971
mev: no interrupt if it is too later#2971zzzckck merged 9 commits intobnb-chain:develop_for_v1.5.8from
Conversation
with 3s block interval, if there is a simulating bid and with a short time left for simulate, then don't interrupt the current simulating bid. The new bid will be pending and can still run once the current simulating bid completes
no replyError if the bid has no time to interrupt
|
LGTM |
| GasUsed uint64 | ||
| GasFee *big.Int | ||
| BuilderFee *big.Int | ||
| Committed bool // whether the bid has been committed to simulate or not |
There was a problem hiding this comment.
I strongly suggest that Committed to be lower case just like the rawBid, just to prevent that the external component inject a value to Committed.
There was a problem hiding this comment.
accept, can be improved
| const ( | ||
| // maxBidPerBuilderPerBlock is the max bid number per builder | ||
| maxBidPerBuilderPerBlock = 3 | ||
| NoInterruptTimeLeft = 400 * time.Millisecond |
There was a problem hiding this comment.
This is not good to be a const, prefer it to be a config, but with default value with 400 * time.Millisecond. This will provider more flexability.
There was a problem hiding this comment.
accept, can be improved
| replyErr = genDiscardedReply(simulatingBid) | ||
| // new bid will be discarded, as it is useless now. | ||
| toCommit = false | ||
| replyErr = genDiscardedReply(bidRuntime) |
There was a problem hiding this comment.
seems generate a wrong reply to builders, as bidRuntime here is not a better bid, I mean the input should not be bidRuntime
There was a problem hiding this comment.
accept, will update
| func (b *bidSimulator) canBeInterrupted(targetTime uint64) bool { | ||
| if targetTime == 0 { | ||
| // invalid targetTime, disable the interrupt check | ||
| return true |
There was a problem hiding this comment.
I think should return false here, this is the case where block header can not be find
| } else { | ||
| replyErr = genDiscardedReply(simulatingBid) | ||
| // new bid will be discarded, as it is useless now. | ||
| toCommit = false |
There was a problem hiding this comment.
this will break the recommit logic, for a recommit bid, you will not do delBestBidToRun, the bidRuntime.isExpectedBetterThan(bestBidRuntime) will return false, bestBidToRun.Committed is ture, so code goes here, validator will report false issue to builder, and recommit wont work.
There was a problem hiding this comment.
the bidRuntime.isExpectedBetterThan(bestBidRuntime) will return false
== why? I think it will return true if recommit the same bid
Description
This is an improvement to the current MEV logic, especially on large traffic to avoid too frequent bid interrupt that could lead to no bid can be simulated.
with 3s block interval, if there is a simulating bid and with a short time left for simulate, then don't interrupt the current simulating bid. The new bid will be pending and can still run once the current simulating bid completes
Rationale
NA
Example
NA
Changes
NA