diff --git a/Cargo.lock b/Cargo.lock index ba1321e0..1ee172a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10959,11 +10959,13 @@ dependencies = [ "frame-support", "frame-system", "hex", + "itertools 0.12.1", "native", "native-api", "on-chain-table", "pallet-balances", "pallet-commitments", + "pallet-migrations", "pallet-permissions", "pallet-session", "pallet-staking", @@ -19429,6 +19431,7 @@ dependencies = [ "pallet-indexing", "pallet-indices", "pallet-keystore", + "pallet-migrations", "pallet-multisig", "pallet-offences", "pallet-permissions", diff --git a/Cargo.toml b/Cargo.toml index 0de973f7..c950d0fe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -144,6 +144,7 @@ pallet-balances = { version = "39.0.0", default-features = false } pallet-grandpa = { version = "38.0.0", default-features = false } pallet-sudo = { version = "38.0.0", default-features = false } pallet-multisig = { version = "38.0.0", default-features = false } +pallet-migrations = { version = "8.0.0", default-features = false } pallet-timestamp = { version = "37.0.0", default-features = false } pallet-transaction-payment-rpc-runtime-api = { version = "38.0.0", default-features = false } scale-info = { version = "2.11.1", default-features = false } diff --git a/pallets/indexing/Cargo.toml b/pallets/indexing/Cargo.toml index 79b496af..96ee5d5d 100644 --- a/pallets/indexing/Cargo.toml +++ b/pallets/indexing/Cargo.toml @@ -23,11 +23,13 @@ scale-info = { features = [ frame-benchmarking = { optional = true, workspace = true } frame-support.workspace = true frame-system.workspace = true +itertools.workspace = true pallet-commitments.workspace = true pallet-permissions.workspace = true pallet-tables.workspace = true pallet-system-tables.workspace = true pallet-session.workspace = true +pallet-migrations.workspace = true hex.workspace = true sxt-core.workspace = true sp-runtime = { workspace = true, default-features = false } @@ -70,6 +72,7 @@ std = [ "pallet-balances/std", "pallet-staking/std", "pallet-system-tables/std", + "pallet-migrations/std", ] runtime-benchmarks = [ "frame-benchmarking/runtime-benchmarks", @@ -78,9 +81,11 @@ runtime-benchmarks = [ "pallet-staking/runtime-benchmarks", "pallet-system-tables/runtime-benchmarks", "sp-staking/runtime-benchmarks", + "pallet-migrations/runtime-benchmarks", "dep:proof-of-sql-commitment-map", ] try-runtime = [ "frame-support/try-runtime", "frame-system/try-runtime", + "pallet-migrations/try-runtime", ] diff --git a/pallets/indexing/src/benchmarking.rs b/pallets/indexing/src/benchmarking.rs index ac8d89cb..e2898711 100644 --- a/pallets/indexing/src/benchmarking.rs +++ b/pallets/indexing/src/benchmarking.rs @@ -3,6 +3,7 @@ use alloc::vec; use frame_benchmarking::v2::*; use frame_system::RawOrigin; +use sp_core::Hasher; use super::*; #[cfg(test)] @@ -13,12 +14,15 @@ use crate::Pallet as Indexing; #[allow(clippy::multiple_bound_locations)] #[instance_benchmarks(where I: NativeApi)] mod benchmarks { + use frame_support::migrations::SteppedMigration; + use frame_support::weights::WeightMeter; use native_api::NativeApi; use pallet_tables::{CommitmentCreationCmd, UpdateTable}; use proof_of_sql_commitment_map::CommitmentSchemeFlags; use sxt_core::permissions::{IndexingPalletPermission, PermissionLevel, PermissionList}; use sxt_core::tables::{ InsertQuorumSize, + QuorumScope, Source, TableIdentifier, TableName, @@ -162,6 +166,66 @@ mod benchmarks { assert!(Indexing::::final_data(batch_id).is_some()); } + #[benchmark] + fn migration_v0_v1_step() { + let submitters_by_scope = (0..MAX_SUBMITTERS * 2) + .map(|submitter_num| { + let scope = if submitter_num % 2 == 0 { + QuorumScope::Public + } else { + QuorumScope::Privileged + }; + + (account("submitter", submitter_num, 0), scope) + }) + .fold( + SubmittersByScope::default(), + |submitters_by_scope, (submitter, scope)| { + submitters_by_scope + .with_submitter(submitter, &scope) + .unwrap() + }, + ); + let hash = <::Hashing as Hasher>::hash(&[]); + crate::migrations::v1::v0::Submissions::::insert( + BatchId::default(), + hash, + submitters_by_scope, + ); + let mut meter = WeightMeter::new(); + + #[block] + { + crate::migrations::v1::LazyMigrationV1::, I>::step( + None, &mut meter, + ) + .unwrap(); + } + + // Check that the new storage is decodable: + (0..MAX_SUBMITTERS * 2).for_each(|submitter_num| { + let scope = if submitter_num % 2 == 0 { + QuorumScope::Public + } else { + QuorumScope::Privileged + }; + assert_eq!( + crate::SubmissionsV1::::get(( + BatchId::default(), + scope, + account::("submitter", submitter_num, 0) + )) + .unwrap(), + hash + ); + }); + // uses twice the weight once for migration and then for checking if there is another key. + assert_eq!( + meter.consumed(), + weights::SubstrateWeight::::migration_v0_v1_step() * 2 + ); + } + impl_benchmark_test_suite!( PalletWithApi, crate::mock::new_test_ext(), diff --git a/pallets/indexing/src/lib.rs b/pallets/indexing/src/lib.rs index 37023d58..16919a44 100644 --- a/pallets/indexing/src/lib.rs +++ b/pallets/indexing/src/lib.rs @@ -31,9 +31,12 @@ mod error_conversions; /// Native wrapper around the indexing pallet. pub mod native_pallet; +pub mod migrations; + #[allow(clippy::manual_inspect)] #[frame_support::pallet] pub mod pallet { + use alloc::collections::{BTreeMap, BTreeSet}; use alloc::string::String; use alloc::vec::Vec; @@ -44,6 +47,7 @@ pub mod pallet { use frame_support::{Blake2_128, Blake2_128Concat}; use frame_system::pallet_prelude::*; use hex::FromHex; + use itertools::{Either, Itertools}; use native_api::NativeApi; use on_chain_table::OnChainTable; use sp_core::{H256, U256}; @@ -80,19 +84,17 @@ pub mod pallet { type WeightInfo: WeightInfo; } - /// Double Map of Submissions using the batch-id as the first key and the submitter's - /// public key as the second key to hold the hash of the submitted data. - /// Each submission for a given batch id will have an entry here + /// Hashed data submissions that have yet to reach quorum. #[pallet::storage] #[pallet::getter(fn submissions)] - pub type Submissions, I: 'static = ()> = StorageDoubleMap< + pub type SubmissionsV1, I: 'static = ()> = StorageNMap< _, - Blake2_128Concat, - BatchId, - Blake2_128Concat, + ( + NMapKey, + NMapKey, + NMapKey, + ), ::Hash, - SubmittersByScope, - ValueQuery, // Allows us to receive a default instead of None >; #[pallet::storage] @@ -170,8 +172,6 @@ pub mod pallet { LateBatch, /// Invalid Table identifier was supplied InvalidTable, - /// This user has already submitted data for this batch id - AlreadySubmitted, /// The table could not be deserialized using a Stream Reader NativeDeserializationError, /// There was no record batch contained in the data @@ -190,6 +190,8 @@ pub mod pallet { NativeRecordBatchDuplicateIdentifiers, /// Error serializing the OnChainTable NativeSerializationError, + /// Maximum submissions already reached for this batch id + MaxSubmittersReached, /// Error deserializing the table as an OnChainTable TableDeserializationError, /// Error deserializing the table as an OnChainTable @@ -327,22 +329,16 @@ pub mod pallet { T: Config, I: NativeApi, { - // We don't need to save the full data. We just need a count associated with each submission - let match_submissions = Submissions::::get(&batch_id, data_hash); - - // Check if this user has already submitted this data - let current_num_matching_submissions = match_submissions.len_of_scope(quorum_scope); - - let new_match_submissions = match_submissions - .with_submitter(who.clone(), quorum_scope) - // Just return the unchanged submissions if maximum is exceeded. - .unwrap_or_else(|(submitters, _)| submitters); - - if new_match_submissions.len_of_scope(quorum_scope) == current_num_matching_submissions { - Err(Error::::AlreadySubmitted)? + let submission_map_with_this = + SubmissionsV1::::iter_prefix((&batch_id, quorum_scope)) + .take(MAX_SUBMITTERS as usize) + .chain(core::iter::once((who.clone(), data_hash))) + .collect::>(); + + if submission_map_with_this.len() > MAX_SUBMITTERS as usize { + Err(Error::MaxSubmittersReached::)?; } - - Submissions::::insert(&batch_id, data_hash, &new_match_submissions); + SubmissionsV1::::insert((&batch_id, quorum_scope, &who), data_hash); let submission = DataSubmission { table: table.clone(), @@ -350,38 +346,41 @@ pub mod pallet { data_hash, quorum_scope: *quorum_scope, }; - // Emit an event noting who submitted what Pallet::::deposit_event(Event::DataSubmitted { who, submission }); - match table_insert_quorum.of_scope(quorum_scope) { - Some(quorum_size) - if new_match_submissions.len_of_scope(quorum_scope) as u8 > *quorum_size => - { - // Iterate over the submitters who submitted differing data and collect - // their account ids - let dissenters = Submissions::::iter_prefix(&batch_id) - .filter(|(hash, _)| hash != &data_hash) - .flat_map(|(_, submitters)| submitters.into_iter_scope(quorum_scope)) - // de-dup collection - .collect::>() - .into_iter() - // resulting set should contain up to MAX_SUBMITTERS items *after* de-dup - .take(MAX_SUBMITTERS as usize) - .collect::>() - .try_into() - .expect("source Vec is constructed to not exceed maximum submitter list size"); + let (agreements_unbounded, dissents_unbounded): (BTreeSet<_>, BTreeSet<_>) = + submission_map_with_this + .into_iter() + .partition_map(|(account_id, hash)| { + if hash == data_hash { + Either::Left(account_id) + } else { + Either::Right(account_id) + } + }); + match table_insert_quorum.of_scope(quorum_scope) { + Some(quorum_size) if agreements_unbounded.len() as u8 > *quorum_size => { let block_number = >::block_number(); + const ALREADY_VALIDATED_SUBMITTER_COUNT: &str = + "we've already validated that the submitter count is within bounds"; + let agreements = agreements_unbounded + .try_into() + .expect(ALREADY_VALIDATED_SUBMITTER_COUNT); + let dissents = dissents_unbounded + .try_into() + .expect(ALREADY_VALIDATED_SUBMITTER_COUNT); + // Decide on the quorum let data_quorum = DataQuorum { table, batch_id, data_hash, block_number: block_number.into(), - agreements: new_match_submissions.of_scope(quorum_scope).clone(), - dissents: dissenters, + agreements, + dissents, quorum_scope: *quorum_scope, }; @@ -405,8 +404,11 @@ pub mod pallet { T: Config, I: NativeApi, { - Submissions::::iter_key_prefix(&quorum.batch_id) - .for_each(|key| Submissions::::remove(&quorum.batch_id, key)); + SubmissionsV1::::clear_prefix( + (&quorum.batch_id,), + MAX_SUBMITTERS * QuorumScope::VARIANT_COUNT as u32, + None, + ); FinalData::::insert(&quorum.batch_id, &quorum); diff --git a/pallets/indexing/src/migrations/mod.rs b/pallets/indexing/src/migrations/mod.rs new file mode 100644 index 00000000..3abefb82 --- /dev/null +++ b/pallets/indexing/src/migrations/mod.rs @@ -0,0 +1,11 @@ +//! Storage migrations for the indexing pallet + +pub mod v1; + +/// A unique identifier across all pallets. +/// +/// This constant represents a unique identifier for the migrations of this pallet. +/// It helps differentiate migrations for this pallet from those of others. Note that we don't +/// directly pull the crate name from the environment, since that would change if the crate were +/// ever to be renamed and could cause historic migrations to run again. +pub const PALLET_MIGRATIONS_ID: &[u8; 26] = b"pallet-indexing-migrations"; diff --git a/pallets/indexing/src/migrations/v1/mod.rs b/pallets/indexing/src/migrations/v1/mod.rs new file mode 100644 index 00000000..e5525edf --- /dev/null +++ b/pallets/indexing/src/migrations/v1/mod.rs @@ -0,0 +1,122 @@ +//! Migration from pallet-indexing storage v0 to v1 +//! +//! This migration handles changes to the `Submitters`, which went from being a mapping of data +//! hashes to submitters to the reverse. + +use frame_support::migrations::{MigrationId, SteppedMigration, SteppedMigrationError}; +use frame_support::pallet_prelude::PhantomData; +use frame_support::weights::WeightMeter; +use sxt_core::indexing::BatchId; +use sxt_core::tables::QuorumScope; + +use super::PALLET_MIGRATIONS_ID; +use crate::pallet::{Config, SubmissionsV1}; + +mod tests; + +/// Module containing the OLD (v0) storage Submissions. +#[allow(missing_docs)] +pub mod v0 { + use frame_support::pallet_prelude::ValueQuery; + use frame_support::{storage_alias, Blake2_128Concat}; + use sxt_core::indexing::{BatchId, SubmittersByScope}; + + use super::Config; + use crate::pallet::Pallet; + + /// The Submissions that is being migrated from. + #[storage_alias] + pub type Submissions, I: 'static> = StorageDoubleMap< + Pallet, + Blake2_128Concat, + BatchId, + Blake2_128Concat, + ::Hash, + SubmittersByScope<::AccountId>, + ValueQuery, + >; +} + +/// Migrates [`crate::Submissions`]'s map structure +/// +/// From: +/// `batch_id -> data_hash -> quorum_scope -> submitter_list` +/// (Though that last mapping is just in the form of a struct) +/// +/// To: +/// `batch_id -> quorum_scope -> submitter -> data_hash` +pub struct LazyMigrationV1, W: crate::weights::WeightInfo, I: 'static = ()>( + PhantomData<(T, W, I)>, +); + +impl, W: crate::weights::WeightInfo, I: 'static> SteppedMigration + for LazyMigrationV1 +{ + type Cursor = (BatchId, ::Hash); + type Identifier = MigrationId<26>; + + fn id() -> Self::Identifier { + MigrationId { + pallet_id: *PALLET_MIGRATIONS_ID, + version_from: 0, + version_to: 1, + } + } + + fn step( + mut cursor: Option, + meter: &mut WeightMeter, + ) -> Result, SteppedMigrationError> { + let required = W::migration_v0_v1_step(); + + if meter.remaining().any_lt(required) { + return Err(SteppedMigrationError::InsufficientWeight { required }); + } + + // We loop here to do as much progress as possible per step. + loop { + if meter.try_consume(required).is_err() { + break; + } + + let mut iter = if let Some((last_batch_id, last_hash)) = cursor { + // If a cursor is provided, start iterating from the stored value + v0::Submissions::::iter_from(v0::Submissions::::hashed_key_for( + last_batch_id, + last_hash, + )) + } else { + // If no cursor is provided, start iterating from the beginning. + v0::Submissions::::iter() + }; + + // If there's a next item in the iterator, perform the migration. + if let Some((batch_id, data_hash, submitters_by_scope)) = iter.next() { + submitters_by_scope + .iter_scope(&QuorumScope::Public) + .for_each(|submitter| { + SubmissionsV1::::insert( + (batch_id.clone(), QuorumScope::Public, submitter), + data_hash, + ); + }); + submitters_by_scope + .iter_scope(&QuorumScope::Privileged) + .for_each(|submitter| { + SubmissionsV1::::insert( + (batch_id.clone(), QuorumScope::Privileged, submitter), + data_hash, + ); + }); + + v0::Submissions::::remove(&batch_id, data_hash); + + cursor = Some((batch_id, data_hash)) // Return the processed key as the new cursor. + } else { + cursor = None; // Signal that the migration is complete (no more items to process). + break; + } + } + Ok(cursor) + } +} diff --git a/pallets/indexing/src/migrations/v1/tests.rs b/pallets/indexing/src/migrations/v1/tests.rs new file mode 100644 index 00000000..ac2a315f --- /dev/null +++ b/pallets/indexing/src/migrations/v1/tests.rs @@ -0,0 +1,191 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#![cfg(all(test, not(feature = "runtime-benchmarks")))] + +use codec::Decode; +use frame_support::traits::OnRuntimeUpgrade; +use frame_system::ensure_signed; +use native_api::Api; +use pallet_migrations::WeightInfo as _; +use sp_core::Hasher; +use sxt_core::indexing::{BatchId, SubmittersByScope}; +use sxt_core::tables::QuorumScope; + +use crate::migrations::v1; +use crate::mock::{ + new_test_ext, + run_to_block, + AllPalletsWithSystem, + MigratorServiceWeight, + RuntimeOrigin, + System, + Test, +}; +use crate::weights::WeightInfo as _; + +fn submissions_from_seed( + seed: u64, +) -> ( + BatchId, + ::Hash, + SubmittersByScope<::AccountId>, +) { + let batch_id = BatchId::try_from(seed.to_le_bytes().to_vec()).unwrap(); + let data_hash = <::Hashing as Hasher>::hash(&seed.to_le_bytes()); + + let num_privileged_submissions = seed % 32; + + // this math is chosen to + // - have some variety with num_privileged_submissions + // - always have at least 1 public submission (simplifies test logic) + let num_public_submissions = ((seed * 2) % 31) + 1; + + let privileged_submitters_by_scope = (0..num_privileged_submissions) + .map(|submitter_seed_index| { + let submitter_index = seed * 32 + submitter_seed_index; + ensure_signed(RuntimeOrigin::signed(submitter_index)).unwrap() + }) + .fold(SubmittersByScope::default(), |acc, submitter| { + acc.with_submitter(submitter, &QuorumScope::Privileged) + .unwrap() + }); + let submitters_by_scope = (0..num_public_submissions) + .map(|submitter_seed_index| { + let submitter_index = seed * 32 + submitter_seed_index; + ensure_signed(RuntimeOrigin::signed(submitter_index)).unwrap() + }) + .fold(privileged_submitters_by_scope, |acc, submitter| { + acc.with_submitter(submitter, &QuorumScope::Public).unwrap() + }); + + (batch_id, data_hash, submitters_by_scope) +} + +#[test] +fn lazy_migration_works() { + new_test_ext().execute_with(|| { + (0..64u64) + .map(submissions_from_seed) + .for_each(|(batch_id, data_hash, submitters)| { + v1::v0::Submissions::::insert(batch_id, data_hash, submitters) + }); + + // Give it enough weight do do exactly 16 iterations: + let limit = ::WeightInfo::progress_mbms_none() + + pallet_migrations::Pallet::::exec_migration_max_weight() + + crate::weights::SubstrateWeight::::migration_v0_v1_step() * 8; + MigratorServiceWeight::set(&limit); + + System::set_block_number(1); + AllPalletsWithSystem::on_runtime_upgrade(); // onboard MBMs + + // check migration progress across many blocks + let mut last_num_migrated = 0; + for block in 2..=9 { + run_to_block(block); + + let mut num_migrated = 0; + (0..64u64) + .map(submissions_from_seed) + .for_each(|(batch_id, data_hash, submitters)| { + let first_submitter = submitters + .iter_scope(&QuorumScope::Public) + .next() + .expect("seeding ensures at least one public submitter"); + let is_migrated = crate::SubmissionsV1::::get(( + &batch_id, + QuorumScope::Public, + first_submitter, + )) + .is_some(); + + if is_migrated { + submitters + .iter_scope(&QuorumScope::Public) + .for_each(|submitter| { + assert_eq!( + crate::SubmissionsV1::::get(( + &batch_id, + QuorumScope::Public, + submitter + )), + Some(data_hash) + ); + }); + submitters + .iter_scope(&QuorumScope::Privileged) + .for_each(|submitter| { + assert_eq!( + crate::SubmissionsV1::::get(( + &batch_id, + QuorumScope::Privileged, + submitter + )), + Some(data_hash) + ); + }); + assert!(!v1::v0::Submissions::::contains_key( + &batch_id, data_hash + ),); + num_migrated += 1 + } else { + assert_eq!( + v1::v0::Submissions::::get(&batch_id, data_hash), + submitters + ); + } + }); + + assert_eq!(num_migrated, last_num_migrated + 8); + last_num_migrated = num_migrated; + } + + // Check that everything is migrated now + (0..64u64) + .map(submissions_from_seed) + .for_each(|(batch_id, data_hash, submitters)| { + submitters + .iter_scope(&QuorumScope::Public) + .for_each(|submitter| { + assert_eq!( + crate::SubmissionsV1::::get(( + &batch_id, + QuorumScope::Public, + submitter + )), + Some(data_hash) + ); + }); + submitters + .iter_scope(&QuorumScope::Privileged) + .for_each(|submitter| { + assert_eq!( + crate::SubmissionsV1::::get(( + &batch_id, + QuorumScope::Privileged, + submitter + )), + Some(data_hash) + ); + }); + assert!(!v1::v0::Submissions::::contains_key( + &batch_id, data_hash + ),); + }); + }); +} diff --git a/pallets/indexing/src/mock.rs b/pallets/indexing/src/mock.rs index ecb97138..e3ef9501 100644 --- a/pallets/indexing/src/mock.rs +++ b/pallets/indexing/src/mock.rs @@ -1,7 +1,8 @@ use frame_election_provider_support::bounds::{ElectionBounds, ElectionBoundsBuilder}; use frame_election_provider_support::{onchain, SequentialPhragmen}; -use frame_support::pallet_prelude::ConstU32; -use frame_support::traits::{ConstU128, VariantCountOf}; +use frame_support::migrations::MultiStepMigrator; +use frame_support::pallet_prelude::{ConstU32, Weight}; +use frame_support::traits::{ConstU128, OnFinalize, OnInitialize, VariantCountOf}; use frame_support::{derive_impl, parameter_types}; use native_api::Api; use proof_of_sql_static_setups::io::get_or_init_from_files_with_four_points_unchecked; @@ -25,6 +26,7 @@ frame_support::construct_runtime!( SystemTables: pallet_system_tables, Balances: pallet_balances, Staking: pallet_staking, + Migrator: pallet_migrations, } ); @@ -40,6 +42,22 @@ impl frame_system::Config for Test { type Block = Block; type Lookup = IdentityLookup; type Hash = H256; + type MultiBlockMigrator = Migrator; +} + +frame_support::parameter_types! { + pub storage MigratorServiceWeight: Weight = Weight::from_parts(100, 100); // do not use in prod +} + +#[derive_impl(pallet_migrations::config_preludes::TestDefaultConfig)] +impl pallet_migrations::Config for Test { + #[cfg(not(feature = "runtime-benchmarks"))] + type Migrations = ( + crate::migrations::v1::LazyMigrationV1, Api>, + ); + #[cfg(feature = "runtime-benchmarks")] + type Migrations = pallet_migrations::mock_helpers::MockedMigrations; + type MaxServiceWeight = MigratorServiceWeight; } impl pallet_balances::Config for Test { @@ -191,3 +209,16 @@ pub fn new_test_ext() -> sp_io::TestExternalities { storage.into() } + +#[allow(dead_code)] +pub fn run_to_block(n: u64) { + assert!(System::block_number() < n); + while System::block_number() < n { + let b = System::block_number(); + AllPalletsWithSystem::on_finalize(b); + // Done by Executive: + ::MultiBlockMigrator::step(); + System::set_block_number(b + 1); + AllPalletsWithSystem::on_initialize(b + 1); + } +} diff --git a/pallets/indexing/src/tests.rs b/pallets/indexing/src/tests.rs index 8f951c39..ada60876 100644 --- a/pallets/indexing/src/tests.rs +++ b/pallets/indexing/src/tests.rs @@ -10,20 +10,19 @@ use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::__private::RuntimeDebug; use frame_support::dispatch::DispatchResult; use frame_support::pallet_prelude::TypeInfo; -use frame_support::{assert_err, assert_ok}; +use frame_support::{assert_err, assert_noop, assert_ok}; use frame_system::ensure_signed; use native_api::Api; use pallet_tables::{CommitmentCreationCmd, UpdateTable}; use proof_of_sql_commitment_map::CommitmentSchemeFlags; use sp_core::Hasher; use sp_runtime::BoundedVec; +use sxt_core::indexing::MAX_SUBMITTERS; use sxt_core::permissions::{IndexingPalletPermission, PermissionLevel, PermissionList}; use sxt_core::tables::{ - CommitmentScheme, CreateStatement, InsertQuorumSize, QuorumScope, - SourceAndMode, TableIdentifier, TableName, TableNamespace, @@ -207,71 +206,8 @@ fn inserting_data_succeeds_when_data_is_good() { // Verify that the submission was stored as expected // and the hash was generated from the submitted data assert_eq!( - Indexing::submissions(test_batch.clone(), hash).len_of_scope(&QuorumScope::Public), - 1 - ); - }) -} - -#[test] -fn submission_fails_when_data_is_already_submitted() { - new_test_ext().execute_with(|| { - System::set_block_number(1); - let (table_id, test_create) = sample_table_definition(); - Tables::create_tables( - RuntimeOrigin::root(), - vec![UpdateTable { - ident: table_id.clone(), - create_statement: test_create, - table_type: TableType::CoreBlockchain, - commitment: CommitmentCreationCmd::Empty(CommitmentSchemeFlags { - hyper_kzg: false, - dynamic_dory: true, - }), - source: sxt_core::tables::Source::Ethereum, - }] - .try_into() - .unwrap(), - ) - .unwrap(); - let signer = RuntimeOrigin::signed(1); - let who = ensure_signed(signer.clone()).unwrap(); - let permissions = PermissionList::try_from(vec![PermissionLevel::IndexingPallet( - IndexingPalletPermission::SubmitDataForPublicQuorum, - )]) - .unwrap(); - pallet_permissions::Permissions::::insert(who, permissions.clone()); - - let test_batch = BatchId::try_from(b"test_batch".to_vec()).unwrap(); - let test_data = row_data(); - - assert_ok!(Indexing::submit_data( - signer.clone(), - table_id.clone(), - test_batch.clone(), - test_data.clone(), - ),); - - let mut hash_input = test_data.encode(); - hash_input.extend(None::.encode()); - let hash = <::Hashing as Hasher>::hash(&hash_input); - - // Verify that the submission was stored as expected - // and the hash was generated from the submitted data - assert_eq!( - Indexing::submissions(test_batch.clone(), hash).len_of_scope(&QuorumScope::Public), - 1 - ); - - // Verify that submitting the same thing again returns the expected error - assert_err!( - Indexing::submit_data( - signer.clone(), - table_id.clone(), - test_batch.clone(), - test_data.clone(), - ), - crate::Error::::AlreadySubmitted + Indexing::submissions((test_batch.clone(), QuorumScope::Public, who)).unwrap(), + hash ); }) } @@ -298,13 +234,8 @@ fn data_submission_fails_if_no_permissions() { crate::Error::::UnauthorizedSubmitter, ); - let hash = <::Hashing as Hasher>::hash(&test_data); - // Verify that the submission was not stored - assert_eq!( - Indexing::submissions(test_batch.clone(), hash).len_of_scope(&QuorumScope::Public), - 0 - ); + assert!(Indexing::submissions((test_batch.clone(), QuorumScope::Public, 1)).is_none()); }) } @@ -382,9 +313,10 @@ fn data_is_decided_on_after_required_submissions() { assert_eq!(fd.quorum_scope, QuorumScope::Public); // Verify that the old data was successfully removed for this batch - let submitters = Indexing::submissions(test_submission.batch_id.clone(), test_data_hash); - assert!(submitters.scope_is_empty(&QuorumScope::Public)); - assert!(submitters.scope_is_empty(&QuorumScope::Privileged)); + let submitters_count = + crate::SubmissionsV1::::iter_prefix((test_submission.batch_id.clone(),)) + .count(); + assert_eq!(submitters_count, 0); }) } @@ -476,12 +408,10 @@ fn correct_data_is_decided_on_after_required_submissions() { assert_eq!(final_data.unwrap().data_hash, data_hash); // Verify that the old data was successfully removed for this batch - for _i in 1..4 { - assert!( - Indexing::submissions(test_submission.batch_id.clone(), data_hash) - .scope_is_empty(&QuorumScope::Public) - ) - } + let submitters_count = + crate::SubmissionsV1::::iter_prefix((test_submission.batch_id.clone(),)) + .count(); + assert_eq!(submitters_count, 0); }) } @@ -800,8 +730,8 @@ fn submit_data_with_mothership_key_work() { // Verify that the submission was stored as expected // and the hash was generated from the submitted data assert_eq!( - Indexing::submissions(test_batch.clone(), hash).len_of_scope(&QuorumScope::Public), - 1 + Indexing::submissions((test_batch.clone(), QuorumScope::Public, 1)).unwrap(), + hash ); }) } @@ -860,9 +790,10 @@ fn we_can_reach_privileged_quorum() { assert_eq!(fd.quorum_scope, QuorumScope::Privileged); // Verify that the old data was successfully removed for this batch - let submitters = Indexing::submissions(test_submission.batch_id.clone(), test_data_hash); - assert!(submitters.scope_is_empty(&QuorumScope::Public)); - assert!(submitters.scope_is_empty(&QuorumScope::Privileged)); + let submitters_count = + crate::SubmissionsV1::::iter_prefix((test_submission.batch_id.clone(),)) + .count(); + assert_eq!(submitters_count, 0); }) } @@ -928,16 +859,42 @@ fn we_can_manage_quorum_state_for_both_scopes() { // public submission assert_ok!(submit_test_data(public_submitter, test_submission.clone())); - let submissions = Indexing::submissions(&test_submission.batch_id, test_data_hash); - assert_eq!(submissions.len_of_scope(&QuorumScope::Public), 1); - assert!(submissions.scope_is_empty(&QuorumScope::Privileged)); + assert_eq!( + crate::SubmissionsV1::::iter_prefix(( + &test_submission.batch_id, + QuorumScope::Public + )) + .count(), + 1 + ); + assert_eq!( + crate::SubmissionsV1::::iter_prefix(( + &test_submission.batch_id, + QuorumScope::Privileged + )) + .count(), + 0 + ); assert!(Indexing::final_data(&test_submission.batch_id).is_none()); // both submission assert_ok!(submit_test_data(both_submitter, test_submission.clone())); - let submissions = Indexing::submissions(&test_submission.batch_id, test_data_hash); - assert_eq!(submissions.len_of_scope(&QuorumScope::Public), 2); - assert_eq!(submissions.len_of_scope(&QuorumScope::Privileged), 1); + assert_eq!( + crate::SubmissionsV1::::iter_prefix(( + &test_submission.batch_id, + QuorumScope::Public + )) + .count(), + 2 + ); + assert_eq!( + crate::SubmissionsV1::::iter_prefix(( + &test_submission.batch_id, + QuorumScope::Privileged + )) + .count(), + 1 + ); assert!(Indexing::final_data(&test_submission.batch_id).is_none()); // privileged submission @@ -952,9 +909,10 @@ fn we_can_manage_quorum_state_for_both_scopes() { assert_eq!(final_data.quorum_scope, QuorumScope::Privileged); // Verify that the old data was successfully removed for this batch - let submitters = Indexing::submissions(test_submission.batch_id.clone(), test_data_hash); - assert!(submitters.scope_is_empty(&QuorumScope::Public)); - assert!(submitters.scope_is_empty(&QuorumScope::Privileged)); + assert_eq!( + crate::SubmissionsV1::::iter_prefix((&test_submission.batch_id,)).count(), + 0 + ); assert_eq!( System::read_events_for_pallet::>() @@ -1023,9 +981,10 @@ fn reaching_quorum_for_both_scopes_simultaneously_produces_one_quorum_reached_ev assert_eq!(final_data.quorum_scope, QuorumScope::Public); // Verify that the old data was successfully removed for this batch - let submitters = Indexing::submissions(test_submission.batch_id.clone(), test_data_hash); - assert!(submitters.scope_is_empty(&QuorumScope::Public)); - assert!(submitters.scope_is_empty(&QuorumScope::Privileged)); + assert_eq!( + crate::SubmissionsV1::::iter_prefix((&test_submission.batch_id,)).count(), + 0 + ); assert_eq!( System::read_events_for_pallet::>() @@ -1085,9 +1044,10 @@ fn we_cannot_submit_for_table_disabled_quorum_scope() { submit_test_data(public_submitter, test_submission.clone()), crate::Error::::UnauthorizedSubmitter ); - let submissions = Indexing::submissions(&test_submission.batch_id, test_data_hash); - assert!(submissions.scope_is_empty(&QuorumScope::Public)); - assert!(submissions.scope_is_empty(&QuorumScope::Privileged)); + assert_eq!( + crate::SubmissionsV1::::iter_prefix((&test_submission.batch_id,)).count(), + 0 + ); assert!(Indexing::final_data(&test_submission.batch_id).is_none()); }) } @@ -1141,9 +1101,10 @@ fn we_cannot_submit_with_privilege_to_different_table() { submit_test_data(privileged_submitter, test_submission.clone()), crate::Error::::UnauthorizedSubmitter ); - let submissions = Indexing::submissions(&test_submission.batch_id, test_data_hash); - assert!(submissions.scope_is_empty(&QuorumScope::Public)); - assert!(submissions.scope_is_empty(&QuorumScope::Privileged)); + assert_eq!( + crate::SubmissionsV1::::iter_prefix((&test_submission.batch_id,)).count(), + 0 + ); assert!(Indexing::final_data(&test_submission.batch_id).is_none()); }) } @@ -1364,3 +1325,128 @@ fn no_block_number_stored_when_implicit_and_empty_data() { assert_eq!(stored, None); }); } + +#[test] +fn submitters_can_overwrite_their_submission() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + let (table_id, create_statement) = sample_table_definition(); + Tables::create_tables( + RuntimeOrigin::root(), + vec![UpdateTable { + ident: table_id.clone(), + create_statement, + table_type: TableType::CoreBlockchain, + commitment: CommitmentCreationCmd::Empty(CommitmentSchemeFlags::all()), + source: sxt_core::tables::Source::Ethereum, + }] + .try_into() + .unwrap(), + ) + .unwrap(); + + let permissions = PermissionList::try_from(vec![PermissionLevel::IndexingPallet( + IndexingPalletPermission::SubmitDataForPublicQuorum, + )]) + .unwrap(); + let signer = RuntimeOrigin::signed(1); + let who = ensure_signed(signer.clone()).unwrap(); + pallet_permissions::Permissions::::insert(who, permissions); + + let batch_id = BatchId::try_from(b"test_batch".to_vec()).unwrap(); + let data = row_data(); + let data_hash = hash_row_data_with_block_number::(&data, None); + + Indexing::submit_data( + signer.clone(), + table_id.clone(), + batch_id.clone(), + data.clone(), + ) + .unwrap(); + assert_eq!( + crate::SubmissionsV1::::get((&batch_id, QuorumScope::Public, &who)).unwrap(), + data_hash + ); + + let different_data = diff_row_data(); + let different_data_hash = hash_row_data_with_block_number::(&different_data, None); + Indexing::submit_data(signer, table_id, batch_id.clone(), different_data.clone()).unwrap(); + assert_eq!( + crate::SubmissionsV1::::get((&batch_id, QuorumScope::Public, &who)).unwrap(), + different_data_hash + ); + }); +} + +#[test] +fn submitters_cannot_exceed_maximum() { + new_test_ext().execute_with(|| { + System::set_block_number(1); + let (table_id, create_statement) = sample_table_definition(); + Tables::create_tables( + RuntimeOrigin::root(), + vec![UpdateTable { + ident: table_id.clone(), + create_statement, + table_type: TableType::CoreBlockchain, + commitment: CommitmentCreationCmd::Empty(CommitmentSchemeFlags::all()), + source: sxt_core::tables::Source::Ethereum, + }] + .try_into() + .unwrap(), + ) + .unwrap(); + + let batch_id = BatchId::try_from(b"test_batch".to_vec()).unwrap(); + + // artificially fill submissions for batch + (1..=MAX_SUBMITTERS).for_each(|submitter_num| { + let signer = RuntimeOrigin::signed(submitter_num as u64); + let who = ensure_signed(signer.clone()).unwrap(); + let artificial_data_hash = <::Hashing as Hasher>::hash( + &submitter_num.to_le_bytes(), + ); + + crate::SubmissionsV1::::insert( + (batch_id.clone(), QuorumScope::Public, who), + artificial_data_hash, + ); + }); + + // we cannot insert one more + let permissions = PermissionList::try_from(vec![PermissionLevel::IndexingPallet( + IndexingPalletPermission::SubmitDataForPublicQuorum, + )]) + .unwrap(); + + let signer = RuntimeOrigin::signed((MAX_SUBMITTERS + 1) as u64); + let who = ensure_signed(signer.clone()).unwrap(); + pallet_permissions::Permissions::::insert(who, permissions.clone()); + let data = row_data(); + + assert_noop!( + Indexing::submit_data(signer.clone(), table_id.clone(), batch_id.clone(), data,), + crate::Error::::MaxSubmittersReached + ); + + // submitters can still re-submit new hashes + let signer = RuntimeOrigin::signed(1); + let who = ensure_signed(signer.clone()).unwrap(); + pallet_permissions::Permissions::::insert(who, permissions); + let data = row_data(); + let data_hash = hash_row_data_with_block_number::(&data, None); + + Indexing::submit_data( + signer.clone(), + table_id.clone(), + batch_id.clone(), + data.clone(), + ) + .unwrap(); + assert_eq!( + crate::SubmissionsV1::::get((&batch_id, QuorumScope::Public, &who)).unwrap(), + data_hash + ); + }); +} diff --git a/pallets/indexing/src/weights.rs b/pallets/indexing/src/weights.rs index 3312bb92..3dc8402f 100644 --- a/pallets/indexing/src/weights.rs +++ b/pallets/indexing/src/weights.rs @@ -54,6 +54,11 @@ pub trait WeightInfo { /// Storage: `Indexing::BlockNumbers` (r:0 w:1) /// Proof: `Indexing::BlockNumbers` (`max_values`: None, `max_size`: Some(156), added: 2631, mode: `MaxEncodedLen`) fn submit_data_quorum_reached() -> Weight; + /// Storage: UNKNOWN KEY `0x5f0eaa9161a01e3d007e08083fa1748247aabc8065823acc682503190fa1fbd7` (r:2 w:1) + /// Proof: UNKNOWN KEY `0x5f0eaa9161a01e3d007e08083fa1748247aabc8065823acc682503190fa1fbd7` (r:2 w:1) + /// Storage: `Indexing::SubmissionsV1` (r:0 w:64) + /// Proof: `Indexing::SubmissionsV1` (`max_values`: None, `max_size`: Some(150), added: 2625, mode: `MaxEncodedLen`) + fn migration_v0_v1_step() -> Weight; } pub struct SubstrateWeight(PhantomData); @@ -114,4 +119,18 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(7)) .saturating_add(T::DbWeight::get().writes(4)) } + /// Storage: UNKNOWN KEY `0x5f0eaa9161a01e3d007e08083fa1748247aabc8065823acc682503190fa1fbd7` (r:2 w:1) + /// Proof: UNKNOWN KEY `0x5f0eaa9161a01e3d007e08083fa1748247aabc8065823acc682503190fa1fbd7` (r:2 w:1) + /// Storage: `Indexing::SubmissionsV1` (r:0 w:64) + /// Proof: `Indexing::SubmissionsV1` (`max_values`: None, `max_size`: Some(150), added: 2625, mode: `MaxEncodedLen`) + fn migration_v0_v1_step() -> Weight { + // Proof Size summary in bytes: + // Measured: `2256` + // Estimated: `8196` + // Minimum execution time: 303_120_000 picoseconds. + Weight::from_parts(311_376_000, 0) + .saturating_add(Weight::from_parts(0, 8196)) + .saturating_add(T::DbWeight::get().reads(2)) + .saturating_add(T::DbWeight::get().writes(65)) + } } diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 8f7e05c6..66f5b6a3 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -30,6 +30,7 @@ pallet-balances.workspace = true pallet-grandpa.workspace = true pallet-sudo.workspace = true pallet-multisig.workspace = true +pallet-migrations.workspace = true pallet-timestamp.workspace = true pallet-transaction-payment.workspace = true sp-api.workspace = true @@ -123,6 +124,7 @@ std = [ "pallet-grandpa/std", "pallet-sudo/std", "pallet-multisig/std", + "pallet-migrations/std", "pallet-timestamp/std", "pallet-transaction-payment-rpc-runtime-api/std", "pallet-transaction-payment/std", @@ -173,6 +175,7 @@ runtime-benchmarks = [ "pallet-grandpa/runtime-benchmarks", "pallet-sudo/runtime-benchmarks", "pallet-multisig/runtime-benchmarks", + "pallet-migrations/runtime-benchmarks", "pallet-timestamp/runtime-benchmarks", "sp-runtime/runtime-benchmarks", "pallet-staking/runtime-benchmarks", @@ -197,6 +200,7 @@ try-runtime = [ "pallet-grandpa/try-runtime", "pallet-sudo/try-runtime", "pallet-multisig/try-runtime", + "pallet-migrations/try-runtime", "pallet-timestamp/try-runtime", "pallet-transaction-payment/try-runtime", "sp-runtime/try-runtime", diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index cb5a716c..45e1e481 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -165,7 +165,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: 227, + spec_version: 228, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 1, @@ -290,7 +290,7 @@ impl frame_system::Config for Runtime { type OnSetCode = (); type MaxConsumers = frame_support::traits::ConstU32<16>; type SingleBlockMigrations = (); - type MultiBlockMigrator = (); + type MultiBlockMigrator = MultiBlockMigrations; type PreInherents = (); type PostInherents = (); type PostTransactions = (); @@ -328,6 +328,31 @@ impl pallet_statement::Config for Runtime { type MaxAllowedBytes = MaxAllowedBytes; } +parameter_types! { + pub MbmServiceWeight: Weight = Perbill::from_percent(80) * BlockWeights::get().max_block; +} + +impl pallet_migrations::Config for Runtime { + type RuntimeEvent = RuntimeEvent; + #[cfg(not(feature = "runtime-benchmarks"))] + type Migrations = ( + pallet_indexing::migrations::v1::LazyMigrationV1< + Runtime, + pallet_indexing::SubstrateWeight, + native_api::Api, + >, + ); + // Benchmarks need mocked migrations to guarantee that they succeed. + #[cfg(feature = "runtime-benchmarks")] + type Migrations = pallet_migrations::mock_helpers::MockedMigrations; + type CursorMaxLen = ConstU32<65_536>; + type IdentifierMaxLen = ConstU32<256>; + type MigrationStatusHandler = (); + type FailedMigrationHandler = frame_support::migrations::FreezeChainOnFailedMigration; + type MaxServiceWeight = MbmServiceWeight; + type WeightInfo = pallet_migrations::weights::SubstrateWeight; +} + impl pallet_utility::Config for Runtime { type RuntimeEvent = RuntimeEvent; type RuntimeCall = RuntimeCall; @@ -866,6 +891,9 @@ mod runtime { #[runtime::pallet_index(71)] pub type Statement = pallet_statement; + #[runtime::pallet_index(72)] + pub type MultiBlockMigrations = pallet_migrations; + // Custom pallets start at index 100 to ensure room for future consensus work #[runtime::pallet_index(100)] pub type Permissions = pallet_permissions::Pallet; @@ -941,6 +969,7 @@ mod benches { [pallet_staking, Staking] [pallet_sudo, Sudo] [pallet_multisig, Multisig] + [pallet_migrations, MultiBlockMigrations] [frame_system, SystemBench::] [pallet_timestamp, Timestamp] [pallet_utility, Utility] diff --git a/sxt-core/src/tables.rs b/sxt-core/src/tables.rs index 87088609..88c53e71 100644 --- a/sxt-core/src/tables.rs +++ b/sxt-core/src/tables.rs @@ -2,7 +2,6 @@ extern crate alloc; use alloc::format; use alloc::string::{String, ToString}; use alloc::vec::Vec; -use core::fmt::Display; use core::str::{from_utf8, Utf8Error}; use codec::{Decode, Encode, MaxEncodedLen}; @@ -326,6 +325,13 @@ pub enum QuorumScope { Privileged, } +impl QuorumScope { + /// Number of scopes. + /// + /// Replace with core::mem::variant_count when it is stable/no_std. + pub const VARIANT_COUNT: usize = 2; +} + /// Quorum sizes to exceed to insert to a table for all [`QuorumScope`]s. #[derive( Copy,