From df7104840362265ea4900fad6bea24d947897c18 Mon Sep 17 00:00:00 2001 From: Tudor Malene <tudor.malene@gmail.com> Date: Tue, 12 Nov 2024 16:08:09 +0000 Subject: [PATCH] fix canonical block for a weird corner case --- go/enclave/components/block_processor.go | 9 +++- go/enclave/enclave.go | 2 - go/enclave/nodetype/sequencer.go | 3 -- go/enclave/nodetype/validator.go | 3 -- go/enclave/storage/enclavedb/batch.go | 3 ++ go/enclave/storage/enclavedb/block.go | 3 ++ go/enclave/storage/storage.go | 51 +++++++++++-------- .../simulation/simulation_in_mem_test.go | 2 +- 8 files changed, 44 insertions(+), 32 deletions(-) diff --git a/go/enclave/components/block_processor.go b/go/enclave/components/block_processor.go index 48f432ef9a..5f7bac53bb 100644 --- a/go/enclave/components/block_processor.go +++ b/go/enclave/components/block_processor.go @@ -121,7 +121,7 @@ func (bp *l1BlockProcessor) tryAndInsertBlock(ctx context.Context, br *common.Bl } bp.logger.Trace("BlockHeader inserted successfully", - log.BlockHeightKey, block.Number, log.BlockHashKey, block.Hash(), "ingestionType", ingestionType) + log.BlockHeightKey, block.Number, log.BlockHashKey, block.Hash(), "parentHash", block.ParentHash, "ingestionType", ingestionType) return ingestionType, nil } @@ -136,6 +136,11 @@ func (bp *l1BlockProcessor) ingestBlock(ctx context.Context, block *types.Header } return nil, fmt.Errorf("could not retrieve head block. Cause: %w", err) } + + if prevL1Head.Hash() == block.Hash() { + return &BlockIngestionType{OldCanonicalBlock: true}, nil + } + // we do a basic sanity check, comparing the received block to the head block on the chain if block.ParentHash != prevL1Head.Hash() { isCanon, err := bp.storage.IsBlockCanonical(ctx, block.Hash()) @@ -158,6 +163,8 @@ func (bp *l1BlockProcessor) ingestBlock(ctx context.Context, block *types.Header if chainFork.IsFork() { bp.logger.Info("Fork detected in the l1 chain", "can", chainFork.CommonAncestor.Hash(), "noncan", prevL1Head.Hash()) + } else { + bp.logger.Error("Should not happen. Weird Fork detected in the l1 chain", "fork", chainFork) } return &BlockIngestionType{ChainFork: chainFork, PreGenesis: false}, nil } diff --git a/go/enclave/enclave.go b/go/enclave/enclave.go index 9516e0768a..e9e0b0b830 100644 --- a/go/enclave/enclave.go +++ b/go/enclave/enclave.go @@ -193,7 +193,6 @@ func NewEnclave(config *enclaveconfig.EnclaveConfig, genesis *genesis.Genesis, m batchExecutor, registry, rProducer, - rConsumer, rollupCompression, gethEncodingService, logger, @@ -217,7 +216,6 @@ func NewEnclave(config *enclaveconfig.EnclaveConfig, genesis *genesis.Genesis, m blockProcessor, batchExecutor, registry, - rConsumer, chainConfig, storage, sigVerifier, diff --git a/go/enclave/nodetype/sequencer.go b/go/enclave/nodetype/sequencer.go index 3bd45bdd0f..36b95c608e 100644 --- a/go/enclave/nodetype/sequencer.go +++ b/go/enclave/nodetype/sequencer.go @@ -47,7 +47,6 @@ type sequencer struct { batchProducer components.BatchExecutor batchRegistry components.BatchRegistry rollupProducer components.RollupProducer - rollupConsumer components.RollupConsumer rollupCompression *components.RollupCompression gethEncoding gethencoding.EncodingService @@ -68,7 +67,6 @@ func NewSequencer( batchExecutor components.BatchExecutor, registry components.BatchRegistry, rollupProducer components.RollupProducer, - rollupConsumer components.RollupConsumer, rollupCompression *components.RollupCompression, gethEncodingService gethencoding.EncodingService, logger gethlog.Logger, @@ -86,7 +84,6 @@ func NewSequencer( batchProducer: batchExecutor, batchRegistry: registry, rollupProducer: rollupProducer, - rollupConsumer: rollupConsumer, rollupCompression: rollupCompression, gethEncoding: gethEncodingService, logger: logger, diff --git a/go/enclave/nodetype/validator.go b/go/enclave/nodetype/validator.go index af90d7adc4..58ca57742b 100644 --- a/go/enclave/nodetype/validator.go +++ b/go/enclave/nodetype/validator.go @@ -27,7 +27,6 @@ type obsValidator struct { blockProcessor components.L1BlockProcessor batchExecutor components.BatchExecutor batchRegistry components.BatchRegistry - rollupConsumer components.RollupConsumer chainConfig *params.ChainConfig @@ -44,7 +43,6 @@ func NewValidator( consumer components.L1BlockProcessor, batchExecutor components.BatchExecutor, registry components.BatchRegistry, - rollupConsumer components.RollupConsumer, chainConfig *params.ChainConfig, storage storage.Storage, sigValidator *components.SignatureValidator, @@ -58,7 +56,6 @@ func NewValidator( blockProcessor: consumer, batchExecutor: batchExecutor, batchRegistry: registry, - rollupConsumer: rollupConsumer, chainConfig: chainConfig, storage: storage, sigValidator: sigValidator, diff --git a/go/enclave/storage/enclavedb/batch.go b/go/enclave/storage/enclavedb/batch.go index a66be299f5..39a6ca5267 100644 --- a/go/enclave/storage/enclavedb/batch.go +++ b/go/enclave/storage/enclavedb/batch.go @@ -41,6 +41,9 @@ func WriteBatchHeader(ctx context.Context, dbtx *sql.Tx, batch *core.Batch, conv } func UpdateCanonicalBatch(ctx context.Context, dbtx *sql.Tx, isCanonical bool, blocks []common.L1BlockHash) error { + if len(blocks) == 0 { + return nil + } args := make([]any, 0) args = append(args, isCanonical) for _, blockHash := range blocks { diff --git a/go/enclave/storage/enclavedb/block.go b/go/enclave/storage/enclavedb/block.go index 390537c5b5..b56e05e6e5 100644 --- a/go/enclave/storage/enclavedb/block.go +++ b/go/enclave/storage/enclavedb/block.go @@ -32,6 +32,9 @@ func WriteBlock(ctx context.Context, dbtx *sql.Tx, b *types.Header) error { } func UpdateCanonicalBlock(ctx context.Context, dbtx *sql.Tx, isCanonical bool, blocks []common.L1BlockHash) error { + if len(blocks) == 0 { + return nil + } args := make([]any, 0) args = append(args, isCanonical) for _, blockHash := range blocks { diff --git a/go/enclave/storage/storage.go b/go/enclave/storage/storage.go index 852d1ecc6e..bc053f0ecb 100644 --- a/go/enclave/storage/storage.go +++ b/go/enclave/storage/storage.go @@ -248,31 +248,38 @@ func (s *storageImpl) StoreBlock(ctx context.Context, block *types.Header, chain return err } + var nonCanonical, canonical []common.L1BlockHash if chainFork != nil && chainFork.IsFork() { s.logger.Info(fmt.Sprintf("Update Fork. %s", chainFork)) - err := enclavedb.UpdateCanonicalBlock(ctx, dbTx, false, chainFork.NonCanonicalPath) - if err != nil { - return err - } - err = enclavedb.UpdateCanonicalBlock(ctx, dbTx, true, chainFork.CanonicalPath) - if err != nil { - return err - } - err = enclavedb.UpdateCanonicalBatch(ctx, dbTx, false, chainFork.NonCanonicalPath) - if err != nil { - return err - } - err = enclavedb.UpdateCanonicalBatch(ctx, dbTx, true, chainFork.CanonicalPath) - if err != nil { - return err - } + nonCanonical = chainFork.NonCanonicalPath + canonical = chainFork.CanonicalPath + } else { + // handle the case when this block was canonical at some point, then reverted + canonical = []common.L1BlockHash{block.Hash()} + } - // sanity check that there is always a single canonical batch or block per layer - // called after forks, for the latest 50 blocks - err = enclavedb.CheckCanonicalValidity(ctx, dbTx, blockId-50) - if err != nil { - s.logger.Crit("Should not happen.", log.ErrKey, err) - } + err = enclavedb.UpdateCanonicalBlock(ctx, dbTx, false, nonCanonical) + if err != nil { + return err + } + err = enclavedb.UpdateCanonicalBlock(ctx, dbTx, true, canonical) + if err != nil { + return err + } + err = enclavedb.UpdateCanonicalBatch(ctx, dbTx, false, nonCanonical) + if err != nil { + return err + } + err = enclavedb.UpdateCanonicalBatch(ctx, dbTx, true, canonical) + if err != nil { + return err + } + + // sanity check that there is always a single canonical batch or block per layer + // called after forks, for the latest 50 blocks + err = enclavedb.CheckCanonicalValidity(ctx, dbTx, blockId-50) + if err != nil { + s.logger.Crit("Should not happen.", log.ErrKey, err) } if err := dbTx.Commit(); err != nil { diff --git a/integration/simulation/simulation_in_mem_test.go b/integration/simulation/simulation_in_mem_test.go index 052eaebf99..d523ea43c5 100644 --- a/integration/simulation/simulation_in_mem_test.go +++ b/integration/simulation/simulation_in_mem_test.go @@ -26,7 +26,7 @@ func TestInMemoryMonteCarloSimulation(t *testing.T) { simParams := params.SimParams{ NumberOfNodes: numberOfNodes, // todo (#718) - try reducing this back to 50 milliseconds once faster-finality model is optimised - AvgBlockDuration: 180 * time.Millisecond, + AvgBlockDuration: 40 * time.Millisecond, SimulationTime: 45 * time.Second, L1EfficiencyThreshold: 0.2, MgmtContractLib: ethereummock.NewMgmtContractLibMock(),