Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 17 additions & 2 deletions db/state/execctx/domain_shared.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we return non-nil domains with error?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is anti-pattern: it's responsibility of NewSharedDomains() func to close object - if error happened.

But we do return non-nil now - I forgot why (I did it). Probably because constructor has biz-logic now and we want threat some errors as non-critical, but keep existing sd object - beceause now sd object is quite long-living (mark made it survive between blocks as i remember).

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
}

Expand Down Expand Up @@ -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
}

Expand Down
73 changes: 73 additions & 0 deletions db/state/execctx/domain_shared_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
9 changes: 0 additions & 9 deletions execution/commitment/commitmentdb/commitment_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Loading