Skip to content
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.

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

[batcher] Cleanup batcher channel inclusion block logic #12363

39 changes: 8 additions & 31 deletions op-batcher/batcher/channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ type channel struct {
// Set of confirmed txID -> inclusion block. For determining if the channel is timed out
confirmedTransactions map[string]eth.BlockID

// True if confirmed TX list is updated. Set to false after updated min/max inclusion blocks.
confirmedTxUpdated bool
// Inclusion block number of first confirmed TX
minInclusionBlock uint64
// Inclusion block number of last confirmed TX
Expand All @@ -47,6 +45,7 @@ func newChannel(log log.Logger, metr metrics.Metricer, cfg ChannelConfig, rollup
channelBuilder: cb,
pendingTransactions: make(map[string]txData),
confirmedTransactions: make(map[string]eth.BlockID),
minInclusionBlock: math.MaxUint64,
}, nil
}

Expand Down Expand Up @@ -82,9 +81,12 @@ func (s *channel) TxConfirmed(id string, inclusionBlock eth.BlockID) (bool, []*t
}
delete(s.pendingTransactions, id)
s.confirmedTransactions[id] = inclusionBlock
s.confirmedTxUpdated = true
s.channelBuilder.FramePublished(inclusionBlock.Number)

// Update min/max inclusion blocks for timeout check
s.minInclusionBlock = min(s.minInclusionBlock, inclusionBlock.Number)
s.maxInclusionBlock = max(s.maxInclusionBlock, inclusionBlock.Number)

// If this channel timed out, put the pending blocks back into the local saved blocks
// and then reset this state so it can try to build a new channel.
if s.isTimedOut() {
Expand All @@ -107,43 +109,18 @@ func (s *channel) Timeout() uint64 {
return s.channelBuilder.Timeout()
}

// updateInclusionBlocks finds the first & last confirmed tx and saves its inclusion numbers
func (s *channel) updateInclusionBlocks() {
if len(s.confirmedTransactions) == 0 || !s.confirmedTxUpdated {
return
}
// If there are confirmed transactions, find the first + last confirmed block numbers
min := uint64(math.MaxUint64)
max := uint64(0)
for _, inclusionBlock := range s.confirmedTransactions {
if inclusionBlock.Number < min {
min = inclusionBlock.Number
}
if inclusionBlock.Number > max {
max = inclusionBlock.Number
}
}
s.minInclusionBlock = min
s.maxInclusionBlock = max
s.confirmedTxUpdated = false
}

// pendingChannelIsTimedOut returns true if submitted channel has timed out.
// isTimedOut returns true if submitted channel has timed out.
// A channel has timed out if the difference in L1 Inclusion blocks between
// the first & last included block is greater than or equal to the channel timeout.
func (s *channel) isTimedOut() bool {
// Update min/max inclusion blocks for timeout check
s.updateInclusionBlocks()
// Prior to the granite hard fork activating, the use of the shorter ChannelTimeout here may cause the batcher
// to believe the channel timed out when it was valid. It would then resubmit the blocks needlessly.
// This wastes batcher funds but doesn't cause any problems for the chain progressing safe head.
return s.maxInclusionBlock-s.minInclusionBlock >= s.cfg.ChannelTimeout
return len(s.confirmedTransactions) > 0 && s.maxInclusionBlock-s.minInclusionBlock >= s.cfg.ChannelTimeout
}

// pendingChannelIsFullySubmitted returns true if the channel has been fully submitted.
// isFullySubmitted returns true if the channel has been fully submitted.
func (s *channel) isFullySubmitted() bool {
// Update min/max inclusion blocks for timeout check
s.updateInclusionBlocks()
return s.IsFull() && len(s.pendingTransactions)+s.PendingFrames() == 0
}

Expand Down
18 changes: 9 additions & 9 deletions op-batcher/batcher/channel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,19 @@ func TestChannelTimeout(t *testing.T) {
channel := m.currentChannel
require.NotNil(t, channel)

// add some pending txs, to be confirmed below
channel.pendingTransactions[zeroFrameTxID(0).String()] = txData{}
channel.pendingTransactions[zeroFrameTxID(1).String()] = txData{}
channel.pendingTransactions[zeroFrameTxID(2).String()] = txData{}

// There are no confirmed transactions so
// the pending channel cannot be timed out
timeout := channel.isTimedOut()
require.False(t, timeout)

// Manually set a confirmed transactions
// To avoid other methods clearing state
channel.confirmedTransactions[zeroFrameTxID(0).String()] = eth.BlockID{Number: 0}
channel.confirmedTransactions[zeroFrameTxID(1).String()] = eth.BlockID{Number: 99}
channel.confirmedTxUpdated = true
// Manually confirm transactions
channel.TxConfirmed(zeroFrameTxID(0).String(), eth.BlockID{Number: 0})
channel.TxConfirmed(zeroFrameTxID(1).String(), eth.BlockID{Number: 99})

// Since the ChannelTimeout is 100, the
// pending channel should not be timed out
Expand All @@ -62,10 +65,7 @@ func TestChannelTimeout(t *testing.T) {

// Add a confirmed transaction with a higher number
// than the ChannelTimeout
channel.confirmedTransactions[zeroFrameTxID(2).String()] = eth.BlockID{
Number: 101,
}
channel.confirmedTxUpdated = true
channel.TxConfirmed(zeroFrameTxID(2).String(), eth.BlockID{Number: 101})

// Now the pending channel should be timed out
timeout = channel.isTimedOut()
Expand Down