light: fix txpool causes panic on reorgOnNewHead #54
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
panic: runtime error: invalid memory address or nil pointer dereference Mar 04 12:22:51 sf-etclabs sh[26575]: [signal SIGSEGV: segmentation violation code=0x1 addr=0x1c0 pc=0xc5937c] Mar 04 12:22:51 sf-etclabs sh[26575]: goroutine 57 [running]: Mar 04 12:22:51 sf-etclabs sh[26575]: github.com/ethereum/go-ethereum/light.(*TxPool).reorgOnNewHead(0xc00038cc60, 0x11a2960, 0xc0204bd920, 0xc0209ec480, 0xc0204bd920, 0xc02096eb00, 0x8) Mar 04 12:22:51 sf-etclabs sh[26575]: /root/go/src/github.com/ethereum/go-ethereum/light/txpool.go:230 +0x23c Mar 04 12:22:51 sf-etclabs sh[26575]: github.com/ethereum/go-ethereum/light.(*TxPool).setNewHead(0xc00038cc60, 0xc0209ec480) Mar 04 12:22:51 sf-etclabs sh[26575]: /root/go/src/github.com/ethereum/go-ethereum/light/txpool.go:311 +0x13a Mar 04 12:22:51 sf-etclabs sh[26575]: github.com/ethereum/go-ethereum/light.(*TxPool).eventLoop(0xc00038cc60) Mar 04 12:22:51 sf-etclabs sh[26575]: /root/go/src/github.com/ethereum/go-ethereum/light/txpool.go:292 +0x53 Mar 04 12:22:51 sf-etclabs sh[26575]: created by github.com/ethereum/go-ethereum/light.NewTxPool Mar 04 12:22:51 sf-etclabs sh[26575]: /root/go/src/github.com/ethereum/go-ethereum/light/txpool.go:109 +0x3eb Signed-off-by: meows <b5c6@protonmail.com>
|
@tzdybal If and when LGTY, we can merge it and then I'll also make the corresponding PR at ethereum/go-ethereum. |
|
beep @tzdybal |
tzdybal
suggested changes
Mar 11, 2020
light/txpool.go
Outdated
| for _, hash := range oldHashes { | ||
| pool.rollbackTxs(hash, txc) | ||
|
|
||
| if newHeader == nil { |
There was a problem hiding this comment.
This statement doesn't make much sense, especially in here.
Above, we assign newHeader to newh, and operate on newh. If newHeader was indeed nil, this would fail (except degenerated case when both newh/newHeader and oldHeader are nil).
If it actually can happen, or we want to be extra defensive, move the check above line 229.
There was a problem hiding this comment.
I was assuming oldh was nil...
There was a problem hiding this comment.
PTAL at changes thru 687e626.
Have added some test cases and simplified the checks.
The check for newHeader nilness should happen earlier, somewhere up the Chain Insertion line. Signed-off-by: meows <b5c6@protonmail.com>
In real life, I think the ValidateHeaderChain check is sufficient, but tests use the InsertChain method without the Validate_ method prior, so that's why I think both these checks are needed. Signed-off-by: meows <b5c6@protonmail.com>
Adds a few edge-cases tests in TestTxPool which test: - non-nil txpool header-hash (by provenance) returns a nil value from chaindb This could be from a bad chaindb or an otherwise fucked pool head hash. + The otherwise fucked case is also tested. - pool attempts to set head to a nil value Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
Signed-off-by: meows <b5c6@protonmail.com>
tzdybal
approved these changes
Mar 12, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #53
Signed-off-by: meows b5c6@protonmail.com