core: exempt empty no-op blocks from sidechain ghost-state rejection

core/blockchain.go

@@ -3678,7 +3678,7 @@
 				continue
 			}
 
-			if canonical != nil && canonical.Root() == block.Root() {
+			if canonical != nil && canonical.Root() == block.Root() && bc.isSidechainGhostState(block, canonical) {
 				// This is most likely a shadow-state attack. When a fork is imported into the
 				// database, and it eventually reaches a block height which is not pruned, we
 				// just found that the state already exist! This means that the sidechain block
@@ -3797,6 +3797,34 @@
 	return nil, 0, nil
 }
 
+// isSidechainGhostState decides whether a sidechain block whose state root
+// already matches the canonical block at the same height is a shadow-state
+// attack (true) or a legitimate no-op block (false). It is only meaningful when
+// canonical.Root() == block.Root() already holds.
+//
+// A block that performs no state transition (e.g. an empty block) inherits its
+// parent's state root, so two distinct no-op blocks at the same height (differing
+// only in seal/timestamp/coinbase) legitimately share a state root: there is no
+// forged state to skip-verify. We exempt that case, but only when the *canonical*
+// chain was also a no-op at this height, measured against the canonical parent's
+// trusted (already-verified) header. Gating on the canonical side, not just the
+// side block's own parent, closes the forge-both-roots bypass: the side block's
+// parent may itself be unverified/attacker-controlled here, whereas the canonical
+// parent header cannot be. If either parent header is unavailable, we keep the
+// conservative behavior and treat it as an attack.
+func (bc *BlockChain) isSidechainGhostState(block *types.Block, canonical *types.Block) bool {
+	canonParent := bc.GetHeaderByNumber(canonical.NumberU64() - 1)
+	sideParent := bc.GetHeader(block.ParentHash(), block.NumberU64()-1)
+	if canonParent == nil || sideParent == nil {
+		return true
+	}
+	// Legitimate only if neither the canonical block nor the side block changed
+	// state relative to its own parent, i.e. both are genuine no-ops.
+	canonNoOp := canonParent.Root == canonical.Root()
+	sideNoOp := sideParent.Root == block.Root()
+	return !(canonNoOp && sideNoOp)
+}
+
 // recoverAncestors finds the closest ancestor with available state and re-execute
 // all the ancestor blocks since that.
 // recoverAncestors is only used post-merge.

core/blockchain_test.go

@@ -3108,6 +3108,240 @@ func testSideImportPrunedBlocks(t *testing.T, scheme string) {
 	}
 }
 
+// TestSideImportEmptyBlockGhostState is a regression test for
+// https://github.com/0xPolygon/bor/issues/2224. Two distinct empty
+// (zero-transaction) blocks at the same height legitimately share a state root,
+// because neither performs a state transition: both inherit their parent's root.
+// insertSideChain used to reject any sidechain block whose root matched the
+// canonical block at the same height as a "ghost-state attack", which fired a
+// false positive on such empty blocks and wedged sync after a validator restart.
+func TestSideImportEmptyBlockGhostState(t *testing.T) {
+	testSideImportEmptyBlockGhostState(t, rawdb.HashScheme)
+	testSideImportEmptyBlockGhostState(t, rawdb.PathScheme)
+}
+
+// noRewardEngine wraps the faker but skips the block reward, so empty blocks
+// make no state transition and share their parent's state root — matching how
+// bor's reward-less empty blocks behave (the condition that triggers #2224).
+type noRewardEngine struct{ *ethash.Ethash }
+
+func (e *noRewardEngine) Finalize(chain consensus.ChainHeaderReader, header *types.Header, state vm.StateDB, body *types.Body, receipts []*types.Receipt) ([]*types.Receipt, error) {
+	return receipts, nil
+}
+
+func (e *noRewardEngine) FinalizeAndAssemble(chain consensus.ChainHeaderReader, header *types.Header, state *state.StateDB, body *types.Body, receipts []*types.Receipt) (*types.Block, []*types.Receipt, time.Duration, error) {
+	header.Root = state.IntermediateRoot(chain.Config().IsEIP158(header.Number))
+	return types.NewBlock(header, &types.Body{Transactions: body.Transactions, Uncles: body.Uncles, Withdrawals: body.Withdrawals}, receipts, trie.NewStackTrie(nil)), receipts, 0, nil
+}
+
+// ghostStateTestKey funds the sender used to drive state transitions in the
+// ghost-state sidechain tests.
+var ghostStateTestKey, _ = crypto.HexToECDSA("b71c71a67e1177ad4e901695e1b4b9ee17ae16c6668d313eac2f96dbcda3f291")
+
+// setupPrunedGhostStateChain builds a canonical chain whose blocks all perform a
+// state transition (so old states get pruned), inserts it, and returns the chain
+// plus everything a sidechain test needs to fork at forkIdx. forkBlockEmpty
+// controls whether the canonical block at forkIdx is left empty (no state
+// transition) or carries the same value-transfer tx as every other block. It
+// asserts the fork parent's state is pruned, so re-importing a sibling routes
+// through insertSideChain via ErrPrunedAncestor.
+func setupPrunedGhostStateChain(t *testing.T, scheme string, forkBlockEmpty bool) (chain *BlockChain, genDb ethdb.Database, blocks []*types.Block, forkParent *types.Block, forkIdx int, forkTxNonce uint64) {
+	t.Helper()
+
+	engine := &noRewardEngine{ethash.NewFaker()}
+	addr := crypto.PubkeyToAddress(ghostStateTestKey.PublicKey)
+	signer := types.LatestSigner(params.TestChainConfig)
+	genesis := &Genesis{
+		Config:  params.TestChainConfig,
+		Alloc:   types.GenesisAlloc{addr: {Balance: big.NewInt(1e18)}},
+		BaseFee: big.NewInt(params.InitialBaseFee),
+	}
+
+	states := state.TriesInMemory
+	if scheme == rawdb.PathScheme {
+		states = state.TriesInMemory + 1
+	}
+	total := 2 * state.TriesInMemory
+	// forkIdx indexes into blocks (blocks[i] is height i+1) and sits comfortably
+	// below the pruning boundary.
+	forkIdx = total - states - 5
+
+	nonce := uint64(0)
+	genDb, blocks, _ = GenerateChainWithGenesis(genesis, engine, total, func(i int, b *BlockGen) {
+		b.SetCoinbase(common.Address{1})
+		if i == forkIdx {
+			forkTxNonce = nonce
+			if forkBlockEmpty {
+				return // no state transition: root stays equal to parent's
+			}
+		}
+		tx, _ := types.SignTx(types.NewTransaction(nonce, common.Address{0xaa}, big.NewInt(1), params.TxGas, b.BaseFee(), nil), signer, ghostStateTestKey)
+		b.AddTx(tx)
+		nonce++
+	})
+
+	datadir := t.TempDir()
+	pdb, err := pebble.New(datadir, 0, 0, "", false)
+	if err != nil {
+		t.Fatalf("Failed to create persistent key-value database: %v", err)
+	}
+	db, err := rawdb.Open(pdb, rawdb.OpenOptions{Ancient: path.Join(datadir, "ancient")})
+	if err != nil {
+		t.Fatalf("Failed to create persistent freezer database: %v", err)
+	}
+	t.Cleanup(func() { db.Close() })
+
+	chain, err = NewBlockChain(db, genesis, engine, DefaultConfig().WithStateScheme(scheme))
+	if err != nil {
+		t.Fatalf("failed to create tester chain: %v", err)
+	}
+	t.Cleanup(chain.Stop)
+
+	if n, err := chain.InsertChain(blocks, false); err != nil {
+		t.Fatalf("block %d: failed to insert into chain: %v", n, err)
+	}
+
+	forkParent = blocks[forkIdx-1]
+	if chain.HasState(forkParent.Root()) {
+		t.Fatalf("test setup: fork parent (block %d) state not pruned", forkParent.NumberU64())
+	}
+	return chain, genDb, blocks, forkParent, forkIdx, forkTxNonce
+}
+
+func testSideImportEmptyBlockGhostState(t *testing.T, scheme string) {
+	chain, genDb, blocks, forkParent, forkIdx, _ := setupPrunedGhostStateChain(t, scheme, true)
+
+	// Empty sibling at the fork height: different coinbase → different hash, but
+	// same (parent's) state root as both its parent and the canonical block.
+	sideBlocks, _ := GenerateChain(params.TestChainConfig, forkParent, chain.engine, genDb, 1, func(i int, b *BlockGen) {
+		b.SetCoinbase(common.Address{2})
+	})
+	side := sideBlocks[0]
+	canonical := blocks[forkIdx]
+	if side.Hash() == canonical.Hash() {
+		t.Fatalf("side block hash must differ from canonical")
+	}
+	if side.Root() != canonical.Root() {
+		t.Fatalf("test setup: side root %x must equal canonical root %x", side.Root(), canonical.Root())
+	}
+	if side.Root() != forkParent.Root() {
+		t.Fatalf("test setup: empty side block must inherit parent root")
+	}
+
+	if _, err := chain.InsertChain(sideBlocks, false); err != nil {
+		t.Fatalf("empty side block falsely rejected: %v", err)
+	}
+}
+
+// TestSideImportGhostStateStillRejected is the security counterpart to
+// TestSideImportEmptyBlockGhostState: it proves the #2224 fix only narrowed the
+// false positive and did not open the real hole. A side block that DID perform a
+// state transition (its root differs from its parent's) but claims a state root
+// equal to the canonical block at that height is the genuine shadow-state attack
+// and must still be rejected.
+func TestSideImportGhostStateStillRejected(t *testing.T) {
+	testSideImportGhostStateStillRejected(t, rawdb.HashScheme)
+	testSideImportGhostStateStillRejected(t, rawdb.PathScheme)
+}
+
+func testSideImportGhostStateStillRejected(t *testing.T, scheme string) {
+	chain, genDb, blocks, forkParent, forkIdx, forkTxNonce := setupPrunedGhostStateChain(t, scheme, false)
+	signer := types.LatestSigner(params.TestChainConfig)
+
+	// Side block on the canonical fork parent, carrying the SAME tx as the
+	// canonical fork block (with a different coinbase). It reaches the canonical
+	// root via a real state transition, so its root differs from its parent's:
+	// side.Root() == canonical.Root() but side.Root() != forkParent.Root().
+	sideBlocks, _ := GenerateChain(params.TestChainConfig, forkParent, chain.engine, genDb, 1, func(i int, b *BlockGen) {
+		b.SetCoinbase(common.Address{2})
+		tx, _ := types.SignTx(types.NewTransaction(forkTxNonce, common.Address{0xaa}, big.NewInt(1), params.TxGas, b.BaseFee(), nil), signer, ghostStateTestKey)
+		b.AddTx(tx)
+	})
+	side := sideBlocks[0]
+	canonical := blocks[forkIdx]
+	if side.Root() != canonical.Root() {
+		t.Fatalf("test setup: side root %x must equal canonical root %x", side.Root(), canonical.Root())
+	}
+	if side.Root() == forkParent.Root() {
+		t.Fatalf("test setup: side block must change state (root must differ from parent)")
+	}
+
+	if _, err := chain.InsertChain(sideBlocks, false); err == nil || err.Error() != "sidechain ghost-state attack" {
+		t.Fatalf("expected ghost-state attack rejection, got: %v", err)
+	}
+}
+
+// TestSidechainGhostStateCanonicalGate covers the hardening requested in PR #2293
+// review: the no-op exemption must be gated on the *canonical* chain also being a
+// no-op at that height, not only on the side block's own (possibly attacker-forged)
+// parent. This defends against an attacker who forges both side parent and side
+// child roots to match a canonical root at a height where the canonical chain
+// actually changed state (the forge-both-roots bypass). Because the side block's
+// parent can be unverified here while the canonical parent header is trusted,
+// isSidechainGhostState decides on the canonical parent. This exercises the
+// decision directly, since forging a header past GenerateChain's honest state
+// transitions is not otherwise reachable.
+func TestSidechainGhostStateCanonicalGate(t *testing.T) {
+	chain, _, blocks, _, forkIdx, _ := setupPrunedGhostStateChain(t, rawdb.HashScheme, false)
+
+	// canonical is a real state-transition block: its root differs from its parent.
+	canonical := blocks[forkIdx]
+	canonParent := blocks[forkIdx-1]
+	if canonical.Root() == canonParent.Root() {
+		t.Fatalf("test setup: canonical block must change state relative to its parent")
+	}
+
+	// Forge a side parent that already sits at the canonical root, then a no-op
+	// side child inheriting it: sideNoOp is true. But the canonical chain changed
+	// state here (canonNoOp false), so the exemption must be denied. The forged
+	// side parent is written to the header chain so GetHeader resolves it, mirroring
+	// how insertSideChain writes sidechain headers without state verification.
+	forgedParent := types.NewBlockWithHeader(&types.Header{
+		Number:     canonParent.Number(),
+		ParentHash: canonParent.ParentHash(),
+		Root:       canonical.Root(),
+		Coinbase:   common.Address{9},
+	})
+	rawdb.WriteHeader(chain.db, forgedParent.Header())
+	forged := types.NewBlockWithHeader(&types.Header{
+		Number:     canonical.Number(),
+		ParentHash: forgedParent.Hash(),
+		Root:       canonical.Root(),
+		Coinbase:   common.Address{2},
+	})
+	if forged.Hash() == canonical.Hash() {
+		t.Fatalf("test setup: forged side block must differ from canonical")
+	}
+	// Sanity: the side block is a no-op vs its (forged) parent, the precise input
+	// the pre-hardening check would have wrongly exempted.
+	if chain.GetHeader(forged.ParentHash(), forged.NumberU64()-1).Root != forged.Root() {
+		t.Fatalf("test setup: forged side block must be a no-op vs its parent")
+	}
+	if !chain.isSidechainGhostState(forged, canonical) {
+		t.Fatalf("attack not detected: no-op side block at a state-changing canonical height must be rejected")
+	}
+}
+
+// TestSidechainGhostStateMissingParentIsAttack covers the conservative fallback:
+// when either parent header is unavailable, isSidechainGhostState cannot prove the
+// blocks are genuine no-ops, so it must treat the match as an attack (return true).
+func TestSidechainGhostStateMissingParentIsAttack(t *testing.T) {
+	chain, _, blocks, _, forkIdx, _ := setupPrunedGhostStateChain(t, rawdb.HashScheme, false)
+
+	canonical := blocks[forkIdx]
+
+	// A side block whose parent header is not in the database at all: GetHeader
+	// returns nil, so the exemption is denied without inspecting roots.
+	orphan := types.NewBlockWithHeader(&types.Header{
+		Number:     canonical.Number(),
+		ParentHash: common.Hash{0xde, 0xad},
+		Root:       canonical.Root(),
+	})
+	if !chain.isSidechainGhostState(orphan, canonical) {
+		t.Fatalf("missing side parent must be treated as an attack")
+	}
+}
+
 // TestDeleteCreateRevert tests a weird state transition corner case that we hit
 // while changing the internals of statedb. The workflow is that a contract is
 // self destructed, then in a followup transaction (but same block) it's created