From b86d3ca01483d3b5a64bea58387ac140092f97b0 Mon Sep 17 00:00:00 2001 From: Trevor Lovell Date: Mon, 17 Nov 2025 16:57:29 -0700 Subject: [PATCH 1/2] feat: reorganize submissions-finding-quorum storage with SubmissionsV1 Currently, submissions storage is keyed from the data hash to the accounts, rather than keyed from an account to data hash. This design choice makes it possible for submitters to disagree with themselves, and input an unbounded number of unique submissions for the same batch id. The solution for this is to change the map from being.. ``` BatchId -> DataHash -> QuorumScope -> [AccountId] ``` to... ``` BatchId -> QuorumScope -> AccountId -> DataHash ``` This change adds a new piece of storage SubmissionsV1 to reflect that second mapping, and switches the pallet-indexing logic over to using it. No data migration is provided, since submissions are fairly disposable in practice. A future PR will handle clearing out the old submissions (and even pruning the new submissions). --- Cargo.lock | 1 + pallets/indexing/Cargo.toml | 1 + pallets/indexing/src/lib.rs | 132 ++++++++++---- pallets/indexing/src/tests.rs | 334 ++++++++++++++++++++++------------ 4 files changed, 315 insertions(+), 153 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88c1c02b..669b7d85 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10403,6 +10403,7 @@ dependencies = [ "frame-support", "frame-system", "hex", + "itertools 0.12.1", "native", "native-api", "on-chain-table", diff --git a/pallets/indexing/Cargo.toml b/pallets/indexing/Cargo.toml index 97c9bc57..bf951ba2 100644 --- a/pallets/indexing/Cargo.toml +++ b/pallets/indexing/Cargo.toml @@ -37,6 +37,7 @@ postcard.workspace = true proof-of-sql-static-setups = { workspace = true, features = ["io"], optional = true } on-chain-table = { workspace = true, default-features = false } proof-of-sql-commitment-map = { workspace = true, optional = true } +itertools.workspace = true [dev-dependencies] sp-staking.workspace = true diff --git a/pallets/indexing/src/lib.rs b/pallets/indexing/src/lib.rs index 34f6dcce..f67c0ca4 100644 --- a/pallets/indexing/src/lib.rs +++ b/pallets/indexing/src/lib.rs @@ -34,22 +34,25 @@ pub mod native_pallet; #[allow(clippy::manual_inspect)] #[frame_support::pallet] pub mod pallet { + use alloc::collections::{BTreeMap, BTreeSet}; use alloc::string::String; use alloc::vec::Vec; - use codec::Decode; + use codec::{Decode, EncodeLike}; use commitment_sql::InsertAndCommitmentMetadata; use frame_support::dispatch::RawOrigin; use frame_support::pallet_prelude::*; use frame_support::{Blake2_128, Blake2_128Concat}; use frame_system::pallet_prelude::*; use hex::FromHex; + use itertools::Itertools; use native_api::NativeApi; use on_chain_table::OnChainTable; use sp_core::crypto::Ss58Codec; use sp_core::{H256, U256}; use sp_runtime::traits::{Bounded, Hash, StaticLookup, UniqueSaturatedInto}; - use sp_runtime::{BoundedVec, SaturatedConversion}; + use sp_runtime::{BoundedVec, Either, SaturatedConversion}; + use sxt_core::heavy::Heavy; use sxt_core::permissions::{IndexingPalletPermission, PermissionLevel}; use sxt_core::tables::{ InsertQuorumSize, @@ -83,7 +86,6 @@ pub mod pallet { /// Storage map of `BatchId` and data hash to submitters that have agreed to the batch/hash. #[pallet::storage] - #[pallet::getter(fn submissions)] pub type Submissions, I: 'static = ()> = StorageDoubleMap< _, Blake2_128Concat, @@ -94,6 +96,37 @@ pub mod pallet { ValueQuery, // Allows us to receive a default instead of None >; + /// Storage map of `(BatchId, QuorumScope, AccountId)` to data hash for batches that are still + /// finding quorum. + /// + /// Will get pruned once `Config::MaxBatchesFindingQuorum` is reached. + #[pallet::storage] + #[pallet::getter(fn submissions_v1)] + pub type SubmissionsV1, I: 'static = ()> = StorageNMap< + _, + ( + NMapKey, + NMapKey, + NMapKey, + ), + ::Hash, + >; + + /// Storage map of `BatchId`s by batch index, in the order of first submission. + /// + /// Will get pruned once `Config::MaxBatchesFindingQuorum` is reached. + #[pallet::storage] + #[pallet::getter(fn batch_queue_get)] + pub type BatchQueue, I: 'static = ()> = + CountedStorageMap<_, Blake2_128Concat, u32, BatchId>; + + /// The lowest index currently in the `BatchQueue`. + /// + /// Will increment as the `BatchQueue` is pruned. + #[pallet::storage] + #[pallet::getter(fn batch_queue_bottom)] + pub type BatchQueueBottom, I: 'static = ()> = StorageValue<_, u32, ValueQuery>; + /// Storage map of `BatchId`s to `DataQuorum`s for batches that have reached quorum. #[pallet::storage] #[pallet::getter(fn final_data)] @@ -199,6 +232,8 @@ pub mod pallet { TableSerializationError, /// Submitter Injection Failed SubmitterInjectionFailed, + /// Maximum submissions already reached for this batch id + MaxSubmittersReached, } #[pallet::call] @@ -402,22 +437,26 @@ 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); + // There is no `StorageNMap::contains_key_prefix` + if SubmissionsV1::::iter_prefix((&batch_id,)) + .next() + .is_none() + { + let batch_index = Pallet::::batch_queue_bottom() + BatchQueue::::count(); + BatchQueue::::insert(batch_index, &batch_id); + } - 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); + 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 new_match_submissions.len_of_scope(quorum_scope) == current_num_matching_submissions { - Err(Error::::AlreadySubmitted)? + 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(), @@ -425,38 +464,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(); + // Technically we don't need to check this, we know at this point that both lists + // sizes will sum up to the size of submission_map_with_this, which we already + // checked is below the number of max submitters. We still avoid the panic out of + // an abundance of caution. + let (agreements, dissents) = agreements_unbounded + .try_into() + .and_then(|agreements| Ok((agreements, dissents_unbounded.try_into()?))) + .map_err(|_| Error::MaxSubmittersReached::)?; + // 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, }; @@ -482,8 +524,7 @@ pub mod pallet { I: NativeApi, { // Clean up submissions for this batch - Submissions::::iter_key_prefix(&quorum.batch_id) - .for_each(|key| Submissions::::remove(&quorum.batch_id, key)); + let _ = remove_batch_id_from_submissions_v1::(&quorum.batch_id); // Record final decision FinalData::::insert(&quorum.batch_id, &quorum); @@ -625,4 +666,21 @@ pub mod pallet { ); Ok(()) } + + /// Removes the given `batch_id` from the `SubmissionsV1` storage and returns the weight of the + /// clear_prefix. + fn remove_batch_id_from_submissions_v1(batch_id: impl EncodeLike) -> Heavy<()> + where + T: Config, + I: NativeApi, + { + let removal_limit = MAX_SUBMITTERS + .saturating_mul(QuorumScope::VARIANT_COUNT.try_into().unwrap_or_default()); + + let removal_results = SubmissionsV1::::clear_prefix((batch_id,), removal_limit, None); + + T::DbWeight::get() + .reads_writes(removal_results.loops.into(), removal_results.unique.into()) + .into() + } } diff --git a/pallets/indexing/src/tests.rs b/pallets/indexing/src/tests.rs index c3bf3e35..fd9a27d8 100644 --- a/pallets/indexing/src/tests.rs +++ b/pallets/indexing/src/tests.rs @@ -10,13 +10,14 @@ 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, @@ -218,7 +219,7 @@ fn inserting_data_succeeds_when_data_is_good() { IndexingPalletPermission::SubmitDataForPublicQuorum, )]) .unwrap(); - pallet_permissions::Permissions::::insert(who, permissions.clone()); + pallet_permissions::Permissions::::insert(&who, permissions.clone()); let test_batch = BatchId::try_from(b"test_batch".to_vec()).unwrap(); let test_data = row_data(); @@ -236,74 +237,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(internal_batch_id, 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(sp_runtime::AccountId32::new([1; 32])); - 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); - - let internal_batch_id = build_inner_batch_id::(&test_batch, &table_id); - - // Verify that the submission was stored as expected - // and the hash was generated from the submitted data - assert_eq!( - Indexing::submissions(internal_batch_id.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_v1((internal_batch_id.clone(), QuorumScope::Public, who)), + Some(hash) ); }) } @@ -319,7 +254,8 @@ fn data_submission_fails_if_no_permissions() { let test_data = RowData::try_from(b"some arbitrary row data".to_vec()).unwrap(); // Create a non permissioned signer - let signer = RuntimeOrigin::signed(sp_runtime::AccountId32::new([1; 32])); + let account = sp_runtime::AccountId32::new([1; 32]); + let signer = RuntimeOrigin::signed(account.clone()); assert_err!( Indexing::submit_data( signer.clone(), @@ -330,13 +266,13 @@ fn data_submission_fails_if_no_permissions() { crate::Error::::UnauthorizedSubmitter, ); - let hash = <::Hashing as Hasher>::hash(&test_data); + let _ = <::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 - ); + let internal_batch_id = build_inner_batch_id::(&test_batch, &test_identifier); + let submitters_count = + crate::SubmissionsV1::::iter_prefix((internal_batch_id.clone(),)).count(); + assert_eq!(submitters_count, 0); }) } @@ -419,9 +355,9 @@ 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(&internal_batch_id, test_data_hash); - assert!(submitters.scope_is_empty(&QuorumScope::Public)); - assert!(submitters.scope_is_empty(&QuorumScope::Privileged)); + let submitters_count = + crate::SubmissionsV1::::iter_prefix((internal_batch_id.clone(),)).count(); + assert_eq!(submitters_count, 0); }) } @@ -518,10 +454,9 @@ 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(&internal_batch_id, data_hash) - .scope_is_empty(&QuorumScope::Public)) - } + let submitters_count = + crate::SubmissionsV1::::iter_prefix((internal_batch_id.clone(),)).count(); + assert_eq!(submitters_count, 0); }) } @@ -807,7 +742,7 @@ fn submit_data_with_mothership_key_work() { PermissionLevel::IndexingPallet(IndexingPalletPermission::SubmitDataForPublicQuorum); assert_ok!(pallet_permissions::Pallet::::add_proxy_permission( RuntimeOrigin::signed(admin), - signer_key, + signer_key.clone(), permission, )); @@ -827,8 +762,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(internal_batch_id, hash).len_of_scope(&QuorumScope::Public), - 1 + Indexing::submissions_v1((internal_batch_id, QuorumScope::Public, signer_key)), + Some(hash) ); }) } @@ -889,9 +824,11 @@ 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(&internal_batch_id, test_data_hash); - assert!(submitters.scope_is_empty(&QuorumScope::Public)); - assert!(submitters.scope_is_empty(&QuorumScope::Privileged)); + let internal_batch_id = + build_inner_batch_id::(&test_submission.batch_id, &test_submission.table); + let submitters_count = + crate::SubmissionsV1::::iter_prefix((internal_batch_id.clone(),)).count(); + assert_eq!(submitters_count, 0); }) } @@ -961,17 +898,43 @@ fn we_can_manage_quorum_state_for_both_scopes() { let internal_batch_id = build_inner_batch_id::(&test_submission.batch_id, &table_id); - let submissions = Indexing::submissions(&internal_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(( + &internal_batch_id, + QuorumScope::Public + )) + .count(), + 1 + ); + assert_eq!( + crate::SubmissionsV1::::iter_prefix(( + &internal_batch_id, + QuorumScope::Privileged + )) + .count(), + 0 + ); assert!(Indexing::final_data(&internal_batch_id).is_none()); // both submission assert_ok!(submit_test_data(both_submitter, test_submission.clone())); - let submissions = Indexing::submissions(&internal_batch_id, test_data_hash); - assert_eq!(submissions.len_of_scope(&QuorumScope::Public), 1); - assert_eq!(submissions.len_of_scope(&QuorumScope::Privileged), 1); + assert_eq!( + crate::SubmissionsV1::::iter_prefix(( + &internal_batch_id, + QuorumScope::Public + )) + .count(), + 1 + ); + assert_eq!( + crate::SubmissionsV1::::iter_prefix(( + &internal_batch_id, + QuorumScope::Privileged + )) + .count(), + 1 + ); assert!(Indexing::final_data(&internal_batch_id).is_none()); // privileged submission @@ -986,9 +949,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(&internal_batch_id, test_data_hash); - assert!(submitters.scope_is_empty(&QuorumScope::Public)); - assert!(submitters.scope_is_empty(&QuorumScope::Privileged)); + assert_eq!( + crate::SubmissionsV1::::iter_prefix((&internal_batch_id,)).count(), + 0 + ); assert_eq!( System::read_events_for_pallet::>() @@ -1060,9 +1024,10 @@ fn reaching_quorum_for_both_scopes_simultaneously_produces_privileged_quorum_rea 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((&internal_batch_id,)).count(), + 0 + ); assert_eq!( System::read_events_for_pallet::>() @@ -1104,8 +1069,6 @@ fn we_cannot_submit_for_table_disabled_quorum_scope() { batch_id: BatchId::try_from(b"test_batch".to_vec()).unwrap(), data: row_data(), }; - let test_data_hash = - <::Hashing as Hasher>::hash(&test_submission.data); let public_permission = PermissionLevel::IndexingPallet(IndexingPalletPermission::SubmitDataForPublicQuorum); @@ -1122,10 +1085,13 @@ 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!(Indexing::final_data(&test_submission.batch_id).is_none()); + let internal_batch_id = + build_inner_batch_id::(&test_submission.batch_id, &test_submission.table); + assert_eq!( + crate::SubmissionsV1::::iter_prefix((&internal_batch_id,)).count(), + 0 + ); + assert!(Indexing::final_data(&internal_batch_id).is_none()); }) } @@ -1178,9 +1144,12 @@ 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)); + let internal_batch_id = + build_inner_batch_id::(&test_submission.batch_id, &test_submission.table); + assert_eq!( + crate::SubmissionsV1::::iter_prefix((&internal_batch_id,)).count(), + 0 + ); assert!(Indexing::final_data(&test_submission.batch_id).is_none()); }) } @@ -1464,9 +1433,9 @@ fn we_can_reach_quorum_before_and_after_changing_quorum_size() { build_inner_batch_id::(&test_submission.batch_id, &table_id); // Verify that the old data was successfully removed for this batch - let submitters = Indexing::submissions(&internal_batch_id, test_data_hash); - assert!(submitters.scope_is_empty(&QuorumScope::Public)); - assert!(submitters.scope_is_empty(&QuorumScope::Privileged)); + let submitters_count = + crate::SubmissionsV1::::iter_prefix((internal_batch_id.clone(),)).count(); + assert_eq!(submitters_count, 0); // Now update the quorum to make this table public let new_quorum = InsertQuorumSize { @@ -1523,9 +1492,9 @@ fn we_can_reach_quorum_before_and_after_changing_quorum_size() { assert_eq!(fd.quorum_scope, QuorumScope::Public); // Verify that the old data was successfully removed for this batch - let submitters = Indexing::submissions(&internal_batch_id_2, test_data_hash); - assert!(submitters.scope_is_empty(&QuorumScope::Public)); - assert!(submitters.scope_is_empty(&QuorumScope::Privileged)); + let submitters_count = + crate::SubmissionsV1::::iter_prefix((internal_batch_id_2.clone(),)).count(); + assert_eq!(submitters_count, 0); }); } @@ -1570,3 +1539,136 @@ fn we_can_submit_to_permissionless_table_with_no_permissions() { assert!(Indexing::final_data(&internal_batch_id).is_some()); }) } + +#[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(sp_runtime::AccountId32::new([1; 32])); + 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(); + let internal_batch_id = build_inner_batch_id::(&batch_id, &table_id); + assert_eq!( + crate::SubmissionsV1::::get((&internal_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((&internal_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(); + + let internal_batch_id = build_inner_batch_id::(&batch_id, &table_id); + // artificially fill submissions for batch + (1..=MAX_SUBMITTERS).for_each(|submitter_num| { + let signer = + RuntimeOrigin::signed(sp_runtime::AccountId32::new([submitter_num as u8; 32])); + let who = ensure_signed(signer.clone()).unwrap(); + let artificial_data_hash = <::Hashing as Hasher>::hash( + &submitter_num.to_le_bytes(), + ); + + crate::SubmissionsV1::::insert( + (internal_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(sp_runtime::AccountId32::new( + [(MAX_SUBMITTERS + 1) as u8; 32], + )); + 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(sp_runtime::AccountId32::new([1; 32])); + 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((&internal_batch_id, QuorumScope::Public, &who)) + .unwrap(), + data_hash + ); + }); +} From 143d1382aec5736f5ab1e8aa902a4cba89de8353 Mon Sep 17 00:00:00 2001 From: Trevor Lovell Date: Wed, 3 Dec 2025 15:32:28 -0700 Subject: [PATCH 2/2] fix: update pallet-indexing weights Recent changes have been made that switch pallet-indexing to using a newer form of submissions storage to find quorum. This change re-benchmarks the weights (which didn't really change much). --- pallets/indexing/src/weights.rs | 70 +++++++++++++++++++-------------- runtime/src/lib.rs | 2 +- 2 files changed, 42 insertions(+), 30 deletions(-) diff --git a/pallets/indexing/src/weights.rs b/pallets/indexing/src/weights.rs index cfa5f1fe..3c51cf6f 100644 --- a/pallets/indexing/src/weights.rs +++ b/pallets/indexing/src/weights.rs @@ -2,7 +2,7 @@ //! Autogenerated weights for `pallet_indexing` //! //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 43.0.0 -//! DATE: 2025-10-31, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! DATE: 2025-12-03, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` //! WORST CASE MAP SIZE: `1000000` //! HOSTNAME: `trevor-benchmark`, CPU: `AMD EPYC 7763 64-Core Processor` //! WASM-EXECUTION: `Compiled`, CHAIN: `None`, DB CACHE: `1024` @@ -50,16 +50,22 @@ impl WeightInfo for SubstrateWeight { /// Proof: `Indexing::FinalData` (`max_values`: None, `max_size`: Some(2337), added: 4812, mode: `MaxEncodedLen`) /// Storage: `Tables::Schemas` (r:1 w:0) /// Proof: `Tables::Schemas` (`max_values`: None, `max_size`: Some(8358), added: 10833, mode: `MaxEncodedLen`) - /// Storage: `Indexing::Submissions` (r:1 w:1) - /// Proof: `Indexing::Submissions` (`max_values`: None, `max_size`: Some(2151), added: 4626, mode: `MaxEncodedLen`) + /// Storage: `Indexing::SubmissionsV1` (r:2 w:1) + /// Proof: `Indexing::SubmissionsV1` (`max_values`: None, `max_size`: Some(150), added: 2625, mode: `MaxEncodedLen`) + /// Storage: `Indexing::BatchQueueBottom` (r:1 w:0) + /// Proof: `Indexing::BatchQueueBottom` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Indexing::CounterForBatchQueue` (r:1 w:1) + /// Proof: `Indexing::CounterForBatchQueue` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Indexing::BatchQueue` (r:1 w:1) + /// Proof: `Indexing::BatchQueue` (`max_values`: None, `max_size`: Some(57), added: 2532, mode: `MaxEncodedLen`) fn submit_data_quorum_not_reached() -> Weight { // Proof Size summary in bytes: - // Measured: `611` + // Measured: `1086` // Estimated: `276827554` - // Minimum execution time: 362_637_000 picoseconds. - Weight::from_parts(365_193_000, 276827554) - .saturating_add(T::DbWeight::get().reads(7_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) + // Minimum execution time: 393_946_000 picoseconds. + Weight::from_parts(396_661_000, 276827554) + .saturating_add(T::DbWeight::get().reads(11_u64)) + .saturating_add(T::DbWeight::get().writes(3_u64)) } /// Storage: `Tables::TableInsertQuorums` (r:1 w:0) /// Proof: `Tables::TableInsertQuorums` (`max_values`: None, `max_size`: Some(152), added: 2627, mode: `MaxEncodedLen`) @@ -71,18 +77,18 @@ impl WeightInfo for SubstrateWeight { /// Proof: `Indexing::FinalData` (`max_values`: None, `max_size`: Some(2337), added: 4812, mode: `MaxEncodedLen`) /// Storage: `Tables::Schemas` (r:1 w:0) /// Proof: `Tables::Schemas` (`max_values`: None, `max_size`: Some(8358), added: 10833, mode: `MaxEncodedLen`) - /// Storage: `Indexing::Submissions` (r:2 w:1) - /// Proof: `Indexing::Submissions` (`max_values`: None, `max_size`: Some(2151), added: 4626, mode: `MaxEncodedLen`) + /// Storage: `Indexing::SubmissionsV1` (r:5 w:4) + /// Proof: `Indexing::SubmissionsV1` (`max_values`: None, `max_size`: Some(150), added: 2625, mode: `MaxEncodedLen`) /// Storage: `Commitments::CommitmentStorageMap` (r:2 w:2) /// Proof: `Commitments::CommitmentStorageMap` (`max_values`: None, `max_size`: Some(45497), added: 47972, mode: `MaxEncodedLen`) fn submit_data_quorum_reached() -> Weight { // Proof Size summary in bytes: - // Measured: `46053` + // Measured: `46691` // Estimated: `276827554` - // Minimum execution time: 121_426_138_000 picoseconds. - Weight::from_parts(125_614_953_000, 276827554) - .saturating_add(T::DbWeight::get().reads(10_u64)) - .saturating_add(T::DbWeight::get().writes(4_u64)) + // Minimum execution time: 121_523_547_000 picoseconds. + Weight::from_parts(125_296_478_000, 276827554) + .saturating_add(T::DbWeight::get().reads(13_u64)) + .saturating_add(T::DbWeight::get().writes(7_u64)) } } @@ -98,16 +104,22 @@ impl WeightInfo for () { /// Proof: `Indexing::FinalData` (`max_values`: None, `max_size`: Some(2337), added: 4812, mode: `MaxEncodedLen`) /// Storage: `Tables::Schemas` (r:1 w:0) /// Proof: `Tables::Schemas` (`max_values`: None, `max_size`: Some(8358), added: 10833, mode: `MaxEncodedLen`) - /// Storage: `Indexing::Submissions` (r:1 w:1) - /// Proof: `Indexing::Submissions` (`max_values`: None, `max_size`: Some(2151), added: 4626, mode: `MaxEncodedLen`) + /// Storage: `Indexing::SubmissionsV1` (r:2 w:1) + /// Proof: `Indexing::SubmissionsV1` (`max_values`: None, `max_size`: Some(150), added: 2625, mode: `MaxEncodedLen`) + /// Storage: `Indexing::BatchQueueBottom` (r:1 w:0) + /// Proof: `Indexing::BatchQueueBottom` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Indexing::CounterForBatchQueue` (r:1 w:1) + /// Proof: `Indexing::CounterForBatchQueue` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Indexing::BatchQueue` (r:1 w:1) + /// Proof: `Indexing::BatchQueue` (`max_values`: None, `max_size`: Some(57), added: 2532, mode: `MaxEncodedLen`) fn submit_data_quorum_not_reached() -> Weight { // Proof Size summary in bytes: - // Measured: `611` + // Measured: `1086` // Estimated: `276827554` - // Minimum execution time: 362_637_000 picoseconds. - Weight::from_parts(365_193_000, 276827554) - .saturating_add(RocksDbWeight::get().reads(7_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) + // Minimum execution time: 393_946_000 picoseconds. + Weight::from_parts(396_661_000, 276827554) + .saturating_add(RocksDbWeight::get().reads(11_u64)) + .saturating_add(RocksDbWeight::get().writes(3_u64)) } /// Storage: `Tables::TableInsertQuorums` (r:1 w:0) /// Proof: `Tables::TableInsertQuorums` (`max_values`: None, `max_size`: Some(152), added: 2627, mode: `MaxEncodedLen`) @@ -119,17 +131,17 @@ impl WeightInfo for () { /// Proof: `Indexing::FinalData` (`max_values`: None, `max_size`: Some(2337), added: 4812, mode: `MaxEncodedLen`) /// Storage: `Tables::Schemas` (r:1 w:0) /// Proof: `Tables::Schemas` (`max_values`: None, `max_size`: Some(8358), added: 10833, mode: `MaxEncodedLen`) - /// Storage: `Indexing::Submissions` (r:2 w:1) - /// Proof: `Indexing::Submissions` (`max_values`: None, `max_size`: Some(2151), added: 4626, mode: `MaxEncodedLen`) + /// Storage: `Indexing::SubmissionsV1` (r:5 w:4) + /// Proof: `Indexing::SubmissionsV1` (`max_values`: None, `max_size`: Some(150), added: 2625, mode: `MaxEncodedLen`) /// Storage: `Commitments::CommitmentStorageMap` (r:2 w:2) /// Proof: `Commitments::CommitmentStorageMap` (`max_values`: None, `max_size`: Some(45497), added: 47972, mode: `MaxEncodedLen`) fn submit_data_quorum_reached() -> Weight { // Proof Size summary in bytes: - // Measured: `46053` + // Measured: `46691` // Estimated: `276827554` - // Minimum execution time: 121_426_138_000 picoseconds. - Weight::from_parts(125_614_953_000, 276827554) - .saturating_add(RocksDbWeight::get().reads(10_u64)) - .saturating_add(RocksDbWeight::get().writes(4_u64)) + // Minimum execution time: 121_523_547_000 picoseconds. + Weight::from_parts(125_296_478_000, 276827554) + .saturating_add(RocksDbWeight::get().reads(13_u64)) + .saturating_add(RocksDbWeight::get().writes(7_u64)) } } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index ae202909..f901e499 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -366,7 +366,7 @@ pub const AVERAGE_INSERT_TARGET_COST: u128 = MILLICENTS pub const TARGET_BYTE_FEE: u128 = AVERAGE_INSERT_TARGET_COST.saturating_div(AVERAGE_INSERT_SIZE_BYTES); /// Approximated Average Insert Weight from actual transactions on testnet -pub const AVERAGE_INSERT_CALL_WEIGHT: u128 = 125_614_953_000; +pub const AVERAGE_INSERT_CALL_WEIGHT: u128 = 125_296_478_000; pub const WEIGHT_FEE: u128 = AVERAGE_INSERT_TARGET_COST.saturating_div(AVERAGE_INSERT_CALL_WEIGHT); parameter_types! {