Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Fix incorrect prune state of a signed_block issue #10344

Merged
merged 6 commits into from
May 11, 2021

Conversation

nickjjzhao
Copy link
Contributor

@nickjjzhao nickjjzhao commented May 6, 2021

Change Description

The variable prune_state in a signed_block should be updated after some trxs in the block get pruned that happens in prune_trxs(). Otherwise in some situation to_signed_block_v0() will try to convert a signed_block with pruned trxs to a signed_block_v0 , which is not supposed to, due to incorrect state of the variable.

Change Type

Select ONE:

  • Documentation
  • Stability bug fix
  • Other
  • Other - special case

Testing Changes

Select ANY that apply:

  • New Tests
  • Existing Tests
  • Test Framework
  • CI System
  • Other

Consensus Changes

  • Consensus Changes

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@nickjjzhao nickjjzhao changed the title Update var prune_state after trxs in a signed_block get pruned Fix incorrect prune state of a signed_block issue May 7, 2021
@heifner
Copy link
Contributor

heifner commented May 10, 2021

Don't forget to back-port to 2.1

@@ -186,7 +186,7 @@
Utils.Print("Post light validation waiting info for each node:\nproducer: {}\nfull: {},\nlight: {}".format(json.dumps(pvnPostInfo, indent=1), json.dumps(fvnPostInfo, indent=1), json.dumps(lvnPostInfo, indent=1)))
Utils.Print("Post full validation waiting info for each node:\nproducer: {}\nfull: {},\nlight: {}".format(json.dumps(pvnPost2Info, indent=1), json.dumps(fvnPost2Info, indent=1), json.dumps(lvnPost2Info, indent=1)))

assert not headAdvanced, "the full validation node is still syncing"
assert (not headAdvanced) or (fvnPost2Info["head_block_num"] < cfTrxBlockNum), "the full validation node is still syncing"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why were these two additions needed? waitForHeadToAdvance() should be enough, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It happened locally for full validation node that headAdvanced was true but the node hadn't synced up to the pruned block yet. Similar to the light validation node, its headAdvanced was false but the node had already passed the pruned block. I think we should treat these situations as success not failure so I added two extra conditions. I might be wrong.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, that makes sense.

@nickjjzhao nickjjzhao merged commit b1f0dfb into develop May 11, 2021
@nickjjzhao nickjjzhao deleted the jjz-update-prune_state branch May 11, 2021 22:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants