Skip to content

Commit

Permalink
Some fixes for UTs
Browse files Browse the repository at this point in the history
  • Loading branch information
fmiguelgarcia committed May 15, 2023
1 parent 999de7e commit 99d9721
Show file tree
Hide file tree
Showing 19 changed files with 301 additions and 276 deletions.
217 changes: 87 additions & 130 deletions Cargo.lock

Large diffs are not rendered by default.

4 changes: 1 addition & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
[workspace]
members = [
"pallets/executive",
"pallets/system",
"pallets/dactr",
"pallets/bridges/nomad/updater-manager",
Expand All @@ -14,13 +13,11 @@ members = [
[patch."https://github.com/paritytech/substrate.git"]
frame-system = { path = "pallets/system" }
frame-system-benchmarking = { path = "pallets/system/benchmarking" }
frame-executive = { path = "pallets/executive" }
frame-system-rpc-runtime-api = { path = "pallets/system/rpc/runtime-api" }

[patch.crates-io]
# Customized Local pallets
frame-system = { path = "pallets/system" }
frame-executive = { path = "pallets/executive" }
frame-system-rpc-runtime-api = { path = "pallets/system/rpc/runtime-api" }
frame-system-benchmarking = { path = "pallets/system/benchmarking" }

Expand Down Expand Up @@ -93,6 +90,7 @@ sp-weights = { git = "https://github.com/paritytech/substrate.git", branch = "po
sc-sysinfo = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.37" }

## Frame
frame-executive = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.37" }
frame-support = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.37" }
frame-try-runtime = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.37" }
pallet-balances = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.37" }
Expand Down
34 changes: 6 additions & 28 deletions node/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,10 @@ pub type BlockImport = da::BlockImport<
pub mod da {
use std::{collections::HashMap, sync::Arc};

use da_primitives::{
asdr::AppExtrinsic, BlockLengthColumns, BlockLengthRows, OpaqueExtrinsic, BLOCK_CHUNK_SIZE,
};
use da_primitives::{BlockLengthColumns, BlockLengthRows, OpaqueExtrinsic, BLOCK_CHUNK_SIZE};
use da_runtime::{
apis::{DataAvailApi, ExtensionBuilder},
Header as DaHeader, Runtime, UncheckedExtrinsic,
Header as DaHeader, Runtime,
};
use derive_more::Constructor;
use frame_support::ensure;
Expand Down Expand Up @@ -141,6 +139,7 @@ pub mod da {
) -> Result<ImportResult, Self::Error> {
let no_extrinsics = vec![];
let extrinsics = block.body.as_ref().unwrap_or(&no_extrinsics);

let raw_ext_iter = extrinsics.iter().map(|opaque| opaque.0.as_slice());
let data_root = submitted_data::extrinsics_root::<Runtime, _>(raw_ext_iter);

Expand All @@ -153,47 +152,26 @@ pub mod da {
)
.expect("Valid BlockLength at genesis .qed");

let app_extrinsics = extrinsics
.iter()
.filter_map(|opaque| UncheckedExtrinsic::try_from(opaque).ok())
.map(AppExtrinsic::from)
.collect::<Vec<_>>();

let best_hash = self.client.info().best_hash;
let block_id = BlockId::Hash(best_hash);
let generated_ext = self
.client
.runtime_api()
.build_extension(
&block_id,
app_extrinsics,
extrinsics.clone(),
data_root,
block_len,
block.header.number,
)
.map_err(|e| {
ConsensusError::ClientImport(format!("Build extension fails due to: {e:?}"))
})?;
/*
let seed = self.client.runtime_api().babe_vrf(&block_id).map_err(|_| {
ConsensusError::ClientImport(format!(
"DA Protocol cannot fetch Babe VRF for block {block_id:?}"
))
})?;
let metrics = IgnoreMetrics {};
let generated_ext = build_extension::<IgnoreMetrics>(
&app_extrinsics,
data_root,
block_len,
block.header.number,
seed,
&metrics,
);*/

ensure!(
extension == &generated_ext,
ConsensusError::ClientImport("DA Extension do NOT match".into())
ConsensusError::ClientImport(
format!("DA Extension does NOT match\nExpected: {extension:?}\nGenerated:{generated_ext:?}"))
);

self.inner
Expand Down
1 change: 1 addition & 0 deletions pallets/bridges/nomad/da-bridge/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl system::Config for Test {
type SS58Prefix = ();
type SubmittedDataExtractor = ();
type SystemWeightInfo = ();
type UncheckedExtrinsic = UncheckedExtrinsic;
type Version = ();
}

Expand Down
1 change: 1 addition & 0 deletions pallets/bridges/nomad/home/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ impl system::Config for Test {
type SS58Prefix = ();
type SubmittedDataExtractor = ();
type SystemWeightInfo = ();
type UncheckedExtrinsic = UncheckedExtrinsic;
type Version = ();
}

Expand Down
52 changes: 26 additions & 26 deletions pallets/bridges/nomad/home/src/weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
// Storage: NomadHome RootToIndex (r:1 w:0)
fn improper_update() -> Weight {
// Minimum execution time: 495_656 nanoseconds.
Weight::from_ref_time(513_119_000 as u64)
.saturating_add(T::DbWeight::get().reads(2 as u64))
.saturating_add(T::DbWeight::get().writes(1 as u64))
Weight::from_ref_time(513_119_000u64)
.saturating_add(T::DbWeight::get().reads(2u64))
.saturating_add(T::DbWeight::get().writes(1u64))
}
// Storage: NomadHome Base (r:1 w:0)
// Storage: NomadHome Nonces (r:1 w:1)
Expand All @@ -60,28 +60,28 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
/// The range of component `b` is `[1, 2048]`.
fn dispatch(b: u32, ) -> Weight {
// Minimum execution time: 114_230 nanoseconds.
Weight::from_ref_time(117_260_068 as u64)
Weight::from_ref_time(117_260_068u64)
// Standard Error: 1_140
.saturating_add(Weight::from_ref_time(9_021 as u64).saturating_mul(b as u64))
.saturating_add(T::DbWeight::get().reads(3 as u64))
.saturating_add(T::DbWeight::get().writes(4 as u64))
.saturating_add(Weight::from_ref_time(9_021u64).saturating_mul(b as u64))
.saturating_add(T::DbWeight::get().reads(3u64))
.saturating_add(T::DbWeight::get().writes(4u64))
}
// Storage: NomadHome Base (r:1 w:1)
// Storage: NomadHome RootToIndex (r:32 w:32)
// Storage: NomadHome IndexToRoot (r:31 w:32)
fn update() -> Weight {
// Minimum execution time: 845_072 nanoseconds.
Weight::from_ref_time(881_385_000 as u64)
.saturating_add(T::DbWeight::get().reads(64 as u64))
.saturating_add(T::DbWeight::get().writes(65 as u64))
Weight::from_ref_time(881_385_000u64)
.saturating_add(T::DbWeight::get().reads(64u64))
.saturating_add(T::DbWeight::get().writes(65u64))
}
// Storage: NomadHome Base (r:1 w:1)
// Storage: UpdaterManager Updater (r:1 w:1)
fn set_updater() -> Weight {
// Minimum execution time: 45_128 nanoseconds.
Weight::from_ref_time(46_213_000 as u64)
.saturating_add(T::DbWeight::get().reads(2 as u64))
.saturating_add(T::DbWeight::get().writes(2 as u64))
Weight::from_ref_time(46_213_000u64)
.saturating_add(T::DbWeight::get().reads(2u64))
.saturating_add(T::DbWeight::get().writes(2u64))
}
}

Expand All @@ -91,9 +91,9 @@ impl WeightInfo for () {
// Storage: NomadHome RootToIndex (r:1 w:0)
fn improper_update() -> Weight {
// Minimum execution time: 495_656 nanoseconds.
Weight::from_ref_time(513_119_000 as u64)
.saturating_add(RocksDbWeight::get().reads(2 as u64))
.saturating_add(RocksDbWeight::get().writes(1 as u64))
Weight::from_ref_time(513_119_000u64)
.saturating_add(RocksDbWeight::get().reads(2u64))
.saturating_add(RocksDbWeight::get().writes(1u64))
}
// Storage: NomadHome Base (r:1 w:0)
// Storage: NomadHome Nonces (r:1 w:1)
Expand All @@ -103,27 +103,27 @@ impl WeightInfo for () {
/// The range of component `b` is `[1, 2048]`.
fn dispatch(b: u32, ) -> Weight {
// Minimum execution time: 114_230 nanoseconds.
Weight::from_ref_time(117_260_068 as u64)
Weight::from_ref_time(117_260_068u64)
// Standard Error: 1_140
.saturating_add(Weight::from_ref_time(9_021 as u64).saturating_mul(b as u64))
.saturating_add(RocksDbWeight::get().reads(3 as u64))
.saturating_add(RocksDbWeight::get().writes(4 as u64))
.saturating_add(Weight::from_ref_time(9_021u64).saturating_mul(b as u64))
.saturating_add(RocksDbWeight::get().reads(3u64))
.saturating_add(RocksDbWeight::get().writes(4u64))
}
// Storage: NomadHome Base (r:1 w:1)
// Storage: NomadHome RootToIndex (r:32 w:32)
// Storage: NomadHome IndexToRoot (r:31 w:32)
fn update() -> Weight {
// Minimum execution time: 845_072 nanoseconds.
Weight::from_ref_time(881_385_000 as u64)
.saturating_add(RocksDbWeight::get().reads(64 as u64))
.saturating_add(RocksDbWeight::get().writes(65 as u64))
Weight::from_ref_time(881_385_000u64)
.saturating_add(RocksDbWeight::get().reads(64u64))
.saturating_add(RocksDbWeight::get().writes(65u64))
}
// Storage: NomadHome Base (r:1 w:1)
// Storage: UpdaterManager Updater (r:1 w:1)
fn set_updater() -> Weight {
// Minimum execution time: 45_128 nanoseconds.
Weight::from_ref_time(46_213_000 as u64)
.saturating_add(RocksDbWeight::get().reads(2 as u64))
.saturating_add(RocksDbWeight::get().writes(2 as u64))
Weight::from_ref_time(46_213_000u64)
.saturating_add(RocksDbWeight::get().reads(2u64))
.saturating_add(RocksDbWeight::get().writes(2u64))
}
}
5 changes: 3 additions & 2 deletions pallets/bridges/nomad/updater-manager/src/mock.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use da_primitives::Header;
use frame_support::{parameter_types, weights::Weight};
use frame_system::{self as system, test_utils::TestRandomness};
use frame_system::{self as system, mocking::MockUncheckedExtrinsic, test_utils::TestRandomness};
use primitive_types::H256;
use sp_runtime::traits::{BlakeTwo256, ConstU32, IdentityLookup};

use crate as updater_manager;

type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
type UncheckedExtrinsic = MockUncheckedExtrinsic<Test>;
type Block = frame_system::mocking::MockBlock<Test>;

// Configure a mock runtime to test the pallet.
Expand Down Expand Up @@ -55,6 +55,7 @@ impl system::Config for Test {
type SS58Prefix = ();
type SubmittedDataExtractor = ();
type SystemWeightInfo = ();
type UncheckedExtrinsic = UncheckedExtrinsic;
type Version = ();
}

Expand Down
29 changes: 11 additions & 18 deletions pallets/dactr/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,29 @@ use frame_support::{
parameter_types,
weights::{ConstantMultiplier, IdentityFee, Weight},
};
use frame_system::{header_builder::da::HeaderExtensionBuilder, test_utils::TestRandomness};
use frame_system::{
header_builder::da::HeaderExtensionBuilder, mocking::MockUncheckedExtrinsic,
test_utils::TestRandomness,
};
use pallet_transaction_payment::CurrencyAdapter;
use sp_core::H256;
use sp_runtime::{
generic,
traits::{BlakeTwo256, ConstU32, IdentityLookup},
};
use sp_runtime::traits::{BlakeTwo256, ConstU32, IdentityLookup};

use crate::{self as da_control, *};

/// An unchecked extrinsic type to be used in tests.
pub type UncheckedExtrinsic<T, Signature = (), Extra = ()> = generic::UncheckedExtrinsic<
<T as frame_system::Config>::AccountId,
<T as frame_system::Config>::RuntimeCall,
Signature,
Extra,
>;
type UncheckedExtrinsic = MockUncheckedExtrinsic<Test>;

/// An implementation of `sp_runtime::traits::Block` to be used in tests.
pub type Block<T> = generic::Block<
Header<<T as frame_system::Config>::BlockNumber, sp_runtime::traits::BlakeTwo256>,
UncheckedExtrinsic<T>,
>;
type Block = frame_system::mocking::MockBlock<Test>;

type BlockNumber = u32;

frame_support::construct_runtime!(
pub enum Test where
Block = Block<Test>,
NodeBlock = Block<Test>,
UncheckedExtrinsic = UncheckedExtrinsic<Test>,
Block = Block,
NodeBlock = Block,
UncheckedExtrinsic = UncheckedExtrinsic,
{
System: frame_system,
Balances: pallet_balances,
Expand Down Expand Up @@ -81,6 +73,7 @@ impl frame_system::Config for Test {
type SS58Prefix = ();
type SubmittedDataExtractor = ();
type SystemWeightInfo = ();
type UncheckedExtrinsic = UncheckedExtrinsic;
type Version = ();
}

Expand Down
16 changes: 3 additions & 13 deletions pallets/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ impl<
COnRuntimeUpgrade: OnRuntimeUpgrade,
> Executive<System, Block, Context, UnsignedValidator, AllPalletsWithSystem, COnRuntimeUpgrade>
where
Block::Extrinsic: Checkable<Context> + Codec + GetAppId,
Block::Extrinsic: Checkable<Context> + Codec,
CheckedOf<Block::Extrinsic, Context>: Applyable + GetDispatchInfo,
CallOf<Block::Extrinsic, Context>:
Dispatchable<Info = DispatchInfo, PostInfo = PostDispatchInfo>,
Expand Down Expand Up @@ -566,25 +566,15 @@ where
sp_io::init_tracing();
let encoded = uxt.encode();
let encoded_len = encoded.len();
Self::apply_extrinsic_with_len(uxt, encoded_len, encoded)
}

/// Actually apply an extrinsic given its `encoded_len`; this doesn't note its hash.
fn apply_extrinsic_with_len(
uxt: Block::Extrinsic,
encoded_len: usize,
to_note: Vec<u8>,
) -> ApplyExtrinsicResult {
let app_id = uxt.app_id();
sp_tracing::enter_span!(sp_tracing::info_span!("apply_extrinsic",
ext=?sp_core::hexdisplay::HexDisplay::from(&uxt.encode())));
ext=?sp_core::hexdisplay::HexDisplay::from(&encoded)));
// Verify that the signature is good.
let xt = uxt.check(&Default::default())?;

// We don't need to make sure to `note_extrinsic` only after we know it's going to be
// executed to prevent it from leaking in storage since at this point, it will either
// execute or panic (and revert storage changes).
<frame_system::Pallet<System>>::note_extrinsic(app_id, to_note);
<frame_system::Pallet<System>>::note_extrinsic(encoded);

// AUDIT: Under no circumstances may this function panic from here onwards.

Expand Down
2 changes: 1 addition & 1 deletion pallets/system/benchmarking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ benchmarks! {
let block_length = Default::default();

}: {
let _header = T::HeaderExtensionBuilder::build(app_extrinsics, data_root, block_length);
let _header = T::HeaderExtensionBuilder::build(app_extrinsics, data_root, block_length, 0);
}

impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test);
Expand Down
8 changes: 6 additions & 2 deletions pallets/system/benchmarking/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,18 @@
#![cfg(test)]

use da_primitives::Header as DaHeader;
use frame_system::{header_builder::da::HeaderExtensionBuilder, test_utils::TestRandomness};
use frame_system::{
header_builder::da::HeaderExtensionBuilder, mocking::MockUncheckedExtrinsic,
test_utils::TestRandomness,
};
use sp_core::H256;
use sp_runtime::traits::{BlakeTwo256, IdentityLookup};

type AccountId = u64;
type AccountIndex = u32;
type BlockNumber = u32;

type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
type UncheckedExtrinsic = MockUncheckedExtrinsic<Test>;
type Block = frame_system::mocking::MockBlock<Test>;

frame_support::construct_runtime!(
Expand Down Expand Up @@ -68,6 +71,7 @@ impl frame_system::Config for Test {
type SS58Prefix = ();
type SubmittedDataExtractor = ();
type SystemWeightInfo = ();
type UncheckedExtrinsic = UncheckedExtrinsic;
type Version = ();
}

Expand Down
2 changes: 2 additions & 0 deletions pallets/system/src/header_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ pub fn build_extension<M: Metrics>(
// **NOTE:** Header extension V2 is not yet enable by default.
if cfg!(feature = "header_extension_v2") {
use da_primitives::kate_commitment::v2::KateCommitment;
#[allow(unused_mut)]
let mut kate = KateCommitment::new(rows, cols, data_root, commitment);

#[cfg(feature = "header_commitment_corruption")]
Expand All @@ -158,6 +159,7 @@ pub fn build_extension<M: Metrics>(
}
.into()
} else {
#[allow(unused_mut)]
let mut kate = da_primitives::kate_commitment::v1::KateCommitment {
rows,
cols,
Expand Down
Loading

0 comments on commit 99d9721

Please sign in to comment.