Skip to content

cmd/geth: fix importBlock#2244

Merged
zzzckck merged 1 commit intobnb-chain:developfrom
weiihann:develop/fix-import-block
Apr 15, 2024
Merged

cmd/geth: fix importBlock#2244
zzzckck merged 1 commit intobnb-chain:developfrom
weiihann:develop/fix-import-block

Conversation

@weiihann
Copy link
Copy Markdown
Contributor

@weiihann weiihann commented Feb 29, 2024

Description

This PR fixes a bug when importing blocks in an offline manner (i.e. ./bsc import block_data).

Rationale

Expected Behaviour

Command: ./bsc import block_data

...
INFO [02-29|15:00:58.337] Importing blockchain                     file=0_1000000
INFO [02-29|15:00:58.343] Generated state snapshot                 accounts=13 slots=0 storage=843.00B dangling=0 elapsed=9.620ms
INFO [02-29|15:01:06.365] Imported new chain segment               number=805 hash=544976..99b62e miner=0x685B1ded8013785d6623CC18D214320b6Bb64759 blocks=805 txs=25 mgas=4.490 elapsed=8.$
INFO [02-29|15:01:14.371] Imported new chain segment               number=1600 hash=05b2b9..f21f4e miner=0x35E7a025f4da968De7e4D7E4004197917F4070F1 blocks=795 txs=12 mgas=1.240 elapsed=8$
INFO [02-29|15:01:22.376] Imported new chain segment               number=2465 hash=81374e..879eed miner=0x68Bf0B8b6FB4E317a0f9D6F03eAF8CE6675BC60D blocks=865 txs=0  mgas=0.000 elapsed=8$
INFO [02-29|15:01:22.654] Imported new chain segment               number=2500 hash=717e6a..0b30f9 miner=0x2a7cdd959bFe8D9487B2a43B33565295a698F7e2 blocks=35  txs=0  mgas=0.000 elapsed=2$
...

Actual Behaviour

Command: ./bsc import block_data

...
INFO [02-29|14:55:53.094] Importing blockchain                     file=0_1000000
INFO [02-29|14:55:53.098] Generated state snapshot                 accounts=13 slots=0 storage=843.00B dangling=0 elapsed=10.023ms
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x2 addr=0x0 pc=0x10516ce80]

goroutine 1 [running]:
github.com/ethereum/go-ethereum/internal/ethapi.(*BlockChainAPI).Call(0x0, {0x106ed6c28, 0x14001484c30}, {0x0, 0x140015af188, 0x140016f1c90, 0x0, 0x0, 0x0, 0x0, ...}, ...)
	github.com/ethereum/go-ethereum/internal/ethapi/api.go:1221 +0x60
github.com/ethereum/go-ethereum/consensus/parlia.(*Parlia).getCurrentValidatorsBeforeLuban(0x1400072f800, {0xd3, 0xe8, 0x6f, 0xee, 0x61, 0xcc, 0x6a, 0x89, 0xaf, ...}, ...)
	github.com/ethereum/go-ethereum/consensus/parlia/lubanFork.go:37 +0x32c
github.com/ethereum/go-ethereum/consensus/parlia.(*Parlia).getCurrentValidators(0x1400072f800, {0xd3, 0xe8, 0x6f, 0xee, 0x61, 0xcc, 0x6a, 0x89, 0xaf, ...}, ...)
	github.com/ethereum/go-ethereum/consensus/parlia/parlia.go:1527 +0x544
github.com/ethereum/go-ethereum/consensus/parlia.(*Parlia).verifyValidators(0x1400072f800, 0x14002317680)
	github.com/ethereum/go-ethereum/consensus/parlia/parlia.go:988 +0xdc
github.com/ethereum/go-ethereum/consensus/parlia.(*Parlia).Finalize(0x1400072f800, {0x106ee1230, 0x14000320c00}, 0x14002317680, 0xcb57d9960655a450?, 0x3af00049?, {0x1c9c380?, 0x140028c6be0?, 0x5f49cd4b?}, {0x140028c6c20?, ...}, ...)
	github.com/ethereum/go-ethereum/consensus/parlia/parlia.go:1114 +0x268
github.com/ethereum/go-ethereum/core.(*StateProcessor).Process(0x14001486b80, 0x1400150df40, 0x14002214780, {{0x0, 0x0}, 0x0, 0x0, 0x0, {0x0, 0x0, ...}})
	github.com/ethereum/go-ethereum/core/state_processor.go:136 +0xdcc
github.com/ethereum/go-ethereum/core.(*BlockChain).insertChain(0x14000320c00, {0x14001f85000?, 0x9c4, 0x9c4}, 0x1)
	github.com/ethereum/go-ethereum/core/blockchain.go:2049 +0x1650
github.com/ethereum/go-ethereum/core.(*BlockChain).InsertChain(0x14000320c00, {0x14001f85000?, 0x9c4, 0x9c4})
	github.com/ethereum/go-ethereum/core/blockchain.go:1827 +0x82c
github.com/ethereum/go-ethereum/cmd/utils.ImportChain(0x0?, {0x16ba23777, 0x9})
	github.com/ethereum/go-ethereum/cmd/utils/cmd.go:218 +0x528
main.importChain(0x140015b7480)
	github.com/ethereum/go-ethereum/cmd/geth/chaincmd.go:493 +0x2bc
github.com/ethereum/go-ethereum/internal/flags.MigrateGlobalFlags.func2.1(0x14000661b68?)
	github.com/ethereum/go-ethereum/internal/flags/helpers.go:91 +0x38
github.com/urfave/cli/v2.(*Command).Run(0x108505cc0, 0x140015b7480, {0x140016c0ce0, 0x2, 0x2})
	github.com/urfave/cli/v2@v2.25.7/command.go:274 +0x730
github.com/urfave/cli/v2.(*Command).Run(0x1400056c160, 0x14001502440, {0x1400017c000, 0x9, 0x9})
	github.com/urfave/cli/v2@v2.25.7/command.go:267 +0x940
github.com/urfave/cli/v2.(*App).RunContext(0x14001522000, {0x106ed6ad8?, 0x108578740}, {0x1400017c000, 0x9, 0x9})
	github.com/urfave/cli/v2@v2.25.7/app.go:332 +0x518
github.com/urfave/cli/v2.(*App).Run(...)
	github.com/urfave/cli/v2@v2.25.7/app.go:309
main.main()
	github.com/ethereum/go-ethereum/cmd/geth/main.go:289 +0x4c

The error above is primarily triggered by the following function:

bsc/consensus/parlia/parlia.go

Lines 993 to 998 in bd13416

func (p *Parlia) verifyValidators(header *types.Header) error {
if header.Number.Uint64()%p.config.Epoch != 0 {
return nil
}
newValidators, voteAddressMap, err := p.getCurrentValidators(header.ParentHash, new(big.Int).Sub(header.Number, big.NewInt(1)))

Essentially, for every epoch, the Parlia consensus engine would verify the validators set by calling an RPC method. However, when using the ./bsc import block_data tool, the RPC API is not available as it purely does block importing and execution. Hence, this results in the error as shown above.

There are a few solutions to this problem:

  1. Re-engineer the getCurrentValidators method to not use the RPC method. We may risk breaking crucial components with this approach.
  2. Launch the node alongside the RPC API when executing the ./bsc import tool. We only need the RPC API, so other services launched are trivial and add unnecessary processing overhead.
  3. Simply add an option in Parlia to skip verifying validators.

The third option is selected. While it may not be the most elegant, it ensures that this tool is working properly with a minimal change to the codebase, and without breaking things.

Changes

  • Add a new field noVerifyValidators to Parlia

@zzzckck
Copy link
Copy Markdown
Collaborator

zzzckck commented Mar 21, 2024

There are a few solutions to this problem:
1.Re-engineer the getCurrentValidators method to not use the RPC method. We may risk breaking crucial components with this approach.
2.Launch the node alongside the RPC API when executing the ./bsc import tool. We only need the RPC API, so other services launched are trivial and add unnecessary processing overhead.
3.Simply add an option in Parlia to skip verifying validators.

///===== Actually, I would prefer Solution 1 or Solution 2 if possible.

@weiihann weiihann force-pushed the develop/fix-import-block branch 2 times, most recently from cc6c52e to 4ce7033 Compare March 29, 2024 08:33
@weiihann
Copy link
Copy Markdown
Contributor Author

weiihann commented Mar 29, 2024

Went with the second approach. UX is slightly worse because logs related to starting a node will also appear. But the good thing is that code changes are minimal, no need to modify major components and there is not much performance impact.

zzzckck
zzzckck reviewed Apr 3, 2024
View reviewed changes
@weiihann weiihann force-pushed the develop/fix-import-block branch from 4ce7033 to 100f33f Compare April 3, 2024 06:10
@zzzckck
Copy link
Copy Markdown
Collaborator

zzzckck commented Apr 11, 2024

We can change the commit message now.

@weiihann weiihann force-pushed the develop/fix-import-block branch from 100f33f to 6eed590 Compare April 15, 2024 07:04
@weiihann weiihann changed the title cmd, parlia: option to disable verify validators cmd/geth: fix importBlock Apr 15, 2024
@zzzckck zzzckck merged commit 837de88 into bnb-chain:develop Apr 15, 2024
This was referenced Apr 26, 2024
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zlacfzy zlacfzy zlacfzy approved these changes

@zzzckck zzzckck zzzckck approved these changes

@unclezoro unclezoro Awaiting requested review from unclezoro

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@weiihann @zzzckck @zlacfzy