Skip to content

mev: no interrupt if it is too later #2971

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zzzckck merged 9 commits into from
Mar 22, 2025
Merged

mev: no interrupt if it is too later #2971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
678cefb
mev: no interrupt if it is too later
zzzckck Mar 22, 2025
b2c6c6a
mev: bid interrupr code improve
zzzckck Mar 22, 2025
6cbe627
mev: code improve
zzzckck Mar 22, 2025
9b58f26
mev: remove invalid bestBidToRun
zzzckck Mar 22, 2025
db73bde
mev: code
zzzckck Mar 22, 2025
5c8f462
mev: code improve for replyErr
zzzckck Mar 22, 2025
b97c4e1
mev: code improve
zzzckck Mar 22, 2025
edb80cf
mev: fix env is nill, hardcode blocktime
zzzckck Mar 22, 2025
eaa62ed
mev: reply ms
zzzckck Mar 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions core/types/bid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ type Bid struct {
GasUsed uint64
GasFee *big.Int
BuilderFee *big.Int
Committed bool // whether the bid has been committed to simulate or not
Copy link
Copy Markdown
Contributor

@unclezoro unclezoro Mar 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

@zzzckck zzzckck Mar 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accept, can be improved


rawBid RawBid
}
Expand Down
106 changes: 91 additions & 15 deletions miner/bid_simulator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
const (
// maxBidPerBuilderPerBlock is the max bid number per builder
maxBidPerBuilderPerBlock = 3
NoInterruptTimeLeft = 400 * time.Millisecond
Copy link
Copy Markdown
Contributor

@unclezoro unclezoro Mar 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

@zzzckck zzzckck Mar 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accept, can be improved

)

var (
Expand Down Expand Up @@ -111,8 +112,9 @@ type bidSimulator struct {
pendingMu sync.RWMutex
pending map[uint64]map[common.Address]map[common.Hash]struct{} // blockNumber -> builder -> bidHash -> struct{}

bestBidMu sync.RWMutex
bestBid map[common.Hash]*BidRuntime // prevBlockHash -> bidRuntime
bestBidMu sync.RWMutex
bestBid map[common.Hash]*BidRuntime // prevBlockHash -> bidRuntime
bestBidToRun map[common.Hash]*types.Bid // prevBlockHash -> *types.Bid

simBidMu sync.RWMutex
simulatingBid map[common.Hash]*BidRuntime // prevBlockHash -> bidRuntime, in the process of simulation
Expand Down Expand Up @@ -143,6 +145,7 @@ func newBidSimulator(
newBidCh: make(chan newBidPackage, 100),
pending: make(map[uint64]map[common.Address]map[common.Hash]struct{}),
bestBid: make(map[common.Hash]*BidRuntime),
bestBidToRun: make(map[common.Hash]*types.Bid),
simulatingBid: make(map[common.Hash]*BidRuntime),
}

Expand Down Expand Up @@ -255,6 +258,7 @@ func (b *bidSimulator) ExistBuilder(builder common.Address) bool {
return ok
}

// best bid here is based on packedBlockReward after the bid is simulated
func (b *bidSimulator) SetBestBid(prevBlockHash common.Hash, bid *BidRuntime) {
b.bestBidMu.Lock()
defer b.bestBidMu.Unlock()
Expand All @@ -275,6 +279,34 @@ func (b *bidSimulator) GetBestBid(prevBlockHash common.Hash) *BidRuntime {
return b.bestBid[prevBlockHash]
}

// best bid to run is based on bid's expectedBlockReward before the bid is simulated
func (b *bidSimulator) SetBestBidToRun(prevBlockHash common.Hash, bid *types.Bid) {
b.bestBidMu.Lock()
defer b.bestBidMu.Unlock()

b.bestBidToRun[prevBlockHash] = bid
}

// in case the bid is invalid(invalid GasUsed,Reward,GasPrice...), remove it.
func (b *bidSimulator) DelBestBidToRun(prevBlockHash common.Hash, delBid *types.Bid) {
b.bestBidMu.Lock()
defer b.bestBidMu.Unlock()
cur := b.bestBidToRun[prevBlockHash]
if cur == nil || delBid == nil {
return
}
if cur.Hash() == delBid.Hash() {
delete(b.bestBidToRun, prevBlockHash)
}
}

func (b *bidSimulator) GetBestBidToRun(prevBlockHash common.Hash) *types.Bid {
b.bestBidMu.RLock()
defer b.bestBidMu.RUnlock()

return b.bestBidToRun[prevBlockHash]
}

func (b *bidSimulator) SetSimulatingBid(prevBlockHash common.Hash, bid *BidRuntime) {
b.simBidMu.Lock()
defer b.simBidMu.Unlock()
Expand Down Expand Up @@ -318,6 +350,15 @@ func (b *bidSimulator) mainLoop() {
}
}

func (b *bidSimulator) canBeInterrupted(targetTime uint64) bool {
if targetTime == 0 {
// invalid targetTime, disable the interrupt check
return true
Copy link
Copy Markdown
Contributor

@unclezoro unclezoro Mar 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think should return false here, this is the case where block header can not be find

}
left := time.Until(time.Unix(int64(targetTime), 0))
return left >= NoInterruptTimeLeft
}

func (b *bidSimulator) newBidLoop() {
var (
interruptCh chan int32
Expand All @@ -331,6 +372,7 @@ func (b *bidSimulator) newBidLoop() {
close(interruptCh)
}
interruptCh = make(chan int32, 1)
bidRuntime.bid.Committed = true
select {
case b.simBidCh <- &simBidReq{interruptCh: interruptCh, bid: bidRuntime}:
log.Debug("BidSimulator: commit", "builder", bidRuntime.bid.Builder, "bidHash", bidRuntime.bid.Hash().Hex())
Expand Down Expand Up @@ -359,27 +401,53 @@ func (b *bidSimulator) newBidLoop() {
}

var replyErr error
// simulatingBid will be nil if there is no bid in simulation, compare with the bestBid instead
if simulatingBid := b.GetSimulatingBid(newBid.bid.ParentHash); simulatingBid != nil {
// simulatingBid always better than bestBid, so only compare with simulatingBid if a simulatingBid exists
if bidRuntime.isExpectedBetterThan(simulatingBid) {
commit(commitInterruptBetterBid, bidRuntime)
toCommit := true
bestBidToRun := b.GetBestBidToRun(newBid.bid.ParentHash)
if bestBidToRun != nil {
bestBidRuntime, _ := newBidRuntime(bestBidToRun, b.config.ValidatorCommission)
if bidRuntime.isExpectedBetterThan(bestBidRuntime) {
// new bid has better expectedBlockReward, use bidRuntime
log.Info("new bid has better expectedBlockReward")
} else if !bestBidToRun.Committed {
// bestBidToRun is not committed yet, this newBid will trigger bestBidToRun to commit
log.Info("to simulate the best non-committed bid")
bidRuntime = bestBidRuntime
replyErr = genDiscardedReply(bidRuntime)
} else {
replyErr = genDiscardedReply(simulatingBid)
// new bid will be discarded, as it is useless now.
toCommit = false
Copy link
Copy Markdown
Contributor

@unclezoro unclezoro Mar 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator Author

@zzzckck zzzckck Mar 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the bidRuntime.isExpectedBetterThan(bestBidRuntime) will return false
== why? I think it will return true if recommit the same bid

replyErr = genDiscardedReply(bidRuntime)
Copy link
Copy Markdown
Contributor

@unclezoro unclezoro Mar 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems generate a wrong reply to builders, as bidRuntime here is not a better bid, I mean the input should not be bidRuntime

Copy link
Copy Markdown
Collaborator Author

@zzzckck zzzckck Mar 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accept, will update

}
} else {
// bestBid is nil means the bid is the first bid, otherwise the bid should compare with the bestBid
if bestBid := b.GetBestBid(newBid.bid.ParentHash); bestBid == nil ||
bidRuntime.isExpectedBetterThan(bestBid) {
commit(commitInterruptBetterBid, bidRuntime)
}

if toCommit {
b.SetBestBidToRun(bidRuntime.bid.ParentHash, bidRuntime.bid)
// try to commit the new bid
// but if there is a simulating bid and with a short time left, don't interrupt it
if simulatingBid := b.GetSimulatingBid(newBid.bid.ParentHash); simulatingBid != nil {
parentHeader := b.chain.GetHeaderByHash(newBid.bid.ParentHash)
var blockTime uint64 = 0
if parentHeader != nil {
const blockInterval uint64 = 3 // todo: to improve this hard code value
blockTime = parentHeader.Time + blockInterval
}

if b.canBeInterrupted(blockTime) {
commit(commitInterruptBetterBid, bidRuntime)
} else {
if newBid.bid.Hash() == bidRuntime.bid.Hash() {
left := time.Until(time.Unix(int64(blockTime), 0))
replyErr = fmt.Errorf("bid is pending as no enough time to interrupt, left:%d, NoInterruptTimeLeft:%d",
left.Milliseconds(), NoInterruptTimeLeft.Milliseconds())
}
}
} else {
replyErr = genDiscardedReply(bestBid)
commit(commitInterruptBetterBid, bidRuntime)
}
}

if newBid.feedback != nil {
newBid.feedback <- replyErr

log.Info("[BID ARRIVED]",
"block", newBid.bid.BlockNumber,
"builder", newBid.bid.Builder,
Expand Down Expand Up @@ -419,6 +487,12 @@ func (b *bidSimulator) clearLoop() {
delete(b.bestBid, k)
}
}
delete(b.bestBidToRun, parentHash)
for k, v := range b.bestBidToRun {
if v.BlockNumber <= blockNumber-b.chain.TriesInMemory() {
delete(b.bestBidToRun, k)
}
}
b.bestBidMu.Unlock()

b.simBidMu.Lock()
Expand Down Expand Up @@ -561,6 +635,8 @@ func (b *bidSimulator) simBid(interruptCh chan int32, bidRuntime *BidRuntime) {
log.Debug("BidSimulator: recommit", "builder", bidRuntime.bid.Builder, "bidHash", bidRuntime.bid.Hash().Hex())
default:
}
} else {
b.DelBestBidToRun(parentHash, bidRuntime.bid)
}
}(startTS)

Expand Down