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

CSUB-348 Simplify associated signing types #815

Merged
merged 3 commits into from
Jan 3, 2023
Merged
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
15 changes: 10 additions & 5 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,16 @@ updates:
schedule:
interval: "weekly"

- package-ecosystem: "cargo"
directory: "/pallets/offchain-task-scheduler"
schedule:
interval: "weekly"

- package-ecosystem: "cargo"
directory: "/pallets/offchain-task-scheduler/runtime-api"
schedule:
interval: "weekly"

- package-ecosystem: "cargo"
directory: "/pallets/rewards"
schedule:
Expand Down Expand Up @@ -88,8 +98,3 @@ updates:
ignore:
- dependency-name: "*"
update-types: ["version-update:semver-patch"]

- package-ecosystem: "cargo"
directory: "/pallets/offchain-task-scheduler"
schedule:
interval: "weekly"
2 changes: 1 addition & 1 deletion creditcoin-js/creditcoin.json

Large diffs are not rendered by default.

16 changes: 2 additions & 14 deletions integration-tests/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@
resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.20.1.tgz#1148bb33ab252b165a06698fde7576092a78b4a9"
integrity sha512-mrzLkl6U9YLF8qpqI7TB82PESyEGjm/0Ly91jG575eVxMMlb8fYfOXFZIJ8XfLrJZQbm7dlKry2bJmXBUEkdFg==
dependencies:
regenerator-runtime "^0.13.10"
"@babel/helper-plugin-utils" "^7.19.0"

"@babel/template@^7.18.10", "@babel/template@^7.3.3":
version "7.18.10"
Expand Down Expand Up @@ -890,7 +890,7 @@
slash "^3.0.0"
write-file-atomic "^4.0.1"

"@jest/types@^29.2.1", "@jest/types@^29.3.1":
"@jest/types@^29.3.1":
version "29.3.1"
resolved "https://registry.yarnpkg.com/@jest/types/-/types-29.3.1.tgz#7c5a80777cb13e703aeec6788d044150341147e3"
integrity sha512-d0S0jmmTpjnhCmNpApgX3jrUZgZ22ivKJRvL2lli5hpCRoNnp1f85r2/wpKfXuYu8E7Jjh1hGfhPyup1NM5AmA==
Expand Down Expand Up @@ -3141,18 +3141,6 @@ jest-snapshot@^29.3.1:
pretty-format "^29.3.1"
semver "^7.3.5"

jest-util@^29.0.0:
version "29.2.1"
resolved "https://registry.yarnpkg.com/jest-util/-/jest-util-29.2.1.tgz#f26872ba0dc8cbefaba32c34f98935f6cf5fc747"
integrity sha512-P5VWDj25r7kj7kl4pN2rG/RN2c1TLfYYYZYULnS/35nFDjBai+hBeo3MDrYZS7p6IoY3YHZnt2vq4L6mKnLk0g==
dependencies:
"@jest/types" "^29.3.1"
"@types/node" "*"
chalk "^4.0.0"
ci-info "^3.2.0"
graceful-fs "^4.2.9"
picomatch "^2.2.3"

jest-util@^29.3.1:
version "29.3.1"
resolved "https://registry.yarnpkg.com/jest-util/-/jest-util-29.3.1.tgz#1dda51e378bbcb7e3bc9d8ab651445591ed373e1"
Expand Down
3 changes: 0 additions & 3 deletions pallets/creditcoin/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@ impl pallet_offchain_task_scheduler::Config for Test {
type RuntimeEvent = RuntimeEvent;
type UnverifiedTaskTimeout = ConstU64<5>;
type AuthorityId = AuthorityId;
type AccountIdFrom = AccountId;
type InternalPublic = sp_core::sr25519::Public;
type PublicSigning = <Signature as Verify>::Signer;
type TaskCall = RuntimeCall;
type WeightInfo = pallet_offchain_task_scheduler::weights::WeightInfo<Self>;
type Task = pallet_creditcoin::Task<AccountId, BlockNumber, Hash, Moment>;
Expand Down
7 changes: 3 additions & 4 deletions pallets/creditcoin/src/ocw/tasks/collect_coins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,6 @@ pub(crate) mod tests {
fn ocw_fail_collect_coins_works() {
let mut ext = ExtBuilder::default();
let acct_pubkey = ext.generate_authority();
let acct = AccountId::from(acct_pubkey.into_account().0);
let expected_collected_coins_id =
crate::CollectedCoinsId::new::<crate::mock::Test>(&CHAIN, &[0]);
ext.build_offchain_and_execute_with_state(|_state, pool| {
Expand All @@ -512,9 +511,9 @@ pub(crate) mod tests {
cause: Cause::AbiMismatch,
deadline: Test::unverified_transfer_deadline(),
};
assert_ok!(TaskSchedulerPallet::<Test>::offchain_signed_tx(acct.clone(), |_| call
.clone()
.into(),));
assert_ok!(TaskSchedulerPallet::<Test>::offchain_signed_tx(acct_pubkey.into(), |_| {
call.clone().into()
},));
crate::mock::roll_to(2);

assert_matches!(pool.write().transactions.pop(), Some(tx) => {
Expand Down
5 changes: 4 additions & 1 deletion pallets/offchain-task-scheduler/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ pub trait TaskDefault<T: SystemConfig> {
}

benchmarks! {
where_clause { where T::Task: TaskDefault<T> }
where_clause { where
T::Task: TaskDefault<T>,
<T::AuthorityId as AppCrypto<T::Public, T::Signature>>::RuntimeAppPublic: Into<T::Public>,
}
on_initialize {
//insert t transfers
let t in 0..1024;
Expand Down
36 changes: 17 additions & 19 deletions pallets/offchain-task-scheduler/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,28 @@ pub const KEY_TYPE: KeyTypeId = KeyTypeId(*b"gots");
pub mod crypto {
use super::AppCrypto;
use crate::KEY_TYPE;
use sp_core::crypto::Wraps;
use sp_runtime::{
app_crypto::{app_crypto, sr25519},
MultiSignature, MultiSigner,
};

app_crypto!(sr25519, KEY_TYPE);

#[derive(Clone, PartialEq, Eq, core::fmt::Debug)]
pub struct AuthorityId;

impl AppCrypto<MultiSigner, MultiSignature> for AuthorityId {
type RuntimeAppPublic = Public;
type GenericPublic = sp_core::sr25519::Public;
type GenericPublic = <Public as Wraps>::Inner;
type GenericSignature = sp_core::sr25519::Signature;
}

impl From<Public> for MultiSigner {
fn from(public: Public) -> MultiSigner {
sp_core::sr25519::Public::from(public).into()
}
}
}

#[frame_support::pallet]
Expand Down Expand Up @@ -76,19 +84,7 @@ pub mod pallet {
+ Debug;
type UnverifiedTaskTimeout: Get<<Self as SystemConfig>::BlockNumber>;
type WeightInfo: WeightInfo;
type AuthorityId: AppCrypto<
Self::Public,
<Self as frame_system::offchain::SigningTypes>::Signature,
>;
type AccountIdFrom: From<sp_core::sr25519::Public>
+ IsType<Self::AccountId>
+ Clone
+ core::fmt::Debug
+ PartialEq
+ AsRef<[u8; 32]>;

type InternalPublic: sp_core::crypto::UncheckedFrom<[u8; 32]>;
type PublicSigning: From<Self::InternalPublic> + Into<Self::Public>;
type AuthorityId: AppCrypto<Self::Public, Self::Signature>;
type TaskCall: Dispatchable<RuntimeOrigin = Self::RuntimeOrigin> + Clone;
}

Expand Down Expand Up @@ -124,7 +120,10 @@ pub mod pallet {
}

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T>
where
<T::AuthorityId as AppCrypto<T::Public, T::Signature>>::RuntimeAppPublic: Into<T::Public>,
{
fn on_initialize(block_number: T::BlockNumber) -> Weight {
log::debug!("Cleaning up expired entries");

Expand All @@ -144,8 +143,8 @@ pub mod pallet {
}

fn offchain_worker(block_number: T::BlockNumber) {
let auth_id = match Self::authority_id() {
Some(id) => id,
let signer = match Self::authority_pubkey() {
Some(pubkey) => pubkey,
None => {
log::debug!(target: "task", "Not an authority, skipping offchain work");
return;
Expand All @@ -166,8 +165,7 @@ pub mod pallet {
use tasks::error::TaskError::*;
match task.forward_task(deadline) {
Ok(call) => {
match Self::submit_txn_with_synced_nonce(auth_id.clone(), |_| call.clone())
{
match Self::submit_txn_with_synced_nonce(signer.clone(), |_| call.clone()) {
Ok(_) => guard.forget(),
Err(e) => {
log::error!("Failed to send a dispatchable transaction: {:?}", e)
Expand Down
3 changes: 0 additions & 3 deletions pallets/offchain-task-scheduler/src/mock/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,6 @@ impl crate::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type UnverifiedTaskTimeout = ConstU64<5>;
type AuthorityId = crate::crypto::AuthorityId;
type AccountIdFrom = AccountId;
type InternalPublic = sp_core::sr25519::Public;
type PublicSigning = <Signature as Verify>::Signer;
type TaskCall = RuntimeCall;
type WeightInfo = crate::weights::WeightInfo<Self>;
type Task = super::task::MockTask<u32>;
Expand Down
50 changes: 28 additions & 22 deletions pallets/offchain-task-scheduler/src/ocw.rs
Original file line number Diff line number Diff line change
@@ -1,42 +1,48 @@
pub(crate) mod nonce;

use super::Error;
use super::{crypto, log, Authorities, Config, Pallet};
use super::{log, Authorities, Config, Pallet};
use alloc::vec;
use frame_support::dispatch::Vec;
use frame_support::traits::IsType;
use frame_system::offchain::AppCrypto;
use frame_system::offchain::{Account, SendSignedTransaction, Signer};
use frame_system::{Config as SystemConfig, Pallet as System};
use frame_system::Pallet as System;
use nonce::lock_key;
pub use nonce::nonce_key;
use sp_runtime::offchain::storage::StorageValueRef;
use sp_runtime::traits::IdentifyAccount;
use sp_runtime::traits::One;
use sp_runtime::traits::Saturating;
use sp_runtime::RuntimeAppPublic;

impl<T: Config> Pallet<T> {
pub fn authority_id() -> Option<T::AccountIdFrom> {
let local_keys = crypto::Public::all()
.into_iter()
.map(|p| sp_core::sr25519::Public::from(p).into())
.collect::<Vec<T::AccountIdFrom>>();
pub fn authority_pubkey() -> Option<T::Public>
where
<T::AuthorityId as AppCrypto<T::Public, T::Signature>>::RuntimeAppPublic: Into<T::Public>,
{
let local_keys =
<T::AuthorityId as AppCrypto<T::Public, T::Signature>>::RuntimeAppPublic::all()
.into_iter()
.map(|p| {
let pkey = p.into();
(pkey.clone(), pkey.into_account())
})
.collect::<Vec<(T::Public, T::AccountId)>>();

log::trace!(target: "task", "local keys {local_keys:?}");

Authorities::<T>::iter_keys().find_map(|auth| {
let acct = auth.clone().into();
local_keys.contains(&acct).then(|| T::AccountIdFrom::from(auth))
local_keys
.iter()
.find_map(|(pkey, acc)| if auth == *acc { Some(pkey.clone()) } else { None })
})
}

pub fn offchain_signed_tx(
auth_id: T::AccountIdFrom,
auth_pubkey: T::Public,
call: impl Fn(&Account<T>) -> T::TaskCall,
) -> Result<(), Error<T>> {
use sp_core::crypto::UncheckedFrom;
let auth_bytes: &[u8; 32] = auth_id.as_ref();
let public: T::PublicSigning = T::InternalPublic::unchecked_from(*auth_bytes).into();
let signer = Signer::<T, T::AuthorityId>::any_account().with_filter(vec![public.into()]);
let signer = Signer::<T, T::AuthorityId>::any_account().with_filter(vec![auth_pubkey]);
let result = signer.send_signed_transaction(call);

if let Some((acc, res)) = result {
Expand All @@ -53,17 +59,17 @@ impl<T: Config> Pallet<T> {
}

pub fn submit_txn_with_synced_nonce(
auth_id: T::AccountIdFrom,
pubkey: T::Public,
call: impl Fn(&Account<T>) -> T::TaskCall,
) -> Result<(), Error<T>> {
let acc_id: &<T as SystemConfig>::AccountId = auth_id.into_ref();
let mut account_data = System::<T>::account(acc_id);
let auth_id: &T::AccountId = &pubkey.clone().into_account();
let mut account_data = System::<T>::account(auth_id);

let key = &lock_key(auth_id.into_ref());
let key = &lock_key(auth_id);
let mut lock = Pallet::<T>::nonce_lock_new(key);
let _guard = lock.lock();

let key = &nonce_key(auth_id.into_ref());
let key = &nonce_key(auth_id);
let synced_nonce_storage = StorageValueRef::persistent(key);
let synced_nonce = synced_nonce_storage.get::<T::Index>().ok().flatten();

Expand All @@ -73,11 +79,11 @@ impl<T: Config> Pallet<T> {
if let Some(nonce) = synced_nonce {
if nonce > account_data.nonce {
account_data.nonce = nonce;
frame_system::Account::<T>::insert(acc_id, account_data.clone());
frame_system::Account::<T>::insert(auth_id, account_data.clone());
}
}

Pallet::<T>::offchain_signed_tx(auth_id, call)
Pallet::<T>::offchain_signed_tx(pubkey, call)
.map(|_| synced_nonce_storage.set(&account_data.nonce.saturating_add(One::one())))
}
}
Expand Down
6 changes: 3 additions & 3 deletions pallets/offchain-task-scheduler/src/ocw/nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,9 @@ mod tests {

std::thread::spawn(move || {
let mut ext_builder = ExtBuilder::default().with_keystore();
let acct_sr25519_pubkey = ext_builder.generate_authority();
let acct = {
let acct_pubkey = ext_builder.generate_authority();
<Runtime as SystemConfig>::AccountId::from(acct_pubkey.into_account().0)
<Runtime as SystemConfig>::AccountId::from(acct_sr25519_pubkey.into_account().0)
};
ext_builder.offchain = Some(offchain);
ext_builder.with_pool();
Expand All @@ -158,7 +158,7 @@ mod tests {

for _ in 0..LOOP {
assert_ok!(crate::Pallet::<Runtime>::submit_txn_with_synced_nonce(
acct.clone(),
acct_sr25519_pubkey.into(),
|_| call.clone(),
));

Expand Down
8 changes: 4 additions & 4 deletions pallets/offchain-task-scheduler/src/ocw/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ fn effective_guard_lifetime_until_task_expiration() {
fn offchain_signed_tx_works() {
let mut ext_builder = ExtBuilder::default().with_keystore();
let acct_pubkey = ext_builder.generate_authority();
let auth = AccountId::from(acct_pubkey.into_account().0);
let auth = acct_pubkey;
let pool = ext_builder.with_pool();
ext_builder.build().execute_with(|| {
roll_to::<Trivial>(1);
Expand All @@ -247,7 +247,7 @@ fn offchain_signed_tx_works() {
remark: 0.encode(),
});

assert_ok!(Pallet::<Runtime>::offchain_signed_tx(auth.clone(), |_| call.clone()));
assert_ok!(Pallet::<Runtime>::offchain_signed_tx(auth.into(), |_| call.clone()));
roll_to::<Trivial>(2);

assert_matches!(pool.write().transactions.pop(), Some(tx) => {
Expand All @@ -261,7 +261,7 @@ fn offchain_signed_tx_works() {
fn offchain_signed_tx_send_fails() {
let mut ext_builder = ExtBuilder::default().with_keystore();
let acct_pubkey = ext_builder.generate_authority();
let auth = AccountId::from(acct_pubkey.into_account().0);
let auth = acct_pubkey;
ext_builder.with_pool();
ext_builder.build().execute_with(|| {
roll_to::<Trivial>(1);
Expand All @@ -273,7 +273,7 @@ fn offchain_signed_tx_send_fails() {
use frame_support::assert_err;
with_failing_submit_transaction(|| {
assert_err!(
Pallet::<Runtime>::offchain_signed_tx(auth.clone(), |_| call.clone()),
Pallet::<Runtime>::offchain_signed_tx(auth.into(), |_| call.clone()),
crate::Error::OffchainSignedTxFailed
);
})
Expand Down
1 change: 1 addition & 0 deletions pallets/offchain-task-scheduler/src/tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub trait TaskV2<Runtime: SystemConfig> {
type Call;
type EvaluationError;
type SchedulerError;

/// A task generates its own id. This Id is used as a task id in the scheduler and also to check onchain storage persistence.
fn to_id(&self) -> Runtime::Hash;
//A task will know how to check onchain storage persistence.
Expand Down
6 changes: 1 addition & 5 deletions runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use frame_support::{
traits::{ConstU32, ConstU8},
weights::{WeightToFeeCoefficient, WeightToFeeCoefficients, WeightToFeePolynomial},
};

use pallet_creditcoin::weights::WeightInfo as creditcoin_weights;
use pallet_creditcoin::WeightInfo;
use pallet_offchain_task_scheduler::crypto::AuthorityId;
Expand Down Expand Up @@ -149,9 +148,6 @@ impl pallet_offchain_task_scheduler::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type UnverifiedTaskTimeout = ConstU32<60>;
type AuthorityId = AuthorityId;
type AccountIdFrom = AccountId;
type InternalPublic = sp_core::sr25519::Public;
type PublicSigning = <Signature as Verify>::Signer;
type TaskCall = RuntimeCall;
type WeightInfo = pallet_offchain_task_scheduler::weights::WeightInfo<Runtime>;
type Task = pallet_creditcoin::Task<AccountId, BlockNumber, Hash, Moment>;
Expand Down Expand Up @@ -605,7 +601,7 @@ impl_runtime_apis! {
}

impl frame_system::offchain::SigningTypes for Runtime {
type Public = <Signature as Verify>::Signer;
type Public = Signer;
type Signature = Signature;
}

Expand Down
Loading