Skip to content

fix: provide parent header during EuclidV2 transition verification #1187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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
2 changes: 1 addition & 1 deletion consensus/clique/clique.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func (c *Clique) VerifyHeader(chain consensus.ChainHeaderReader, header *types.H
// VerifyHeaders is similar to VerifyHeader, but verifies a batch of headers. The
// method returns a quit channel to abort the operations and a results channel to
// retrieve the async verifications (the order is that of the input slice).
func (c *Clique) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool) (chan<- struct{}, <-chan error) {
func (c *Clique) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool, parent *types.Header) (chan<- struct{}, <-chan error) {
abort := make(chan struct{})
results := make(chan error, len(headers))

Expand Down
2 changes: 1 addition & 1 deletion consensus/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ type Engine interface {
// concurrently. The method returns a quit channel to abort the operations and
// a results channel to retrieve the async verifications (the order is that of
// the input slice).
VerifyHeaders(chain ChainHeaderReader, headers []*types.Header, seals []bool) (chan<- struct{}, <-chan error)
VerifyHeaders(chain ChainHeaderReader, headers []*types.Header, seals []bool, parent *types.Header) (chan<- struct{}, <-chan error)

// VerifyUncles verifies that the given block's uncles conform to the consensus
// rules of a given engine.
Expand Down
2 changes: 1 addition & 1 deletion consensus/ethash/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ func (ethash *Ethash) VerifyHeader(chain consensus.ChainHeaderReader, header *ty
// VerifyHeaders is similar to VerifyHeader, but verifies a batch of headers
// concurrently. The method returns a quit channel to abort the operations and
// a results channel to retrieve the async verifications.
func (ethash *Ethash) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool) (chan<- struct{}, <-chan error) {
func (ethash *Ethash) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool, parent *types.Header) (chan<- struct{}, <-chan error) {
// If we're running a full engine faking, accept any input as valid
if ethash.config.PowMode == ModeFullFake || len(headers) == 0 {
abort, results := make(chan struct{}), make(chan error, len(headers))
Expand Down
9 changes: 7 additions & 2 deletions consensus/system_contract/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,18 @@ func (s *SystemContract) VerifyHeader(chain consensus.ChainHeaderReader, header
// concurrently. The method returns a quit channel to abort the operations and
// a results channel to retrieve the async verifications (the order is that of
// the input slice).
func (s *SystemContract) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool) (chan<- struct{}, <-chan error) {
func (s *SystemContract) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool, parent *types.Header) (chan<- struct{}, <-chan error) {
abort := make(chan struct{})
results := make(chan error, len(headers))

go func() {
for i, header := range headers {
err := s.verifyHeader(chain, header, headers[:i])
parents := headers[:i]
if len(parents) == 0 && parent != nil {
parents = []*types.Header{parent}
}

err := s.verifyHeader(chain, header, parents)
if err != nil {
log.Error("Error verifying headers", "err", err)
}
Expand Down
53 changes: 53 additions & 0 deletions consensus/system_contract/system_contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ package system_contract
import (
"context"
"fmt"
"math/big"
"sync"
"time"

"github.com/scroll-tech/go-ethereum"
"github.com/scroll-tech/go-ethereum/common"
"github.com/scroll-tech/go-ethereum/core/types"
"github.com/scroll-tech/go-ethereum/log"
"github.com/scroll-tech/go-ethereum/params"
"github.com/scroll-tech/go-ethereum/rollup/sync_service"
Expand Down Expand Up @@ -140,3 +143,53 @@ func (s *SystemContract) localSignerAddress() common.Address {

return s.signer
}

// FakeEthClient implements a minimal version of sync_service.EthClient for testing purposes.
type FakeEthClient struct {
mu sync.Mutex
// Value is the fixed Value to return from StorageAt.
// We'll assume StorageAt returns a 32-byte Value representing an Ethereum address.
Value common.Address
}

// BlockNumber returns 0.
func (f *FakeEthClient) BlockNumber(ctx context.Context) (uint64, error) {
return 0, nil
}

// ChainID returns a zero-value chain ID.
func (f *FakeEthClient) ChainID(ctx context.Context) (*big.Int, error) {
return big.NewInt(0), nil
}

// FilterLogs returns an empty slice of logs.
func (f *FakeEthClient) FilterLogs(ctx context.Context, q ethereum.FilterQuery) ([]types.Log, error) {
return []types.Log{}, nil
}

// HeaderByNumber returns nil.
func (f *FakeEthClient) HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error) {
return nil, nil
}

// SubscribeFilterLogs returns a nil subscription.
func (f *FakeEthClient) SubscribeFilterLogs(ctx context.Context, query ethereum.FilterQuery, ch chan<- types.Log) (ethereum.Subscription, error) {
return nil, nil
}

// TransactionByHash returns (nil, false, nil).
func (f *FakeEthClient) TransactionByHash(ctx context.Context, txHash common.Hash) (*types.Transaction, bool, error) {
return nil, false, nil
}

// BlockByHash returns nil.
func (f *FakeEthClient) BlockByHash(ctx context.Context, hash common.Hash) (*types.Block, error) {
return nil, nil
}

// StorageAt returns the byte representation of f.value.
func (f *FakeEthClient) StorageAt(ctx context.Context, account common.Address, key common.Hash, blockNumber *big.Int) ([]byte, error) {
f.mu.Lock()
defer f.mu.Unlock()
return f.Value.Bytes(), nil
}
64 changes: 6 additions & 58 deletions consensus/system_contract/system_contract_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@ package system_contract
import (
"context"
"math/big"
"sync"
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/scroll-tech/go-ethereum"
"github.com/scroll-tech/go-ethereum/accounts"
"github.com/scroll-tech/go-ethereum/common"
"github.com/scroll-tech/go-ethereum/core/types"
Expand All @@ -19,14 +17,14 @@ import (
"github.com/scroll-tech/go-ethereum/trie"
)

var _ sync_service.EthClient = &fakeEthClient{}
var _ sync_service.EthClient = &FakeEthClient{}

func TestSystemContract_FetchSigner(t *testing.T) {
log.Root().SetHandler(log.DiscardHandler())

expectedSigner := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678")

fakeClient := &fakeEthClient{value: expectedSigner}
fakeClient := &FakeEthClient{Value: expectedSigner}

config := &params.SystemContractConfig{
SystemContractAddress: common.HexToAddress("0xFAKE"),
Expand Down Expand Up @@ -55,7 +53,7 @@ func TestSystemContract_AuthorizeCheck(t *testing.T) {

expectedSigner := common.HexToAddress("0x1234567890abcdef1234567890abcdef12345678")

fakeClient := &fakeEthClient{value: expectedSigner}
fakeClient := &FakeEthClient{Value: expectedSigner}
config := &params.SystemContractConfig{
SystemContractAddress: common.HexToAddress("0xFAKE"),
Period: 10,
Expand Down Expand Up @@ -106,8 +104,8 @@ func TestSystemContract_SignsAfterUpdate(t *testing.T) {
updatedSigner := common.HexToAddress("0x2222222222222222222222222222222222222222")

// Create a fake client that starts by returning the wrong signer.
fakeClient := &fakeEthClient{
value: oldSigner,
fakeClient := &FakeEthClient{
Value: oldSigner,
}

config := &params.SystemContractConfig{
Expand All @@ -130,7 +128,7 @@ func TestSystemContract_SignsAfterUpdate(t *testing.T) {

// Now, simulate an update: change the fake client's returned value to updatedSigner.
fakeClient.mu.Lock()
fakeClient.value = updatedSigner
fakeClient.Value = updatedSigner
fakeClient.mu.Unlock()

// fetch new value from L1 (simulating a background poll)
Expand Down Expand Up @@ -171,53 +169,3 @@ func TestSystemContract_SignsAfterUpdate(t *testing.T) {
t.Fatal("Timed out waiting for Seal to return a sealed block")
}
}

// fakeEthClient implements a minimal version of sync_service.EthClient for testing purposes.
type fakeEthClient struct {
mu sync.Mutex
// value is the fixed value to return from StorageAt.
// We'll assume StorageAt returns a 32-byte value representing an Ethereum address.
value common.Address
}

// BlockNumber returns 0.
func (f *fakeEthClient) BlockNumber(ctx context.Context) (uint64, error) {
return 0, nil
}

// ChainID returns a zero-value chain ID.
func (f *fakeEthClient) ChainID(ctx context.Context) (*big.Int, error) {
return big.NewInt(0), nil
}

// FilterLogs returns an empty slice of logs.
func (f *fakeEthClient) FilterLogs(ctx context.Context, q ethereum.FilterQuery) ([]types.Log, error) {
return []types.Log{}, nil
}

// HeaderByNumber returns nil.
func (f *fakeEthClient) HeaderByNumber(ctx context.Context, number *big.Int) (*types.Header, error) {
return nil, nil
}

// SubscribeFilterLogs returns a nil subscription.
func (f *fakeEthClient) SubscribeFilterLogs(ctx context.Context, query ethereum.FilterQuery, ch chan<- types.Log) (ethereum.Subscription, error) {
return nil, nil
}

// TransactionByHash returns (nil, false, nil).
func (f *fakeEthClient) TransactionByHash(ctx context.Context, txHash common.Hash) (*types.Transaction, bool, error) {
return nil, false, nil
}

// BlockByHash returns nil.
func (f *fakeEthClient) BlockByHash(ctx context.Context, hash common.Hash) (*types.Block, error) {
return nil, nil
}

// StorageAt returns the byte representation of f.value.
func (f *fakeEthClient) StorageAt(ctx context.Context, account common.Address, key common.Hash, blockNumber *big.Int) ([]byte, error) {
f.mu.Lock()
defer f.mu.Unlock()
return f.value.Bytes(), nil
}
43 changes: 8 additions & 35 deletions consensus/wrapper/consensus.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package wrapper

import (
"math/big"
"time"

"github.com/scroll-tech/go-ethereum/common"
"github.com/scroll-tech/go-ethereum/consensus"
Expand Down Expand Up @@ -59,34 +58,9 @@ func (ue *UpgradableEngine) VerifyHeader(chain consensus.ChainHeaderReader, head
return ue.chooseEngine(header.Time).VerifyHeader(chain, header, seal)
}

func waitForHeader(chain consensus.ChainHeaderReader, header *types.Header) {
hash, number := header.Hash(), header.Number.Uint64()

// poll every 2 seconds, should succeed after a few tries
ticker := time.NewTicker(2 * time.Second)
defer ticker.Stop()

// we give up after 2 minutes
timeout := time.After(120 * time.Second)

for {
select {
case <-ticker.C:
// try reading from chain, if the header is present then we are ready
if h := chain.GetHeader(hash, number); h != nil {
return
}

case <-timeout:
log.Warn("Unable to find last pre-EuclidV2 header in chain", "hash", hash.Hex(), "number", number)
return
}
}
}

// VerifyHeaders verifies a batch of headers concurrently. In our use-case,
// headers can only be all system, all clique, or start with clique and then switch once to system.
func (ue *UpgradableEngine) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool) (chan<- struct{}, <-chan error) {
func (ue *UpgradableEngine) VerifyHeaders(chain consensus.ChainHeaderReader, headers []*types.Header, seals []bool, parent *types.Header) (chan<- struct{}, <-chan error) {
abort := make(chan struct{})
results := make(chan error, len(headers))

Expand All @@ -102,12 +76,12 @@ func (ue *UpgradableEngine) VerifyHeaders(chain consensus.ChainHeaderReader, hea

// If the first header is system, then all headers must be system.
if firstEngine == ue.system {
return firstEngine.VerifyHeaders(chain, headers, seals)
return firstEngine.VerifyHeaders(chain, headers, seals, nil)
}

// If first and last headers are both clique, then all headers are clique.
if firstEngine == lastEngine {
return firstEngine.VerifyHeaders(chain, headers, seals)
return firstEngine.VerifyHeaders(chain, headers, seals, nil)
}

// Otherwise, headers start as clique then switch to system. Since we assume
Expand Down Expand Up @@ -135,7 +109,7 @@ func (ue *UpgradableEngine) VerifyHeaders(chain consensus.ChainHeaderReader, hea

// Verify clique headers.
log.Info("Start EuclidV2 transition verification in Clique section", "startBlockNumber", cliqueHeaders[0].Number, "endBlockNumber", cliqueHeaders[len(cliqueHeaders)-1].Number)
abortClique, cliqueResults := ue.clique.VerifyHeaders(chain, cliqueHeaders, cliqueSeals)
abortClique, cliqueResults := ue.clique.VerifyHeaders(chain, cliqueHeaders, cliqueSeals, nil)

// Note: cliqueResults is not closed so we cannot directly iterate over it
for i := 0; i < len(cliqueHeaders); i++ {
Expand All @@ -149,14 +123,13 @@ func (ue *UpgradableEngine) VerifyHeaders(chain consensus.ChainHeaderReader, hea
}
}

// `VerifyHeader` will try to call `chain.GetHeader`, which will race with `InsertHeaderChain`.
// This might result in "unknown ancestor" header validation error.
// This should be temporary, so we solve this by waiting here.
waitForHeader(chain, cliqueHeaders[len(cliqueHeaders)-1])
// Since the Clique part of the header chain might not yet be stored in the local chain,
// provide a hint to the SystemContract consensus engine.
lastCliqueHeader := cliqueHeaders[len(cliqueHeaders)-1]

// Verify system contract headers.
log.Info("Start EuclidV2 transition verification in SystemContract section", "startBlockNumber", systemHeaders[0].Number, "endBlockNumber", systemHeaders[len(systemHeaders)-1].Number)
abortSystem, systemResults := ue.system.VerifyHeaders(chain, systemHeaders, systemSeals)
abortSystem, systemResults := ue.system.VerifyHeaders(chain, systemHeaders, systemSeals, lastCliqueHeader)

// Note: systemResults is not closed so we cannot directly iterate over it
for i := 0; i < len(systemHeaders); i++ {
Expand Down
10 changes: 5 additions & 5 deletions core/block_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ func TestHeaderVerification(t *testing.T) {

if valid {
engine := ethash.NewFaker()
_, results = engine.VerifyHeaders(chain, []*types.Header{headers[i]}, []bool{true})
_, results = engine.VerifyHeaders(chain, []*types.Header{headers[i]}, []bool{true}, nil)
} else {
engine := ethash.NewFakeFailer(headers[i].Number.Uint64())
_, results = engine.VerifyHeaders(chain, []*types.Header{headers[i]}, []bool{true})
_, results = engine.VerifyHeaders(chain, []*types.Header{headers[i]}, []bool{true}, nil)
}
// Wait for the verification result
select {
Expand Down Expand Up @@ -107,11 +107,11 @@ func testHeaderConcurrentVerification(t *testing.T, threads int) {

if valid {
chain, _ := NewBlockChain(testdb, nil, params.TestChainConfig, ethash.NewFaker(), vm.Config{}, nil, nil)
_, results = chain.engine.VerifyHeaders(chain, headers, seals)
_, results = chain.engine.VerifyHeaders(chain, headers, seals, nil)
chain.Stop()
} else {
chain, _ := NewBlockChain(testdb, nil, params.TestChainConfig, ethash.NewFakeFailer(uint64(len(headers)-1)), vm.Config{}, nil, nil)
_, results = chain.engine.VerifyHeaders(chain, headers, seals)
_, results = chain.engine.VerifyHeaders(chain, headers, seals, nil)
chain.Stop()
}
// Wait for all the verification results
Expand Down Expand Up @@ -176,7 +176,7 @@ func testHeaderConcurrentAbortion(t *testing.T, threads int) {
chain, _ := NewBlockChain(testdb, nil, params.TestChainConfig, ethash.NewFakeDelayer(time.Millisecond), vm.Config{}, nil, nil)
defer chain.Stop()

abort, results := chain.engine.VerifyHeaders(chain, headers, seals)
abort, results := chain.engine.VerifyHeaders(chain, headers, seals, nil)
close(abort)

// Deplete the results channel
Expand Down
2 changes: 1 addition & 1 deletion core/blockchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -1546,7 +1546,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, verifySeals bool) (int, er
headers[i] = block.Header()
seals[i] = verifySeals
}
abort, results := bc.engine.VerifyHeaders(bc, headers, seals)
abort, results := bc.engine.VerifyHeaders(bc, headers, seals, nil)
defer close(abort)

// Peek the error for the first block to decide the directing import logic
Expand Down
2 changes: 1 addition & 1 deletion core/headerchain.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ func (hc *HeaderChain) ValidateHeaderChain(chain []*types.Header, checkFreq int)
seals[len(seals)-1] = true
}

abort, results := hc.engine.VerifyHeaders(hc, chain, seals)
abort, results := hc.engine.VerifyHeaders(hc, chain, seals, nil)
defer close(abort)

// Iterate over the headers and ensure they all check out
Expand Down
Loading
Loading