From 32b1ecc99b8f4db2bf0068afb728f1bb5c77611f Mon Sep 17 00:00:00 2001 From: Rodrigo Quelhas Date: Wed, 15 Mar 2023 14:59:04 +0000 Subject: [PATCH 1/4] fix: std conflicts --- pallets/hyperdrive/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pallets/hyperdrive/src/lib.rs b/pallets/hyperdrive/src/lib.rs index fbf38f6d..5bfd8465 100644 --- a/pallets/hyperdrive/src/lib.rs +++ b/pallets/hyperdrive/src/lib.rs @@ -30,6 +30,8 @@ pub mod pallet { use frame_system::pallet_prelude::*; use sp_arithmetic::traits::{CheckedRem, Zero}; use sp_runtime::traits::Hash; + use sp_std::prelude::*; + use sp_std::vec; use types::*; From acbe4eb37fd847d7a48a34015170a0a32a8a5ca8 Mon Sep 17 00:00:00 2001 From: Rodrigo Quelhas Date: Wed, 15 Mar 2023 23:04:13 +0000 Subject: [PATCH 2/4] feat(hyperdrive): rework merkle root submission --- pallets/acurast/src/lib.rs | 2 +- pallets/hyperdrive/src/benchmarking.rs | 5 +- pallets/hyperdrive/src/lib.rs | 78 +++++++++++++++++-------- pallets/hyperdrive/src/tests.rs | 68 ++++++++++----------- pallets/marketplace/src/benchmarking.rs | 2 +- 5 files changed, 91 insertions(+), 64 deletions(-) diff --git a/pallets/acurast/src/lib.rs b/pallets/acurast/src/lib.rs index 079d3629..3664f90d 100644 --- a/pallets/acurast/src/lib.rs +++ b/pallets/acurast/src/lib.rs @@ -22,13 +22,13 @@ pub type JobRegistrationFor = #[frame_support::pallet] pub mod pallet { use acurast_common::*; + use core::ops::AddAssign; use frame_support::{ dispatch::DispatchResultWithPostInfo, ensure, pallet_prelude::*, traits::UnixTime, Blake2_128Concat, PalletId, }; use frame_system::pallet_prelude::*; use sp_std::prelude::*; - use core::ops::AddAssign; use crate::{traits::*, utils::*, JobRegistrationFor}; diff --git a/pallets/hyperdrive/src/benchmarking.rs b/pallets/hyperdrive/src/benchmarking.rs index 58820347..93688bdb 100644 --- a/pallets/hyperdrive/src/benchmarking.rs +++ b/pallets/hyperdrive/src/benchmarking.rs @@ -89,10 +89,11 @@ benchmarks_instance_pallet! { submit_state_merkle_root { // add the transmitters and submit before benchmarked extrinsic let (caller, _) = update_state_transmitters_helper::(1, true); - }: _(RawOrigin::Signed(caller.clone()), 5.into(), HASH.into()) + }: _(RawOrigin::Signed(caller.clone()), 1.into(), HASH.into()) verify { assert_event::(Event::StateMerkleRootSubmitted{ - block: 5.into(), + source: caller.clone(), + snapshot: 1.into(), state_merkle_root: HASH.into() }.into()); } diff --git a/pallets/hyperdrive/src/lib.rs b/pallets/hyperdrive/src/lib.rs index 5bfd8465..361719a8 100644 --- a/pallets/hyperdrive/src/lib.rs +++ b/pallets/hyperdrive/src/lib.rs @@ -30,6 +30,7 @@ pub mod pallet { use frame_system::pallet_prelude::*; use sp_arithmetic::traits::{CheckedRem, Zero}; use sp_runtime::traits::Hash; + use sp_std::collections::btree_set::BTreeSet; use sp_std::prelude::*; use sp_std::vec; @@ -81,6 +82,7 @@ pub mod pallet { + MaxEncodedLen + TypeInfo + Zero + + From + CheckedRem; /// The hashing system (algorithm) being used in the runtime (e.g. Blake2). type TargetChainHashing: Hash + TypeInfo; @@ -108,11 +110,12 @@ pub mod pallet { removed: Vec, }, StateMerkleRootSubmitted { - block: T::TargetChainBlockNumber, + source: T::AccountId, + snapshot: T::TargetChainBlockNumber, state_merkle_root: T::TargetChainHash, }, StateMerkleRootAccepted { - block: T::TargetChainBlockNumber, + snapshot: T::TargetChainBlockNumber, state_merkle_root: T::TargetChainHash, }, } @@ -131,6 +134,17 @@ pub mod pallet { ValueQuery, >; + #[pallet::type_value] + pub fn FirstSnapshot, I: 'static>() -> T::TargetChainBlockNumber { + 1u8.into() + } + + /// This storage field contains the latest validated snapshot number. + #[pallet::storage] + #[pallet::getter(fn latest_snapshot)] + pub type CurrentSnapshot, I: 'static = ()> = + StorageValue<_, T::TargetChainBlockNumber, ValueQuery, FirstSnapshot>; + #[pallet::storage] #[pallet::getter(fn state_merkle_root)] pub type StateMerkleRootCount, I: 'static = ()> = StorageDoubleMap< @@ -139,15 +153,15 @@ pub mod pallet { T::TargetChainBlockNumber, Identity, T::TargetChainHash, - u8, + BTreeSet, >; #[pallet::error] pub enum Error { /// A known transmitter submits outside the window of activity he is permissioned to. SubmitOutsideTransmitterActivityWindow, - SubmitOutsideTransmissionRate, CalculationOverflow, + UnexpectedSnapshot, } #[pallet::call] @@ -208,10 +222,17 @@ pub mod pallet { #[pallet::weight(< T as Config>::WeightInfo::submit_state_merkle_root())] pub fn submit_state_merkle_root( origin: OriginFor, - block: T::TargetChainBlockNumber, + snapshot: T::TargetChainBlockNumber, state_merkle_root: T::TargetChainHash, ) -> DispatchResult { let who = ensure_signed(origin)?; + let expected_snapshot = Self::latest_snapshot(); + + // Ensure merkle roots are submitted sequentially + ensure!( + snapshot == expected_snapshot, + Error::::UnexpectedSnapshot + ); let activity_window = >::get(&who); let current_block = >::block_number(); @@ -221,32 +242,42 @@ pub mod pallet { && current_block < activity_window.end_block, Error::::SubmitOutsideTransmitterActivityWindow ); - ensure!( - block - .checked_rem(&T::TransmissionRate::get()) - .ok_or(Error::::CalculationOverflow)? - .is_zero(), - Error::::SubmitOutsideTransmissionRate - ); // insert merkle root proposal since all checks passed // allows for constant-time validity checks - let accepted = - StateMerkleRootCount::::mutate(&block, &state_merkle_root, |count| { - let count_ = count.unwrap_or(0) + 1; - *count = Some(count_); - count_ >= >::TransmissionQuorum::get() - }); + let accepted = StateMerkleRootCount::::mutate( + &snapshot, + &state_merkle_root, + |submissions| { + // This can be improved once [let chains feature](https://github.com/rust-lang/rust/issues/53667) lands + if let Some(transmitters) = submissions { + if !transmitters.contains(&who) { + transmitters.insert(who.clone()); + } + } else { + let mut set = BTreeSet::::new(); + set.insert(who.clone()); + *submissions = Some(set); + } + + let submissions_count = submissions + .as_ref() + .map_or(0usize, |transmitters| transmitters.len()); + return submissions_count >= T::TransmissionQuorum::get().into(); + }, + ); // Emit event to inform that the state merkle root has been sumitted Self::deposit_event(Event::StateMerkleRootSubmitted { - block, + source: who, + snapshot, state_merkle_root, }); if accepted { + CurrentSnapshot::::set(expected_snapshot + T::TransmissionRate::get()); Self::deposit_event(Event::StateMerkleRootAccepted { - block, + snapshot, state_merkle_root, }); } @@ -261,9 +292,10 @@ pub mod pallet { block: T::TargetChainBlockNumber, state_merkle_root: T::TargetChainHash, ) -> bool { - StateMerkleRootCount::::get(&block, &state_merkle_root).map_or(false, |count| { - count >= >::TransmissionQuorum::get() - }) + StateMerkleRootCount::::get(&block, &state_merkle_root) + .map_or(false, |submissions| { + submissions.len() >= T::TransmissionQuorum::get().into() + }) } } } diff --git a/pallets/hyperdrive/src/tests.rs b/pallets/hyperdrive/src/tests.rs index 2d604d13..45d9bcfd 100644 --- a/pallets/hyperdrive/src/tests.rs +++ b/pallets/hyperdrive/src/tests.rs @@ -149,35 +149,28 @@ fn submit_outside_activity_window() { assert_err!( TezosHyperdrive::submit_state_merkle_root( RuntimeOrigin::signed(alice_account_id()), - 5, + 1, HASH ), Error::::SubmitOutsideTransmitterActivityWindow ); - System::set_block_number(10); - assert_ok!(TezosHyperdrive::submit_state_merkle_root( - RuntimeOrigin::signed(alice_account_id()), - 10, - HASH - )); - - System::set_block_number(19); - assert_ok!(TezosHyperdrive::submit_state_merkle_root( - RuntimeOrigin::signed(alice_account_id()), - 10, - HASH - )); - System::set_block_number(20); assert_err!( TezosHyperdrive::submit_state_merkle_root( - RuntimeOrigin::signed(bob_account_id()), - 5, + RuntimeOrigin::signed(alice_account_id()), + 1, HASH ), Error::::SubmitOutsideTransmitterActivityWindow ); + + System::set_block_number(10); + assert_ok!(TezosHyperdrive::submit_state_merkle_root( + RuntimeOrigin::signed(alice_account_id()), + 1, + HASH + )); }); } @@ -206,7 +199,7 @@ fn submit_outside_transmission_rate() { 6, HASH ), - Error::::SubmitOutsideTransmitterActivityWindow + Error::::UnexpectedSnapshot ); }); } @@ -240,30 +233,33 @@ fn submit_state_merkle_root() { System::set_block_number(10); - // first submission for target chain block 10 + // first submission for target chain snapshot 1 assert_ok!(TezosHyperdrive::submit_state_merkle_root( RuntimeOrigin::signed(alice_account_id()), - 10, + 1, HASH )); // does not validate until quorum reached - assert_eq!(TezosHyperdrive::validate_state_merkle_root(10, HASH), false); + assert_eq!(TezosHyperdrive::validate_state_merkle_root(1, HASH), false); - // intermitted submission for different block is allowed! - assert_ok!(TezosHyperdrive::submit_state_merkle_root( - RuntimeOrigin::signed(bob_account_id()), - 15, - HASH - )); + // intermitted submission for different snapshot is not allowed! + assert_err!( + TezosHyperdrive::submit_state_merkle_root( + RuntimeOrigin::signed(bob_account_id()), + 2, + HASH + ), + Error::::UnexpectedSnapshot + ); - // second submission for target chain block 10 + // second submission for target chain snapshot 1 assert_ok!(TezosHyperdrive::submit_state_merkle_root( RuntimeOrigin::signed(bob_account_id()), - 10, + 1, HASH )); // does validate since quorum reached - assert_eq!(TezosHyperdrive::validate_state_merkle_root(10, HASH), true); + assert_eq!(TezosHyperdrive::validate_state_merkle_root(1, HASH), true); assert_eq!( events(), @@ -289,19 +285,17 @@ fn submit_state_merkle_root() { removed: vec![], }), RuntimeEvent::TezosHyperdrive(crate::Event::StateMerkleRootSubmitted { - block: 10, - state_merkle_root: HASH - }), - RuntimeEvent::TezosHyperdrive(crate::Event::StateMerkleRootSubmitted { - block: 15, + source: alice_account_id(), + snapshot: 1, state_merkle_root: HASH }), RuntimeEvent::TezosHyperdrive(crate::Event::StateMerkleRootSubmitted { - block: 10, + source: bob_account_id(), + snapshot: 1, state_merkle_root: HASH }), RuntimeEvent::TezosHyperdrive(crate::Event::StateMerkleRootAccepted { - block: 10, + snapshot: 1, state_merkle_root: HASH }) ] diff --git a/pallets/marketplace/src/benchmarking.rs b/pallets/marketplace/src/benchmarking.rs index 2ccfc664..34854093 100644 --- a/pallets/marketplace/src/benchmarking.rs +++ b/pallets/marketplace/src/benchmarking.rs @@ -11,7 +11,7 @@ use sp_runtime::BoundedVec; use sp_std::prelude::*; pub use pallet::Config; -use pallet_acurast::{Event as AcurastEvent, JobRegistrationFor, Script, MultiOrigin}; +use pallet_acurast::{Event as AcurastEvent, JobRegistrationFor, MultiOrigin, Script}; use pallet_acurast::{Pallet as Acurast, Schedule}; pub use crate::stub::*; From 79027393b1e81a87b50965c1101d396d38a0c575 Mon Sep 17 00:00:00 2001 From: Rodrigo Quelhas Date: Wed, 15 Mar 2023 23:04:37 +0000 Subject: [PATCH 3/4] chore: enforce code formatting --- .github/workflows/ci.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 875a0da5..a45729fe 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,6 +33,12 @@ jobs: # - name: Check and Lint Code # run: cargo clippy -- -D warnings + - name: Check formatting + uses: actions-rs/cargo@v1 + with: + command: fmt + args: --check + - name: Run cargo check for release uses: actions-rs/cargo@v1 with: From 9f7a901f051e12c588dec1c87a65ed99fce5776d Mon Sep 17 00:00:00 2001 From: Rodrigo Quelhas Date: Thu, 16 Mar 2023 09:06:56 +0000 Subject: [PATCH 4/4] chore: explicitly set the nightly toochain version --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a45729fe..3004913b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,7 +22,7 @@ jobs: uses: actions-rs/toolchain@v1 with: profile: minimal - toolchain: nightly + toolchain: nightly-2023-03-14 components: rustfmt, clippy target: wasm32-unknown-unknown override: true