From fae4bfa29e86fe06102f517f4b6842eccf4f9ef7 Mon Sep 17 00:00:00 2001 From: Robert Habermeier Date: Fri, 1 Oct 2021 22:24:35 +0200 Subject: [PATCH] Fix an off-by-one: revert rather than revert-to (#3991) * fix off-by-one in disputes reversion code * bump Rococo spec version --- .../implementers-guide/src/runtime/disputes.md | 3 ++- runtime/parachains/src/disputes.rs | 17 +++++++++++------ runtime/rococo/src/lib.rs | 2 +- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/roadmap/implementers-guide/src/runtime/disputes.md b/roadmap/implementers-guide/src/runtime/disputes.md index 9ccf3a3923f8..83ca86b36b04 100644 --- a/roadmap/implementers-guide/src/runtime/disputes.md +++ b/roadmap/implementers-guide/src/runtime/disputes.md @@ -109,4 +109,5 @@ Frozen: Option, * `revert_and_freeze(BlockNumber)`: 1. If `is_frozen()` return. - 1. Set `Frozen` to `Some(BlockNumber)` to indicate a rollback to the given block number is necessary. + 1. Set `Frozen` to `Some(BlockNumber)` to indicate a rollback to the block number. + 1. Issue a `Revert(BlockNumber + 1)` log to indicate a rollback of the block's child in the header chain, which is the same as a rollback to the block number. diff --git a/runtime/parachains/src/disputes.rs b/runtime/parachains/src/disputes.rs index b63305efb27a..1ec5515667fb 100644 --- a/runtime/parachains/src/disputes.rs +++ b/runtime/parachains/src/disputes.rs @@ -304,8 +304,8 @@ pub mod pallet { DisputeTimedOut(CandidateHash), /// A dispute has concluded with supermajority against a candidate. /// Block authors should no longer build on top of this head and should - /// instead revert to the block at the given height which is the last - /// known valid block in this chain. + /// instead revert the block at the given height. This should be the + /// number of the child of the last known valid block in the chain. Revert(T::BlockNumber), } @@ -1127,9 +1127,14 @@ impl Pallet { pub(crate) fn revert_and_freeze(revert_to: T::BlockNumber) { if Self::last_valid_block().map_or(true, |last| last > revert_to) { Frozen::::set(Some(revert_to)); - Self::deposit_event(Event::Revert(revert_to)); + + // The `Revert` log is about reverting a block, not reverting to a block. + // If we want to revert to block X in the current chain, we need to revert + // block X+1. + let revert = revert_to + One::one(); + Self::deposit_event(Event::Revert(revert)); frame_system::Pallet::::deposit_log( - ConsensusLog::Revert(revert_to.saturated_into()).into(), + ConsensusLog::Revert(revert.saturated_into()).into(), ); } } @@ -2284,8 +2289,8 @@ mod tests { Pallet::::revert_and_freeze(0); assert_eq!(Frozen::::get(), Some(0)); - assert_eq!(System::digest().logs[0], ConsensusLog::Revert(0).into()); - System::assert_has_event(Event::Revert(0).into()); + assert_eq!(System::digest().logs[0], ConsensusLog::Revert(1).into()); + System::assert_has_event(Event::Revert(1).into()); }) } diff --git a/runtime/rococo/src/lib.rs b/runtime/rococo/src/lib.rs index 8033a2d580d2..b3f96fbc1c63 100644 --- a/runtime/rococo/src/lib.rs +++ b/runtime/rococo/src/lib.rs @@ -105,7 +105,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("rococo"), impl_name: create_runtime_str!("parity-rococo-v1.8"), authoring_version: 0, - spec_version: 9103, + spec_version: 9104, impl_version: 0, #[cfg(not(feature = "disable-runtime-api"))] apis: RUNTIME_API_VERSIONS,