Skip to content

fix[l2geth]: off-by-one sometimes breaking replica sync #1082

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
merged 2 commits into from
Jun 14, 2021
Merged

fix[l2geth]: off-by-one sometimes breaking replica sync #1082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c05909a
fix[l2geth]: off-by-one sometimes breaking replica sync
smartcontracts Jun 14, 2021
65d7c80
chore: add changeset
smartcontracts Jun 14, 2021
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
5 changes: 5 additions & 0 deletions .changeset/lazy-toes-teach.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@eth-optimism/l2geth': patch
---

Fixes an off-by-one error that would sometimes break replica syncing when stopping and restarting geth.
2 changes: 1 addition & 1 deletion l2geth/miner/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func newWorker(config *Config, chainConfig *params.ChainConfig, engine consensus
unconfirmed: newUnconfirmedBlocks(eth.BlockChain(), miningLogAtDepth),
pendingTasks: make(map[common.Hash]*task),
txsCh: make(chan core.NewTxsEvent, txChanSize),
rollupCh: make(chan core.NewTxsEvent, txChanSize),
rollupCh: make(chan core.NewTxsEvent, 1),
Copy link
Contributor

@annieke annieke Jun 14, 2021

Choose a reason for hiding this comment

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

does this change functionality so that we only process 1 tx at a time?

Copy link
Contributor

@tynes tynes Jun 14, 2021

Choose a reason for hiding this comment

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

Its only possible to really process 1 transaction at a time due to the single threaded nature of evm execution

Copy link
Contributor

@tynes tynes Jun 14, 2021

Choose a reason for hiding this comment

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

The change in functionality here is reducing the buffer size of the channel which will cause writing to the channel to block when its full. Technically the updated recovery logic should allow us to revert this change. This change will add a slight performance hit

chainHeadCh: make(chan core.ChainHeadEvent, chainHeadChanSize),
chainSideCh: make(chan core.ChainSideEvent, chainSideChanSize),
newWorkCh: make(chan *newWorkReq),
Expand Down
19 changes: 10 additions & 9 deletions l2geth/rollup/sync_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,21 +243,22 @@ func (s *SyncService) initializeLatestL1(ctcDeployHeight *big.Int) error {
s.SetLatestL1Timestamp(context.Timestamp)
s.SetLatestL1BlockNumber(context.BlockNumber)
} else {
// Prevent underflows
if *index != 0 {
*index = *index - 1
}
log.Info("Found latest index", "index", *index)
block := s.bc.GetBlockByNumber(*index)
block := s.bc.GetBlockByNumber(*index + 1)
if block == nil {
block = s.bc.CurrentBlock()
idx := block.Number().Uint64()
if idx > *index {
blockNum := block.Number().Uint64()
if blockNum > *index {
// This is recoverable with a reorg but should never happen
return fmt.Errorf("Current block height greater than index")
}
s.SetLatestIndex(&idx)
log.Info("Block not found, resetting index", "new", idx, "old", *index)
var idx *uint64
if blockNum > 0 {
num := blockNum - 1
idx = &num
}
s.SetLatestIndex(idx)
log.Info("Block not found, resetting index", "new", stringify(idx), "old", *index)
}
txs := block.Transactions()
if len(txs) != 1 {
Expand Down