diff --git a/Cargo.lock b/Cargo.lock index 3ba3bb4109..b4ee58cef0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6409,6 +6409,7 @@ dependencies = [ "hex-literal 1.0.0", "k256", "log", + "pallet-balances", "parity-scale-codec", "scale-info", "sidechain-domain", diff --git a/changelog.md b/changelog.md index 6a341be614..7b610c39df 100644 --- a/changelog.md +++ b/changelog.md @@ -8,6 +8,8 @@ This changelog is based on [Keep A Changelog](https://keepachangelog.com/en/1.1. ## Changed +* `pallet-block-producer-metadata` is updated with a configurable fee for inserting the metadata, to make attacks on unbounded storage economically infeasible + ## Added ## Fixed diff --git a/demo/runtime/src/lib.rs b/demo/runtime/src/lib.rs index 96535a2453..982bbaa824 100644 --- a/demo/runtime/src/lib.rs +++ b/demo/runtime/src/lib.rs @@ -349,7 +349,7 @@ impl pallet_balances::Config for Runtime { type WeightInfo = pallet_balances::weights::SubstrateWeight; type FreezeIdentifier = (); type MaxFreezes = (); - type RuntimeHoldReason = (); + type RuntimeHoldReason = RuntimeHoldReason; type RuntimeFreezeReason = RuntimeFreezeReason; type DoneSlashHandler = (); } @@ -591,6 +591,11 @@ impl pallet_block_producer_fees::Config for Runtime { type BenchmarkHelper = PalletBlockProducerFeesBenchmarkHelper; } +parameter_types! { + /// Amount of tokens to hold when upserting block producer metadata. + pub const MetadataHoldAmount: Balance = 1_000_000; +} + impl pallet_block_producer_metadata::Config for Runtime { type WeightInfo = pallet_block_producer_metadata::weights::SubstrateWeight; @@ -600,6 +605,10 @@ impl pallet_block_producer_metadata::Config for Runtime { Sidechain::genesis_utxo() } + type Currency = Balances; + type HoldAmount = MetadataHoldAmount; + type RuntimeHoldReason = RuntimeHoldReason; + #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = PalletBlockProducerMetadataBenchmarkHelper; } diff --git a/toolkit/block-producer-metadata/pallet/Cargo.toml b/toolkit/block-producer-metadata/pallet/Cargo.toml index b2b5651b1b..ea14e4c3b2 100644 --- a/toolkit/block-producer-metadata/pallet/Cargo.toml +++ b/toolkit/block-producer-metadata/pallet/Cargo.toml @@ -23,7 +23,7 @@ hex-literal = { workspace = true, optional = true } sp-core = { workspace = true, optional = true } sp-block-producer-metadata = { workspace = true } sp-runtime = { workspace = true, optional = true } -sp-io = { workspace = true, optional = true} +sp-io = { workspace = true, optional = true } [dev-dependencies] sp-core = { workspace = true } @@ -31,6 +31,7 @@ sp-io = { workspace = true } sp-runtime = { workspace = true } hex-literal = { workspace = true } k256 = { workspace = true } +pallet-balances = { workspace = true } [features] default = ["std"] @@ -45,6 +46,7 @@ std = [ "sp-std/std", "sp-core?/std", "sp-block-producer-metadata/std", + "pallet-balances/std", ] runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", @@ -53,5 +55,5 @@ runtime-benchmarks = [ "sp-runtime/runtime-benchmarks", "hex-literal", "sp-core", - "sp-io" + "sp-io", ] diff --git a/toolkit/block-producer-metadata/pallet/src/benchmarking.rs b/toolkit/block-producer-metadata/pallet/src/benchmarking.rs index 25e03a943f..e4740379be 100644 --- a/toolkit/block-producer-metadata/pallet/src/benchmarking.rs +++ b/toolkit/block-producer-metadata/pallet/src/benchmarking.rs @@ -72,9 +72,10 @@ pub trait BenchmarkHelper { fn cross_chain_signature() -> CrossChainSignature; } -#[benchmarks] +#[benchmarks(where ::Currency: frame_support::traits::tokens::fungible::Mutate<::AccountId>)] mod benchmarks { use super::*; + use frame_support::traits::{Get, tokens::fungible::Mutate}; #[benchmark] fn upsert_metadata() { @@ -82,8 +83,12 @@ mod benchmarks { let cross_chain_pub_key = T::BenchmarkHelper::cross_chain_pub_key(); let cross_chain_signature = T::BenchmarkHelper::cross_chain_signature(); + // Create an account and fund it with sufficient balance + let caller: T::AccountId = account("caller", 0, 0); + let _ = T::Currency::mint_into(&caller, T::HoldAmount::get() * 2u32.into()); + #[extrinsic_call] - _(RawOrigin::None, metadata, cross_chain_signature, cross_chain_pub_key); + _(RawOrigin::Signed(caller), metadata, cross_chain_signature, cross_chain_pub_key); } impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test); diff --git a/toolkit/block-producer-metadata/pallet/src/lib.rs b/toolkit/block-producer-metadata/pallet/src/lib.rs index 09fc863031..3e8401f675 100644 --- a/toolkit/block-producer-metadata/pallet/src/lib.rs +++ b/toolkit/block-producer-metadata/pallet/src/lib.rs @@ -46,6 +46,9 @@ //! type WeightInfo = pallet_block_producer_metadata::weights::SubstrateWeight; //! //! type BlockProducerMetadata = BlockProducerMetadata; +//! type Currency = Balances; +//! type HoldAmount = MetadataHoldAmount; +//! type RuntimeHoldReason = RuntimeHoldReason; //! //! fn genesis_utxo() -> sidechain_domain::UtxoId { //! Sidechain::genesis_utxo() @@ -55,6 +58,8 @@ //! //! Here, besides providing the metadata type and using weights already provided with the pallet, we are also //! wiring the `genesis_utxo` function to fetch the chain's genesis UTXO from the `pallet_sidechain` pallet. +//! Currency, HoldAmount, and RuntimeHoldReason types are required to configure the deposit mechanism for occupying storage. +//! Removing Metadata is not supported, so this deposit is currently ethernal. //! //! At this point, the pallet is ready to be used. //! @@ -80,6 +85,9 @@ //! `sign-block-producer-metadata` command provided by the chain's node. This command returns the signature //! and the metadata encoded as hex bytes. //! +//! When metadata is inserted for the first time, a deposit is held from the caller's account. Updates to existing +//! metadata do not require additional deposits. +//! //! After the signature has been obtained, the user should submit the `upsert_metadata` extrinsic (eg. using PolkadotJS) //! providing: //! - *metadata value*: when using PolkadotJS UI, care must be taken to submit the same values that were passed to the CLI @@ -102,20 +110,27 @@ mod mock; #[cfg(test)] mod tests; +use frame_support::traits::tokens::fungible::Inspect; use parity_scale_codec::Encode; use sidechain_domain::{CrossChainKeyHash, CrossChainPublicKey}; use sp_block_producer_metadata::MetadataSignedMessage; +type BalanceOf = + <::Currency as Inspect<::AccountId>>::Balance; + #[frame_support::pallet] pub mod pallet { use super::*; use crate::weights::WeightInfo; - use frame_support::pallet_prelude::*; - use frame_system::pallet_prelude::OriginFor; + use frame_support::{ + pallet_prelude::*, + traits::{Get, tokens::fungible::MutateHold}, + }; + use frame_system::{ensure_signed, pallet_prelude::OriginFor}; use sidechain_domain::{CrossChainSignature, UtxoId}; /// Current version of the pallet - pub const PALLET_VERSION: u32 = 1; + pub const PALLET_VERSION: u32 = 2; #[pallet::pallet] pub struct Pallet(_); @@ -131,6 +146,16 @@ pub mod pallet { /// Should return the chain's genesis UTXO fn genesis_utxo() -> UtxoId; + /// The currency used for holding tokens + type Currency: MutateHold; + + /// The amount of tokens to hold when upserting metadata + #[pallet::constant] + type HoldAmount: Get>; + + /// The runtime's hold reason type + type RuntimeHoldReason: From; + /// Helper providing mock values for use in benchmarks #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper: benchmarking::BenchmarkHelper; @@ -145,16 +170,26 @@ pub mod pallet { QueryKind = OptionQuery, >; + /// Hold reasons for this pallet + #[pallet::composite_enum] + pub enum HoldReason { + /// Tokens held as deposit for block producer metadata + MetadataDeposit, + } + /// Error type returned by this pallet's extrinsic #[pallet::error] pub enum Error { /// Signals that the signature submitted to `upsert_metadata` does not match the metadata and public key InvalidMainchainSignature, + /// Insufficient balance to hold tokens as fee for upserting block producer metadata + InsufficientBalance, } #[pallet::call] impl Pallet { /// Inserts or updates metadata for the block producer identified by `cross_chain_pub_key`. + /// Holds a constant amount from the caller's account as a deposit for including metadata on the chain. /// /// Arguments: /// - `metadata`: new metadata value @@ -165,11 +200,12 @@ pub mod pallet { #[pallet::call_index(0)] #[pallet::weight(T::WeightInfo::upsert_metadata())] pub fn upsert_metadata( - _origin: OriginFor, + origin: OriginFor, metadata: T::BlockProducerMetadata, signature: CrossChainSignature, cross_chain_pub_key: CrossChainPublicKey, ) -> DispatchResult { + let origin_account = ensure_signed(origin)?; let genesis_utxo = T::genesis_utxo(); let cross_chain_key_hash = cross_chain_pub_key.hash(); @@ -185,6 +221,15 @@ pub mod pallet { ensure!(is_valid_signature, Error::::InvalidMainchainSignature); + if BlockProducerMetadataStorage::::get(cross_chain_key_hash).is_none() { + T::Currency::hold( + &HoldReason::MetadataDeposit.into(), + &origin_account, + T::HoldAmount::get(), + ) + .map_err(|_| Error::::InsufficientBalance)?; + } + BlockProducerMetadataStorage::::insert(cross_chain_key_hash, metadata); Ok(()) } diff --git a/toolkit/block-producer-metadata/pallet/src/mock.rs b/toolkit/block-producer-metadata/pallet/src/mock.rs index d0b03d4de4..45261f226e 100644 --- a/toolkit/block-producer-metadata/pallet/src/mock.rs +++ b/toolkit/block-producer-metadata/pallet/src/mock.rs @@ -1,6 +1,6 @@ -use frame_support::traits::ConstU32; +use frame_support::traits::{ConstU32, ConstU128}; use frame_support::{ - construct_runtime, + construct_runtime, parameter_types, traits::{ConstU16, ConstU64}, }; use hex_literal::hex; @@ -16,10 +16,12 @@ use sp_runtime::{ pub type Block = frame_system::mocking::MockBlock; pub type AccountId = AccountId32; +pub type Balance = u128; construct_runtime! { pub enum Test { System: frame_system, + Balances: pallet_balances, BlockProducerMetadata: crate::pallet } } @@ -39,7 +41,7 @@ impl frame_system::Config for Test { type BlockHashCount = ConstU64<250>; type Version = (); type PalletInfo = PalletInfo; - type AccountData = (); + type AccountData = pallet_balances::AccountData; type OnNewAccount = (); type OnKilledAccount = (); type SystemWeightInfo = (); @@ -57,6 +59,27 @@ impl frame_system::Config for Test { type PostTransactions = (); } +impl pallet_balances::Config for Test { + type MaxLocks = ConstU32<50>; + type MaxReserves = (); + type ReserveIdentifier = [u8; 8]; + type Balance = Balance; + type RuntimeEvent = RuntimeEvent; + type DustRemoval = (); + type ExistentialDeposit = ConstU128<1>; + type AccountStore = System; + type WeightInfo = pallet_balances::weights::SubstrateWeight; + type FreezeIdentifier = (); + type MaxFreezes = (); + type RuntimeHoldReason = RuntimeHoldReason; + type RuntimeFreezeReason = (); + type DoneSlashHandler = (); +} + +parameter_types! { + pub const MetadataHoldAmount: Balance = 1000; +} + #[derive( Clone, Debug, MaxEncodedLen, Encode, Decode, DecodeWithMemTracking, PartialEq, Eq, TypeInfo, )] @@ -90,16 +113,28 @@ impl crate::benchmarking::BenchmarkHelper } } +pub(crate) const FUNDED_ACCOUNT: AccountId32 = AccountId32::new([1; 32]); + impl crate::pallet::Config for Test { type WeightInfo = (); type BlockProducerMetadata = BlockProducerUrlMetadata; fn genesis_utxo() -> UtxoId { UtxoId::new(hex!("59104061ffa0d66f9ba0135d6fc6a884a395b10f8ae9cb276fc2c3bfdfedc260"), 1) } + type Currency = Balances; + type HoldAmount = MetadataHoldAmount; + type RuntimeHoldReason = RuntimeHoldReason; #[cfg(feature = "runtime-benchmarks")] type BenchmarkHelper = PalletBlockProducerMetadataBenchmarkHelper; } pub fn new_test_ext() -> sp_io::TestExternalities { - frame_system::GenesisConfig::::default().build_storage().unwrap().into() + let mut t = frame_system::GenesisConfig::::default().build_storage().unwrap(); + pallet_balances::GenesisConfig:: { + balances: vec![(FUNDED_ACCOUNT, 100_000)], + dev_accounts: None, + } + .assimilate_storage(&mut t) + .unwrap(); + t.into() } diff --git a/toolkit/block-producer-metadata/pallet/src/tests.rs b/toolkit/block-producer-metadata/pallet/src/tests.rs index e7475eb2f1..cb23ad6b21 100644 --- a/toolkit/block-producer-metadata/pallet/src/tests.rs +++ b/toolkit/block-producer-metadata/pallet/src/tests.rs @@ -1,5 +1,5 @@ use super::*; -use frame_support::assert_ok; +use frame_support::{assert_noop, assert_ok, traits::tokens::fungible::InspectHold}; use frame_system::pallet_prelude::OriginFor; use hex_literal::hex; use mock::*; @@ -8,10 +8,12 @@ use sidechain_domain::*; use sp_runtime::{AccountId32, BoundedVec}; #[test] -fn saves_new_metadata() { +fn saves_new_metadata_and_holds_fee() { new_test_ext().execute_with(|| { + let initial_balance = Balances::free_balance(&FUNDED_ACCOUNT); + assert_ok!(super::Pallet::::upsert_metadata( - OriginFor::::signed(AccountId32::new([1; 32])), + OriginFor::::signed(FUNDED_ACCOUNT), url_metadata_1(), cross_chain_signature_1(), cross_chain_pub_key(), @@ -21,14 +23,24 @@ fn saves_new_metadata() { Pallet::::get_metadata_for(&cross_chain_pub_key()), Some(url_metadata_1()) ); + + let final_balance = Balances::free_balance(&FUNDED_ACCOUNT); + assert_eq!(final_balance, initial_balance - MetadataHoldAmount::get()); + + // Check that the amount is held, not burned + let held_balance = Balances::balance_on_hold( + &RuntimeHoldReason::BlockProducerMetadata(crate::HoldReason::MetadataDeposit), + &FUNDED_ACCOUNT, + ); + assert_eq!(held_balance, MetadataHoldAmount::get()); }) } #[test] -fn updates_metadata() { +fn updates_metadata_without_holding_additional_fee() { new_test_ext().execute_with(|| { assert_ok!(super::Pallet::::upsert_metadata( - OriginFor::::signed(AccountId32::new([1; 32])), + OriginFor::::signed(FUNDED_ACCOUNT), url_metadata_1(), cross_chain_signature_1(), cross_chain_pub_key(), @@ -38,9 +50,14 @@ fn updates_metadata() { Pallet::::get_metadata_for(&cross_chain_pub_key()), Some(url_metadata_1()) ); + let balance_after_insert = Balances::free_balance(&FUNDED_ACCOUNT); + let held_after_insert = Balances::balance_on_hold( + &RuntimeHoldReason::BlockProducerMetadata(crate::HoldReason::MetadataDeposit), + &FUNDED_ACCOUNT, + ); assert_ok!(super::Pallet::::upsert_metadata( - OriginFor::::signed(AccountId32::new([1; 32])), + OriginFor::::signed(FUNDED_ACCOUNT), url_metadata_2(), cross_chain_signature_2(), cross_chain_pub_key(), @@ -50,6 +67,15 @@ fn updates_metadata() { Pallet::::get_metadata_for(&cross_chain_pub_key()), Some(url_metadata_2()) ); + + let balance_after_update = Balances::free_balance(&FUNDED_ACCOUNT); + let held_after_update = Balances::balance_on_hold( + &RuntimeHoldReason::BlockProducerMetadata(crate::HoldReason::MetadataDeposit), + &FUNDED_ACCOUNT, + ); + + assert_eq!(balance_after_insert, balance_after_update); + assert_eq!(held_after_insert, held_after_update); }) } @@ -58,7 +84,7 @@ fn rejects_invalid_signature() { new_test_ext().execute_with(|| { assert_eq!( super::Pallet::::upsert_metadata( - OriginFor::::signed(AccountId32::new([1; 32])), + OriginFor::::signed(FUNDED_ACCOUNT), url_metadata_2(), cross_chain_signature_1(), cross_chain_pub_key(), @@ -69,6 +95,23 @@ fn rejects_invalid_signature() { }) } +#[test] +fn fails_with_insufficient_balance() { + new_test_ext().execute_with(|| { + let poor_account = AccountId32::new([13; 32]); + + assert_noop!( + super::Pallet::::upsert_metadata( + OriginFor::::signed(poor_account), + url_metadata_1(), + cross_chain_signature_1(), + cross_chain_pub_key(), + ), + Error::::InsufficientBalance + ); + }) +} + fn url_metadata_1() -> BlockProducerUrlMetadata { BlockProducerUrlMetadata { url: BoundedVec::try_from("https://cool.stuff/spo.json".as_bytes().to_vec()).unwrap(), diff --git a/toolkit/block-producer-metadata/pallet/src/weights.rs b/toolkit/block-producer-metadata/pallet/src/weights.rs index f5fd58cf4d..725a806e4f 100644 --- a/toolkit/block-producer-metadata/pallet/src/weights.rs +++ b/toolkit/block-producer-metadata/pallet/src/weights.rs @@ -1,10 +1,10 @@ //! Autogenerated weights for pallet_block_producer_metadata //! -//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 46.0.0 -//! DATE: 2025-03-27, STEPS: `500`, REPEAT: `200`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 47.0.0 +//! DATE: 2025-06-17, STEPS: `500`, REPEAT: `200`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` -//! HOSTNAME: `Nikolaoss-MacBook-Pro.local`, CPU: `` +//! HOSTNAME: `framework-13`, CPU: `AMD Ryzen 7 7840U w/ Radeon 780M Graphics` //! EXECUTION: , WASM-EXECUTION: Compiled, CHAIN: None, DB CACHE: 1024 // Executed Command: @@ -13,7 +13,7 @@ // benchmark // pallet // --runtime -// target/release/wbuild/sidechain-runtime/sidechain_runtime.compact.compressed.wasm +// target/release/wbuild/partner-chains-demo-runtime/partner_chains_demo_runtime.compact.compressed.wasm // --pallet // pallet_block_producer_metadata // --extrinsic @@ -44,16 +44,18 @@ pub struct SubstrateWeight(PhantomData); impl WeightInfo for SubstrateWeight { /// Storage: `Sidechain::GenesisUtxo` (r:1 w:0) /// Proof: `Sidechain::GenesisUtxo` (`max_values`: Some(1), `max_size`: Some(34), added: 529, mode: `MaxEncodedLen`) - /// Storage: `BlockProducerMetadata::BlockProducerMetadataStorage` (r:0 w:1) + /// Storage: `BlockProducerMetadata::BlockProducerMetadataStorage` (r:1 w:1) /// Proof: `BlockProducerMetadata::BlockProducerMetadataStorage` (`max_values`: None, `max_size`: Some(590), added: 3065, mode: `MaxEncodedLen`) + /// Storage: `System::Account` (r:1 w:1) + /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) fn upsert_metadata() -> Weight { // Proof Size summary in bytes: - // Measured: `90` - // Estimated: `1519` - // Minimum execution time: 203_000_000 picoseconds. - Weight::from_parts(209_000_000, 1519) - .saturating_add(T::DbWeight::get().reads(1_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) + // Measured: `197` + // Estimated: `4055` + // Minimum execution time: 231_755_000 picoseconds. + Weight::from_parts(234_791_000, 4055) + .saturating_add(T::DbWeight::get().reads(3_u64)) + .saturating_add(T::DbWeight::get().writes(2_u64)) } } @@ -61,15 +63,17 @@ impl WeightInfo for SubstrateWeight { impl WeightInfo for () { /// Storage: `Sidechain::GenesisUtxo` (r:1 w:0) /// Proof: `Sidechain::GenesisUtxo` (`max_values`: Some(1), `max_size`: Some(34), added: 529, mode: `MaxEncodedLen`) - /// Storage: `BlockProducerMetadata::BlockProducerMetadataStorage` (r:0 w:1) + /// Storage: `BlockProducerMetadata::BlockProducerMetadataStorage` (r:1 w:1) /// Proof: `BlockProducerMetadata::BlockProducerMetadataStorage` (`max_values`: None, `max_size`: Some(590), added: 3065, mode: `MaxEncodedLen`) + /// Storage: `System::Account` (r:1 w:1) + /// Proof: `System::Account` (`max_values`: None, `max_size`: Some(128), added: 2603, mode: `MaxEncodedLen`) fn upsert_metadata() -> Weight { // Proof Size summary in bytes: - // Measured: `90` - // Estimated: `1519` - // Minimum execution time: 203_000_000 picoseconds. - Weight::from_parts(209_000_000, 1519) - .saturating_add(RocksDbWeight::get().reads(1_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) + // Measured: `197` + // Estimated: `4055` + // Minimum execution time: 231_755_000 picoseconds. + Weight::from_parts(234_791_000, 4055) + .saturating_add(RocksDbWeight::get().reads(3_u64)) + .saturating_add(RocksDbWeight::get().writes(2_u64)) } }