Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Task(bump) Pallet Salary #5163

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.

Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ frame-system = { workspace = true }
frame-system-benchmarking = { optional = true, workspace = true }
frame-system-rpc-runtime-api = { workspace = true }
frame-try-runtime = { optional = true, workspace = true }
frame-metadata-hash-extension = { workspace = true }
pallet-asset-rate = { workspace = true }
pallet-alliance = { workspace = true }
pallet-aura = { workspace = true }
Expand Down Expand Up @@ -191,6 +192,7 @@ std = [
"frame-system-rpc-runtime-api/std",
"frame-system/std",
"frame-try-runtime?/std",
"frame-metadata-hash-extension/std",
"log/std",
"pallet-alliance/std",
"pallet-asset-rate/std",
Expand Down Expand Up @@ -247,6 +249,10 @@ std = [
"xcm/std",
]

experimental = [
"pallet-salary/experimental",
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should use experimental by default for the westend testnet.


# A feature that should be enabled when the runtime should be built for on-chain
# deployment. This will disable stuff that shouldn't be part of the on-chain wasm
# to make it smaller, like logging for example.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ pub type AmbassadorSalaryPaymaster = PayOverXcm<
>;

impl pallet_salary::Config<AmbassadorSalaryInstance> for Runtime {
type RuntimeTask = <Runtime as frame_system::Config>::RuntimeTask;
type WeightInfo = weights::pallet_salary_ambassador_salary::WeightInfo<Runtime>;
type RuntimeEvent = RuntimeEvent;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ pub type FellowshipSalaryPaymaster = PayOverXcm<
>;

impl pallet_salary::Config<FellowshipSalaryInstance> for Runtime {
type RuntimeTask = <Runtime as frame_system::Config>::RuntimeTask;
type WeightInfo = weights::pallet_salary_fellowship_salary::WeightInfo<Runtime>;
type RuntimeEvent = RuntimeEvent;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ use sp_runtime::{
use sp_version::NativeVersion;
use sp_version::RuntimeVersion;

/// FRAME signed extension for verifying the metadata hash.
pub use frame_metadata_hash_extension;

use codec::{Decode, Encode, MaxEncodedLen};
use cumulus_primitives_core::{AggregateMessageOrigin, ClaimQueueOffset, CoreSelector, ParaId};
use frame_support::{
Expand All @@ -91,7 +94,10 @@ use parachains_common::{
AccountId, AuraId, Balance, BlockNumber, Hash, Header, Nonce, Signature,
AVERAGE_ON_INITIALIZE_RATIO, NORMAL_DISPATCH_RATIO,
};
use sp_runtime::RuntimeDebug;
use sp_runtime::{
traits::{SaturatedConversion, Verify},
RuntimeDebug,
};
use testnet_parachains_constants::westend::{
account::*, consensus::*, currency::*, fee::WeightToFee, time::*,
};
Expand Down Expand Up @@ -170,6 +176,94 @@ parameter_types! {
pub const SS58Prefix: u8 = 42;
}

pub type SignedPayload = generic::SignedPayload<RuntimeCall, TxExtension>;

impl frame_system::offchain::SigningTypes for Runtime {
type Public = <Signature as Verify>::Signer;
type Signature = Signature;
}
Comment on lines +179 to +184
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed no?

Suggested change
pub type SignedPayload = generic::SignedPayload<RuntimeCall, TxExtension>;
impl frame_system::offchain::SigningTypes for Runtime {
type Public = <Signature as Verify>::Signer;
type Signature = Signature;
}


impl<C> frame_system::offchain::CreateTransactionBase<C> for Runtime
where
RuntimeCall: From<C>,
{
type RuntimeCall = RuntimeCall;
type Extrinsic = UncheckedExtrinsic;
}

impl<LocalCall> frame_system::offchain::CreateTransaction<LocalCall> for Runtime
where
RuntimeCall: From<LocalCall>,
{
type Extension = TxExtension;

fn create_transaction(call: RuntimeCall, extension: TxExtension) -> UncheckedExtrinsic {
UncheckedExtrinsic::new_transaction(call, extension)
}
}

/// Submits a transaction with the node's public and signature type. Adheres to the signed extension
/// format of the chain.
impl<LocalCall> frame_system::offchain::CreateSignedTransaction<LocalCall> for Runtime
where
RuntimeCall: From<LocalCall>,
{
fn create_signed_transaction<
C: frame_system::offchain::AppCrypto<Self::Public, Self::Signature>,
>(
call: RuntimeCall,
public: <Signature as Verify>::Signer,
account: AccountId,
nonce: <Runtime as frame_system::Config>::Nonce,
) -> Option<UncheckedExtrinsic> {
use sp_runtime::traits::StaticLookup;
// take the biggest period possible.
let period =
BlockHashCount::get().checked_next_power_of_two().map(|c| c / 2).unwrap_or(2) as u64;

let current_block = System::block_number()
.saturated_into::<u64>()
// The `System::block_number` is initialized with `n+1`,
// so the actual block number is `n`.
.saturating_sub(1);
let tip = 0;
let tx_ext: TxExtension = (
frame_system::CheckNonZeroSender::<Runtime>::new(),
frame_system::CheckSpecVersion::<Runtime>::new(),
frame_system::CheckTxVersion::<Runtime>::new(),
frame_system::CheckGenesis::<Runtime>::new(),
frame_system::CheckMortality::<Runtime>::from(generic::Era::mortal(
period,
current_block,
)),
frame_system::CheckNonce::<Runtime>::from(nonce),
frame_system::CheckWeight::<Runtime>::new(),
pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::from(tip),
frame_metadata_hash_extension::CheckMetadataHash::<Runtime>::new(true),
)
.into();
let raw_payload = SignedPayload::new(call, tx_ext)
.map_err(|e| {
log::warn!("Unable to create signed payload: {:?}", e);
})
.ok()?;
let signature = raw_payload.using_encoded(|payload| C::sign(payload, public))?;
let (call, tx_ext, _) = raw_payload.deconstruct();
let address = <Runtime as frame_system::Config>::Lookup::unlookup(account);
let transaction = UncheckedExtrinsic::new_signed(call, address, signature, tx_ext);
Some(transaction)
}
}

Comment on lines +205 to +257
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not needed no?

Suggested change
/// Submits a transaction with the node's public and signature type. Adheres to the signed extension
/// format of the chain.
impl<LocalCall> frame_system::offchain::CreateSignedTransaction<LocalCall> for Runtime
where
RuntimeCall: From<LocalCall>,
{
fn create_signed_transaction<
C: frame_system::offchain::AppCrypto<Self::Public, Self::Signature>,
>(
call: RuntimeCall,
public: <Signature as Verify>::Signer,
account: AccountId,
nonce: <Runtime as frame_system::Config>::Nonce,
) -> Option<UncheckedExtrinsic> {
use sp_runtime::traits::StaticLookup;
// take the biggest period possible.
let period =
BlockHashCount::get().checked_next_power_of_two().map(|c| c / 2).unwrap_or(2) as u64;
let current_block = System::block_number()
.saturated_into::<u64>()
// The `System::block_number` is initialized with `n+1`,
// so the actual block number is `n`.
.saturating_sub(1);
let tip = 0;
let tx_ext: TxExtension = (
frame_system::CheckNonZeroSender::<Runtime>::new(),
frame_system::CheckSpecVersion::<Runtime>::new(),
frame_system::CheckTxVersion::<Runtime>::new(),
frame_system::CheckGenesis::<Runtime>::new(),
frame_system::CheckMortality::<Runtime>::from(generic::Era::mortal(
period,
current_block,
)),
frame_system::CheckNonce::<Runtime>::from(nonce),
frame_system::CheckWeight::<Runtime>::new(),
pallet_transaction_payment::ChargeTransactionPayment::<Runtime>::from(tip),
frame_metadata_hash_extension::CheckMetadataHash::<Runtime>::new(true),
)
.into();
let raw_payload = SignedPayload::new(call, tx_ext)
.map_err(|e| {
log::warn!("Unable to create signed payload: {:?}", e);
})
.ok()?;
let signature = raw_payload.using_encoded(|payload| C::sign(payload, public))?;
let (call, tx_ext, _) = raw_payload.deconstruct();
let address = <Runtime as frame_system::Config>::Lookup::unlookup(account);
let transaction = UncheckedExtrinsic::new_signed(call, address, signature, tx_ext);
Some(transaction)
}
}

impl<LocalCall> frame_system::offchain::CreateInherent<LocalCall> for Runtime
where
RuntimeCall: From<LocalCall>,
{
fn create_inherent(call: RuntimeCall) -> UncheckedExtrinsic {
UncheckedExtrinsic::new_bare(call)
}
}

// Configure FRAME pallets to include in runtime.
#[derive_impl(frame_system::config_preludes::ParaChainDefaultConfig)]
impl frame_system::Config for Runtime {
Expand Down Expand Up @@ -741,7 +835,8 @@ pub type TxExtension = (
frame_system::CheckEra<Runtime>,
frame_system::CheckNonce<Runtime>,
frame_system::CheckWeight<Runtime>,
cumulus_primitives_storage_weight_reclaim::StorageWeightReclaim<Runtime>,
pallet_transaction_payment::ChargeTransactionPayment<Runtime>,
frame_metadata_hash_extension::CheckMetadataHash<Runtime>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this needed? I understand it is better to have it. But I am afraid it breaks stuff.

);
/// Unchecked extrinsic type as expected by this runtime.
pub type UncheckedExtrinsic =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -187,4 +187,16 @@ impl<T: frame_system::Config> pallet_salary::WeightInfo for WeightInfo<T> {
.saturating_add(T::DbWeight::get().reads(3))
.saturating_add(T::DbWeight::get().writes(3))
}
/// Storage: `Ambassador::Status` (r:1 w:1)
/// Proof: `AmbassadorSalary::Status` (`max_values`: Some(1), `max_size`: Some(56), added: 551, mode: `MaxEncodedLen`)
fn bump_offchain() -> Weight {
// Proof Size summary in bytes:
// Measured: `258`
// Estimated: `1541`
// Minimum execution time: 7_000_000 picoseconds.
Weight::from_parts(8_000_000, 0)
.saturating_add(Weight::from_parts(0, 1541))
.saturating_add(T::DbWeight::get().reads(1))
.saturating_add(T::DbWeight::get().writes(1))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -186,4 +186,16 @@ impl<T: frame_system::Config> pallet_salary::WeightInfo for WeightInfo<T> {
.saturating_add(T::DbWeight::get().reads(3))
.saturating_add(T::DbWeight::get().writes(3))
}
/// Storage: `FellowshipSalary::Status` (r:1 w:1)
/// Proof: `FellowshipSalary::Status` (`max_values`: Some(1), `max_size`: Some(56), added: 551, mode: `MaxEncodedLen`)
fn bump_offchain() -> Weight {
// Proof Size summary in bytes:
// Measured: `258`
// Estimated: `1541`
// Minimum execution time: 7_000_000 picoseconds.
Weight::from_parts(8_000_000, 0)
.saturating_add(Weight::from_parts(0, 1541))
.saturating_add(T::DbWeight::get().reads(1))
.saturating_add(T::DbWeight::get().writes(1))
}
}
6 changes: 6 additions & 0 deletions polkadot/xcm/xcm-builder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,3 +75,9 @@ std = [
"xcm-executor/std",
"xcm/std",
]

experimental = [
"frame-support/experimental",
"frame-system/experimental",
"pallet-salary/experimental",
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this new feature?

21 changes: 20 additions & 1 deletion polkadot/xcm/xcm-builder/src/tests/pay/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use frame_support::{
};
use frame_system::{EnsureRoot, EnsureSigned};
use polkadot_primitives::{AccountIndex, BlakeTwo256, Signature};
use sp_runtime::{generic, traits::MaybeEquivalence, AccountId32, BuildStorage};
use sp_runtime::{generic, testing::TestXt, traits::MaybeEquivalence, AccountId32, BuildStorage};
use xcm_executor::{traits::ConvertLocation, XcmExecutor};

pub type TxExtension = (
Expand All @@ -47,6 +47,8 @@ pub type Block = generic::Block<Header, UncheckedExtrinsic>;
pub type BlockNumber = u32;
pub type AccountId = AccountId32;

pub type Extrinsic = TestXt<RuntimeCall, ()>;

construct_runtime!(
pub enum Test {
System: frame_system,
Expand All @@ -57,6 +59,23 @@ construct_runtime!(
}
);

impl<LocalCall> frame_system::offchain::CreateTransactionBase<LocalCall> for Test
where
RuntimeCall: From<LocalCall>,
{
type RuntimeCall = RuntimeCall;
type Extrinsic = Extrinsic;
}

impl<LocalCall> frame_system::offchain::CreateInherent<LocalCall> for Test
where
RuntimeCall: From<LocalCall>,
{
fn create_inherent(call: Self::RuntimeCall) -> Self::Extrinsic {
Extrinsic::new_bare(call)
}
}

#[derive_impl(frame_system::config_preludes::TestDefaultConfig)]
impl frame_system::Config for Test {
type Block = Block;
Expand Down
1 change: 1 addition & 0 deletions polkadot/xcm/xcm-builder/src/tests/pay/salary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ impl GetSalary<Rank, AccountId, Balance> for FixedSalary {
}

impl pallet_salary::Config for Test {
type RuntimeTask = <Test as frame_system::Config>::RuntimeTask;
type WeightInfo = ();
type RuntimeEvent = RuntimeEvent;
type Paymaster = SalaryPayOverXcm;
Expand Down
17 changes: 17 additions & 0 deletions prdoc/pr_5163.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
title: Task(bump) Pallet Salary

doc:
- audience: Runtime Dev
description: |
This PR places `pallet_salary::bump` on an offchain task, this acts as an alternative to
munaul execution (i.e tasks are executed in a random order).

crates:
- name: collectives-westend-runtime
bump: major
- name: staging-xcm-builder
bump: patch
- name: pallet-salary
bump: patch
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bounding of CreateInherent breaks semver.

Suggested change
bump: patch
bump: major

- name: polkadot-sdk
bump: patch
1 change: 1 addition & 0 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1965,6 +1965,7 @@ impl GetSalary<u16, AccountId, Balance> for SalaryForRank {
}

impl pallet_salary::Config for Runtime {
type RuntimeTask = RuntimeTask;
type WeightInfo = ();
type RuntimeEvent = RuntimeEvent;
type Paymaster = PayFromAccount<Balances, TreasuryAccount>;
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/salary/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,4 @@ try-runtime = [
"pallet-ranked-collective?/try-runtime",
"sp-runtime/try-runtime",
]
experimental = ["frame-support/experimental", "frame-system/experimental"]
16 changes: 16 additions & 0 deletions substrate/frame/salary/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,22 @@ mod benchmarks {
assert!(!matches!(Claimant::<T, I>::get(&caller).unwrap().status, Attempted { .. }));
}

#[benchmark]
fn bump_offchain() {
let caller = whitelisted_caller();
ensure_member_with_salary::<T, I>(&caller);
Salary::<T, I>::init(RawOrigin::Signed(caller.clone()).into()).unwrap();
Salary::<T, I>::induct(RawOrigin::Signed(caller.clone()).into()).unwrap();
System::<T>::set_block_number(System::<T>::block_number() + Salary::<T, I>::cycle_period());

#[block]
{
Task::<T, I>::bump_offchain().unwrap();
}

assert_eq!(Salary::<T, I>::status().unwrap().cycle_index, 1u32.into());
}

impl_benchmark_test_suite! {
Salary,
crate::tests::unit::new_test_ext(),
Expand Down
Loading
Loading