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.

Sign up for GitHub

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

Jump to bottom

approval-voting: finality stalls in zombienet test 0006-parachains-max-tranche0 #3826

Closed
sandreim opened this issue Mar 25, 2024 · 3 comments · Fixed by #3831
Closed

approval-voting: finality stalls in zombienet test 0006-parachains-max-tranche0 #3826

sandreim opened this issue Mar 25, 2024 · 3 comments · Fixed by #3831
Assignees
@ordian
Labels
I2-bug The node fails to follow expected behavior.

Comments

@sandreim
Copy link
Contributor

Looks like optimizations done in #3747 are causing a finality stall. I have reverted locally and confirmed this PR introduced it, but haven't really looked for a solution.
What is specific in this test is that needed_approvals is set to n_validators - 1 assuming backing groups of 1.

Additionally 0001-parachains-pvf test is also affected by same issue.

First issue which causes the stall:
2024-03-25 15:05:41.001 WARN tokio-runtime-worker parachain::approval-voting: Failed to create assignment bitfield block_hash=0x2879a329794ab4bd147508f837d7245bce2cc22f3e102331dc865c1a4ebb9e29 err=NullAssignment

Second issue which can disrupt connectivity and hinder approval vote propagation

2024-03-25 15:05:42.060 DEBUG tokio-runtime-worker parachain::approval-distribution: Duplicate assignment peer_id=PeerId("12D3KooWNgfmn7z3F1TsqfXRCYqdCApucPyv6XnfQNt6MLbdSsAm") message_subject=MessageSubject(0x7467bd478b3f36afa7be045b1ae79aef482410b8046335fb8d8f5f42fbea6ee8, Bitfield(BitVec<u8, bitvec::order::Lsb0> { addr: 0x1386e69c0, head: 000, bits: 3, capacity: 8 } [0, 0, 1], PhantomData<u32>), ValidatorIndex(3))
2024-03-25 15:05:42.060 DEBUG tokio-runtime-worker parachain::reputation-aggregator: Reduce reputation peer=PeerId("12D3KooWNgfmn7z3F1TsqfXRCYqdCApucPyv6XnfQNt6MLbdSsAm") rep=CostMinorRepeated("Peer sent identical messages")
2024-03-25 15:05:42.060 DEBUG tokio-runtime-worker parachain::approval-distribution: Duplicate assignment peer_id=PeerId("12D3KooWNgfmn7z3F1TsqfXRCYqdCApucPyv6XnfQNt6MLbdSsAm") message_subject=MessageSubject(0x7467bd478b3f36afa7be045b1ae79aef482410b8046335fb8d8f5f42fbea6ee8, Bitfield(BitVec<u8, bitvec::order::Lsb0> { addr: 0x1386e69c0, head: 000, bits: 3, capacity: 8 } [0, 0, 1], PhantomData<u32>), ValidatorIndex(6))
2024-03-25 15:05:42.060 DEBUG tokio-runtime-worker parachain::reputation-aggregator: Reduce reputation peer=PeerId("12D3KooWNgfmn7z3F1TsqfXRCYqdCApucPyv6XnfQNt6MLbdSsAm") rep=CostMinorRepeated("Peer sent identical messages")
2024-03-25 15:05:42.060 DEBUG tokio-runtime-worker parachain::approval-distribution: Duplicate assignment peer_id=PeerId("12D3KooWNgfmn7z3F1TsqfXRCYqdCApucPyv6XnfQNt6MLbdSsAm") message_subject=MessageSubject(0x7467bd478b3f36afa7be045b1ae79aef482410b8046335fb8d8f5f42fbea6ee8, Bitfield(BitVec<u8, bitvec::order::Lsb0> { addr: 0x158790cb0, head: 000, bits: 2, capacity: 8 } [0, 1], PhantomData<u32>), ValidatorIndex(3))
@sandreim sandreim added the I2-bug The node fails to follow expected behavior. label Mar 25, 2024
@sandreim
Copy link
Contributor Author

cc @ordian

@sandreim sandreim changed the title approval-voiting: finality stalls in zombienet test 0006-parachains-max-tranche0 approval-voting: finality stalls in zombienet test 0006-parachains-max-tranche0 Mar 25, 2024
@sandreim sandreim mentioned this issue Mar 25, 2024
Merged
3 tasks
@sandreim
Copy link
Contributor Author

sandreim commented Mar 25, 2024 •
edited
Loading

Additional data point: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/5637724 in #3805 fails for same reason

l Time)
2024-03-25T10:26:56.460Z zombie::network-node returning for: polkadot_parachain_approval_checking_finality_lag from ns: _raw
2024-03-25T10:26:56.461Z zombie::network-node returning: 60
2024-03-25T10:26:56.461Z zombie::network-node using comparator isBelow for 60, 3

@ordian ordian self-assigned this Mar 25, 2024
@ordian ordian moved this from Backlog to In Progress in parachains team board Mar 25, 2024
@ordian ordian mentioned this issue Mar 25, 2024
Merged
2 tasks
@pepoviola
Copy link
Contributor

This seems to affect also

Thx!

github-merge-queue bot pushed a commit that referenced this issue Mar 26, 2024
@ordian @bkchr
fix regression in approval-voting introduced in #3747 (#3831)
3fc5b82 
Fixes #3826.

The docs on the `candidates` field of `BlockEntry` were incorrectly
stating that they are sorted by core index. The (incorrect) optimization
was introduced in #3747 based on this assumption. The actual ordering is
based on `CandidateIncluded` events ordering in the runtime. We revert
this optimization here.

- [x] verify the underlying issue
- [x] add a regression test

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
@github-project-automation github-project-automation bot moved this from In Progress to Completed in parachains team board Mar 26, 2024
dharjeezy pushed a commit to dharjeezy/polkadot-sdk that referenced this issue Apr 9, 2024
@ordian @bkchr @dharjeezy
fix regression in approval-voting introduced in paritytech#3747 (pari…
e999e4c 
…tytech#3831)

Fixes paritytech#3826.

The docs on the `candidates` field of `BlockEntry` were incorrectly
stating that they are sorted by core index. The (incorrect) optimization
was introduced in paritytech#3747 based on this assumption. The actual ordering is
based on `CandidateIncluded` events ordering in the runtime. We revert
this optimization here.

- [x] verify the underlying issue
- [x] add a regression test

---------

Co-authored-by: Bastian Köcher <git@kchr.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ordian ordian

Labels
I2-bug The node fails to follow expected behavior.
Projects
parachains team board
Status: Completed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix regression in approval-voting introduced in #3747 paritytech/polkadot-sdk
3 participants
@pepoviola @ordian @sandreim