axieinfinity / ronin

A DPoS blockchain.
GNU Lesser General Public License v3.0
67 stars 30 forks source link

v2/consortium_test: fix: do not insert inserted blocks #612

Closed Francesco4203 closed 1 day ago

Francesco4203 commented 3 days ago

Before this PR, in TestIsPeriodBlock and TestIsTrippEffective, the chain of blocks is inserted while it still includes some previously inserted blocks, potentially cause the following error when running on path scheme:

--- FAIL: TestIsTrippEffective (0.19s)
panic: triedb parent [0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421] layer missing [recovered]
        panic: triedb parent [0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421] layer missing

goroutine 4016 [running]:
testing.tRunner.func1.2({0x105c97d20, 0x140002d1770})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/testing/testing.go:1631 +0x1c4
testing.tRunner.func1()
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/testing/testing.go:1634 +0x33c
panic({0x105c97d20?, 0x140002d1770?})
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/runtime/panic.go:770 +0x124
github.com/ethereum/go-ethereum/consensus/consortium/v2.testIsTrippEffective(0x140000c3a00, {0x10592235d, 0x4})
        /Users/long.nguyen/Documents/GitHub/ronin/consensus/consortium/v2/consortium_test.go:2437 +0xe5c
github.com/ethereum/go-ethereum/consensus/consortium/v2.TestIsTrippEffective(0x140000c3a00)
        /Users/long.nguyen/Documents/GitHub/ronin/consensus/consortium/v2/consortium_test.go:2341 +0x40
testing.tRunner(0x140000c3a00, 0x105df9818)
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/testing/testing.go:1689 +0xec
created by testing.(*T).Run in goroutine 1
        /opt/homebrew/Cellar/go/1.22.3/libexec/src/testing/testing.go:1742 +0x318
exit status 2
FAIL    github.com/ethereum/go-ethereum/consensus/consortium/v2 1.631s

This PR is to fix it by creating a new list of blocks that only contains newly created blocks. Run test after fixing:

PASS
ok      github.com/ethereum/go-ethereum/consensus/consortium/v2 1.530s
huyngopt1994 commented 2 days ago

@Francesco4203 so for clear, that the test is failed related to limit of maxdifflayer logic Pathbase, for Hashbase they still look into the keyvalue for checking rootState by Hash?, so could u verify this limit again? after that add comment in the test for clearing why do we choose this number.