Skip to content

[op-node] Check that BlockNumber is non-nil in the receipt validation#3881

Merged
tynes merged 1 commit intoethereum-optimism:developfrom
mdehoog:michael/op-node-reorg-panic
Nov 5, 2022
Merged

[op-node] Check that BlockNumber is non-nil in the receipt validation#3881
tynes merged 1 commit intoethereum-optimism:developfrom
mdehoog:michael/op-node-reorg-panic

Conversation

@mdehoog
Copy link
Copy Markdown
Contributor

@mdehoog mdehoog commented Nov 4, 2022

Description

We saw op-node panic in an internal PoA testnet, I believe due to a reorg that happened during a deploy (having multiple PoA signers with the same key during the deploy). This caused some of the tx receipts returned from the L1 have a null blockNumber value, causing a panic: runtime error: invalid memory address or nil pointer dereference error.

Tests

There doesn't appear to be any existing tests for this logic, but happy to add some if I'm missing where those are.

Additional context

Stacktrace:

[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0xe276b4]
panic: runtime error: invalid memory address or nil pointer dereference

goroutine 217 [running]:
math/big.(*Int).Uint64(...)
	/usr/local/go/src/math/big/int.go:384
github.com/ethereum-optimism/optimism/op-node/sources.makeReceiptsFn.func1({0xc0038d7780?, 0x1371e40?, 0x19?}, {0xc0003899f8, 0x1, 0x1})
	/app/op-node/sources/receipts.go:33 +0xd8
github.com/ethereum-optimism/optimism/op-node/sources.(*IterativeBatchCall[...]).Result(0xc003aa8000)
	/app/op-node/sources/batching.go:170 +0x169
github.com/ethereum-optimism/optimism/op-node/sources.(*EthClient).FetchReceipts(0xc000180140, {0x1718b80, 0xc0038fad80}, {0x5f, 0xb0, 0x7e, 0x59, 0x1c, 0x67, 0x50, ...})
	/app/op-node/sources/eth_client.go:265 +0x1fc
github.com/ethereum-optimism/optimism/op-node/rollup/derive.PreparePayloadAttributes({0x1718b80?, 0xc0038fad80?}, 0xc0002e2438, {0x7ff8045043e8?, 0xc00007e0c0?}, {{0xd9, 0xe6, 0x83, 0xa9, 0x9d, ...}, ...}, ...)
	/app/op-node/rollup/derive/attributes.go:34 +0x97
github.com/ethereum-optimism/optimism/op-node/rollup/driver.(*Sequencer).StartBuildingBlock(_, {_, _}, {{0xd9, 0xe6, 0x83, 0xa9, 0x9d, 0x31, 0x31, ...}, ...}, ...)
	/app/op-node/rollup/driver/sequencer.go:53 +0x50b
github.com/ethereum-optimism/optimism/op-node/rollup/driver.(*Sequencer).CreateNewBlock(_, {_, _}, {{0xd9, 0xe6, 0x83, 0xa9, 0x9d, 0x31, 0x31, ...}, ...}, ...)
	/app/op-node/rollup/driver/sequencer.go:100 +0x105
github.com/ethereum-optimism/optimism/op-node/rollup/driver.(*Driver).createNewL2Block(0xc0003f4000, {0x1718b80, 0xc003a3cc00})
	/app/op-node/rollup/driver/state.go:161 +0x789
github.com/ethereum-optimism/optimism/op-node/rollup/driver.(*Driver).eventLoop(0xc0003f4000)
	/app/op-node/rollup/driver/state.go:270 +0x5ce
created by github.com/ethereum-optimism/optimism/op-node/rollup/driver.(*Driver).Start
	/app/op-node/rollup/driver/state.go:79 +0x85

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Nov 4, 2022

⚠️ No Changeset found

Latest commit: 06be534

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mslipper
Copy link
Copy Markdown
Contributor

mslipper commented Nov 4, 2022

Authorized the build, should be green in a minute.

@tynes
Copy link
Copy Markdown
Contributor

tynes commented Nov 4, 2022

This should be safe to merge

@tynes tynes merged commit badda57 into ethereum-optimism:develop Nov 5, 2022
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.

5 participants