Skip to content
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

Re-enable SN epoch integration test #4134

Merged
merged 2 commits into from
Apr 4, 2023
Merged

Conversation

jordanschalm
Copy link
Member

@jordanschalm jordanschalm commented Mar 30, 2023

Context

The test failure was caused by this fatal error from the Consensus Node brought up to join in the second epoch:
··· DOCK: 2023-03-30 19:48:34.635587 +0000 UTC (clogs ) consensus_test_3 (16726f0fad5781fa651f6a048c007e9f6a4202f8111811cbec685ac96774c32a) - �{"level":"fatal","node_role":"consensus","node_id":"522f1406e6e0c2267dbcc49519539c18d866a584df709e1a277bdbc28bd2d40c","error":"component sealing engine initialization failed: could not repopulate assignment collectors tree: internal error while traversing fork: visitor errored on block 6058fc1a85eaff3f46fe1d2a530ea7fe648cef771c75594bcfeef66238450bda at height 57: could not process incorporated result from block 6058fc1a85eaff3f46fe1d2a530ea7fe648cef771c75594bcfeef66238450bda: could not process incorporated incRes: could not process incorporated result 741f4ba4ad5ff7918b7aaa615306f136ae2e2b9b13a729e115e9fb48c2713b03: could not determine chunk assignment: failed to retrieve source of randomness: could not retrieve quorum certificate for (6058fc1a85eaff3f46fe1d2a530ea7fe648cef771c75594bcfeef66238450bda): could not retrieve resource: key not found","time":"2023-03-30T19:48:34.634479451Z","message":"unhandled irrecoverable error"}

The problem is that, the sealing engine attempts to populate its state using the sealing segment, but it cannot read the QC for some of these blocks. In #3947, we changed to read QCs from storage directly in all cases. However, these QCs were not stored in QC storage for all blocks, in particular sealing segment blocks.

Changes

  • Fixes the bug by inserting QCs explicitly for all sealing segment blocks and adds a test case
  • Re-enables the integration test

- fix a bug introduced in #3947,
  which resulted in some QCs being un-queriable
@codecov-commenter
Copy link

codecov-commenter commented Mar 30, 2023

Codecov Report

Merging #4134 (9c8b78a) into master (814e98d) will decrease coverage by 0.01%.
The diff coverage is 48.22%.

@@            Coverage Diff             @@
##           master    #4134      +/-   ##
==========================================
- Coverage   53.51%   53.51%   -0.01%     
==========================================
  Files         833      835       +2     
  Lines       77896    78192     +296     
==========================================
+ Hits        41687    41842     +155     
- Misses      32874    32992     +118     
- Partials     3335     3358      +23     
Flag Coverage Δ
unittests 53.51% <48.22%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/bootstrap/cmd/keys.go 53.60% <0.00%> (ø)
cmd/scaffold.go 14.75% <0.00%> (-0.34%) ⬇️
cmd/util/ledger/reporters/account_reporter.go 0.00% <0.00%> (ø)
...nsensus/hotstuff/committees/consensus_committee.go 69.72% <ø> (ø)
...ngine/common/follower/pending_tree/pending_tree.go 82.10% <ø> (ø)
fvm/derived/derived_block_data.go 33.33% <0.00%> (-0.96%) ⬇️
fvm/derived/table_invalidator.go 100.00% <ø> (ø)
fvm/environment/account_key_updater.go 19.48% <0.00%> (ø)
fvm/environment/facade_env.go 87.11% <0.00%> (ø)
fvm/errors/accounts.go 0.00% <0.00%> (ø)
... and 46 more

... and 3 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@durkmurder durkmurder left a comment

Choose a reason for hiding this comment

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

Good catch!

@jordanschalm
Copy link
Member Author

bors merge

@bors bors bot merged commit 96f42bf into master Apr 4, 2023
@bors bors bot deleted the jordan/6570-sn-flaky-epoch-test branch April 4, 2023 14:48
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.

4 participants