From c3b1a9836c3cde2e1028162091a96efefb9a1d0e Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Thu, 31 Oct 2019 14:33:34 +0100 Subject: [PATCH] grandpa: Use storage proofs for Grandpa authorities (#3734) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * grandpa: Write Grandpa authorities to well known key. Instead of requiring execution proofs for Grandpa authorities, this enables much simpler storage proofs. * grandpa: Introduce named AuthorityList type. * grandpa: Storage migration for srml-grandpa module. * Remove no-longer-used GrandpaApi runtime API. * grandpa: Write AuthorityList to storage with encoding version. We expect the AuthorityList type may change (eg. key changes). To make upgrades smoother, include a version in the stored value. * Bump node runtime spec version. * Update srml/grandpa/src/lib.rs Co-Authored-By: André Silva --- Cargo.lock | 1 - core/finality-grandpa/primitives/Cargo.toml | 2 - core/finality-grandpa/primitives/src/lib.rs | 78 ++++++++++++----- core/finality-grandpa/src/authorities.rs | 8 +- core/finality-grandpa/src/aux_schema.rs | 9 +- .../src/communication/tests.rs | 3 +- core/finality-grandpa/src/finality_proof.rs | 86 ++++++++++--------- core/finality-grandpa/src/lib.rs | 43 ++++++---- core/finality-grandpa/src/light_import.rs | 58 ++++++------- core/finality-grandpa/src/tests.rs | 39 +++------ node-template/runtime/src/lib.rs | 9 +- node-template/src/service.rs | 6 +- node/cli/src/service.rs | 16 ++-- node/runtime/src/lib.rs | 11 +-- srml/grandpa/Cargo.toml | 1 + srml/grandpa/src/lib.rs | 66 +++++++++----- srml/grandpa/src/mock.rs | 4 +- srml/grandpa/src/tests.rs | 18 ++++ 18 files changed, 263 insertions(+), 195 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 13fc0dcd51a77..5bee6302ec8ef 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5275,7 +5275,6 @@ dependencies = [ "sr-primitives 2.0.0", "sr-std 2.0.0", "substrate-application-crypto 2.0.0", - "substrate-client 2.0.0", ] [[package]] diff --git a/core/finality-grandpa/primitives/Cargo.toml b/core/finality-grandpa/primitives/Cargo.toml index 02439d4150d81..90d7af5777c7c 100644 --- a/core/finality-grandpa/primitives/Cargo.toml +++ b/core/finality-grandpa/primitives/Cargo.toml @@ -5,7 +5,6 @@ authors = ["Parity Technologies "] edition = "2018" [dependencies] -client = { package = "substrate-client", path = "../../client", default-features = false } app-crypto = { package = "substrate-application-crypto", path = "../../application-crypto", default-features = false } codec = { package = "parity-scale-codec", version = "1.0.0", default-features = false, features = ["derive"] } sr-primitives = { path = "../../sr-primitives", default-features = false } @@ -15,7 +14,6 @@ serde = { version = "1.0.101", optional = true, features = ["derive"] } [features] default = ["std"] std = [ - "client/std", "codec/std", "sr-primitives/std", "rstd/std", diff --git a/core/finality-grandpa/primitives/src/lib.rs b/core/finality-grandpa/primitives/src/lib.rs index 27139bbeeffa8..3f5b4cb7f6ed2 100644 --- a/core/finality-grandpa/primitives/src/lib.rs +++ b/core/finality-grandpa/primitives/src/lib.rs @@ -23,9 +23,9 @@ extern crate alloc; #[cfg(feature = "std")] use serde::Serialize; -use codec::{Encode, Decode, Codec}; +use codec::{Encode, Decode, Input, Codec}; use sr_primitives::{ConsensusEngineId, RuntimeDebug}; -use client::decl_runtime_apis; +use rstd::borrow::Cow; use rstd::vec::Vec; mod app { @@ -46,6 +46,10 @@ pub type AuthoritySignature = app::Signature; /// The `ConsensusEngineId` of GRANDPA. pub const GRANDPA_ENGINE_ID: ConsensusEngineId = *b"FRNK"; +/// The storage key for the current set of weighted Grandpa authorities. +/// The value stored is an encoded VersionedAuthorityList. +pub const GRANDPA_AUTHORITIES_KEY: &'static [u8] = b":grandpa_authorities"; + /// The weight of an authority. pub type AuthorityWeight = u64; @@ -58,12 +62,15 @@ pub type SetId = u64; /// The round indicator. pub type RoundNumber = u64; +/// A list of Grandpa authorities with associated weights. +pub type AuthorityList = Vec<(AuthorityId, AuthorityWeight)>; + /// A scheduled change of authority set. #[cfg_attr(feature = "std", derive(Serialize))] #[derive(Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug)] pub struct ScheduledChange { /// The new authorities after the change, along with their respective weights. - pub next_authorities: Vec<(AuthorityId, AuthorityWeight)>, + pub next_authorities: AuthorityList, /// The number of blocks to delay. pub delay: N, } @@ -154,24 +161,51 @@ pub const PENDING_CHANGE_CALL: &str = "grandpa_pending_change"; /// WASM function call to get current GRANDPA authorities. pub const AUTHORITIES_CALL: &str = "grandpa_authorities"; -decl_runtime_apis! { - /// APIs for integrating the GRANDPA finality gadget into runtimes. - /// This should be implemented on the runtime side. - /// - /// This is primarily used for negotiating authority-set changes for the - /// gadget. GRANDPA uses a signaling model of changing authority sets: - /// changes should be signaled with a delay of N blocks, and then automatically - /// applied in the runtime after those N blocks have passed. - /// - /// The consensus protocol will coordinate the handoff externally. - #[api_version(2)] - pub trait GrandpaApi { - /// Get the current GRANDPA authorities and weights. This should not change except - /// for when changes are scheduled and the corresponding delay has passed. - /// - /// When called at block B, it will return the set of authorities that should be - /// used to finalize descendants of this block (B+1, B+2, ...). The block B itself - /// is finalized by the authorities from block B-1. - fn grandpa_authorities() -> Vec<(AuthorityId, AuthorityWeight)>; +/// The current version of the stored AuthorityList type. The encoding version MUST be updated any +/// time the AuthorityList type changes. +const AUTHORITIES_VERISON: u8 = 1; + +/// An AuthorityList that is encoded with a version specifier. The encoding version is updated any +/// time the AuthorityList type changes. This ensures that encodings of different versions of an +/// AuthorityList are differentiable. Attempting to decode an authority list with an unknown +/// version will fail. +#[derive(Default)] +pub struct VersionedAuthorityList<'a>(Cow<'a, AuthorityList>); + +impl<'a> From for VersionedAuthorityList<'a> { + fn from(authorities: AuthorityList) -> Self { + VersionedAuthorityList(Cow::Owned(authorities)) + } +} + +impl<'a> From<&'a AuthorityList> for VersionedAuthorityList<'a> { + fn from(authorities: &'a AuthorityList) -> Self { + VersionedAuthorityList(Cow::Borrowed(authorities)) + } +} + +impl<'a> Into for VersionedAuthorityList<'a> { + fn into(self) -> AuthorityList { + self.0.into_owned() + } +} + +impl<'a> Encode for VersionedAuthorityList<'a> { + fn size_hint(&self) -> usize { + (AUTHORITIES_VERISON, self.0.as_ref()).size_hint() + } + + fn using_encoded R>(&self, f: F) -> R { + (AUTHORITIES_VERISON, self.0.as_ref()).using_encoded(f) + } +} + +impl<'a> Decode for VersionedAuthorityList<'a> { + fn decode(value: &mut I) -> Result { + let (version, authorities): (u8, AuthorityList) = Decode::decode(value)?; + if version != AUTHORITIES_VERISON { + return Err("unknown Grandpa authorities version".into()); + } + Ok(authorities.into()) } } diff --git a/core/finality-grandpa/src/authorities.rs b/core/finality-grandpa/src/authorities.rs index 9b83c9feb6871..263f2dc076e39 100644 --- a/core/finality-grandpa/src/authorities.rs +++ b/core/finality-grandpa/src/authorities.rs @@ -22,7 +22,7 @@ use grandpa::voter_set::VoterSet; use codec::{Encode, Decode}; use log::{debug, info}; use substrate_telemetry::{telemetry, CONSENSUS_INFO}; -use fg_primitives::AuthorityId; +use fg_primitives::{AuthorityId, AuthorityList}; use std::cmp::Ord; use std::fmt::Debug; @@ -86,7 +86,7 @@ pub(crate) struct Status { /// A set of authorities. #[derive(Debug, Clone, Encode, Decode, PartialEq)] pub(crate) struct AuthoritySet { - pub(crate) current_authorities: Vec<(AuthorityId, u64)>, + pub(crate) current_authorities: AuthorityList, pub(crate) set_id: u64, // Tree of pending standard changes across forks. Standard changes are // enacted on finality and must be enacted (i.e. finalized) in-order across @@ -103,7 +103,7 @@ where H: PartialEq, N: Ord, { /// Get a genesis set with given authorities. - pub(crate) fn genesis(initial: Vec<(AuthorityId, u64)>) -> Self { + pub(crate) fn genesis(initial: AuthorityList) -> Self { AuthoritySet { current_authorities: initial, set_id: 0, @@ -390,7 +390,7 @@ pub(crate) enum DelayKind { #[derive(Debug, Clone, Encode, PartialEq)] pub(crate) struct PendingChange { /// The new authorities and weights to apply. - pub(crate) next_authorities: Vec<(AuthorityId, u64)>, + pub(crate) next_authorities: AuthorityList, /// How deep in the chain the announcing block must be /// before the change is applied. pub(crate) delay: N, diff --git a/core/finality-grandpa/src/aux_schema.rs b/core/finality-grandpa/src/aux_schema.rs index a2b05a0cd60e1..1aed0b95aba28 100644 --- a/core/finality-grandpa/src/aux_schema.rs +++ b/core/finality-grandpa/src/aux_schema.rs @@ -25,7 +25,7 @@ use fork_tree::ForkTree; use grandpa::round::State as RoundState; use sr_primitives::traits::{Block as BlockT, NumberFor}; use log::{info, warn}; -use fg_primitives::{AuthorityId, AuthorityWeight, SetId, RoundNumber}; +use fg_primitives::{AuthorityList, SetId, RoundNumber}; use crate::authorities::{AuthoritySet, SharedAuthoritySet, PendingChange, DelayKind}; use crate::consensus_changes::{SharedConsensusChanges, ConsensusChanges}; @@ -55,7 +55,7 @@ type V0VoterSetState = (RoundNumber, RoundState); #[derive(Debug, Clone, Encode, Decode, PartialEq)] struct V0PendingChange { - next_authorities: Vec<(AuthorityId, AuthorityWeight)>, + next_authorities: AuthorityList, delay: N, canon_height: N, canon_hash: H, @@ -63,7 +63,7 @@ struct V0PendingChange { #[derive(Debug, Clone, Encode, Decode, PartialEq)] struct V0AuthoritySet { - current_authorities: Vec<(AuthorityId, AuthorityWeight)>, + current_authorities: AuthorityList, set_id: SetId, pending_changes: Vec>, } @@ -266,7 +266,7 @@ pub(crate) fn load_persistent( -> ClientResult> where B: AuxStore, - G: FnOnce() -> ClientResult>, + G: FnOnce() -> ClientResult, { let version: Option = load_decode(backend, VERSION_KEY)?; let consensus_changes = load_decode(backend, CONSENSUS_CHANGES_KEY)? @@ -426,6 +426,7 @@ pub(crate) fn load_authorities(backend: &B) #[cfg(test)] mod test { + use fg_primitives::AuthorityId; use primitives::H256; use test_client; use super::*; diff --git a/core/finality-grandpa/src/communication/tests.rs b/core/finality-grandpa/src/communication/tests.rs index f918f47258d49..af6d842be3c02 100644 --- a/core/finality-grandpa/src/communication/tests.rs +++ b/core/finality-grandpa/src/communication/tests.rs @@ -28,6 +28,7 @@ use codec::Encode; use sr_primitives::traits::NumberFor; use crate::environment::SharedVoterSetState; +use fg_primitives::AuthorityList; use super::gossip::{self, GossipValidator}; use super::{AuthorityId, VoterSet, Round, SetId}; @@ -200,7 +201,7 @@ fn make_test_network() -> ( ) } -fn make_ids(keys: &[Ed25519Keyring]) -> Vec<(AuthorityId, u64)> { +fn make_ids(keys: &[Ed25519Keyring]) -> AuthorityList { keys.iter() .map(|key| key.clone().public().into()) .map(|id| (id, 1)) diff --git a/core/finality-grandpa/src/finality_proof.rs b/core/finality-grandpa/src/finality_proof.rs index bd22a7bbac287..aa7207b422881 100644 --- a/core/finality-grandpa/src/finality_proof.rs +++ b/core/finality-grandpa/src/finality_proof.rs @@ -34,13 +34,14 @@ //! finality proof (that finalizes some block C that is ancestor of the B and descendant //! of the U) could be returned. +use std::iter; use std::sync::Arc; use log::{trace, warn}; use client::{ backend::Backend, blockchain::Backend as BlockchainBackend, CallExecutor, Client, error::{Error as ClientError, Result as ClientResult}, - light::fetcher::{FetchChecker, RemoteCallRequest, StorageProof}, ExecutionStrategy, + light::fetcher::{FetchChecker, RemoteReadRequest, StorageProof}, }; use codec::{Encode, Decode}; use grandpa::BlockNumberOps; @@ -48,9 +49,9 @@ use sr_primitives::{ Justification, generic::BlockId, traits::{NumberFor, Block as BlockT, Header as HeaderT, One}, }; -use primitives::{H256, Blake2Hasher}; +use primitives::{H256, Blake2Hasher, storage::StorageKey}; use substrate_telemetry::{telemetry, CONSENSUS_INFO}; -use fg_primitives::AuthorityId; +use fg_primitives::{AuthorityId, AuthorityList, VersionedAuthorityList, GRANDPA_AUTHORITIES_KEY}; use crate::justification::GrandpaJustification; @@ -59,9 +60,9 @@ const MAX_FRAGMENTS_IN_PROOF: usize = 8; /// GRANDPA authority set related methods for the finality proof provider. pub trait AuthoritySetForFinalityProver: Send + Sync { - /// Call GrandpaApi::grandpa_authorities at given block. - fn authorities(&self, block: &BlockId) -> ClientResult>; - /// Prove call of GrandpaApi::grandpa_authorities at given block. + /// Read GRANDPA_AUTHORITIES_KEY from storage at given block. + fn authorities(&self, block: &BlockId) -> ClientResult; + /// Prove storage read of GRANDPA_AUTHORITIES_KEY at given block. fn prove_authorities(&self, block: &BlockId) -> ClientResult; } @@ -72,33 +73,28 @@ impl, RA> AuthoritySetForFinalityProver fo E: CallExecutor + 'static + Clone + Send + Sync, RA: Send + Sync, { - fn authorities(&self, block: &BlockId) -> ClientResult> { - self.executor().call( - block, - "GrandpaApi_grandpa_authorities", - &[], - ExecutionStrategy::NativeElseWasm, - None, - ).and_then(|call_result| Decode::decode(&mut &call_result[..]) - .map_err(|err| ClientError::CallResultDecode( - "failed to decode GRANDPA authorities set proof".into(), err - ))) + fn authorities(&self, block: &BlockId) -> ClientResult { + let storage_key = StorageKey(GRANDPA_AUTHORITIES_KEY.to_vec()); + self.storage(block, &storage_key)? + .and_then(|encoded| VersionedAuthorityList::decode(&mut encoded.0.as_slice()).ok()) + .map(|versioned| versioned.into()) + .ok_or(ClientError::InvalidAuthoritiesSet) } fn prove_authorities(&self, block: &BlockId) -> ClientResult { - self.execution_proof(block, "GrandpaApi_grandpa_authorities",&[]).map(|(_, proof)| proof) + self.read_proof(block, iter::once(GRANDPA_AUTHORITIES_KEY)) } } /// GRANDPA authority set related methods for the finality proof checker. pub trait AuthoritySetForFinalityChecker: Send + Sync { - /// Check execution proof of Grandpa::grandpa_authorities at given block. + /// Check storage read proof of GRANDPA_AUTHORITIES_KEY at given block. fn check_authorities_proof( &self, hash: Block::Hash, header: Block::Header, proof: StorageProof, - ) -> ClientResult>; + ) -> ClientResult; } /// FetchChecker-based implementation of AuthoritySetForFinalityChecker. @@ -108,22 +104,30 @@ impl AuthoritySetForFinalityChecker for Arc ClientResult> { - let request = RemoteCallRequest { + ) -> ClientResult { + let storage_key = GRANDPA_AUTHORITIES_KEY.to_vec(); + let request = RemoteReadRequest { block: hash, header, - method: "GrandpaApi_grandpa_authorities".into(), - call_data: vec![], + keys: vec![storage_key.clone()], retry_count: None, }; - self.check_execution_proof(&request, proof) - .and_then(|authorities| { - let authorities: Vec<(AuthorityId, u64)> = Decode::decode(&mut &authorities[..]) - .map_err(|err| ClientError::CallResultDecode( - "failed to decode GRANDPA authorities set proof".into(), err - ))?; - Ok(authorities.into_iter().collect()) + self.check_read_proof(&request, proof) + .and_then(|results| { + let maybe_encoded = results.get(&storage_key) + .expect( + "storage_key is listed in the request keys; \ + check_read_proof must return a value for each requested key; + qed" + ); + maybe_encoded + .as_ref() + .and_then(|encoded| { + VersionedAuthorityList::decode(&mut encoded.as_slice()).ok() + }) + .map(|versioned| versioned.into()) + .ok_or(ClientError::InvalidAuthoritiesSet) }) } } @@ -189,7 +193,7 @@ pub struct FinalityEffects { /// New authorities set id that should be applied starting from block. pub new_set_id: u64, /// New authorities set that should be applied starting from block. - pub new_authorities: Vec<(AuthorityId, u64)>, + pub new_authorities: AuthorityList, } /// Single fragment of proof-of-finality. @@ -408,7 +412,7 @@ pub(crate) fn prove_finality, B: BlockchainBackend, B>( blockchain: &B, current_set_id: u64, - current_authorities: Vec<(AuthorityId, u64)>, + current_authorities: AuthorityList, authorities_provider: &dyn AuthoritySetForFinalityChecker, remote_proof: Vec, ) -> ClientResult> @@ -427,7 +431,7 @@ pub(crate) fn check_finality_proof, B>( fn do_check_finality_proof, B, J>( blockchain: &B, current_set_id: u64, - current_authorities: Vec<(AuthorityId, u64)>, + current_authorities: AuthorityList, authorities_provider: &dyn AuthoritySetForFinalityChecker, remote_proof: Vec, ) -> ClientResult> @@ -522,12 +526,12 @@ fn check_finality_proof_fragment, B, J>( /// Authorities set from initial authorities set or finality effects. enum AuthoritiesOrEffects { - Authorities(u64, Vec<(AuthorityId, u64)>), + Authorities(u64, AuthorityList), Effects(FinalityEffects
), } impl AuthoritiesOrEffects
{ - pub fn extract_authorities(self) -> (u64, Vec<(AuthorityId, u64)>) { + pub fn extract_authorities(self) -> (u64, AuthorityList) { match self { AuthoritiesOrEffects::Authorities(set_id, authorities) => (set_id, authorities), AuthoritiesOrEffects::Effects(effects) => (effects.new_set_id, effects.new_authorities), @@ -581,10 +585,10 @@ pub(crate) mod tests { impl AuthoritySetForFinalityProver for (GetAuthorities, ProveAuthorities) where - GetAuthorities: Send + Sync + Fn(BlockId) -> ClientResult>, + GetAuthorities: Send + Sync + Fn(BlockId) -> ClientResult, ProveAuthorities: Send + Sync + Fn(BlockId) -> ClientResult, { - fn authorities(&self, block: &BlockId) -> ClientResult> { + fn authorities(&self, block: &BlockId) -> ClientResult { self.0(*block) } @@ -597,14 +601,14 @@ pub(crate) mod tests { impl AuthoritySetForFinalityChecker for ClosureAuthoritySetForFinalityChecker where - Closure: Send + Sync + Fn(H256, Header, StorageProof) -> ClientResult>, + Closure: Send + Sync + Fn(H256, Header, StorageProof) -> ClientResult, { fn check_authorities_proof( &self, hash: H256, header: Header, - proof: StorageProof, - ) -> ClientResult> { + proof: StorageProof + ) -> ClientResult { self.0(hash, header, proof) } } diff --git a/core/finality-grandpa/src/lib.rs b/core/finality-grandpa/src/lib.rs index 0decea58117b0..67acaa21d760b 100644 --- a/core/finality-grandpa/src/lib.rs +++ b/core/finality-grandpa/src/lib.rs @@ -61,10 +61,7 @@ use client::{ use client::blockchain::HeaderBackend; use codec::Encode; use sr_primitives::generic::BlockId; -use sr_primitives::traits::{ - NumberFor, Block as BlockT, DigestFor, ProvideRuntimeApi -}; -use fg_primitives::{GrandpaApi, AuthorityPair}; +use sr_primitives::traits::{NumberFor, Block as BlockT, DigestFor, Zero}; use keystore::KeyStorePtr; use inherents::InherentDataProviders; use consensus_common::SelectChain; @@ -108,7 +105,7 @@ use environment::{Environment, VoterSetState}; use import::GrandpaBlockImport; use until_imported::UntilGlobalMessageBlocksImported; use communication::NetworkBridge; -use fg_primitives::{AuthoritySignature, SetId, AuthorityWeight}; +use fg_primitives::{AuthorityList, AuthorityPair, AuthoritySignature, SetId}; // Re-export these two because it's just so damn convenient. pub use fg_primitives::{AuthorityId, ScheduledChange}; @@ -295,7 +292,7 @@ pub(crate) struct NewAuthoritySet { pub(crate) canon_number: N, pub(crate) canon_hash: H, pub(crate) set_id: SetId, - pub(crate) authorities: Vec<(AuthorityId, AuthorityWeight)>, + pub(crate) authorities: AuthorityList, } /// Commands issued to the voter. @@ -367,11 +364,30 @@ pub struct LinkHalf, RA, SC> { voter_commands_rx: mpsc::UnboundedReceiver>>, } +/// Provider for the Grandpa authority set configured on the genesis block. +pub trait GenesisAuthoritySetProvider { + /// Get the authority set at the genesis block. + fn get(&self) -> Result; +} + +impl, RA> GenesisAuthoritySetProvider for Client + where + B: Backend + Send + Sync + 'static, + E: CallExecutor + 'static + Clone + Send + Sync, + RA: Send + Sync, +{ + fn get(&self) -> Result { + use finality_proof::AuthoritySetForFinalityProver; + + self.authorities(&BlockId::Number(Zero::zero())) + } +} + /// Make block importer and link half necessary to tie the background voter /// to it. -pub fn block_import, RA, PRA, SC>( +pub fn block_import, RA, SC>( client: Arc>, - api: &PRA, + genesis_authorities_provider: &dyn GenesisAuthoritySetProvider, select_chain: SC, ) -> Result<( GrandpaBlockImport, @@ -381,12 +397,8 @@ where B: Backend + 'static, E: CallExecutor + 'static + Clone + Send + Sync, RA: Send + Sync, - PRA: ProvideRuntimeApi, - PRA::Api: GrandpaApi, SC: SelectChain, { - use sr_primitives::traits::Zero; - let chain_info = client.info(); let genesis_hash = chain_info.chain.genesis_hash; @@ -395,12 +407,11 @@ where genesis_hash, >::zero(), || { - let genesis_authorities = api.runtime_api() - .grandpa_authorities(&BlockId::number(Zero::zero()))?; + let authorities = genesis_authorities_provider.get()?; telemetry!(CONSENSUS_DEBUG; "afg.loading_authorities"; - "authorities_len" => ?genesis_authorities.len() + "authorities_len" => ?authorities.len() ); - Ok(genesis_authorities) + Ok(authorities) } )?; diff --git a/core/finality-grandpa/src/light_import.rs b/core/finality-grandpa/src/light_import.rs index 30af3a06d3f76..30008b51ece1e 100644 --- a/core/finality-grandpa/src/light_import.rs +++ b/core/finality-grandpa/src/light_import.rs @@ -34,17 +34,18 @@ use consensus_common::{ }; use network::config::{BoxFinalityProofRequestBuilder, FinalityProofRequestBuilder}; use sr_primitives::Justification; -use sr_primitives::traits::{ - NumberFor, Block as BlockT, Header as HeaderT, ProvideRuntimeApi, DigestFor, -}; -use fg_primitives::{self, GrandpaApi, AuthorityId}; +use sr_primitives::traits::{NumberFor, Block as BlockT, Header as HeaderT, DigestFor}; +use fg_primitives::{self, AuthorityList}; use sr_primitives::generic::BlockId; use primitives::{H256, Blake2Hasher}; +use crate::GenesisAuthoritySetProvider; use crate::aux_schema::load_decode; use crate::consensus_changes::ConsensusChanges; use crate::environment::canonical_at_height; -use crate::finality_proof::{AuthoritySetForFinalityChecker, ProvableJustification, make_finality_proof_request}; +use crate::finality_proof::{ + AuthoritySetForFinalityChecker, ProvableJustification, make_finality_proof_request, +}; use crate::justification::GrandpaJustification; /// LightAuthoritySet is saved under this key in aux storage. @@ -53,21 +54,23 @@ const LIGHT_AUTHORITY_SET_KEY: &[u8] = b"grandpa_voters"; const LIGHT_CONSENSUS_CHANGES_KEY: &[u8] = b"grandpa_consensus_changes"; /// Create light block importer. -pub fn light_block_import, RA, PRA>( +pub fn light_block_import, RA>( client: Arc>, backend: Arc, + genesis_authorities_provider: &dyn GenesisAuthoritySetProvider, authority_set_provider: Arc>, - api: Arc, ) -> Result, ClientError> where B: Backend + 'static, E: CallExecutor + 'static + Clone + Send + Sync, RA: Send + Sync, - PRA: ProvideRuntimeApi, - PRA::Api: GrandpaApi, { let info = client.info(); - let import_data = load_aux_import_data(info.chain.finalized_hash, &*client, api)?; + let import_data = load_aux_import_data( + info.chain.finalized_hash, + &*client, + genesis_authorities_provider, + )?; Ok(GrandpaLightBlockImport { client, backend, @@ -110,7 +113,7 @@ struct LightImportData> { #[derive(Debug, Encode, Decode)] struct LightAuthoritySet { set_id: u64, - authorities: Vec<(AuthorityId, u64)>, + authorities: AuthorityList, } impl, RA> GrandpaLightBlockImport { @@ -194,7 +197,7 @@ impl, RA> FinalityProofImport impl LightAuthoritySet { /// Get a genesis set with given authorities. - pub fn genesis(initial: Vec<(AuthorityId, u64)>) -> Self { + pub fn genesis(initial: AuthorityList) -> Self { LightAuthoritySet { set_id: fg_primitives::SetId::default(), authorities: initial, @@ -207,12 +210,12 @@ impl LightAuthoritySet { } /// Get latest authorities set. - pub fn authorities(&self) -> Vec<(AuthorityId, u64)> { + pub fn authorities(&self) -> AuthorityList { self.authorities.clone() } /// Set new authorities set. - pub fn update(&mut self, set_id: u64, authorities: Vec<(AuthorityId, u64)>) { + pub fn update(&mut self, set_id: u64, authorities: AuthorityList) { self.set_id = set_id; std::mem::replace(&mut self.authorities, authorities); } @@ -472,17 +475,14 @@ fn do_finalize_block>( } /// Load light import aux data from the store. -fn load_aux_import_data, PRA>( +fn load_aux_import_data>( last_finalized: Block::Hash, aux_store: &B, - api: Arc, + genesis_authorities_provider: &dyn GenesisAuthoritySetProvider, ) -> Result, ClientError> where B: AuxStore, - PRA: ProvideRuntimeApi, - PRA::Api: GrandpaApi, { - use sr_primitives::traits::Zero; let authority_set = match load_decode(aux_store, LIGHT_AUTHORITY_SET_KEY)? { Some(authority_set) => authority_set, None => { @@ -490,7 +490,7 @@ fn load_aux_import_data, PRA>( from genesis on what appears to be first startup."); // no authority set on disk: fetch authorities from genesis state - let genesis_authorities = api.runtime_api().grandpa_authorities(&BlockId::number(Zero::zero()))?; + let genesis_authorities = genesis_authorities_provider.get()?; let authority_set = LightAuthoritySet::genesis(genesis_authorities); let encoded = authority_set.encode(); @@ -546,6 +546,7 @@ fn on_post_finalization_error(error: ClientError, value_type: &str) -> Consensus pub mod tests { use super::*; use consensus_common::ForkChoiceStrategy; + use fg_primitives::AuthorityId; use primitives::{H256, crypto::Public}; use test_client::client::in_mem::Blockchain as InMemoryAuxStore; use test_client::runtime::{Block, Header}; @@ -622,20 +623,19 @@ pub mod tests { } /// Creates light block import that ignores justifications that came outside of finality proofs. - pub fn light_block_import_without_justifications, RA, PRA>( + pub fn light_block_import_without_justifications, RA>( client: Arc>, backend: Arc, + genesis_authorities_provider: &dyn GenesisAuthoritySetProvider, authority_set_provider: Arc>, - api: Arc, ) -> Result, ClientError> where B: Backend + 'static, E: CallExecutor + 'static + Clone + Send + Sync, RA: Send + Sync, - PRA: ProvideRuntimeApi, - PRA::Api: GrandpaApi, { - light_block_import(client, backend, authority_set_provider, api).map(NoJustificationsImport) + light_block_import(client, backend, genesis_authorities_provider, authority_set_provider) + .map(NoJustificationsImport) } fn import_block( @@ -729,14 +729,14 @@ pub mod tests { #[test] fn aux_data_updated_on_start() { let aux_store = InMemoryAuxStore::::new(); - let api = Arc::new(TestApi::new(vec![(AuthorityId::from_slice(&[1; 32]), 1)])); + let api = TestApi::new(vec![(AuthorityId::from_slice(&[1; 32]), 1)]); // when aux store is empty initially assert!(aux_store.get_aux(LIGHT_AUTHORITY_SET_KEY).unwrap().is_none()); assert!(aux_store.get_aux(LIGHT_CONSENSUS_CHANGES_KEY).unwrap().is_none()); // it is updated on importer start - load_aux_import_data(Default::default(), &aux_store, api).unwrap(); + load_aux_import_data(Default::default(), &aux_store, &api).unwrap(); assert!(aux_store.get_aux(LIGHT_AUTHORITY_SET_KEY).unwrap().is_some()); assert!(aux_store.get_aux(LIGHT_CONSENSUS_CHANGES_KEY).unwrap().is_some()); } @@ -744,7 +744,7 @@ pub mod tests { #[test] fn aux_data_loaded_on_restart() { let aux_store = InMemoryAuxStore::::new(); - let api = Arc::new(TestApi::new(vec![(AuthorityId::from_slice(&[1; 32]), 1)])); + let api = TestApi::new(vec![(AuthorityId::from_slice(&[1; 32]), 1)]); // when aux store is non-empty initially let mut consensus_changes = ConsensusChanges::::empty(); @@ -766,7 +766,7 @@ pub mod tests { ).unwrap(); // importer uses it on start - let data = load_aux_import_data(Default::default(), &aux_store, api).unwrap(); + let data = load_aux_import_data(Default::default(), &aux_store, &api).unwrap(); assert_eq!(data.authority_set.authorities(), vec![(AuthorityId::from_slice(&[42; 32]), 2)]); assert_eq!(data.consensus_changes.pending_changes(), &[(42, Default::default())]); } diff --git a/core/finality-grandpa/src/tests.rs b/core/finality-grandpa/src/tests.rs index 2339379a609d4..93c08a9661baf 100644 --- a/core/finality-grandpa/src/tests.rs +++ b/core/finality-grandpa/src/tests.rs @@ -39,7 +39,7 @@ use codec::Decode; use sr_primitives::traits::{ApiRef, ProvideRuntimeApi, Header as HeaderT}; use sr_primitives::generic::{BlockId, DigestItem}; use primitives::{NativeOrEncoded, ExecutionContext, crypto::Public}; -use fg_primitives::{GRANDPA_ENGINE_ID, AuthorityId}; +use fg_primitives::{GRANDPA_ENGINE_ID, AuthorityList}; use state_machine::{backend::InMemory, prove_read, read_proof_check}; use authorities::AuthoritySet; @@ -137,8 +137,8 @@ impl TestNetFactory for GrandpaTestNet { let import = light_block_import_without_justifications( client.clone(), backend.clone(), + &self.test_config, authorities_provider, - Arc::new(self.test_config.clone()) ).expect("Could not create block import for fresh peer."); let finality_proof_req_builder = import.0.create_finality_proof_request_builder(); let proof_import = Box::new(import.clone()); @@ -188,26 +188,24 @@ impl Future for Exit { #[derive(Default, Clone)] pub(crate) struct TestApi { - genesis_authorities: Vec<(AuthorityId, u64)>, + genesis_authorities: AuthorityList, } impl TestApi { - pub fn new(genesis_authorities: Vec<(AuthorityId, u64)>) -> Self { + pub fn new(genesis_authorities: AuthorityList) -> Self { TestApi { genesis_authorities, } } } -pub(crate) struct RuntimeApi { - inner: TestApi, -} +pub(crate) struct RuntimeApi; impl ProvideRuntimeApi for TestApi { type Api = RuntimeApi; fn runtime_api<'a>(&'a self) -> ApiRef<'a, Self::Api> { - RuntimeApi { inner: self.clone() }.into() + RuntimeApi.into() } } @@ -264,26 +262,15 @@ impl ApiExt for RuntimeApi { } } -impl GrandpaApi for RuntimeApi { - fn GrandpaApi_grandpa_authorities_runtime_api_impl( - &self, - _: &BlockId, - _: ExecutionContext, - _: Option<()>, - _: Vec, - ) -> Result>> { - Ok(self.inner.genesis_authorities.clone()).map(NativeOrEncoded::Native) +impl GenesisAuthoritySetProvider for TestApi { + fn get(&self) -> Result { + Ok(self.genesis_authorities.clone()) } } impl AuthoritySetForFinalityProver for TestApi { - fn authorities(&self, block: &BlockId) -> Result> { - let runtime_api = RuntimeApi { inner: self.clone() }; - runtime_api.GrandpaApi_grandpa_authorities_runtime_api_impl(block, ExecutionContext::Syncing, None, Vec::new()) - .map(|v| match v { - NativeOrEncoded::Native(value) => value, - _ => unreachable!("only providing native values"), - }) + fn authorities(&self, _block: &BlockId) -> Result { + Ok(self.genesis_authorities.clone()) } fn prove_authorities(&self, block: &BlockId) -> Result { @@ -303,7 +290,7 @@ impl AuthoritySetForFinalityChecker for TestApi { _hash: ::Hash, header: ::Header, proof: StorageProof, - ) -> Result> { + ) -> Result { let results = read_proof_check::( *header.state_root(), proof, vec![b"authorities"] ) @@ -320,7 +307,7 @@ impl AuthoritySetForFinalityChecker for TestApi { const TEST_GOSSIP_DURATION: Duration = Duration::from_millis(500); -fn make_ids(keys: &[Ed25519Keyring]) -> Vec<(AuthorityId, u64)> { +fn make_ids(keys: &[Ed25519Keyring]) -> AuthorityList { keys.iter().map(|key| key.clone().public().into()).map(|id| (id, 1)).collect() } diff --git a/node-template/runtime/src/lib.rs b/node-template/runtime/src/lib.rs index c0cd2cf33770c..5b440cdd9b705 100644 --- a/node-template/runtime/src/lib.rs +++ b/node-template/runtime/src/lib.rs @@ -23,8 +23,7 @@ use client::{ runtime_api as client_api, impl_runtime_apis }; use aura_primitives::sr25519::AuthorityId as AuraId; -use grandpa::{AuthorityId as GrandpaId, AuthorityWeight as GrandpaWeight}; -use grandpa::fg_primitives; +use grandpa::AuthorityId as GrandpaId; use version::RuntimeVersion; #[cfg(feature = "std")] use version::NativeVersion; @@ -353,10 +352,4 @@ impl_runtime_apis! { opaque::SessionKeys::generate(seed) } } - - impl fg_primitives::GrandpaApi for Runtime { - fn grandpa_authorities() -> Vec<(GrandpaId, GrandpaWeight)> { - Grandpa::grandpa_authorities() - } - } } diff --git a/node-template/src/service.rs b/node-template/src/service.rs index 398795325fd04..7967a1d2d4e8f 100644 --- a/node-template/src/service.rs +++ b/node-template/src/service.rs @@ -48,7 +48,7 @@ macro_rules! new_full_start { .ok_or_else(|| substrate_service::Error::SelectChainRequired)?; let (grandpa_block_import, grandpa_link) = - grandpa::block_import::<_, _, _, runtime::RuntimeApi, _, _>( + grandpa::block_import::<_, _, _, runtime::RuntimeApi, _>( client.clone(), &*client, select_chain )?; @@ -197,8 +197,8 @@ pub fn new_light(config: Configuration( - client.clone(), backend, Arc::new(fetch_checker), client.clone() + let grandpa_block_import = grandpa::light_block_import::<_, _, _, RuntimeApi>( + client.clone(), backend, &*client.clone(), Arc::new(fetch_checker), )?; let finality_proof_import = grandpa_block_import.clone(); let finality_proof_request_builder = diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 47e36bd926275..789dfc02fb683 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -69,10 +69,11 @@ macro_rules! new_full_start { .with_import_queue(|_config, client, mut select_chain, _transaction_pool| { let select_chain = select_chain.take() .ok_or_else(|| substrate_service::Error::SelectChainRequired)?; - let (grandpa_block_import, grandpa_link) = - grandpa::block_import::<_, _, _, node_runtime::RuntimeApi, _, _>( - client.clone(), &*client, select_chain - )?; + let (grandpa_block_import, grandpa_link) = grandpa::block_import( + client.clone(), + &*client, + select_chain, + )?; let justification_import = grandpa_block_import.clone(); let (block_import, babe_link) = babe::block_import( @@ -291,8 +292,11 @@ pub fn new_light(config: NodeConfiguration) let fetch_checker = fetcher .map(|fetcher| fetcher.checker().clone()) .ok_or_else(|| "Trying to start light import queue without active fetch checker")?; - let grandpa_block_import = grandpa::light_block_import::<_, _, _, RuntimeApi, _>( - client.clone(), backend, Arc::new(fetch_checker), client.clone() + let grandpa_block_import = grandpa::light_block_import::<_, _, _, RuntimeApi>( + client.clone(), + backend, + &*client, + Arc::new(fetch_checker), )?; let finality_proof_import = grandpa_block_import.clone(); diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index a0ee20988eaf8..adee13775d7d3 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -29,7 +29,6 @@ use node_primitives::{ AccountId, AccountIndex, Balance, BlockNumber, Hash, Index, Moment, Signature, }; -use grandpa::fg_primitives; use client::{ block_builder::api::{self as block_builder_api, InherentData, CheckInherentsResult}, runtime_api as client_api, impl_runtime_apis @@ -46,7 +45,7 @@ use version::RuntimeVersion; #[cfg(any(feature = "std", test))] use version::NativeVersion; use primitives::OpaqueMetadata; -use grandpa::{AuthorityId as GrandpaId, AuthorityWeight as GrandpaWeight}; +use grandpa::AuthorityId as GrandpaId; use im_online::sr25519::{AuthorityId as ImOnlineId}; use transaction_payment_rpc_runtime_api::RuntimeDispatchInfo; use contracts_rpc_runtime_api::ContractExecResult; @@ -81,7 +80,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // and set impl_version to equal spec_version. If only runtime // implementation changes and behavior does not, then leave spec_version as // is and increment impl_version. - spec_version: 190, + spec_version: 191, impl_version: 191, apis: RUNTIME_API_VERSIONS, }; @@ -611,12 +610,6 @@ impl_runtime_apis! { } } - impl fg_primitives::GrandpaApi for Runtime { - fn grandpa_authorities() -> Vec<(GrandpaId, GrandpaWeight)> { - Grandpa::grandpa_authorities() - } - } - impl babe_primitives::BabeApi for Runtime { fn configuration() -> babe_primitives::BabeConfiguration { // The choice of `c` parameter (where `1 - c` represents the diff --git a/srml/grandpa/Cargo.toml b/srml/grandpa/Cargo.toml index 4b494cfeff8d1..21b48d3cdc51d 100644 --- a/srml/grandpa/Cargo.toml +++ b/srml/grandpa/Cargo.toml @@ -35,3 +35,4 @@ std = [ "session/std", "finality-tracker/std", ] +migrate-authorities = [] diff --git a/srml/grandpa/src/lib.rs b/srml/grandpa/src/lib.rs index f3e876f2c4e0e..b635b88521ba3 100644 --- a/srml/grandpa/src/lib.rs +++ b/srml/grandpa/src/lib.rs @@ -21,9 +21,6 @@ //! //! In the future, it will also handle misbehavior reports, and on-chain //! finality notifications. -//! -//! For full integration with GRANDPA, the `GrandpaApi` should be implemented. -//! The necessary items are re-exported via the `fg_primitives` crate. #![cfg_attr(not(feature = "std"), no_std)] @@ -32,9 +29,7 @@ pub use substrate_finality_grandpa_primitives as fg_primitives; use rstd::prelude::*; use codec::{self as codec, Encode, Decode, Error}; -use support::{ - decl_event, decl_storage, decl_module, dispatch::Result, -}; +use support::{decl_event, decl_storage, decl_module, dispatch::Result, storage}; use sr_primitives::{ generic::{DigestItem, OpaqueDigestItemId}, traits::Zero, Perbill, }; @@ -42,8 +37,10 @@ use sr_staking_primitives::{ SessionIndex, offence::{Offence, Kind}, }; -use fg_primitives::{GRANDPA_ENGINE_ID, ScheduledChange, ConsensusLog, SetId, RoundNumber}; -pub use fg_primitives::{AuthorityId, AuthorityWeight}; +use fg_primitives::{ + GRANDPA_AUTHORITIES_KEY, GRANDPA_ENGINE_ID, ScheduledChange, ConsensusLog, SetId, RoundNumber, +}; +pub use fg_primitives::{AuthorityId, AuthorityList, AuthorityWeight, VersionedAuthorityList}; use system::{ensure_signed, DigestOf}; mod mock; @@ -64,7 +61,7 @@ pub struct OldStoredPendingChange { /// The delay in blocks until it will be applied. pub delay: N, /// The next authority set. - pub next_authorities: Vec<(AuthorityId, AuthorityWeight)>, + pub next_authorities: AuthorityList, } /// A stored pending change. @@ -75,7 +72,7 @@ pub struct StoredPendingChange { /// The delay in blocks until it will be applied. pub delay: N, /// The next authority set. - pub next_authorities: Vec<(AuthorityId, AuthorityWeight)>, + pub next_authorities: AuthorityList, /// If defined it means the change was forced and the given block number /// indicates the median last finalized block when the change was signaled. pub forced: Option, @@ -126,7 +123,7 @@ pub enum StoredState { decl_event!( pub enum Event { /// New authority set has been applied. - NewAuthorities(Vec<(AuthorityId, AuthorityWeight)>), + NewAuthorities(AuthorityList), /// Current authority set has been paused. Paused, /// Current authority set has been resumed. @@ -136,8 +133,12 @@ decl_event!( decl_storage! { trait Store for Module as GrandpaFinality { - /// The current authority set. - Authorities get(fn authorities): Vec<(AuthorityId, AuthorityWeight)>; + /// DEPRECATED + /// + /// This used to store the current authority set, which has been migrated to the well-known + /// GRANDPA_AUTHORITES_KEY unhashed key. + #[cfg(feature = "migrate-authorities")] + pub(crate) Authorities get(fn authorities): AuthorityList; /// State of the current authority set. State get(fn state): StoredState = StoredState::Live; @@ -159,7 +160,7 @@ decl_storage! { SetIdSession get(fn session_for_set): map SetId => Option; } add_extra_genesis { - config(authorities): Vec<(AuthorityId, AuthorityWeight)>; + config(authorities): AuthorityList; build(|config| Module::::initialize_authorities(&config.authorities)) } } @@ -174,6 +175,11 @@ decl_module! { // FIXME: https://github.com/paritytech/substrate/issues/1112 } + fn on_initialize() { + #[cfg(feature = "migrate-authorities")] + Self::migrate_authorities(); + } + fn on_finalize(block_number: T::BlockNumber) { // check for scheduled pending authority set changes if let Some(pending_change) = >::get() { @@ -199,7 +205,7 @@ decl_module! { // enact the change if we've reached the enacting block if block_number == pending_change.scheduled_at + pending_change.delay { - Authorities::put(&pending_change.next_authorities); + Self::set_grandpa_authorities(&pending_change.next_authorities); Self::deposit_event( Event::NewAuthorities(pending_change.next_authorities) ); @@ -241,8 +247,16 @@ decl_module! { impl Module { /// Get the current set of authorities, along with their respective weights. - pub fn grandpa_authorities() -> Vec<(AuthorityId, AuthorityWeight)> { - Authorities::get() + pub fn grandpa_authorities() -> AuthorityList { + storage::unhashed::get_or_default::(GRANDPA_AUTHORITIES_KEY).into() + } + + /// Set the current set of authorities, along with their respective weights. + fn set_grandpa_authorities(authorities: &AuthorityList) { + storage::unhashed::put( + GRANDPA_AUTHORITIES_KEY, + &VersionedAuthorityList::from(authorities), + ); } /// Schedule GRANDPA to pause starting in the given number of blocks. @@ -293,7 +307,7 @@ impl Module { /// No change should be signaled while any change is pending. Returns /// an error if a change is already pending. pub fn schedule_change( - next_authorities: Vec<(AuthorityId, AuthorityWeight)>, + next_authorities: AuthorityList, in_blocks: T::BlockNumber, forced: Option, ) -> Result { @@ -329,10 +343,20 @@ impl Module { >::deposit_log(log.into()); } - fn initialize_authorities(authorities: &[(AuthorityId, AuthorityWeight)]) { + fn initialize_authorities(authorities: &AuthorityList) { if !authorities.is_empty() { - assert!(Authorities::get().is_empty(), "Authorities are already initialized!"); - Authorities::put(authorities); + assert!( + Self::grandpa_authorities().is_empty(), + "Authorities are already initialized!" + ); + Self::set_grandpa_authorities(authorities); + } + } + + #[cfg(feature = "migrate-authorities")] + fn migrate_authorities() { + if Authorities::exists() { + Self::set_grandpa_authorities(&Authorities::take()); } } } diff --git a/srml/grandpa/src/mock.rs b/srml/grandpa/src/mock.rs index fcacbade20490..c6ea2075575c5 100644 --- a/srml/grandpa/src/mock.rs +++ b/srml/grandpa/src/mock.rs @@ -23,7 +23,7 @@ use runtime_io; use support::{impl_outer_origin, impl_outer_event, parameter_types}; use primitives::H256; use codec::{Encode, Decode}; -use crate::{AuthorityId, GenesisConfig, Trait, Module, ConsensusLog}; +use crate::{AuthorityId, AuthorityList, GenesisConfig, Trait, Module, ConsensusLog}; use substrate_finality_grandpa_primitives::GRANDPA_ENGINE_ID; impl_outer_origin!{ @@ -75,7 +75,7 @@ impl_outer_event!{ } } -pub fn to_authorities(vec: Vec<(u64, u64)>) -> Vec<(AuthorityId, u64)> { +pub fn to_authorities(vec: Vec<(u64, u64)>) -> AuthorityList { vec.into_iter() .map(|(id, weight)| (UintAuthorityId(id).to_public_key::(), weight)) .collect() diff --git a/srml/grandpa/src/tests.rs b/srml/grandpa/src/tests.rs index 2efeb4b5bf3cf..3d6a8752c5de3 100644 --- a/srml/grandpa/src/tests.rs +++ b/srml/grandpa/src/tests.rs @@ -308,3 +308,21 @@ fn time_slot_have_sane_ord() { ]; assert!(FIXTURE.windows(2).all(|f| f[0] < f[1])); } + +#[test] +#[cfg(feature = "migrate-authorities")] +fn authorities_migration() { + use sr_primitives::traits::OnInitialize; + + with_externalities(&mut new_test_ext(vec![]), || { + let authorities = to_authorities(vec![(1, 1), (2, 1), (3, 1)]); + + Authorities::put(authorities.clone()); + assert!(Grandpa::grandpa_authorities().is_empty()); + + Grandpa::on_initialize(1); + + assert!(!Authorities::exists()); + assert_eq!(Grandpa::grandpa_authorities(), authorities); + }); +}