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

Fix an off-by-one: revert rather than revert-to #3991

Merged
3 commits merged into from
Oct 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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