-
Notifications
You must be signed in to change notification settings - Fork 31
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
v2/consortium_test: fix: do not insert inserted blocks #612
v2/consortium_test: fix: do not insert inserted blocks #612
Conversation
} | ||
if _, err := chain.InsertChain(bs[:], nil); err != nil { | ||
// only insert newly generated blocks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason this still previously passed for Hash Scheme 🤔
} | ||
if _, err := chain.InsertChain(bs[:], nil); err != nil { | ||
// only insert newly generated blocks | ||
if _, err := chain.InsertChain(newBs[:], nil); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove newBs
by only using bs[399:] here
} | ||
if _, err := chain.InsertChain(bs[:], nil); err != nil { | ||
// only insert newly generated blocks | ||
if _, err := chain.InsertChain(newBs[:], nil); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similarly
@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. |
Before this PR, in
TestIsPeriodBlock
andTestIsTrippEffective
, the chain of blocks is inserted while it still includes some previously inserted blocks, potentially cause the following error when running on path scheme:This PR is to fix it by creating a new list of blocks that only contains newly created blocks.
Run test after fixing: