Skip to content

[codex] split network split playbooks#185

Merged
parithosh merged 1 commit into
masterfrom
codex/split-network-playbooks
May 21, 2026
Merged

[codex] split network split playbooks#185
parithosh merged 1 commit into
masterfrom
codex/split-network-playbooks

Conversation

@parithosh
Copy link
Copy Markdown
Member

Summary

  • Rename the slot-level network split playbook to two-way-network-split-reorg-trigger
  • Add an epoch-based two-way-network-split-non-finality playbook that preserves the unfinalized-epoch assertion with a 2-epoch floor
  • Regenerate the playbook index so both variants are discoverable

Validation

  • make generate-playbook-index
  • go test ./...
  • jq sanity check for the non-finality epoch floor expression

Notes

  • The reorg-trigger variant uses splitDurationSlots and only requires fresh finality after healing.
  • The non-finality variant uses splitDurationEpochs and enforces non-finality before healing.

@qu0b-reviewer
Copy link
Copy Markdown

qu0b-reviewer Bot commented May 21, 2026

🤖 qu0b-reviewer

Summary

This PR splits the two-way-network-split.yaml playbook into two distinct tests: two-way-network-split-non-finality.yaml (epoch-based, asserts finality stops) and two-way-network-split-reorg-trigger.yaml (slot-based, focuses on reorg triggers). Both get registered in _index.yaml. The changes are generally coherent; the substantive fix for minUnfinalizedEpochs with values < 2 is sound. One minor concern remains.

Issues

  • 🟡 playbooks/_index.yaml:2,4 — The generated timestamp comment and YAML frontmatter both changed (2026-05-19T21:36:30Z2026-05-21T07:03:46Z). These are auto-generated by scripts/generate-playbook-index.sh and will produce spurious diffs every time the index is regenerated. Consider either using a stable/constant value (e.g. generated: stable) or instructing the generator to skip writing this line if its only purpose is documentation.

  • 🟡 playbooks/dev/two-way-network-split-reorg-trigger.yaml — Extraction of get_consensus_specs removes SLOTS_PER_EPOCH as a dependency, which is correct for a slot-duration-based playbook. However, if this playbook is ever repurposed to use epoch-based duration in the future (e.g., by adding a splitDurationEpochs configVar), the get_consensus_specs task won't still be there and the jq expression will silently produce no result. This is an internal consistency trap worth documenting in the config section comment.

Suggestions

  • playbooks/_index.yaml:266 — The renamed playbook entry (id: two-way-network-split-reorg-trigger) is adjacent in the diff to the new non-finality entry, but in the actual YAML file they remain ordered non-finality then reorg-trigger. Confirm this ordering is intentional; if alphabetical by tag group is preferred, verify the _index ordering aligns with the playbook file ordering.

Reviewed @ b8ef5961
"If you can't measure it, you can't improve it." — Peter Drucker

@parithosh parithosh marked this pull request as ready for review May 21, 2026 08:54
@parithosh parithosh merged commit 0ad56fb into master May 21, 2026
6 checks passed
@parithosh parithosh deleted the codex/split-network-playbooks branch May 21, 2026 09:04
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.

1 participant