From 0d08ca0a63894864dc1d17a8f47dcc7932bf4777 Mon Sep 17 00:00:00 2001 From: bit2swaz Date: Tue, 30 Jun 2026 23:35:17 +0530 Subject: [PATCH 1/3] core: exempt empty no-op blocks from sidechain ghost-state rejection --- core/blockchain.go | 23 ++++-- core/blockchain_test.go | 163 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 6 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index f6873cdb1a..e6136fe982 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -3686,12 +3686,23 @@ func (bc *BlockChain) insertSideChain(block *types.Block, it *insertIterator, ma // // If left unchecked, we would now proceed importing the blocks, without actually // having verified the state of the previous blocks. - log.Warn("Sidechain ghost-state attack detected", "number", block.NumberU64(), "sideroot", block.Root(), "canonroot", canonical.Root()) - - // If someone legitimately side-mines blocks, they would still be imported as usual. However, - // we cannot risk writing unverified blocks to disk when they obviously target the pruning - // mechanism. - return nil, it.index, errors.New("sidechain ghost-state attack") + // + // Exception: a block that performed no state transition (e.g. an empty block) + // has the same state root as its parent. Two distinct empty blocks at the same + // height (different seal/timestamp/coinbase) therefore legitimately share a + // state root, and that is not a shadow-state attack — there is no forged state + // to skip-verify. Only treat a matching canonical root as an attack when the + // block actually changed state relative to its parent. If the parent header is + // unavailable, keep the conservative behavior and reject. + parent := bc.GetHeader(block.ParentHash(), block.NumberU64()-1) + if parent == nil || parent.Root != block.Root() { + log.Warn("Sidechain ghost-state attack detected", "number", block.NumberU64(), "sideroot", block.Root(), "canonroot", canonical.Root()) + + // If someone legitimately side-mines blocks, they would still be imported as usual. However, + // we cannot risk writing unverified blocks to disk when they obviously target the pruning + // mechanism. + return nil, it.index, errors.New("sidechain ghost-state attack") + } } } if externTd == nil { diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 0a2cd47a9f..0e2e221530 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -3108,6 +3108,169 @@ 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) + } +} + // 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 From 5ac7160d4b5b65681b86b4e12d440d4261008d03 Mon Sep 17 00:00:00 2001 From: bit2swaz Date: Tue, 14 Jul 2026 19:51:00 +0530 Subject: [PATCH 2/3] core: harden and refactor sidechain ghost-state no-op exemption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #2293 review feedback: - Copilot: gate the empty-block exemption on the *canonical* chain also being a no-op at that height, measured against the trusted canonical parent header — not only the side block's own parent, which may be unverified/attacker-controlled in insertSideChain. This closes the forge-both-roots bypass where an attacker crafts matching side parent+child roots to skip state verification. - diffguard (Quality metrics): extract the decision into isSidechainGhostState, dropping insertSideChain's complexity and size deltas back to zero. Add TestSidechainGhostStateCanonicalGate covering the hardening: a no-op side block at a height where the canonical chain changed state is still rejected. Co-Authored-By: Claude Opus 4.8 --- core/blockchain.go | 53 +++++++++++++++++++++++++++-------------- core/blockchain_test.go | 51 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 18 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index e6136fe982..7dbd68dc59 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -3678,7 +3678,7 @@ func (bc *BlockChain) insertSideChain(block *types.Block, it *insertIterator, ma 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 @@ -3686,23 +3686,12 @@ func (bc *BlockChain) insertSideChain(block *types.Block, it *insertIterator, ma // // If left unchecked, we would now proceed importing the blocks, without actually // having verified the state of the previous blocks. - // - // Exception: a block that performed no state transition (e.g. an empty block) - // has the same state root as its parent. Two distinct empty blocks at the same - // height (different seal/timestamp/coinbase) therefore legitimately share a - // state root, and that is not a shadow-state attack — there is no forged state - // to skip-verify. Only treat a matching canonical root as an attack when the - // block actually changed state relative to its parent. If the parent header is - // unavailable, keep the conservative behavior and reject. - parent := bc.GetHeader(block.ParentHash(), block.NumberU64()-1) - if parent == nil || parent.Root != block.Root() { - log.Warn("Sidechain ghost-state attack detected", "number", block.NumberU64(), "sideroot", block.Root(), "canonroot", canonical.Root()) - - // If someone legitimately side-mines blocks, they would still be imported as usual. However, - // we cannot risk writing unverified blocks to disk when they obviously target the pruning - // mechanism. - return nil, it.index, errors.New("sidechain ghost-state attack") - } + log.Warn("Sidechain ghost-state attack detected", "number", block.NumberU64(), "sideroot", block.Root(), "canonroot", canonical.Root()) + + // If someone legitimately side-mines blocks, they would still be imported as usual. However, + // we cannot risk writing unverified blocks to disk when they obviously target the pruning + // mechanism. + return nil, it.index, errors.New("sidechain ghost-state attack") } } if externTd == nil { @@ -3808,6 +3797,34 @@ func (bc *BlockChain) insertSideChain(block *types.Block, it *insertIterator, ma 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. diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 0e2e221530..7273554629 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -3271,6 +3271,57 @@ func testSideImportGhostStateStillRejected(t *testing.T, scheme string) { } } +// 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") + } +} + // 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 From 3b69d47da9a2364b76aa402cb6c1448a174a58bd Mon Sep 17 00:00:00 2001 From: bit2swaz Date: Wed, 15 Jul 2026 21:31:07 +0530 Subject: [PATCH 3/3] test: cover isSidechainGhostState missing-parent fallback for codecov/patch --- core/blockchain.go | 4 ++-- core/blockchain_test.go | 24 ++++++++++++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/core/blockchain.go b/core/blockchain.go index 7dbd68dc59..0542fe6be7 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -3804,7 +3804,7 @@ func (bc *BlockChain) insertSideChain(block *types.Block, it *insertIterator, ma // // 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 +// 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 @@ -3819,7 +3819,7 @@ func (bc *BlockChain) isSidechainGhostState(block *types.Block, canonical *types 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. + // 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) diff --git a/core/blockchain_test.go b/core/blockchain_test.go index 7273554629..30cc4c60ad 100644 --- a/core/blockchain_test.go +++ b/core/blockchain_test.go @@ -3276,7 +3276,7 @@ func testSideImportGhostStateStillRejected(t *testing.T, scheme string) { // 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 +// 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 @@ -3312,7 +3312,7 @@ func TestSidechainGhostStateCanonicalGate(t *testing.T) { 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 + // 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") @@ -3322,6 +3322,26 @@ func TestSidechainGhostStateCanonicalGate(t *testing.T) { } } +// 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