Skip to content

test(drive-abci): rehearse a reproducible scheduled host fault and the existing recovery path - #4808

Open
DCG-Claude wants to merge 3 commits into
v6.0-devfrom
dashvm/fix-03
Open

DCG-Claude wants to merge 3 commits into
v6.0-devfrom
dashvm/fix-03

Conversation

@DCG-Claude

@DCG-Claude DCG-Claude commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

Part of the smart-contract plan in #4626 (workstream issue #4688, task FIX-03). Once contracts run inside blocks, three very different failures share one word: a guest that traps (a paid result), a node that cannot load a compiled module (that node stops), and a defect in how every node drives the scheduled phase (every block fails). The owner's selected policy for anything that changes execution or fees is the normal protocol upgrade, with no emergency pause, no activation-height override, no recovery binary and no guest-skip rule. Before the jobs phase is built, this PR keeps the three classes apart with tests that drive the real block-execution entry points, reproduces the third class at the scheduled-event integration point, rehearses the recovery path that exists for it, and reports the limitation of that path in a reference chapter.

Refs #4688

What was done?

Test-only fault hook at the scheduled-event integration point (packages/rs-drive-abci/src/config.rs, packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rs). PlatformTestConfig gains scheduled_event_host_fault: bool (default false in both Default and default_minimal_verifications(), next to the base's checkpoint_faults) and the constant SCHEDULED_EVENT_HOST_FAULT_MESSAGE. In run_block_proposal_v0, directly after run_dao_platform_events (and its debug-only phase marker) and before process_raw_state_transitions, a block gated by #[cfg(feature = "testing-config")] returns Err(Error::Execution(ExecutionError::CorruptedCodeExecution(SCHEDULED_EVENT_HOST_FAULT_MESSAGE))) when the flag is set. This follows the four existing testing_configs reads inside shipped v0 modules and the test_fault_injection seam in process_raw_state_transitions. Production builds (console,grovedbg,replay) never enable testing-config, so the compiled run_block_proposal_v0 is byte-identical; no vN generation, no version table, no fee schedule, no consensus error, no proto or SDK surface changes.

Unit tests through the dispatcher (packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs): should_fail_the_whole_proposal_at_the_scheduled_event_point_when_the_host_fault_is_armed pins that an empty proposal returns Err with the injected message (a whole-proposal failure, not a ValidationResult error and not a per-transition result); should_not_fail_the_proposal_at_the_scheduled_event_point_when_unarmed pins that the same proposal is valid with the flag off. The synthetic state carries an epoch 0 with its proposers tree and one validator set so the closing per-block events complete.

Strategy module (packages/rs-drive-abci/tests/strategy_tests/test_cases/scheduled_host_fault_tests.rs, registered in test_cases/mod.rs). All requests go through tenderdash_abci::Application on FullAbciApplication; twin platforms are built from one seed and compared block for block.

  • paid_failure_stays_in_the_block_pays_and_the_chain_advances (class 1): the dashpay all-mutable contract with a skipped-position update at block 3 produces a PaidConsensusError with code 10411 and positive gas_used, and the chain reaches block 10 with verify_state_transition_results.
  • node_local_fault_stops_only_that_node_and_it_commits_the_same_block_after_repair (class 2): twins A and B commit five blocks; B executes the next block; A, armed, fails it with the injected exception and its committed root is unchanged; disarmed, A re-executes the identical request to B's app hash and tx_results; both finalize through finalize_block and hold the same committed root and height.
  • reproducible_scheduled_fault_halts_every_node_before_ordinary_transactions (class 3): both twins armed; every prepare_proposal and process_proposal fails on both nodes for two proposers, rounds 0 to 2, with and without transactions, while signalling latest+1, and for an epoch-change block; committed roots, heights, the versions counter (read with fetch_versions_with_counter) and next_epoch_protocol_version() are unchanged.
  • execution_identical_hotfix_replays_the_faulted_proposal_and_the_chain_continues (class 3 recovery): control node C prepares and processes a concrete proposal R (non-empty, signalling latest+1, first block of epoch 1); A, armed, fails R as proposer and twice as validator with nothing left behind; A is disarmed and reopened from disk with TempPlatform::open_with_tempdir; A processes the unchanged R to C's app hash and tx_results; both finalize R, land in epoch 1 with the versions counter holding exactly {latest+1: 1} and the created identities present on both; continue_chain_for_strategy runs five more blocks on each and the committed roots stay equal.

CI gate (.github/workflows/tests-rs-workspace.yml): the two nextest filters now include test(~scheduled_host_fault) in the PR phase and exclude it from the push-only phase, next to the collision module; the comment block above them is extended. actionlint reports the same pre-existing shellcheck notes as on the base branch and nothing new.

Book chapter (book/src/architecture/block-failure-classes.md, listed in book/src/SUMMARY.md under Architecture): the three classes with their surfaces in rs-drive-abci and Tenderdash 1.7 (internal/consensus/block_executor.go, internal/consensus/prevoter.go, internal/consensus/replay.go, internal/state/execution.go, verified on tag v1.7.0), why a signalling upgrade cannot progress on a halted chain (vote written inside the block, tally on an epoch-change block, activation on a later block), the hotfix-and-replay recovery the tests rehearse, the historical evo1 gates and emergency parameter updates described as evidence of coordinated releases rather than a procedure, the limitation, how to run the rehearsal, and rules for the jobs phase.

How Has This Been Tested?

Locally on macOS (Darwin 24.6.0, arm64), each command redirected to a file with its exit code captured:

cargo fmt --all
cargo clippy -p drive-abci --all-features --all-targets -- --no-deps -D warnings
cargo check --workspace --all-targets
cargo test -p drive-abci --lib run_block_proposal
cargo test -p drive-abci --test strategy_tests scheduled_host_fault
cargo test -p drive-abci --test strategy_tests process_proposal_collision
cargo test -p drive-abci --test strategy_tests run_chain_insert_one_new_identity_and_a_contract_with_bad_update
cargo test -p drive-abci --test strategy_tests run_chain_with_temporarily_disabled_contested_documents
/tmp/actionlint .github/workflows/tests-rs-workspace.yml

Results: 5 run_block_proposal unit tests pass; the 4 scheduled host fault simulations pass in under two seconds; the collision, bad-update and voting simulations pass. The book builds in CI (book-preview.yml); mdbook is not installed locally.

Breaking Changes

None. No consensus, protocol, fee, storage or API change. The only library-visible additions are on the testing-config feature (PlatformTestConfig::scheduled_event_host_fault, SCHEDULED_EVENT_HOST_FAULT_MESSAGE). Every PlatformTestConfig literal outside config.rs on the current base uses a spread, so no other file gains the field.

Decisions taken (provisional values)

  1. The scheduled-event integration point is the position after run_dao_platform_events and before process_raw_state_transitions in run_block_proposal_v0, the processing order the scheduling draft proposes (engineering convention, not an owner decision). When the jobs phase lands, the hook moves inside it.
  2. The paid-guest-failure class is represented by its native analogue, a paid consensus error on a contract update, because no guest runtime exists on this base; the same test extends to a scheduled trap once jobs exist.
  3. The node-local class is represented by one node failing process_proposal with the same exception a storage fault produces; the chain-lock Reject path is documented, not driven (driving it needs a second mock Core RPC with the harness's masternode data).
  4. The class-3 fault surfaces as Err(ExecutionError::CorruptedCodeExecution); a panic in the same phase is documented as equivalent in network effect.
  5. Tenderdash behaviour is cited from tag v1.7.0 sources (dashmate pins dashpay/tenderdash:1.7). Write-ahead-log replay is modelled as re-delivering the identical RequestProcessProposal to the restarted node; the proposer-side re-prepare is not part of replay.
  6. The new strategy module joins the PR nextest gate next to the collision module (it runs in under two seconds); maintainers may move it to the push-only phase.
  7. The chapter reports the execution-changing case as having no demonstrated progressing path under the selected policy; it does not propose one.
  8. Hand-built blocks are proposed by the quorum member the simulation would pick next (walking the current quorum in key order), so the validator set does not rotate under the block and the continuation after recovery follows the harness's own proposer order.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Dash-Tasks: FIX-03

🤖 Generated with Claude Code


🤖 Posted autonomously by DashVM (Claude Fable 5.1) on behalf of pasta.

Automated reviewer consensus (Fable 5.1 implementer, GPT-6 Astra reviewer)

Reviewer consensus

Plan Review consensus

  • FIX-03-R1 [major] Do not treat historical exceptions as an authorized recovery policy -> resolved
    • at /Users/dashvm/work/wt/FIX-03/PLAN.md:245 — §3.3, item 6
  • FIX-03-R2 [major] Replay the failed proposal after reopening the database -> resolved
    • at /Users/dashvm/work/wt/FIX-03/PLAN.md:206 — §3.2, hotfix recovery test
    • round 1 FIX-03-R1: accept: Section 3.3 item 6 and section 2 now describe the evo1 height gates as replay preservation of an already committed history and the emergency consensus params and lowered threshold as rules evaluated inside block processing that still needed blocks; none is presented as a procedure. The chapter repor
    • round 1 FIX-03-R2: accept: Section 3.2 replaces the continuation-only recovery test with a replay rehearsal: a concrete non-empty epoch-boundary proposal is prepared on the control, the process request is stored unchanged, the faulted node fails it twice, is disarmed and reopened from disk, replays the identical request, and

Review consensus

  • FIX03-01 [nit] Use an allowed scope for the documentation commit -> noted
    • at .github/workflows/pr.yml:34; commit 3c7a94e

Summary by CodeRabbit

  • Documentation
    • Added guidance on block-execution failure types, how they affect chain progress, and recovery options.
  • Tests
    • Expanded coverage for transaction failures, node-specific execution faults, and failures that prevent block production across nodes.
    • Added checks that recovery allows nodes to resume processing blocks in sync.

@coderabbitai

coderabbitai Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: dashpay/platform/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 1dce7b06-7ab1-4c4b-858f-14a88fffc1f6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: dashpay/platform/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 95a5a091-5b65-4a94-aa2b-90f933cffc32

📥 Commits

Reviewing files that changed from the base of the PR and between 4f59f36 and b123d9b.

📒 Files selected for processing (8)
  • .github/workflows/tests-rs-workspace.yml
  • book/src/SUMMARY.md
  • book/src/architecture/block-failure-classes.md
  • packages/rs-drive-abci/src/config.rs
  • packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs
  • packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rs
  • packages/rs-drive-abci/tests/strategy_tests/test_cases/mod.rs
  • packages/rs-drive-abci/tests/strategy_tests/test_cases/scheduled_host_fault_tests.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The change adds test-only scheduled-event fault injection and strategy tests for three block-failure classes and replay after repair. A new architecture chapter documents those classes and their recovery constraints. The Rust workspace workflow selects the new tests in its fast phase.

Changes

Block failure handling

Layer / File(s) Summary
Scheduled-event fault injection
packages/rs-drive-abci/src/config.rs, packages/rs-drive-abci/src/execution/engine/run_block_proposal/...
A test-only configuration flag injects a CorruptedCodeExecution error after DAO events and before state transitions. Unit tests verify the armed and unarmed cases.
Failure scenarios and replay
packages/rs-drive-abci/tests/strategy_tests/test_cases/...
Strategy tests cover a deterministic paid failure, a node-local failure and repair, reproducible scheduled faults, and replay of an unchanged proposal after reopening a node without the fault.
Failure-class guidance and test selection
book/src/architecture/block-failure-classes.md, book/src/SUMMARY.md, .github/workflows/tests-rs-workspace.yml
The new chapter describes failure classes, protocol-upgrade constraints, and recovery boundaries. The workflow selects the scheduled-host-fault tests in the fast phase and excludes them from full chain simulation.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to b123d

This change adds a test-only fault injection hook, strategy tests, and documentation. Production behavior is not affected because the hook is compiled out of production builds. I found no merge-blocking risk.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b123d

The change does not establish a new production fault trigger or a verified security finding. It does make an important recovery limitation explicit: a fault that stops every block cannot be repaired through the normal block-dependent upgrade process while the chain is halted.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — If deliberately armed in a test-feature build, the flag can fail proposals on one node or, when armed across nodes, model a network-wide halt. The reviewed production feature boundary excludes this new trigger.

Trust Boundaries and Controls

  • observed — The failure branch reads a test flag, not transaction contents. The testing configuration is feature-gated and skipped during configuration deserialization, limiting external input’s ability to arm the hook.

Resilience and Maintainability Implications

  • observed — The tests support durable-state preservation and recovery by reopen-and-replay, but not same-process epoch-change retry. The chapter states the limit of the existing block-dependent upgrade path rather than presenting it as a recovery procedure.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 5 files. (3 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: testing a reproducible scheduled host fault and its recovery path. It is concise, specific, and directly related to the pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 78.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 5 files. (3 skipped: 3 unsupported.)

✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-09-29T08:26:44.794Z

@github-actions github-actions Bot added this to the v5.0.0 milestone Sep 17, 2026
@thepastaclaw

thepastaclaw commented Sep 17, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit b123d9b) · triage: normal · Phase 2 only (queue backlog)

@codecov

codecov Bot commented Sep 17, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 46.45161% with 83 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.40%. Comparing base (5f1e0cc) to head (3c7a94e).
⚠️ Report is 14 commits behind head on v5.0-dev.

Files with missing lines Patch % Lines
...bci/src/execution/engine/run_block_proposal/mod.rs 50.38% 65 Missing ⚠️
packages/rs-drive-abci/src/config.rs 9.09% 10 Missing ⚠️
.../src/execution/engine/run_block_proposal/v0/mod.rs 38.46% 8 Missing ⚠️

❌ Your patch check has failed because the patch coverage (46.45%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@              Coverage Diff              @@
##           v5.0-dev    #4808       +/-   ##
=============================================
- Coverage     86.36%   75.40%   -10.97%     
=============================================
  Files          2766     2799       +33     
  Lines        366105   411517    +45412     
=============================================
- Hits         316191   310296     -5895     
- Misses        49914   101221    +51307     
Components Coverage Δ
dpp 75.13% <ø> (-12.16%) ⬇️
drive 76.86% <ø> (-7.39%) ⬇️
drive-abci 73.60% <46.45%> (-16.06%) ⬇️
sdk ∅ <ø> (∅)
dapi-client ∅ <ø> (∅)
platform-version ∅ <ø> (∅)
platform-value 86.67% <ø> (-6.25%) ⬇️
platform-wallet ∅ <ø> (∅)
drive-proof-verifier 34.71% <ø> (-15.07%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

The codecov patch and project numbers on this head are a reporting artifact, not missing coverage, so I am not rerunning the job or pushing for it.

What the numbers rest on: the Rust workspace job for 3c7a94e ran on the self-hosted mac-runner-pasta and passed all 15515 tests, including the two new run_block_proposal unit tests (which execute the hook lines codecov lists as missed in run_block_proposal/v0/mod.rs) and the four scheduled_host_fault simulations. The file reports codecov built from that run map profiles onto line tables that do not belong to this branch: run_block_proposal/v0/mod.rs is instrumented up to line 551 while the file is 485 lines here, and config.rs up to line 1273 while the file is 1249 lines. Inside the patch that shows up as a struct literal with one field "hit" and its neighbours "missed", which cannot happen in a single test run. The same shifted mapping is what drives the project-wide drop on files this PR does not touch (for example rs-dpp/src/block/finalized_epoch_info/mod.rs reports 78 misses inside functions the job log shows passing).

This is the known self-hosted-runner line-table mismatch seen on other v5.0-dev PRs; a rerun adds a second upload with the same stale mapping. Neither codecov check is in the v5.0-dev rulesets.


🤖 Posted autonomously by DashVM (Claude Fable 5.1) on behalf of pasta.

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ DEGRADED — Final validation — Phase 1 + Phase 2

⚠️ DEGRADED review. The primary review models were unavailable (gpt-6-astra unavailable: All credentials for model gpt-6-astra are cooling down (last error: usage_limit_reached: The usage limit has been reache), so this review ran on stand-in models: gpt-5.6-luna → muse-spark-1.3-contributor, gpt-5.6-sol → muse-spark-1.3-contributor, gpt-5.6-terra → muse-spark-1.3-contributor, gpt-6-astra → muse-spark-1.3-contributor. Both review phases and the independent verifiers still ran, but on weaker models, with Phase 1 capped at high effort. Treat the verdict as provisional; a full-strength re-review will run on the next push once the primary models are back.

Test-only rehearsal of the three block-failure classes with no production impact. I independently verified the safety claims: the fault hook in run_block_proposal_v0 and the new PlatformTestConfig field plus message constant are all #[cfg(feature = "testing-config")] gated, default to false in both constructors, and testing_configs is #[serde(skip)] so the flag is unreachable from config files. No version-table, consensus-error, fee, serialization, or wire-format changes. No in-scope findings from any lane.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: architecture-layering); reviewer 3: muse-spark-1.3-contributor (agent: phase1-reviewer, role: platform-versioning); reviewer 4: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 5: muse-spark-1.3-contributor (agent: phase1-reviewer, role: security-auditor); reviewer 6: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: general); reviewer 7: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: architecture-layering); reviewer 8: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: platform-versioning); reviewer 9: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: rust-quality); reviewer 10: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: phase2-reviewer, role: security-auditor); final verifier: muse-spark-1.3-contributor (standing in for gpt-6-astra) (agent: astra-verifier, role: final-verifier)

  • Degraded mode: gpt-6-astra unavailable: All credentials for model gpt-6-astra are cooling down (last error: usage_limit_reached: The usage limit has been reache (detected by probe, since 2026-09-18T05:22:01Z); stand-ins gpt-5.6-luna → muse-spark-1.3-contributor, gpt-5.6-sol → muse-spark-1.3-contributor, gpt-5.6-terra → muse-spark-1.3-contributor, gpt-6-astra → muse-spark-1.3-contributor; Phase 1 effort capped at high
  • Triage: normal by muse-spark-1.3-contributor (standing in for gpt-6-astra) (effort low) — Large test-only fault hook gated behind testing-config plus docs and strategy tests with no production consensus, funds, crypto, or migration change.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort high); agent phase1-reviewer, muse-spark-1.3-contributor — architecture-layering (completed, effort high); agent phase1-reviewer, muse-spark-1.3-contributor — platform-versioning (completed, effort high); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort high); agent phase1-reviewer, muse-spark-1.3-contributor — security-auditor (completed, effort high); agent phase1-reviewer
  • Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (lane failed), glm-5.3-flash (zai below 15% reserve: 5h 99% left, weekly 14% left)
  • Fresh verifier: muse-spark-1.3-contributor (standing in for gpt-6-astra) — final-verifier; agent astra-verifier
  • Phase 2 reviewers: muse-spark-1.3-contributor (standing in for gpt-6-astra) — general (completed, effort high); agent phase2-reviewer, muse-spark-1.3-contributor (standing in for gpt-6-astra) — architecture-layering (completed, effort high); agent phase2-reviewer, muse-spark-1.3-contributor (standing in for gpt-6-astra) — platform-versioning (completed, effort high); agent phase2-reviewer, muse-spark-1.3-contributor (standing in for gpt-6-astra) — rust-quality (completed, effort high); agent phase2-reviewer, muse-spark-1.3-contributor (standing in for gpt-6-astra) — security-auditor (completed, effort high); agent phase2-reviewer
Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

  • Real scheduled-jobs phase will need full versioning — This PR deliberately rehearses with a test hook before any guest runtime exists; when the jobs phase lands (hook moves inside it, paid trap results, fee schedule, possible new transitions/errors), that work will need a new vN implementation, dispatcher arm, PlatformVersion slots, activation gates, and estimation/execution/proof coverage.
    • Follow-up: Track the versioned rollout of the jobs phase in the smart-contract workstream (issue #4688) when execution or fees change.

@github-actions github-actions Bot added bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. waiting-self-review Waiting for the author to post /self-reviewed labels Sep 20, 2026
@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

/self-reviewed 3c7a94e

@github-actions github-actions Bot added waiting-slot too-many-open-prs Beyond the author's 5 open PRs; waits for one to merge before a human is asked. and removed waiting-self-review Waiting for the author to post /self-reviewed waiting-slot too-many-open-prs Beyond the author's 5 open PRs; waits for one to merge before a human is asked. labels Sep 20, 2026
DCG-Claude and others added 3 commits September 29, 2026 03:15
…e existing recovery path

Adds a test-only hook at the scheduled-event integration point of
run_block_proposal_v0 (after the DAO events, before the ordinary state
transitions) behind the testing-config feature, and a strategy module
that keeps three block failure classes apart: a paid consensus failure
that stays in the block and is charged, a node-local failure that stops
one node until it re-executes the same block, and a reproducible host
fault that fails every proposal on every node before any transaction is
looked at. The last test rehearses the recovery path that exists for
the third class: an execution and fee identical hotfix, a restart from
disk, and the replay of the very proposal that faulted, compared block
for block against a never-faulted twin.

Production builds are byte-identical: the hook does not exist without
the feature.

Refs #4688

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…_block_proposal

Armed, the hook fails the whole proposal with the injected internal
error for an empty proposal, which is the class every node reproduces
and no transaction removal can route around. Unarmed, the same empty
proposal runs through to a valid outcome. The synthetic state gets an
epoch 0 with its proposers tree and one validator set so the closing
per-block events can complete.

Refs #4688

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… recovery path

Adds a reference chapter that keeps a deterministic paid failure, a
node-local failure and a reproducible per-block fault apart, cites where
each surfaces in rs-drive-abci and in Tenderdash 1.7, explains why a
signalling upgrade cannot progress on a halted chain, states the hotfix
and replay recovery the rehearsal tests exercise, and reports that a
repair which changes execution or fees has no demonstrated progressing
path under the selected policy. The historical network gated exceptions
are described as evidence of coordinated releases, not as a procedure.

The scheduled host fault strategy module joins the PR nextest gate next
to the process proposal collision module.

Refs #4688

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

Rebased onto the current v5.0-dev (head is now b123d9b, previously 3c7a94e). All three commits are preserved; the range-diff shows only context drift from upstream.

Conflicts resolved from the blobs, none needed a design change:

  • packages/rs-drive-abci/src/config.rs: upstream added the checkpoint_faults fault-injection field next to where this PR adds scheduled_event_host_fault. Both fields are kept in the struct, in default_minimal_verifications() and in Default.
  • packages/rs-drive-abci/src/execution/engine/run_block_proposal/v0/mod.rs: upstream added a debug-only phase marker after run_dao_platform_events. The hook now sits directly after that marker and still before process_raw_state_transitions, so the integration point is unchanged.
  • packages/rs-drive-abci/tests/strategy_tests/test_cases/voting_tests.rs: upstream switched the test config literal to ..Default::default(), which already covers the new field, so this PR no longer touches that file.

The new strategy module and the book chapter are byte-identical to the pre-rebase tip. Rebuilt with fmt, clippy on drive-abci with all targets and warnings as errors, the workspace check with all targets, the two run_block_proposal unit tests and the four scheduled_host_fault strategy tests, all green.


🤖 Posted autonomously by DashVM (Claude Fable 5.1) on behalf of pasta.

@github-actions github-actions Bot added waiting-bots Waiting for the review bots to report on this head and removed bot-review-skipped A required review bot did not report; it was skipped by the window or by a person. labels Sep 29, 2026

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review — Final validation — Phase 2 only (queue backlog)

Independently reviewed the complete diff at b123d9b and found no actionable in-scope defects. The fault hook is feature-gated and disabled by default, leaving production consensus behavior unchanged; the recovery tests model request redelivery after reopening the database rather than exercising Tenderdash’s WAL implementation. Local validation passed all five run_block_proposal unit tests, four scheduled-host-fault strategy tests, five proposal-collision regression tests, and git diff --check; the worktree remains unchanged.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 2: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 3: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 4: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6-astra (agent: phase2-reviewer, role: general); reviewer 6: gpt-6-astra (agent: phase2-reviewer, role: architecture-layering); reviewer 7: gpt-6-astra (agent: phase2-reviewer, role: platform-versioning); reviewer 8: gpt-6-astra (agent: phase2-reviewer, role: rust-quality); final verifier: gpt-6-astra (agent: astra-verifier, role: final-verifier)

  • Triage: normal by gpt-6-astra (effort low) — The substantial test suite, documentation, CI updates, and testing-config-gated fault hook in run_block_proposal_v0 warrant ordinary review but do not change production consensus rules or another critical surface.
  • Phase 1 reviewers: not run (skipped for throughput: 12 PRs queued, above the 10 limit)
  • Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
  • Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
  • Phase 2 reviewers: gpt-6-astra — general (completed, effort high); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6-astra — general (completed, effort high); agent phase2-reviewer, gpt-6-astra — architecture-layering (completed, effort high); agent phase2-reviewer, gpt-6-astra — platform-versioning (completed, effort high); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort high); agent phase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

/self-reviewed b123d9b

@DCG-Claude

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review


🤖 Posted autonomously by DashVM (Claude Fable 5.1) on behalf of pasta.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@github-actions github-actions Bot added ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. and removed waiting-bots Waiting for the review bots to report on this head labels Sep 29, 2026
@github-actions
github-actions Bot requested a review from ktechmidas September 29, 2026 13:20
@github-actions
github-actions Bot requested a review from shumkov September 29, 2026 13:20
@DCG-Claude
DCG-Claude changed the base branch from v6.0-dev to v5.0-dev September 30, 2026 15:32
@github-actions github-actions Bot removed the ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. label Sep 30, 2026
@github-actions github-actions Bot modified the milestones: v6.0.0, v5.0.0 Sep 30, 2026
@github-actions github-actions Bot added the ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. label Sep 30, 2026
@DCG-Claude
DCG-Claude changed the base branch from v5.0-dev to v6.0-dev September 30, 2026 15:42
@github-actions github-actions Bot modified the milestones: v5.0.0, v6.0.0 Sep 30, 2026
@github-actions github-actions Bot removed the ready-for-human Bots have reported, the author has self-reviewed, and the build is green: this needs a human. label Sep 30, 2026

This branch has not been deployed

No deployments
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.

2 participants