-
Notifications
You must be signed in to change notification settings - Fork 282
fix(l1 follower, rollup verifier): blockhash mismatch #1192
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
base: jt/export-headers-toolkit
Are you sure you want to change the base?
Changes from all commits
4561826
a47ccd8
5774441
66f0aef
7a48a92
30d5a46
2ef390e
de651d9
3e33a8a
7d8bb01
b90e909
51d5c7d
eaad65b
2ee9190
56cc282
8e4a635
ed1f352
32b8209
b54c25f
3b78f62
cc23f6d
0559042
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,9 @@ import ( | |
"errors" | ||
"fmt" | ||
"math/big" | ||
"net/url" | ||
"path" | ||
"path/filepath" | ||
"runtime" | ||
"sync" | ||
"sync/atomic" | ||
|
@@ -61,6 +64,7 @@ import ( | |
"github.com/scroll-tech/go-ethereum/rollup/ccc" | ||
"github.com/scroll-tech/go-ethereum/rollup/da_syncer" | ||
"github.com/scroll-tech/go-ethereum/rollup/l1" | ||
"github.com/scroll-tech/go-ethereum/rollup/missing_header_fields" | ||
"github.com/scroll-tech/go-ethereum/rollup/rollup_sync_service" | ||
"github.com/scroll-tech/go-ethereum/rollup/sync_service" | ||
"github.com/scroll-tech/go-ethereum/rpc" | ||
|
@@ -241,7 +245,12 @@ func New(stack *node.Node, config *ethconfig.Config, l1Client l1.Client) (*Ether | |
if config.EnableDASyncing { | ||
// Do not start syncing pipeline if we are producing blocks for permissionless batches. | ||
if !config.DA.ProduceBlocks { | ||
eth.syncingPipeline, err = da_syncer.NewSyncingPipeline(context.Background(), eth.blockchain, chainConfig, eth.chainDb, l1Client, stack.Config().L1DeploymentBlock, config.DA) | ||
missingHeaderFieldsManager, err := createMissingHeaderFieldsManager(stack, chainConfig) | ||
if err != nil { | ||
return nil, fmt.Errorf("cannot create missing header fields manager: %w", err) | ||
} | ||
|
||
eth.syncingPipeline, err = da_syncer.NewSyncingPipeline(context.Background(), eth.blockchain, chainConfig, eth.chainDb, l1Client, stack.Config().L1DeploymentBlock, config.DA, missingHeaderFieldsManager) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not init There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there's too many dependencies (and implicit behavior) so that imo it's more fitting to do it when setting up the node |
||
if err != nil { | ||
return nil, fmt.Errorf("cannot initialize da syncer: %w", err) | ||
} | ||
|
@@ -337,6 +346,22 @@ func New(stack *node.Node, config *ethconfig.Config, l1Client l1.Client) (*Ether | |
return eth, nil | ||
} | ||
|
||
func createMissingHeaderFieldsManager(stack *node.Node, chainConfig *params.ChainConfig) (*missing_header_fields.Manager, error) { | ||
downloadURL, err := url.Parse(stack.Config().DAMissingHeaderFieldsBaseURL) | ||
if err != nil { | ||
return nil, fmt.Errorf("invalid DAMissingHeaderFieldsBaseURL: %w", err) | ||
} | ||
downloadURL.Path = path.Join(downloadURL.Path, chainConfig.ChainID.String()+".bin") | ||
Comment on lines
+350
to
+354
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the flag is a user input I think the correct way is sanitizing the flag properly with |
||
|
||
expectedSHA256Checksum := chainConfig.Scroll.MissingHeaderFieldsSHA256 | ||
if expectedSHA256Checksum == nil { | ||
return nil, fmt.Errorf("missing expected SHA256 checksum for missing header fields file in chain config") | ||
} | ||
|
||
filePath := filepath.Join(stack.Config().DataDir, fmt.Sprintf("missing-header-fields-%s-%s", chainConfig.ChainID, expectedSHA256Checksum.Hex())) | ||
return missing_header_fields.NewManager(context.Background(), filePath, downloadURL.String(), *expectedSHA256Checksum), nil | ||
} | ||
|
||
func makeExtraData(extra []byte) []byte { | ||
if len(extra) == 0 { | ||
// create default extradata | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason to reset the header's BaseFee?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's due to differences in serialization and therefore the block hash.
big.Int == 0
is serialized whilebig.Int == nil
is not. during testing it became clear that this needs to be done so the block hashes will end up matching