Skip to content
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

cmd/geth: fix importBlock #2244

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

weiihann
Copy link
Contributor

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/[email protected]/command.go:274 +0x730
github.com/urfave/cli/v2.(*Command).Run(0x1400056c160, 0x14001502440, {0x1400017c000, 0x9, 0x9})
	github.com/urfave/cli/[email protected]/command.go:267 +0x940
github.com/urfave/cli/v2.(*App).RunContext(0x14001522000, {0x106ed6ad8?, 0x108578740}, {0x1400017c000, 0x9, 0x9})
	github.com/urfave/cli/[email protected]/app.go:332 +0x518
github.com/urfave/cli/v2.(*App).Run(...)
	github.com/urfave/cli/[email protected]/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:

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
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
Contributor Author

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
Copy link
Collaborator

zzzckck commented Apr 11, 2024

We can change the commit message now.

@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
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants