[Consensus] test to confirm correct threshold clock advancement after GC

[akichidis]

Description

This is a fix for a behaviour that has been observed during the private-testnet test where some nodes started crashing when setting gc_depth = 5. The goal was to test GC with very low gc values to reveal possible issues, such as this one.

This PR is fixing the block acceptance path after commits happening. As we try to unsuspend blocks after commits have happened, which naturally move the gc_round , we should make sure that those blocks are been processed via Core as well and move the threshold clock. Otherwise it's possible - and this mostly becomes visible during bulk processing of blocks - for the following scenario to happen:

This PR is now including the test only that confirm the issue as described below when we bulk processing blocks under GC conditions. The #20906 has moved the threshold clock in DagState eliminating the issue:

• a bunch of blocks are received for processing
• threshold clocks advances to round R according to the so far accepted blocks via the block manager. Some blocks though are not accepted yet as they do have some early dependencies higher than the current gc_round, so they get suspended.
• we attempt to trigger a commit. A commit happens where multiple leaders get committed. This moves the gc_round. Blocks are unsuspended because of the new gc_round. More commits are now happening as we can process the suspended blocks.
• Once the commits finished, gc_round has advanced so much that our threshold clock is well behind. Now our node tries to create a new block but the threshold clock is at round R although gc_round has advanced to R + X. This will make our own block not getting accepted as we produce a block for a quite old round. We see panics from

  sui/consensus/core/src/core.rs

  Line 515 in 5b9269e

  assert_eq!(accepted_blocks.len(), 1);

Test plan

CI/PT

---

Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

• Protocol:
• Nodes (Validators and Full nodes):
• Indexer:
• JSON-RPC:
• GraphQL:
• CLI:
• Rust SDK:
• REST API:

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sui-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 23, 2025 0:12am

3 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
multisig-toolkit	⬜️ Ignored (Inspect)	Visit Preview		Jan 23, 2025 0:12am
sui-kiosk	⬜️ Ignored (Inspect)	Visit Preview		Jan 23, 2025 0:12am
sui-typescript-docs	⬜️ Ignored (Inspect)	Visit Preview		Jan 23, 2025 0:12am

[akichidis]

The alternative I thought was try_commit calling directly the add_accepted_blocks it self for stronger guarantees if we refactor the code etc. Chose to return the accepted blocks and handle independently as I didn't want the try_commit do too much on its own

[mwtian]

I think this issue seems to be a consequence of the threshold clock implementation, which is essentially a separate data structure that needs to be synced with DagState. Keeping track of the flow of accepted blocks seem a bit messy. What about revisiting a previous discussion / PR, where during proposing, the quorum round is read from DagState?

[akichidis]

I think this issue seems to be a consequence of the threshold clock implementation, which is essentially a separate data structure that needs to be synced with DagState. Keeping track of the flow of accepted blocks seem a bit messy. What about revisiting a previous discussion / PR, where during proposing, the quorum round is read from DagState?

@mwtian @arun-koshy could you please re-review? I will try to refactor this on separate PR and find a better fit/solution probably using DagState, but I would like to merge this so I can at least enable GC for simtests/devnet and start getting feedback earlier and then refine further. Let me know if that's ok.