Fix GRANDPA set ID handling

node/src/service.rs

@@ -325,12 +325,19 @@ where
         let warp_sync_config =
             grandpa_warp_sync::config(genesis_hash, config.chain_spec.chain_type());
         log::warn!("{}", warp_sync_config.log_message());
-        let warp_sync: Arc<dyn WarpSyncProvider<Block>> =
-            Arc::new(sc_consensus_grandpa::warp_proof::NetworkProvider::new(
-                backend.clone(),
-                grandpa_link.shared_authority_set().clone(),
-                warp_sync_config.into_hard_forks(),
-            ));
+        let initial_set_id = warp_sync_config.one_time_initial_set_id();
+        let inner = sc_consensus_grandpa::warp_proof::NetworkProvider::new(
+            backend.clone(),
+            grandpa_link.shared_authority_set().clone(),
+            warp_sync_config.into_hard_forks(),
+        );
+        let warp_sync: Arc<dyn WarpSyncProvider<Block>> = match initial_set_id {
+            Some(initial_set_id) => Arc::new(grandpa_warp_sync::InitialSetIdProvider::new(
+                inner,
+                initial_set_id,
+            )),
+            None => Arc::new(inner),
+        };
 
         Some(WarpSyncConfig::WithProvider(warp_sync))
     };

node/src/service/grandpa_warp_sync.rs

@@ -1,21 +1,28 @@
 use node_subtensor_runtime::opaque::Block;
 use sc_chain_spec::ChainType;
 use sc_consensus_grandpa::{AuthoritySetHardFork, warp_proof::HardForks};
-use sp_consensus_grandpa::{AuthorityId, AuthorityList};
+use sc_network_sync::strategy::warp::{EncodedProof, VerificationResult, WarpSyncProvider};
+use sp_consensus_grandpa::{AuthorityId, AuthorityList, SetId};
 use sp_core::{ByteArray, H256};
 
+const FINNEY_GENESIS: H256 = H256(hex_literal::hex!(
+    "2f0555cc76fc2840a25a6ea3b9637146806f1f44b090c175ffde2a7e5ab36c03"
+));
 const TESTNET_GENESIS: H256 = H256(hex_literal::hex!(
     "8f9cf856bf558a14440e75569c9e58594757048d7b3a84b5d25f6bd978263105"
 ));
 
 pub(super) enum Config {
     TestnetCheckpoints(Vec<AuthoritySetHardFork<Block>>),
+    OneTimeInitialSetId(SetId),
     InitialSetId(u64),
 }
 
 pub(super) fn config(genesis_hash: H256, chain_type: ChainType) -> Config {
     if genesis_hash == TESTNET_GENESIS {
         Config::TestnetCheckpoints(testnet_checkpoints())
+    } else if genesis_hash == FINNEY_GENESIS {
+        Config::OneTimeInitialSetId(3)
     } else {
         let set_id = match chain_type {
             ChainType::Live => 3,
@@ -30,22 +37,83 @@ impl Config {
     pub(super) fn log_message(&self) -> String {
         match self {
             Self::TestnetCheckpoints(_) => "Testnet GRANDPA warp sync checkpoints enabled.".into(),
+            Self::OneTimeInitialSetId(set_id) => {
+                format!(
+                    "Finney GRANDPA warp sync one-time initial set ID patch enabled. Set ID = \
+                     {set_id}"
+                )
+            }
             Self::InitialSetId(set_id) => {
                 format!("GRANDPA warp sync initial set ID patch enabled. Set ID = {set_id}")
             }
         }
     }
 
+    pub(super) fn one_time_initial_set_id(&self) -> Option<SetId> {
+        match self {
+            Self::OneTimeInitialSetId(set_id) => Some(*set_id),
+            Self::TestnetCheckpoints(_) | Self::InitialSetId(_) => None,
+        }
+    }
+
     pub(super) fn into_hard_forks(self) -> HardForks<Block> {
         match self {
             Self::TestnetCheckpoints(checkpoints) => {
                 HardForks::new_hard_forked_authorities(checkpoints)
             }
+            Self::OneTimeInitialSetId(_) => HardForks::new_hard_forked_authorities(Vec::new()),
             Self::InitialSetId(set_id) => HardForks::new_initial_set_id(set_id),
         }
     }
 }
 
+pub(super) struct InitialSetIdProvider<P> {
+    inner: P,
+    initial_set_id: SetId,
+}
+
+impl<P> InitialSetIdProvider<P> {
+    pub(super) fn new(inner: P, initial_set_id: SetId) -> Self {
+        Self {
+            inner,
+            initial_set_id,
+        }
+    }
+}
+
+impl<P> WarpSyncProvider<Block> for InitialSetIdProvider<P>
+where
+    P: WarpSyncProvider<Block>,
+{
+    fn generate(
+        &self,
+        start: H256,
+    ) -> Result<EncodedProof, Box<dyn std::error::Error + Send + Sync>> {
+        self.inner.generate(start)
+    }
+
+    fn verify(
+        &self,
+        proof: &EncodedProof,
+        set_id: SetId,
+        authorities: AuthorityList,
+    ) -> Result<VerificationResult<Block>, Box<dyn std::error::Error + Send + Sync>> {
+        // Warp sync starts from set 0. Apply Finney's historical correction there, then trust the
+        // set ID returned by each proof. The SDK's `new_initial_set_id` applies its correction to
+        // every fragment, which over-counts as soon as a proof spans new rotations.
+        let set_id = if set_id == 0 {
+            self.initial_set_id
+        } else {
+            set_id
+        };
+        self.inner.verify(proof, set_id, authorities)
+    }
+
+    fn current_authorities(&self) -> AuthorityList {
+        self.inner.current_authorities()
+    }
+}
+
 #[allow(clippy::expect_used)]
 fn testnet_authorities() -> AuthorityList {
     [
@@ -94,13 +162,50 @@ fn testnet_checkpoints() -> Vec<AuthoritySetHardFork<Block>> {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::sync::atomic::{AtomicU64, Ordering};
+
+    #[derive(Default)]
+    struct RecordingProvider {
+        set_id: AtomicU64,
+    }
+
+    impl WarpSyncProvider<Block> for RecordingProvider {
+        fn generate(
+            &self,
+            _start: H256,
+        ) -> Result<EncodedProof, Box<dyn std::error::Error + Send + Sync>> {
+            Ok(EncodedProof(Vec::new()))
+        }
+
+        fn verify(
+            &self,
+            _proof: &EncodedProof,
+            set_id: SetId,
+            authorities: AuthorityList,
+        ) -> Result<VerificationResult<Block>, Box<dyn std::error::Error + Send + Sync>> {
+            self.set_id.store(set_id, Ordering::Relaxed);
+            Ok(VerificationResult::Partial(
+                set_id.saturating_add(1),
+                authorities,
+                H256::zero(),
+            ))
+        }
+
+        fn current_authorities(&self) -> AuthorityList {
+            Vec::new()
+        }
+    }
 
     #[test]
     fn checkpoints_are_exactly_testnet_genesis_scoped() {
         assert!(matches!(
             config(TESTNET_GENESIS, ChainType::Live),
             Config::TestnetCheckpoints(_)
         ));
+        assert!(matches!(
+            config(FINNEY_GENESIS, ChainType::Live),
+            Config::OneTimeInitialSetId(3)
+        ));
         assert!(matches!(
             config(H256::zero(), ChainType::Live),
             Config::InitialSetId(3)
@@ -157,4 +262,22 @@ mod tests {
             .collect::<Vec<_>>();
         assert_eq!(authority_ids, expected_authority_ids);
     }
+
+    #[test]
+    fn finney_initial_set_id_is_applied_only_at_warp_sync_start() {
+        let provider = InitialSetIdProvider::new(RecordingProvider::default(), 3);
+        let proof = EncodedProof(Vec::new());
+
+        let Ok(first) = provider.verify(&proof, 0, Vec::new()) else {
+            panic!("first proof should verify");
+        };
+        assert!(matches!(first, VerificationResult::Partial(4, _, _)));
+        assert_eq!(provider.inner.set_id.load(Ordering::Relaxed), 3);
+
+        let Ok(second) = provider.verify(&proof, 4, Vec::new()) else {
+            panic!("second proof should verify");
+        };
+        assert!(matches!(second, VerificationResult::Partial(5, _, _)));
+        assert_eq!(provider.inner.set_id.load(Ordering::Relaxed), 4);
+    }
 }

pallets/admin-utils/src/tests/mock.rs

@@ -398,7 +398,9 @@ impl crate::GrandpaInterface<Test> for GrandpaInterfaceImpl {
         in_blocks: BlockNumber,
         forced: Option<BlockNumber>,
     ) -> sp_runtime::DispatchResult {
-        Grandpa::schedule_change(next_authorities, in_blocks, forced)
+        Grandpa::schedule_change(next_authorities, in_blocks, forced)?;
+        pallet_grandpa::CurrentSetId::<Test>::mutate(|set_id| *set_id += 1);
+        Ok(())
     }
 }
 

pallets/admin-utils/src/tests/mod.rs

@@ -1815,6 +1815,7 @@ fn test_sudo_non_root_cannot_set_evm_chain_id() {
 fn test_schedule_grandpa_change() {
     new_test_ext().execute_with(|| {
         assert_eq!(Grandpa::grandpa_authorities(), vec![]);
+        pallet_grandpa::CurrentSetId::<Test>::put(3);
 
         let bob: GrandpaId = ed25519::Pair::from_legacy_string("//Bob", None)
             .public()
@@ -1827,9 +1828,22 @@ fn test_schedule_grandpa_change() {
             None
         ));
 
+        assert_eq!(Grandpa::current_set_id(), 4);
+        assert_noop!(
+            AdminUtils::schedule_grandpa_change(
+                RuntimeOrigin::root(),
+                vec![(bob.clone(), 1)],
+                41,
+                None
+            ),
+            pallet_grandpa::Error::<Test>::ChangePending
+        );
+        assert_eq!(Grandpa::current_set_id(), 4);
+
         Grandpa::on_finalize(42);
 
         assert_eq!(Grandpa::grandpa_authorities(), vec![(bob, 1)]);
+        assert_eq!(Grandpa::current_set_id(), 4);
     });
 }
 

pallets/admin-utils/src/weights.rs

@@ -140,17 +140,19 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 	}
 	/// Storage: `Grandpa::PendingChange` (r:1 w:1)
 	/// Proof: `Grandpa::PendingChange` (`max_values`: Some(1), `max_size`: Some(1294), added: 1789, mode: `MaxEncodedLen`)
+	/// Storage: `Grandpa::CurrentSetId` (r:1 w:1)
+	/// Proof: `Grandpa::CurrentSetId` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`)
 	/// The range of component `a` is `[0, 32]`.
 	fn schedule_grandpa_change(a: u32, ) -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `174`
-		//  Estimated: `2779`
+		//  Estimated: `3282`
 		// Minimum execution time: 7_444_000 picoseconds.
-		Weight::from_parts(8_055_222, 2779)
+		Weight::from_parts(8_055_222, 3282)
 			// Standard Error: 917
 			.saturating_add(Weight::from_parts(18_955, 0).saturating_mul(a.into()))
-			.saturating_add(T::DbWeight::get().reads(1_u64))
-			.saturating_add(T::DbWeight::get().writes(1_u64))
+			.saturating_add(T::DbWeight::get().reads(2_u64))
+			.saturating_add(T::DbWeight::get().writes(2_u64))
 	}
 	/// Storage: `SubtensorModule::MaxDelegateTake` (r:0 w:1)
 	/// Proof: `SubtensorModule::MaxDelegateTake` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)
@@ -1491,17 +1493,19 @@ impl WeightInfo for () {
 	}
 	/// Storage: `Grandpa::PendingChange` (r:1 w:1)
 	/// Proof: `Grandpa::PendingChange` (`max_values`: Some(1), `max_size`: Some(1294), added: 1789, mode: `MaxEncodedLen`)
+	/// Storage: `Grandpa::CurrentSetId` (r:1 w:1)
+	/// Proof: `Grandpa::CurrentSetId` (`max_values`: Some(1), `max_size`: Some(8), added: 503, mode: `MaxEncodedLen`)
 	/// The range of component `a` is `[0, 32]`.
 	fn schedule_grandpa_change(a: u32, ) -> Weight {
 		// Proof Size summary in bytes:
 		//  Measured:  `174`
-		//  Estimated: `2779`
+		//  Estimated: `3282`
 		// Minimum execution time: 7_444_000 picoseconds.
-		Weight::from_parts(8_055_222, 2779)
+		Weight::from_parts(8_055_222, 3282)
 			// Standard Error: 917
 			.saturating_add(Weight::from_parts(18_955, 0).saturating_mul(a.into()))
-			.saturating_add(RocksDbWeight::get().reads(1_u64))
-			.saturating_add(RocksDbWeight::get().writes(1_u64))
+			.saturating_add(RocksDbWeight::get().reads(2_u64))
+			.saturating_add(RocksDbWeight::get().writes(2_u64))
 	}
 	/// Storage: `SubtensorModule::MaxDelegateTake` (r:0 w:1)
 	/// Proof: `SubtensorModule::MaxDelegateTake` (`max_values`: Some(1), `max_size`: None, mode: `Measured`)

runtime/src/lib.rs

@@ -235,7 +235,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
     //   `spec_version`, and `authoring_version` are the same between Wasm and native.
     // This value is set to 100 to notify Polkadot-JS App (https://polkadot.js.org/apps) to use
     //   the compatible custom types.
-    spec_version: 437,
+    spec_version: 438,
     impl_version: 1,
     apis: RUNTIME_API_VERSIONS,
     transaction_version: 1,
@@ -998,7 +998,11 @@ impl pallet_admin_utils::GrandpaInterface<Runtime> for GrandpaInterfaceImpl {
         in_blocks: BlockNumber,
         forced: Option<BlockNumber>,
     ) -> sp_runtime::DispatchResult {
-        Grandpa::schedule_change(next_authorities, in_blocks, forced)
+        Grandpa::schedule_change(next_authorities, in_blocks, forced)?;
+        // This runtime does not use pallet-session, so mirror the bookkeeping performed by
+        // pallet-grandpa's session handler after it successfully schedules an authority change.
+        pallet_grandpa::CurrentSetId::<Runtime>::mutate(|set_id| *set_id += 1);
+        Ok(())
     }
 }