Skip to content

Commit

Permalink
stateful-storage: check hash in signed extension (#2137)
Browse files Browse the repository at this point in the history
# Goal
The goal of this PR is to check the hash for stateful storage
transactions and fail early.

Closes #1860

# Checklist
- [x] e2e Tests added?
- [X] Benchmarks added?
- [x] Spec version incremented?
  • Loading branch information
aramikm authored Aug 30, 2024
1 parent 07f7bd9 commit 978d6af
Show file tree
Hide file tree
Showing 10 changed files with 345 additions and 46 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions e2e/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions e2e/stateful-pallet-storage/handleItemized.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,10 @@ describe('📗 Stateful Pallet Storage', function () {
add_actions,
0
);
await assert.rejects(itemized_add_result_1.fundAndSend(fundingSource), { name: 'StalePageState' });
await assert.rejects(itemized_add_result_1.signAndSend('current'), {
name: 'RpcError',
message: /1010: Invalid Transaction: Custom error: 9/,
});
});
});

Expand Down Expand Up @@ -285,7 +288,10 @@ describe('📗 Stateful Pallet Storage', function () {
};
const remove_actions = [remove_action];
const op = ExtrinsicHelper.applyItemActions(providerKeys, schemaId_deletable, msa_id, remove_actions, 0);
await assert.rejects(op.fundAndSend(fundingSource), { name: 'StalePageState' });
await assert.rejects(op.signAndSend('current'), {
name: 'RpcError',
message: /1010: Invalid Transaction: Custom error: 9/,
});
});
});

Expand Down
14 changes: 7 additions & 7 deletions e2e/stateful-pallet-storage/handlePaginated.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,9 +201,9 @@ describe('📗 Stateful Pallet Storage', function () {
const payload_1 = new Bytes(ExtrinsicHelper.api.registry, 'Hello World From Frequency');

const paginated_add_result_1 = ExtrinsicHelper.upsertPage(providerKeys, schemaId, msa_id, page_id, payload_1, 0);
await assert.rejects(paginated_add_result_1.fundAndSend(fundingSource), {
name: 'StalePageState',
section: 'statefulStorage',
await assert.rejects(paginated_add_result_1.signAndSend('current'), {
name: 'RpcError',
message: /1010: Invalid Transaction: Custom error: 9/,
});
});
});
Expand Down Expand Up @@ -242,10 +242,10 @@ describe('📗 Stateful Pallet Storage', function () {
});

it('🛑 should fail call to remove page with stale target hash', async function () {
const paginated_add_result_1 = ExtrinsicHelper.removePage(providerKeys, schemaId, msa_id, 0, 0);
await assert.rejects(paginated_add_result_1.fundAndSend(fundingSource), {
name: 'StalePageState',
section: 'statefulStorage',
const paginated_add_result_1 = ExtrinsicHelper.removePage(providerKeys, schemaId, msa_id, 0, 1000);
await assert.rejects(paginated_add_result_1.signAndSend('current'), {
name: 'RpcError',
message: /1010: Invalid Transaction: Custom error: 9/,
});
});
});
Expand Down
1 change: 1 addition & 0 deletions node/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ frequency-service = { package = "frequency-service", path = "../service", defaul
pallet-msa = { package = "pallet-msa", path = "../../pallets/msa", default-features = false }
pallet-frequency-tx-payment = { package = "pallet-frequency-tx-payment", path = "../../pallets/frequency-tx-payment", default-features = false }
pallet-handles = { package = "pallet-handles", path = "../../pallets/handles", default-features = false }
pallet-stateful-storage = { package = "pallet-stateful-storage", path = "../../pallets/stateful-storage", default-features = false }
cli-opt = { default-features = false, path = "../cli-opt" }

# Substrate
Expand Down
12 changes: 8 additions & 4 deletions node/cli/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use sp_core::{Encode, Pair};
use sp_keyring::Sr25519Keyring;
use sp_runtime::{OpaqueExtrinsic, SaturatedConversion};

use frequency_runtime::StaleHashCheckExtension;
use pallet_balances::Call as BalancesCall;
use pallet_msa;
use sp_inherents::InherentDataProvider;
Expand Down Expand Up @@ -118,8 +119,10 @@ pub fn create_benchmark_extrinsic(
.unwrap_or(2) as u64;
let extra: runtime::SignedExtra = (
frame_system::CheckNonZeroSender::<runtime::Runtime>::new(),
frame_system::CheckSpecVersion::<runtime::Runtime>::new(),
frame_system::CheckTxVersion::<runtime::Runtime>::new(),
(
frame_system::CheckSpecVersion::<runtime::Runtime>::new(),
frame_system::CheckTxVersion::<runtime::Runtime>::new(),
),
frame_system::CheckGenesis::<runtime::Runtime>::new(),
frame_system::CheckEra::<runtime::Runtime>::from(sp_runtime::generic::Era::mortal(
period,
Expand All @@ -130,6 +133,7 @@ pub fn create_benchmark_extrinsic(
pallet_frequency_tx_payment::ChargeFrqTransactionPayment::<runtime::Runtime>::from(0),
pallet_msa::CheckFreeExtrinsicUse::<runtime::Runtime>::new(),
pallet_handles::handles_signed_extension::HandlesSignedExtension::<runtime::Runtime>::new(),
StaleHashCheckExtension,
frame_metadata_hash_extension::CheckMetadataHash::<runtime::Runtime>::new(false),
cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim::<runtime::Runtime>::new(),
);
Expand All @@ -139,15 +143,15 @@ pub fn create_benchmark_extrinsic(
extra.clone(),
(
(),
runtime::VERSION.spec_version,
runtime::VERSION.transaction_version,
(runtime::VERSION.spec_version, runtime::VERSION.transaction_version),
genesis_hash,
best_hash,
(),
(),
(),
(),
(),
(),
None,
(),
),
Expand Down
5 changes: 2 additions & 3 deletions pallets/stateful-storage/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ use sp_std::prelude::*;

mod stateful_child_tree;
pub mod types;

pub mod weights;

use crate::{stateful_child_tree::StatefulChildTree, types::*};
Expand Down Expand Up @@ -939,7 +938,7 @@ impl<T: Config> Pallet<T> {
}

/// Gets a paginated storage for desired parameters
fn get_paginated_page_for(
pub fn get_paginated_page_for(
msa_id: MessageSourceId,
schema_id: SchemaId,
page_id: PageId,
Expand All @@ -955,7 +954,7 @@ impl<T: Config> Pallet<T> {
}

/// Gets an itemized storage for desired parameters
fn get_itemized_page_for(
pub fn get_itemized_page_for(
msa_id: MessageSourceId,
schema_id: SchemaId,
) -> Result<Option<ItemizedPage<T>>, DispatchError> {
Expand Down
20 changes: 10 additions & 10 deletions runtime/common/src/weights/block_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
// limitations under the License.

//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0
//! DATE: 2024-08-15 (Y/M/D)
//! HOSTNAME: `ip-10-173-11-47`, CPU: `Intel(R) Xeon(R) Platinum 8375C CPU @ 2.90GHz`
//! DATE: 2024-08-21 (Y/M/D)
//! HOSTNAME: `ip-10-173-4-165`, CPU: `Intel(R) Xeon(R) Platinum 8375C CPU @ 2.90GHz`
//!
//! SHORT-NAME: `block`, LONG-NAME: `BlockExecution`, RUNTIME: `Frequency Development (No Relay)`
//! WARMUPS: `10`, REPEAT: `100`
Expand All @@ -43,17 +43,17 @@ parameter_types! {
/// Calculated by multiplying the *Average* with `1.0` and adding `0`.
///
/// Stats nanoseconds:
/// Min, Max: 337_630, 378_812
/// Average: 345_638
/// Median: 342_194
/// Std-Dev: 8479.5
/// Min, Max: 333_594, 560_445
/// Average: 345_593
/// Median: 339_315
/// Std-Dev: 24323.47
///
/// Percentiles nanoseconds:
/// 99th: 372_459
/// 95th: 362_361
/// 75th: 346_630
/// 99th: 398_789
/// 95th: 364_915
/// 75th: 346_466
pub const BlockExecutionWeight: Weight =
Weight::from_parts(WEIGHT_REF_TIME_PER_NANOS.saturating_mul(345_638), 0);
Weight::from_parts(WEIGHT_REF_TIME_PER_NANOS.saturating_mul(345_593), 0);
}

#[cfg(test)]
Expand Down
20 changes: 10 additions & 10 deletions runtime/common/src/weights/extrinsic_weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
// limitations under the License.

//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 32.0.0
//! DATE: 2024-08-15 (Y/M/D)
//! HOSTNAME: `ip-10-173-11-47`, CPU: `Intel(R) Xeon(R) Platinum 8375C CPU @ 2.90GHz`
//! DATE: 2024-08-21 (Y/M/D)
//! HOSTNAME: `ip-10-173-4-165`, CPU: `Intel(R) Xeon(R) Platinum 8375C CPU @ 2.90GHz`
//!
//! SHORT-NAME: `extrinsic`, LONG-NAME: `ExtrinsicBase`, RUNTIME: `Frequency Development (No Relay)`
//! WARMUPS: `10`, REPEAT: `100`
Expand All @@ -43,17 +43,17 @@ parameter_types! {
/// Calculated by multiplying the *Average* with `1.0` and adding `0`.
///
/// Stats nanoseconds:
/// Min, Max: 92_351, 94_986
/// Average: 93_521
/// Median: 93_543
/// Std-Dev: 592.1
/// Min, Max: 90_815, 92_870
/// Average: 91_811
/// Median: 91_829
/// Std-Dev: 403.29
///
/// Percentiles nanoseconds:
/// 99th: 94_946
/// 95th: 94_443
/// 75th: 93_970
/// 99th: 92_749
/// 95th: 92_366
/// 75th: 92_088
pub const ExtrinsicBaseWeight: Weight =
Weight::from_parts(WEIGHT_REF_TIME_PER_NANOS.saturating_mul(93_521), 0);
Weight::from_parts(WEIGHT_REF_TIME_PER_NANOS.saturating_mul(91_811), 0);
}

#[cfg(test)]
Expand Down
Loading

0 comments on commit 978d6af

Please sign in to comment.