From 4008b41230f623dc2dc3547d4170625f26ebe0d5 Mon Sep 17 00:00:00 2001 From: sudeepdino008 Date: Tue, 14 Apr 2026 10:57:49 +0200 Subject: [PATCH] [r3.4] commitment, execctx: fix InsertBlocks failure during block catch-up Cherry-pick of #20546 from main. InsertBlocks on 3.4 uses BeginRw directly (not SharedDomains), so the inserters.go portion of the main PR is a no-op here. This cherry-pick keeps the general SharedDomains contract improvement: SeekCommitment always fully restores the SD, and NewSharedDomains attaches ErrBehindCommitment as an environmental signal probed via TxNums.Last. --- db/state/execctx/domain_shared.go | 19 ++++- db/state/execctx/domain_shared_test.go | 73 +++++++++++++++++++ .../commitmentdb/commitment_context.go | 9 --- 3 files changed, 90 insertions(+), 11 deletions(-) diff --git a/db/state/execctx/domain_shared.go b/db/state/execctx/domain_shared.go index c7ab43ae299..e3abc8ae727 100644 --- a/db/state/execctx/domain_shared.go +++ b/db/state/execctx/domain_shared.go @@ -30,6 +30,7 @@ import ( "github.com/erigontech/erigon/common/log/v3" "github.com/erigontech/erigon/db/kv" "github.com/erigontech/erigon/db/kv/order" + "github.com/erigontech/erigon/db/kv/rawdbv3" "github.com/erigontech/erigon/db/state/changeset" "github.com/erigontech/erigon/db/state/statecfg" "github.com/erigontech/erigon/diagnostics/metrics" @@ -73,11 +74,13 @@ type accHolder interface { func IsDomainAheadOfBlocks(ctx context.Context, tx kv.TemporalRwTx, logger log.Logger) bool { doms, err := NewSharedDomains(ctx, tx, logger) + if doms != nil { + defer doms.Close() + } if err != nil { logger.Debug("domain ahead of blocks", "err", err, "stack", dbg.Stack()) return errors.Is(err, commitmentdb.ErrBehindCommitment) } - defer doms.Close() return false } @@ -116,10 +119,22 @@ func NewSharedDomains(ctx context.Context, tx kv.TemporalTx, logger log.Logger) sd.sdCtx = commitmentdb.NewSharedDomainsCommitmentContext(sd, commitment.ModeDirect, tv, tx.Debug().Dirs().Tmp) - if _, _, err := sd.SeekCommitment(ctx, tx); err != nil { + _, blockNum, err := sd.SeekCommitment(ctx, tx) + if err != nil { return sd, err } + // ErrBehindCommitment is an environmental signal; sd is fully initialized. + if blockNum > 0 { + lastBn, _, err := rawdbv3.TxNums.Last(tx) + if err != nil { + return sd, err + } + if lastBn < blockNum { + return sd, fmt.Errorf("%w: TxNums index is at block %d and behind commitment %d", commitmentdb.ErrBehindCommitment, lastBn, blockNum) + } + } + return sd, nil } diff --git a/db/state/execctx/domain_shared_test.go b/db/state/execctx/domain_shared_test.go index 66ecab183ee..90fd31bc932 100644 --- a/db/state/execctx/domain_shared_test.go +++ b/db/state/execctx/domain_shared_test.go @@ -170,6 +170,79 @@ Loop: goto Loop } +// TestNewSharedDomains_StateAheadOfBlocks verifies that when the persisted +// commitment state is ahead of the TxNums index (catch-up scenario), +// NewSharedDomains returns ErrBehindCommitment but the SharedDomains itself +// is fully initialized (txNum set, patricia trie restored). Catch-up handlers +// like ExecModule.InsertBlocks and forkchoice rely on receiving a usable SD +// alongside the signal error. +func TestNewSharedDomains_StateAheadOfBlocks(t *testing.T) { + if testing.Short() { + t.Skip() + } + t.Parallel() + + stepSize := uint64(8) + require := require.New(t) + db := newTestDb(t, stepSize) + + ctx := context.Background() + + // Phase 1: write some accounts, compute commitment, flush, and append TxNums up to block N. + rwTx, err := db.BeginTemporalRw(ctx) + require.NoError(err) + defer rwTx.Rollback() + + const lastBlock = uint64(10) + for blockNum := uint64(0); blockNum <= lastBlock; blockNum++ { + maxTxNum := blockNum*2 + 1 + require.NoError(rawdbv3.TxNums.Append(rwTx, blockNum, maxTxNum)) + } + + doms, err := execctx.NewSharedDomains(ctx, rwTx, log.New()) + require.NoError(err) + + addr := make([]byte, length.Addr) + for i := uint64(0); i < 4; i++ { + addr[0] = byte(i) + acc := accounts.Account{ + Nonce: i, + Balance: *uint256.NewInt(i + 1), + } + require.NoError(doms.DomainPut(kv.AccountsDomain, rwTx, addr, accounts3.SerialiseV3(&acc), uint64(i), nil)) + } + commitTxNum := lastBlock*2 + 1 + _, err = doms.ComputeCommitment(ctx, rwTx, true, lastBlock, commitTxNum, "", nil) + require.NoError(err) + require.NoError(doms.Flush(ctx, rwTx)) + doms.Close() + require.NoError(rwTx.Commit()) + + // Phase 2: truncate TxNums so that lastBn < commitment block — this is the + // "state ahead of blocks" condition. + rwTx, err = db.BeginTemporalRw(ctx) + require.NoError(err) + defer rwTx.Rollback() + require.NoError(rawdbv3.TxNums.Truncate(rwTx, lastBlock-3)) + lastBn, _, err := rawdbv3.TxNums.Last(rwTx) + require.NoError(err) + require.Less(lastBn, lastBlock, "TxNums must be behind commitment block for the test to be meaningful") + + // Phase 3: NewSharedDomains must return ErrBehindCommitment AND a fully-initialized SD. + doms, err = execctx.NewSharedDomains(ctx, rwTx, log.New()) + require.ErrorIs(err, commitmentdb.ErrBehindCommitment, "expected ErrBehindCommitment signal") + require.NotNil(doms, "SD must be returned alongside the signal error") + defer doms.Close() + + // SD is fully initialized: txNum set to commitment-state's txNum. + require.Equal(commitTxNum, doms.TxNum(), "SD txNum must be set to the commitment state's txNum (full restore)") + + // Basic domain reads work — no panic, no nil deref. + addr[0] = 0 + _, _, err = doms.GetLatest(kv.AccountsDomain, rwTx, addr) + require.NoError(err) +} + func TestSharedDomain_StorageIter(t *testing.T) { if testing.Short() { t.Skip() diff --git a/execution/commitment/commitmentdb/commitment_context.go b/execution/commitment/commitmentdb/commitment_context.go index 9968bcf33d6..dfffa07556c 100644 --- a/execution/commitment/commitmentdb/commitment_context.go +++ b/execution/commitment/commitmentdb/commitment_context.go @@ -445,15 +445,6 @@ func (sdc *SharedDomainsCommitmentContext) SeekCommitment(ctx context.Context, t if err != nil { return 0, 0, err } - if blockNum > 0 { - lastBn, _, err := rawdbv3.TxNums.Last(tx) - if err != nil { - return 0, 0, err - } - if lastBn < blockNum { - return 0, 0, fmt.Errorf("%w: TxNums index is at block %d and behind commitment %d", ErrBehindCommitment, lastBn, blockNum) - } - } if err = sdc.enableConcurrentCommitmentIfPossible(); err != nil { return 0, 0, err }