Skip to content

fix(drive-abci): don't panic on unsupported version error #2679

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

Open
lklimek wants to merge 7 commits into
base: v4.0-dev
Choose a base branch
from
Open

fix(drive-abci): don't panic on unsupported version error #2679

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0078462
chore(abci): don't panic on unsupported version error
lklimek Jun 24, 2025
5e230ff
Merge branch 'v2.0-dev' into fix-no-panic-version-mismatch
lklimek Jul 8, 2025
54db7bb
Merge branch 'v2.1-dev' into fix-no-panic-version-mismatch
lklimek Jul 8, 2025
b6df2ed
Merge branch 'v2.2-dev' into fix-no-panic-version-mismatch
lklimek Nov 12, 2025
5495788
Merge branch 'v3.0-dev' into fix-no-panic-version-mismatch
lklimek Dec 19, 2025
2048e50
Merge branch 'v3.0-dev' into fix-no-panic-version-mismatch
lklimek Feb 10, 2026
43e65ce
Merge branch 'v3.1-dev' into fix-no-panic-version-mismatch
lklimek Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions packages/rs-drive-abci/src/error/execution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ pub enum ExecutionError {
known_versions: Vec<FeatureVersion>,
},

/// We don't have the required protocol version. Most liekly the platform must be upgraded.

@thepastaclaw thepastaclaw Jun 11, 2026

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.

💬 Nitpick: Typo 'liekly' in ProtocolVersionNotSupported doc comment

Doc comment for the new ProtocolVersionNotSupported variant contains liekly (should be likely). This renders on the public rustdoc surface of ExecutionError, so worth fixing while the variant is new.

Suggested change
/// We don't have the required protocol version. Most liekly the platform must be upgraded.
/// We don't have the required protocol version. Most likely the platform must be upgraded.

source: ['claude', 'codex']

#[error("platform protocol version {required} is not supported, max supported is {max_supported}; please upgrade")]
ProtocolVersionNotSupported {
/// The maximum supported protocol version
max_supported: u32,
/// Protocol version requested by the network
required: u32,
},

/// The platform encountered a corrupted cache state error.
#[error("platform corrupted cached state error: {0}")]
CorruptedCachedState(String),
Expand Down
12 changes: 10 additions & 2 deletions packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,23 @@ where

// We should panic if this node is not supported a new protocol version

@thepastaclaw thepastaclaw Jun 11, 2026

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.

🟡 Suggestion: Stale comment still says 'We should panic'

The comment above the PlatformVersion::get(next_protocol_version) block still reads // We should panic if this node is not supported a new protocol version, but this PR's whole purpose is to stop panicking and instead return a ProtocolVersionNotSupported validation error so Tenderdash can reject the proposal and try another proposer. Leaving the old comment in place is actively misleading for future readers and for anyone grepping for panic.

Suggested change
// We should panic if this node is not supported a new protocol version
// If this node does not support the upgraded protocol version, do not panic:
// return a validation error so process_proposal rejects the block and Tenderdash
// can fall back to another proposer while the operator upgrades.
let Ok(next_platform_version) = PlatformVersion::get(next_protocol_version) else {

source: ['claude', 'codex']

let Ok(next_platform_version) = PlatformVersion::get(next_protocol_version) else {
panic!(
let max_supported = PlatformVersion::latest().protocol_version;
tracing::error!(
r#"Failed to upgrade the network protocol version {next_protocol_version}.

Please update your software to the latest version: https://docs.dash.org/platform-protocol-upgrade

Your software version: {}, latest supported protocol version: {}."#,
env!("CARGO_PKG_VERSION"),
PlatformVersion::latest().protocol_version
max_supported
);

return Ok(ValidationResult::new_with_error(Error::Execution(
ExecutionError::ProtocolVersionNotSupported {
max_supported,
required: next_protocol_version,
},
)));
};
Comment on lines 85 to 103

@thepastaclaw thepastaclaw Jun 11, 2026

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.

🟡 Suggestion: Add regression test for unsupported protocol version rejection

The PR description states 'Not tested', but this branch is the entire behavioral guarantee being introduced and is consensus-critical: at an epoch change, an unsupported next_epoch_protocol_version must yield Ok(ValidationResult::new_with_error(ExecutionError::ProtocolVersionNotSupported { .. })) rather than panic or return Err. The Rust Result vs validation-error distinction matters here — process_proposal only takes the reject path through the validation-result variant; a future refactor that flips this to Err would still compile but reintroduce a DoS-style failure mode (or worse, a panic). A focused test that drives run_block_proposal past is_epoch_change_but_not_genesis with next_epoch_protocol_version set above PlatformVersion::latest().protocol_version and asserts both the validation error variant and (ideally) a reject response from process_proposal would lock the invariant in place.

source: ['claude', 'codex']


let old_protocol_version = block_platform_state.current_protocol_version_in_consensus();
Expand Down
Loading