From eb2c8cdf0cf118669c7b4f020565490e8e077728 Mon Sep 17 00:00:00 2001 From: Nikolaos Dymitriadis Date: Wed, 19 Feb 2025 10:25:24 +0100 Subject: [PATCH] change: Feed block production log from session validator management pallet Signed-off-by: Nikolaos Dymitriadis --- Cargo.lock | 3 +- node/node/src/inherent_data.rs | 10 +-- node/node/src/tests/inherent_data_tests.rs | 15 +++-- node/node/src/tests/runtime_api_mock.rs | 14 ++++ node/runtime/Cargo.toml | 2 + node/runtime/src/lib.rs | 44 +++++++++++-- .../src/ariadne_inherent_data_provider.rs | 2 +- .../block-production-log/Cargo.toml | 3 +- .../block-production-log/src/lib.rs | 58 +++++++++------- .../block-production-log/src/test.rs | 66 ++++++++++--------- 10 files changed, 145 insertions(+), 72 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8c15e43d6..cf8188cf8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10030,6 +10030,7 @@ dependencies = [ "sidechain-slots", "sp-api", "sp-block-builder", + "sp-block-production-log", "sp-block-rewards", "sp-consensus-aura", "sp-consensus-grandpa", @@ -10304,7 +10305,7 @@ dependencies = [ "async-trait", "hex", "parity-scale-codec", - "sealed_test", + "sp-api", "sp-core", "sp-inherents", "sp-runtime", diff --git a/node/node/src/inherent_data.rs b/node/node/src/inherent_data.rs index b06f46cf7..b62e72e4b 100644 --- a/node/node/src/inherent_data.rs +++ b/node/node/src/inherent_data.rs @@ -12,11 +12,12 @@ use sidechain_domain::{McBlockHash, ScEpochNumber}; use sidechain_mc_hash::McHashDataSource; use sidechain_mc_hash::McHashInherentDataProvider as McHashIDP; use sidechain_runtime::opaque::SessionKeys; -use sidechain_runtime::CrossChainPublic; use sidechain_runtime::{opaque::Block, BeneficiaryId}; +use sidechain_runtime::{BlockAuthor, CrossChainPublic}; use sidechain_slots::ScSlotConfig; use sp_api::ProvideRuntimeApi; -use sp_block_production_log::BlockProducerIdInherentProvider; +use sp_block_production_log::BlockAuthorInherentProvider; +use sp_block_production_log::BlockProductionLogApi; use sp_block_rewards::BlockBeneficiaryInherentProvider; use sp_blockchain::HeaderBackend; use sp_consensus_aura::{ @@ -56,13 +57,14 @@ where ScEpochNumber, >, T::Api: NativeTokenManagementApi, + T::Api: BlockProductionLogApi>, { type InherentDataProviders = ( AuraIDP, TimestampIDP, McHashIDP, AriadneIDP, - BlockProducerIdInherentProvider, + BlockAuthorInherentProvider, BlockBeneficiaryInherentProvider, NativeTokenIDP, ); @@ -101,7 +103,7 @@ where ) .await?; let block_producer_id_provider = - BlockProducerIdInherentProvider::from_env("SIDECHAIN_BLOCK_BENEFICIARY")?; + BlockAuthorInherentProvider::new(client.as_ref(), parent_hash)?; let block_beneficiary_provider = BlockBeneficiaryInherentProvider::::from_env( "SIDECHAIN_BLOCK_BENEFICIARY", diff --git a/node/node/src/tests/inherent_data_tests.rs b/node/node/src/tests/inherent_data_tests.rs index df7a24313..afbdea27f 100644 --- a/node/node/src/tests/inherent_data_tests.rs +++ b/node/node/src/tests/inherent_data_tests.rs @@ -5,14 +5,14 @@ use authority_selection_inherents::{ authority_selection_inputs::AuthoritySelectionInputs, mock::MockAuthoritySelectionDataSource, }; use hex_literal::hex; -use sidechain_domain::byte_string::SizedByteString; use sidechain_domain::{ MainchainBlock, McBlockHash, McBlockNumber, McEpochNumber, McSlotNumber, NativeTokenAmount, ScEpochNumber, }; use sidechain_mc_hash::mock::MockMcHashDataSource; +use sidechain_runtime::BlockAuthor; use sp_consensus_aura::Slot; -use sp_core::H256; +use sp_core::{ecdsa, H256}; use sp_inherents::CreateInherentDataProviders; use sp_inherents::{InherentData, InherentDataProvider}; use sp_native_token_management::mock::MockNativeTokenDataSource; @@ -97,11 +97,14 @@ async fn block_proposal_cidp_should_be_created_correctly() { .is_some()); assert_eq!( inherent_data - .get_data::>(&sp_block_production_log::INHERENT_IDENTIFIER) + .get_data::(&sp_block_production_log::INHERENT_IDENTIFIER) .unwrap(), - Some(SizedByteString::<32>(hex!( - "0000000000000000000000000000000000000000000000000000000000000001" - ))) + Some(BlockAuthor::ProBono( + ecdsa::Public::from_raw(hex!( + "000000000000000000000000000000000000000000000000000000000000000001" + )) + .into() + )) ); } diff --git a/node/node/src/tests/runtime_api_mock.rs b/node/node/src/tests/runtime_api_mock.rs index bb0fd1924..fa3faa800 100644 --- a/node/node/src/tests/runtime_api_mock.rs +++ b/node/node/src/tests/runtime_api_mock.rs @@ -1,6 +1,7 @@ use super::mock::mock_genesis_utxo; use authority_selection_inherents::authority_selection_inputs::AuthoritySelectionInputs; use authority_selection_inherents::CommitteeMember; +use hex_literal::hex; use sidechain_domain::*; use sidechain_mc_hash::McHashInherentDigest; use sidechain_runtime::opaque::SessionKeys; @@ -8,6 +9,7 @@ use sidechain_runtime::CrossChainPublic; use sp_api::{ApiRef, ProvideRuntimeApi}; use sp_blockchain::HeaderBackend; use sp_core::ecdsa; +use sp_core::{ed25519, sr25519}; use sp_runtime::traits::{Block as BlockT, Header as HeaderT, NumberFor, Zero}; use sp_runtime::Digest; use sp_sidechain::GetGenesisUtxo; @@ -107,6 +109,18 @@ sp_api::mock_impl_runtime_apis! { true } } + + impl sp_block_production_log::BlockProductionLogApi> for TestApi { + fn get_current_author() -> CommitteeMember { + CommitteeMember::permissioned( + ecdsa::Public::from_raw(hex!("000000000000000000000000000000000000000000000000000000000000000001")).into(), + SessionKeys { + aura: sr25519::Public::default().into(), + grandpa: ed25519::Public::default().into() + } + ) + } + } } impl HeaderBackend for TestApi { diff --git a/node/runtime/Cargo.toml b/node/runtime/Cargo.toml index 10fec763a..ae547486f 100644 --- a/node/runtime/Cargo.toml +++ b/node/runtime/Cargo.toml @@ -65,6 +65,7 @@ frame-system-benchmarking = { workspace = true, optional = true } # Local Dependencies sp-block-rewards = { workspace = true } +sp-block-production-log = { workspace = true } pallet-block-production-log = { workspace = true } pallet-block-rewards = { workspace = true } sp-sidechain = { workspace = true } @@ -153,6 +154,7 @@ std = [ "pallet-native-token-management/std", "sp-native-token-management/std", "pallet-session-runtime-stub/std", + "sp-block-production-log/std", ] runtime-benchmarks = [ diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 9989d713f..14829d9ad 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -27,6 +27,9 @@ use opaque::SessionKeys; use pallet_grandpa::AuthorityId as GrandpaId; use pallet_session_validator_management; use pallet_transaction_payment::{ConstFeeMultiplier, FungibleAdapter, Multiplier}; +use parity_scale_codec::MaxEncodedLen; +use parity_scale_codec::{Decode, Encode}; +use scale_info::TypeInfo; use session_manager::ValidatorManagementSessionManager; use sidechain_domain::{ NativeTokenAmount, PermissionedCandidateData, RegistrationData, ScEpochNumber, ScSlotNumber, @@ -34,7 +37,7 @@ use sidechain_domain::{ }; use sp_api::impl_runtime_apis; use sp_consensus_aura::sr25519::AuthorityId as AuraId; -use sp_core::{crypto::KeyTypeId, hexdisplay::HexDisplay, OpaqueMetadata}; +use sp_core::{crypto::KeyTypeId, OpaqueMetadata}; use sp_runtime::{ generic, impl_opaque_keys, traits::{ @@ -431,10 +434,10 @@ impl sp_sidechain::OnNewEpoch for LogBeneficiaries { let slot = pallet_aura::CurrentSlot::::get(); let block_production_log = BlockProductionLog::take_prefix(&slot); if let Some((s, b)) = block_production_log.first() { - log::info!("Block production log head: {} -> {}", **s, HexDisplay::from(&b.0)) + log::info!("Block production log head: {} -> {:?}", **s, b) }; if let Some((s, b)) = block_production_log.last() { - log::info!("Block production log tail: {} -> {}", **s, HexDisplay::from(&b.0)) + log::info!("Block production log tail: {} -> {:?}", **s, b) }; RuntimeDbWeight::get().reads_writes(1, 1) } @@ -455,8 +458,32 @@ impl pallet_block_rewards::Config for Runtime { type GetBlockRewardPoints = sp_block_rewards::SimpleBlockCount; } +#[derive(MaxEncodedLen, Encode, Decode, Clone, TypeInfo, PartialEq, Eq, Debug)] +pub enum BlockAuthor { + Incentivized(CrossChainPublic, StakePoolPublicKey), + ProBono(CrossChainPublic), +} +impl BlockAuthor { + pub fn id(&self) -> &CrossChainPublic { + match self { + Self::Incentivized(id, _ ) => id, + Self::ProBono(id) => id + } + } +} +impl From> for BlockAuthor { + fn from(value: CommitteeMember) -> Self { + match value { + CommitteeMember::Permissioned { id, .. } => BlockAuthor::ProBono(id), + CommitteeMember::Registered { id, stake_pool_pub_key, .. } => { + BlockAuthor::Incentivized(id, stake_pool_pub_key) + }, + } + } +} + impl pallet_block_production_log::Config for Runtime { - type BlockProducerId = BeneficiaryId; + type BlockProducerId = BlockAuthor; type WeightInfo = pallet_block_production_log::weights::SubstrateWeight; fn current_slot() -> sp_consensus_slots::Slot { @@ -871,6 +898,15 @@ impl_runtime_apis! { NativeTokenManagement::initialized() } } + + impl sp_block_production_log::BlockProductionLogApi> for Runtime { + fn get_current_author() -> CommitteeMember { + let mut committee = SessionCommitteeManagement::get_current_committee().1; + let slot = pallet_aura::CurrentSlot::::get(); + // this implementation is compatible with round-robin employed by Aura + committee.remove(*slot as usize % committee.len()) + } + } } #[cfg(test)] diff --git a/toolkit/primitives/authority-selection-inherents/src/ariadne_inherent_data_provider.rs b/toolkit/primitives/authority-selection-inherents/src/ariadne_inherent_data_provider.rs index 28694baf0..8d9e30e03 100644 --- a/toolkit/primitives/authority-selection-inherents/src/ariadne_inherent_data_provider.rs +++ b/toolkit/primitives/authority-selection-inherents/src/ariadne_inherent_data_provider.rs @@ -2,7 +2,6 @@ use crate::authority_selection_inputs::AuthoritySelectionDataSource; use crate::authority_selection_inputs::AuthoritySelectionInputs; use parity_scale_codec::{Decode, Encode}; -use sp_session_validator_management::CommitteeMember as CommitteeMemberT; #[cfg(feature = "std")] use { crate::authority_selection_inputs::AuthoritySelectionInputsCreationError, @@ -13,6 +12,7 @@ use { sp_consensus_slots::Slot, sp_inherents::{InherentData, InherentIdentifier}, sp_runtime::traits::Block as BlockT, + sp_session_validator_management::CommitteeMember as CommitteeMemberT, sp_session_validator_management::{ InherentError, MainChainScripts, SessionValidatorManagementApi, INHERENT_IDENTIFIER, }, diff --git a/toolkit/primitives/block-production-log/Cargo.toml b/toolkit/primitives/block-production-log/Cargo.toml index 137fb7282..e2f02849e 100644 --- a/toolkit/primitives/block-production-log/Cargo.toml +++ b/toolkit/primitives/block-production-log/Cargo.toml @@ -7,6 +7,7 @@ license = "Apache-2.0" [dependencies] async-trait = { workspace = true } parity-scale-codec = { workspace = true } +sp-api = { workspace = true } sp-core = { workspace = true, optional = true } sp-inherents = { workspace = true } sp-runtime = { workspace = true } @@ -14,7 +15,6 @@ thiserror = { workspace = true } [dev-dependencies] hex = { workspace = true } -sealed_test = { workspace = true } [features] default = ["std"] @@ -26,4 +26,5 @@ std = [ "sp-core/std", "sp-inherents/std", "sp-runtime/std", + "sp-api/std", ] diff --git a/toolkit/primitives/block-production-log/src/lib.rs b/toolkit/primitives/block-production-log/src/lib.rs index 6c896d9a0..363431b76 100644 --- a/toolkit/primitives/block-production-log/src/lib.rs +++ b/toolkit/primitives/block-production-log/src/lib.rs @@ -1,12 +1,16 @@ #![cfg_attr(not(feature = "std"), no_std)] +extern crate alloc; + #[cfg(test)] mod test; -use parity_scale_codec::Encode; +use core::error::Error; +use parity_scale_codec::{Decode, Encode}; use sp_inherents::{InherentIdentifier, IsFatalError}; +use sp_runtime::traits::Block as BlockT; #[cfg(feature = "std")] -use {parity_scale_codec::Decode, sp_inherents::InherentData}; +use {sp_api::ProvideRuntimeApi, sp_inherents::InherentData}; pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"blprdlog"; @@ -14,7 +18,7 @@ pub const INHERENT_IDENTIFIER: InherentIdentifier = *b"blprdlog"; #[cfg_attr(not(feature = "std"), derive(Debug))] #[cfg_attr(feature = "std", derive(Decode, thiserror::Error, sp_runtime::RuntimeDebug))] pub enum InherentError { - #[cfg_attr(feature = "std", error("Block Producer Id inherent must be provided every block"))] + #[cfg_attr(feature = "std", error("Block Author inherent must be provided every block"))] InherentRequired, } impl IsFatalError for InherentError { @@ -25,41 +29,40 @@ impl IsFatalError for InherentError { #[cfg(feature = "std")] #[derive(Debug)] -pub struct BlockProducerIdInherentProvider { - pub id: T, +pub struct BlockAuthorInherentProvider { + pub author: Author, } #[cfg(feature = "std")] -impl BlockProducerIdInherentProvider -where - T: TryFrom> + Send + Sync + Encode, - >>::Error: std::fmt::Debug, -{ - pub fn from_env(env_var: &str) -> Result { - let env_var_value = std::env::var(env_var).map_err(|_| { - format!("Block Producer Id environment variable '{env_var}' is not set") - })?; - let bytes = sp_core::bytes::from_hex(&env_var_value) - .map_err(|_| format!("Block Producer Id environment variable '{env_var}' value '{env_var_value}' is not a valid hex string"))?; - let id = T::try_from(bytes.clone()).map_err(|e| { - format!("Could not convert '{env_var_value}' into Block Producer Id. Cause: {e:#?}") - })?; +impl BlockAuthorInherentProvider { + pub fn new( + client: &C, + parent_hash: Block::Hash, + ) -> Result> + where + Member: Decode, + Block: BlockT, + C: ProvideRuntimeApi, + C::Api: BlockProductionLogApi, + Author: From, + { + let author: Author = client.runtime_api().get_current_author(parent_hash)?.into(); - Ok(BlockProducerIdInherentProvider { id }) + Ok(BlockAuthorInherentProvider { author }) } } #[cfg(feature = "std")] #[async_trait::async_trait] -impl sp_inherents::InherentDataProvider for BlockProducerIdInherentProvider +impl sp_inherents::InherentDataProvider for BlockAuthorInherentProvider where - T: TryFrom> + Send + Sync + Encode, + T: Send + Sync + Encode + Decode, { async fn provide_inherent_data( &self, inherent_data: &mut InherentData, ) -> Result<(), sp_inherents::Error> { - inherent_data.put_data(INHERENT_IDENTIFIER, &self.id) + inherent_data.put_data(INHERENT_IDENTIFIER, &self.author) } async fn try_handle_error( @@ -75,3 +78,12 @@ where } } } + +sp_api::decl_runtime_apis! { + pub trait BlockProductionLogApi + where + Member: Decode + { + fn get_current_author() -> Member; + } +} diff --git a/toolkit/primitives/block-production-log/src/test.rs b/toolkit/primitives/block-production-log/src/test.rs index b2fd3bdc6..5eb988f30 100644 --- a/toolkit/primitives/block-production-log/src/test.rs +++ b/toolkit/primitives/block-production-log/src/test.rs @@ -1,42 +1,44 @@ -use crate::BlockProducerIdInherentProvider; -use sealed_test::prelude::*; +use crate::{BlockAuthorInherentProvider, BlockProductionLogApi}; +use sp_api::ApiRef; +use sp_api::ProvideRuntimeApi; +use sp_runtime::traits::Block as BlockT; -const BLOCK_PRODUCER_ID_ENV_VAR_VALUE: &str = - "020a1091341fe5664bfa1782d5e04779689068c916b04cb365ec3153755684d9"; +pub type Block = sp_runtime::generic::Block< + sp_runtime::generic::Header, + sp_runtime::OpaqueExtrinsic, +>; -#[sealed_test] -fn from_env_happy_path() { - std::env::set_var("TEST_VAR_NAME", BLOCK_PRODUCER_ID_ENV_VAR_VALUE); - let provider = BlockProducerIdInherentProvider::<[u8; 32]>::from_env("TEST_VAR_NAME").unwrap(); - assert_eq!(hex::encode(provider.id), BLOCK_PRODUCER_ID_ENV_VAR_VALUE) -} +type Member = u32; +type Author = u64; -#[sealed_test] -fn from_env_happy_path_with_0x_prefix() { - let env_var_value = format!("0x{}", BLOCK_PRODUCER_ID_ENV_VAR_VALUE); - std::env::set_var("TEST_VAR_NAME", env_var_value); - let provider = BlockProducerIdInherentProvider::<[u8; 32]>::from_env("TEST_VAR_NAME").unwrap(); - assert_eq!(hex::encode(provider.id), BLOCK_PRODUCER_ID_ENV_VAR_VALUE) +#[derive(Clone, Default)] +struct TestApi { + author: Member, } -#[sealed_test] -fn error_when_env_var_is_not_set() { - std::env::remove_var("TEST_VAR_NAME"); - let err = BlockProducerIdInherentProvider::<[u8; 32]>::from_env("TEST_VAR_NAME").unwrap_err(); - assert_eq!(err, "Block Producer Id environment variable 'TEST_VAR_NAME' is not set"); +impl ProvideRuntimeApi for TestApi { + type Api = Self; + fn runtime_api(&self) -> ApiRef { + (*self).clone().into() + } } -#[sealed_test] -fn error_when_invalid_hex() { - std::env::set_var("TEST_VAR_NAME", "0xabcdzzzzz"); - let err = BlockProducerIdInherentProvider::<[u8; 32]>::from_env("TEST_VAR_NAME").unwrap_err(); - assert_eq!(err, "Block Producer Id environment variable 'TEST_VAR_NAME' value '0xabcdzzzzz' is not a valid hex string"); +sp_api::mock_impl_runtime_apis! { + impl BlockProductionLogApi for TestApi { + + fn get_current_author() -> Member { + self.author + } + } } -#[sealed_test] -fn error_when_failed_try_from() { - std::env::set_var("TEST_VAR_NAME", BLOCK_PRODUCER_ID_ENV_VAR_VALUE); - // Value is 32 bytes, expected 33 - let err = BlockProducerIdInherentProvider::<[u8; 33]>::from_env("TEST_VAR_NAME").unwrap_err(); - assert!(err.starts_with("Could not convert '020a1091341fe5664bfa1782d5e04779689068c916b04cb365ec3153755684d9' into Block Producer Id. Cause:")); +#[test] +fn provides_author_based_on_runtime_api() { + let mock_api = TestApi { author: 102 }; + + let provider = + BlockAuthorInherentProvider::::new(&mock_api, ::Hash::default()) + .expect("Should not fail"); + + assert_eq!(provider.author, mock_api.author.into()); }