Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Fix an off-by-one: revert rather than revert-to (#3991)
Browse files Browse the repository at this point in the history
* fix off-by-one in disputes reversion code

* bump Rococo spec version
  • Loading branch information
rphmeier committed Oct 1, 2021
1 parent 14de1c1 commit fae4bfa
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
3 changes: 2 additions & 1 deletion roadmap/implementers-guide/src/runtime/disputes.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,5 @@ Frozen: Option<BlockNumber>,

* `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.
17 changes: 11 additions & 6 deletions runtime/parachains/src/disputes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}

Expand Down Expand Up @@ -1127,9 +1127,14 @@ impl<T: Config> Pallet<T> {
pub(crate) fn revert_and_freeze(revert_to: T::BlockNumber) {
if Self::last_valid_block().map_or(true, |last| last > revert_to) {
Frozen::<T>::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::<T>::deposit_log(
ConsensusLog::Revert(revert_to.saturated_into()).into(),
ConsensusLog::Revert(revert.saturated_into()).into(),
);
}
}
Expand Down Expand Up @@ -2284,8 +2289,8 @@ mod tests {
Pallet::<Test>::revert_and_freeze(0);

assert_eq!(Frozen::<Test>::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());
})
}

Expand Down
2 changes: 1 addition & 1 deletion runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit fae4bfa

Please sign in to comment.