From f47cb20eaaacedcd3d2fa13abb1c8ae4ce5c106e Mon Sep 17 00:00:00 2001 From: Francisco Valentim Castilho Date: Sat, 17 Feb 2024 18:18:03 -0300 Subject: [PATCH 1/3] Feat: documented all modules --- INV4/pallet-inv4/src/account_derivation.rs | 26 ++++++++++++++++------ INV4/pallet-inv4/src/dispatch.rs | 11 ++++++++- INV4/pallet-inv4/src/fee_handling.rs | 26 +++++++++++++++++++++- INV4/pallet-inv4/src/inv4_core.rs | 11 +++++++++ INV4/pallet-inv4/src/lib.rs | 2 ++ INV4/pallet-inv4/src/lookup.rs | 13 +++++++++++ INV4/pallet-inv4/src/multisig.rs | 16 ++++++++++++- INV4/pallet-inv4/src/origin.rs | 10 +++++++++ INV4/pallet-inv4/src/voting.rs | 16 +++++++++++++ 9 files changed, 121 insertions(+), 10 deletions(-) diff --git a/INV4/pallet-inv4/src/account_derivation.rs b/INV4/pallet-inv4/src/account_derivation.rs index 85b37b8e..e2abd15c 100644 --- a/INV4/pallet-inv4/src/account_derivation.rs +++ b/INV4/pallet-inv4/src/account_derivation.rs @@ -1,12 +1,25 @@ +//! Core Account Derivation. +//! +//! ## Overview +//! +//! This module defines a method for generating account addresses, and how it's implemented within this +//! pallet. We use a custom derivation scheme to ensure that when a multisig is created, its AccountId +//! remains consistent across different parachains, promoting seamless interaction. +//! +//! ### The module contains: +//! - `CoreAccountDerivation` trait: The interface for our derivation method. +//! - Pallet implementation: The specific logic used to derive AccountIds. + use crate::{Config, Pallet}; use codec::{Compact, Encode}; use frame_support::traits::Get; use sp_io::hashing::blake2_256; use xcm::latest::{BodyId, BodyPart, Junction, Junctions}; - -// Trait providing the XCM location and the derived account of a core. +/// Trait providing the XCM location and the derived account of a core. pub trait CoreAccountDerivation { + /// Derives the core's AccountId. fn derive_core_account(core_id: T::CoreId) -> T::AccountId; + /// Specifies a core's location. fn core_location(core_id: T::CoreId) -> Junctions; } @@ -14,10 +27,10 @@ impl CoreAccountDerivation for Pallet where T::AccountId: From<[u8; 32]>, { + /// HashedDescription of the core location from the perspective of a sibling chain. + /// This derivation allows the local account address to match the account address in other parachains. + /// Reference: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/xcm-builder/src/location_conversion.rs fn derive_core_account(core_id: T::CoreId) -> T::AccountId { - // HashedDescription of the core location from the perspective of a sibling chain. - // This derivation allows the local account address to match the account address in other parachains. - // Reference: https://github.com/paritytech/polkadot-sdk/blob/master/polkadot/xcm/xcm-builder/src/location_conversion.rs blake2_256( &( b"SiblingChain", @@ -28,9 +41,8 @@ where ) .into() } - + /// Core location is defined as a plurality within the parachain. fn core_location(core_id: T::CoreId) -> Junctions { - // Core location is defined as a plurality within the parachain. Junctions::X2( Junction::Parachain(T::ParaId::get()), Junction::Plurality { diff --git a/INV4/pallet-inv4/src/dispatch.rs b/INV4/pallet-inv4/src/dispatch.rs index b73bb7a2..ff026c2d 100644 --- a/INV4/pallet-inv4/src/dispatch.rs +++ b/INV4/pallet-inv4/src/dispatch.rs @@ -1,3 +1,11 @@ +//! Dispatches calls internally, charging fees to the multisig account. +//! +//! ## Overview +//! +//! This module employs a custom `MultisigInternalOrigin` to ensure calls originate +//! from the multisig account itself, automating fee payments. The `dispatch_call` function +//! includes pre and post dispatch handling for streamlined fee management within the multisig context. + use crate::{ fee_handling::{FeeAsset, MultisigFeeHandler}, origin::{INV4Origin, MultisigInternalOrigin}, @@ -8,7 +16,7 @@ use frame_support::{ pallet_prelude::*, }; -// Dispatch a call executing pre/post dispatch for proper fee handling. +/// Dispatch a call executing pre/post dispatch for proper fee handling. pub fn dispatch_call( core_id: ::CoreId, fee_asset: &FeeAsset, @@ -17,6 +25,7 @@ pub fn dispatch_call( where T::AccountId: From<[u8; 32]>, { + // Create new custom origin as the multisig. let internal_origin = MultisigInternalOrigin::new(core_id); let multisig_account = internal_origin.to_account_id(); let origin = INV4Origin::Multisig(internal_origin).into(); diff --git a/INV4/pallet-inv4/src/fee_handling.rs b/INV4/pallet-inv4/src/fee_handling.rs index 7fdfa6ec..3667bcfd 100644 --- a/INV4/pallet-inv4/src/fee_handling.rs +++ b/INV4/pallet-inv4/src/fee_handling.rs @@ -1,3 +1,10 @@ +//! MultisigFeeHandler trait. +//! +//! ## Overview +//! +//! Defines how transaction fees are charged to the multisig account. +//! This trait requires proper runtime implementation to allow the usage of native or non-native assets. + use crate::Config; use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ @@ -10,23 +17,38 @@ use sp_runtime::{ DispatchResult, }; -/// Asset to be used by the multisig for paying fees transaction fees. +/// Represents the asset to be used by the multisig for paying fees transaction fees. +/// +/// This enum plays a role in marking the desired asset in the `MultisigFeeHandler` trait. #[derive(Clone, TypeInfo, Encode, Decode, MaxEncodedLen, Debug, PartialEq, Eq)] pub enum FeeAsset { Native, Relay, } +/// Represents a potential negative asset balance incurred during fee payment operations +/// within a multisig context. +/// +/// This enum handles imbalances in either the native token or +/// a relay chain asset used for fees. +/// +/// - `Native(NativeNegativeImbalance)`: Indicates a deficit balance in the chain's native asset. +/// - `Relay(RelayNegativeImbalance)`: Indicates a deficit balance in an asset originating on the relay chain. +/// +/// This enum plays a role in resolving deficit balances in the `MultisigFeeHandler` trait. pub enum FeeAssetNegativeImbalance { Native(NativeNegativeImbalance), Relay(RelayNegativeImbalance), } /// Fee handler trait. +/// /// This should be implemented properly in the runtime to account for native and non-native assets. pub trait MultisigFeeHandler { + /// Type returned by `pre_dispatch` - implementation dependent. type Pre; + /// Checks if the fee can be paid using the selected asset. fn pre_dispatch( asset: &FeeAsset, who: &T::AccountId, @@ -35,6 +57,7 @@ pub trait MultisigFeeHandler { len: usize, ) -> Result; + /// Charges the call dispatching fee from the multisig directly. fn post_dispatch( asset: &FeeAsset, pre: Option, @@ -44,6 +67,7 @@ pub trait MultisigFeeHandler { result: &DispatchResult, ) -> Result<(), TransactionValidityError>; + /// Charges the fee for creating the core(multisig). fn handle_creation_fee( imbalance: FeeAssetNegativeImbalance< >::NegativeImbalance, diff --git a/INV4/pallet-inv4/src/inv4_core.rs b/INV4/pallet-inv4/src/inv4_core.rs index 89f435c0..bae8efe9 100644 --- a/INV4/pallet-inv4/src/inv4_core.rs +++ b/INV4/pallet-inv4/src/inv4_core.rs @@ -1,3 +1,14 @@ +//! Core creation and internal management. +//! +//! ## Overview +//! +//! This module handles the mechanics of creating multisigs (referred to as "cores") and their lifecycle management. Key functions include: +//! +//! - `inner_create_core`: Sets up a new core, deriving its AccountId, distributing voting tokens, and handling creation fees. +//! - `inner_set_parameters`: Updates the cores's operational rules based on passed proposals. +//! Use with caution as breaking changes caused by bad inputs are not checked. +//! - `is_asset_frozen`: Utility function for checking if wether a core's voting asset is frozen if it exists. + use super::pallet::*; use crate::{ account_derivation::CoreAccountDerivation, diff --git a/INV4/pallet-inv4/src/lib.rs b/INV4/pallet-inv4/src/lib.rs index 6d7d31dc..d24d293f 100644 --- a/INV4/pallet-inv4/src/lib.rs +++ b/INV4/pallet-inv4/src/lib.rs @@ -6,6 +6,8 @@ //! //! ## Overview //! This pallet handles advanced virtual multisigs (internally called cores). +//! Lower level implementation details of this pallet's calls are contained in separate modules each of them +//! containing their own docs. //! //! ### Pallet Functions //! diff --git a/INV4/pallet-inv4/src/lookup.rs b/INV4/pallet-inv4/src/lookup.rs index d18b0f5f..70bb92ef 100644 --- a/INV4/pallet-inv4/src/lookup.rs +++ b/INV4/pallet-inv4/src/lookup.rs @@ -1,13 +1,26 @@ +//! Core's XCM location utilities. +//! +//! ## Overview +//! +//! This module implements the [`StaticLookup`] trait allowing for convenient conversion between a +//! Core's id and it's derived AccountId. +//! This implementation abstracs on top of two lower level functions: +//! - `lookup_core`: Used for accessing the storage and retrieving a core's AccountId. +//! - `lookup_address`: Used for converting from a `MultiAddress::Index` that contains a CoreId to this core's AccountId. + use crate::{Config, CoreByAccount, CoreStorage, Pallet}; use core::marker::PhantomData; use frame_support::error::LookupError; use sp_runtime::{traits::StaticLookup, MultiAddress}; impl Pallet { + /// Queries `CoreStorage` to retrieve the AccountId of a core. pub fn lookup_core(core_id: T::CoreId) -> Option { CoreStorage::::get(core_id).map(|core| core.account) } + /// Matches `MultiAddress` to allow for a `MultiAddress::Index` containing a CoreId to be converted + /// to it's derived AccountId. pub fn lookup_address(a: MultiAddress) -> Option { match a { MultiAddress::Id(i) => Some(i), diff --git a/INV4/pallet-inv4/src/multisig.rs b/INV4/pallet-inv4/src/multisig.rs index 85191dd2..02684f2d 100644 --- a/INV4/pallet-inv4/src/multisig.rs +++ b/INV4/pallet-inv4/src/multisig.rs @@ -1,3 +1,16 @@ +//! Multisig Operations. +//! +//! ## Overview +//! +//! Handles the core actions within an already established multisig. +//! +//! ### Core functionalities: +//! - Minting/Burning voting tokens. +//! - Handling proposal votes. +//! - Dispatching approved proposals when both support and approval meet/exceed their minimum required thresholds. +//! - Canceling proposals. +//! - Adding/Removing multisig members. + use super::pallet::{self, *}; use crate::{ account_derivation::CoreAccountDerivation, @@ -202,7 +215,8 @@ where // Get the voting token balance of the caller let voter_balance: BalanceOf = T::AssetsProvider::balance(core_id, &owner); - // If caller doesn't own the token, they have no voting power + // If caller doesn't own the token, they have no voting power and his + // voting power is directly correlated with his token balance. ensure!(!voter_balance.is_zero(), Error::::NoPermission); // Get the multisig call data from the storage diff --git a/INV4/pallet-inv4/src/origin.rs b/INV4/pallet-inv4/src/origin.rs index d0dd79ff..f94553e1 100644 --- a/INV4/pallet-inv4/src/origin.rs +++ b/INV4/pallet-inv4/src/origin.rs @@ -1,3 +1,11 @@ +//! Custom Multisig Origin (`INV4Origin`). +//! +//! ## Overview +//! +//! This module enhances security and fee handling for multisig operations, defines a custom origin [`INV4Origin`] and +//! includes the [`ensure_multisig`] function to guarantee calls genuinely come from the multisig account. +//! The origin also conviniently automates fee deductions associated with dispatched proposals directly from the multisig account. + use crate::{ account_derivation::CoreAccountDerivation, pallet::{self, Origin, Pallet}, @@ -13,6 +21,7 @@ pub enum INV4Origin { Multisig(MultisigInternalOrigin), } +/// Internal origin for identifying the multisig CoreId. #[derive(PartialEq, Eq, Encode, Decode, TypeInfo, MaxEncodedLen, Clone, RuntimeDebug)] pub struct MultisigInternalOrigin { pub id: T::CoreId, @@ -31,6 +40,7 @@ where } } +/// Matches the origin to ensures the passed origin is indeed from the multisig itself. pub fn ensure_multisig( o: OuterOrigin, ) -> Result, BadOrigin> diff --git a/INV4/pallet-inv4/src/voting.rs b/INV4/pallet-inv4/src/voting.rs index 3f1d57cd..dd671a4a 100644 --- a/INV4/pallet-inv4/src/voting.rs +++ b/INV4/pallet-inv4/src/voting.rs @@ -1,3 +1,12 @@ +//! Voting Mechanism. +//! +//! ## Overview +//! +//! This module provides a weighted voting [`Tally`] implementation used for managing the multisig's proposals. +//! Members each have a balance in voting tokens and this balance differentiate their voting power +//! as every vote utilizes the entire `power` of the said member. +//! This empowers decision-making where certain members possess greater influence. + use crate::{origin::INV4Origin, BalanceOf, Config, CoreStorage, Error, Multisig, Pallet}; use codec::{Decode, Encode, HasCompact, MaxEncodedLen}; use core::marker::PhantomData; @@ -38,6 +47,7 @@ pub struct Tally { } impl Tally { + /// Used manually building a `Tally`. pub fn from_parts( ayes: Votes, nays: Votes, @@ -51,6 +61,7 @@ impl Tally { } } + /// Cheks if a vote is valid then adds the voters balance to his desired vote. pub fn process_vote( &mut self, account: T::AccountId, @@ -202,12 +213,16 @@ impl CustomPolling> for Pallet { } } +/// Represents a proposal vote within a multisig context. +/// +/// This is both the vote and it's strenght as in how many tokens are being voted. #[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub enum Vote { Aye(Votes), Nay(Votes), } +/// This type is used for checkign how many votes a voting option has. pub type VoteRecord = Vote>; impl Pallet @@ -215,6 +230,7 @@ where Result, ::RuntimeOrigin>: From<::RuntimeOrigin>, { + /// Cheks the minimum support and approval required for passing a proposal on said core. pub fn minimum_support_and_required_approval(core_id: T::CoreId) -> Option<(Perbill, Perbill)> { CoreStorage::::get(core_id).map(|core| (core.minimum_support, core.required_approval)) } From b1b568084d5d1ea67d06217ef8087299f645d942 Mon Sep 17 00:00:00 2001 From: Francisco Valentim Castilho Date: Sat, 17 Feb 2024 18:23:34 -0300 Subject: [PATCH 2/3] Fix: visually weird without spacing --- INV4/pallet-inv4/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/INV4/pallet-inv4/src/lib.rs b/INV4/pallet-inv4/src/lib.rs index d24d293f..5c87d40e 100644 --- a/INV4/pallet-inv4/src/lib.rs +++ b/INV4/pallet-inv4/src/lib.rs @@ -6,6 +6,7 @@ //! //! ## Overview //! This pallet handles advanced virtual multisigs (internally called cores). +//! //! Lower level implementation details of this pallet's calls are contained in separate modules each of them //! containing their own docs. //! From df69b041953ebadac0c09f17a444ba03b87085a3 Mon Sep 17 00:00:00 2001 From: Gabriel Facco de Arruda Date: Sun, 18 Feb 2024 11:38:07 -0300 Subject: [PATCH 3/3] Apply suggestions from code review --- INV4/pallet-inv4/src/fee_handling.rs | 6 +++--- INV4/pallet-inv4/src/inv4_core.rs | 5 ++--- INV4/pallet-inv4/src/lib.rs | 2 +- INV4/pallet-inv4/src/lookup.rs | 8 ++++---- INV4/pallet-inv4/src/multisig.rs | 6 ++---- INV4/pallet-inv4/src/origin.rs | 8 +++++--- INV4/pallet-inv4/src/voting.rs | 10 +++++----- 7 files changed, 22 insertions(+), 23 deletions(-) diff --git a/INV4/pallet-inv4/src/fee_handling.rs b/INV4/pallet-inv4/src/fee_handling.rs index 3667bcfd..abec8cf5 100644 --- a/INV4/pallet-inv4/src/fee_handling.rs +++ b/INV4/pallet-inv4/src/fee_handling.rs @@ -17,9 +17,9 @@ use sp_runtime::{ DispatchResult, }; -/// Represents the asset to be used by the multisig for paying fees transaction fees. +/// Represents the asset to be used by the multisig for paying transaction fees. /// -/// This enum plays a role in marking the desired asset in the `MultisigFeeHandler` trait. +/// This enum defines the assets that can be used to pay for transaction fees. #[derive(Clone, TypeInfo, Encode, Decode, MaxEncodedLen, Debug, PartialEq, Eq)] pub enum FeeAsset { Native, @@ -67,7 +67,7 @@ pub trait MultisigFeeHandler { result: &DispatchResult, ) -> Result<(), TransactionValidityError>; - /// Charges the fee for creating the core(multisig). + /// Charges the fee for creating the core (multisig). fn handle_creation_fee( imbalance: FeeAssetNegativeImbalance< >::NegativeImbalance, diff --git a/INV4/pallet-inv4/src/inv4_core.rs b/INV4/pallet-inv4/src/inv4_core.rs index bae8efe9..37f09abf 100644 --- a/INV4/pallet-inv4/src/inv4_core.rs +++ b/INV4/pallet-inv4/src/inv4_core.rs @@ -5,9 +5,8 @@ //! This module handles the mechanics of creating multisigs (referred to as "cores") and their lifecycle management. Key functions include: //! //! - `inner_create_core`: Sets up a new core, deriving its AccountId, distributing voting tokens, and handling creation fees. -//! - `inner_set_parameters`: Updates the cores's operational rules based on passed proposals. -//! Use with caution as breaking changes caused by bad inputs are not checked. -//! - `is_asset_frozen`: Utility function for checking if wether a core's voting asset is frozen if it exists. +//! - `inner_set_parameters`: Updates the core's operational rules. +//! - `is_asset_frozen`: Utility function for checking if a core's voting asset is frozen (can't be transferred by the owner). use super::pallet::*; use crate::{ diff --git a/INV4/pallet-inv4/src/lib.rs b/INV4/pallet-inv4/src/lib.rs index 5c87d40e..e87571db 100644 --- a/INV4/pallet-inv4/src/lib.rs +++ b/INV4/pallet-inv4/src/lib.rs @@ -7,7 +7,7 @@ //! ## Overview //! This pallet handles advanced virtual multisigs (internally called cores). //! -//! Lower level implementation details of this pallet's calls are contained in separate modules each of them +//! Lower level implementation details of this pallet's calls are contained in separate modules, each of them //! containing their own docs. //! //! ### Pallet Functions diff --git a/INV4/pallet-inv4/src/lookup.rs b/INV4/pallet-inv4/src/lookup.rs index 70bb92ef..901bd670 100644 --- a/INV4/pallet-inv4/src/lookup.rs +++ b/INV4/pallet-inv4/src/lookup.rs @@ -1,10 +1,10 @@ -//! Core's XCM location utilities. +//! Custom account lookup implementation. //! //! ## Overview //! -//! This module implements the [`StaticLookup`] trait allowing for convenient conversion between a -//! Core's id and it's derived AccountId. -//! This implementation abstracs on top of two lower level functions: +//! This module implements the [`StaticLookup`] trait allowing for convenient lookup of a core's +//! AccountId from its CoreId. +//! This implementation abstracts on top of two lower level functions: //! - `lookup_core`: Used for accessing the storage and retrieving a core's AccountId. //! - `lookup_address`: Used for converting from a `MultiAddress::Index` that contains a CoreId to this core's AccountId. diff --git a/INV4/pallet-inv4/src/multisig.rs b/INV4/pallet-inv4/src/multisig.rs index 02684f2d..1488f5c1 100644 --- a/INV4/pallet-inv4/src/multisig.rs +++ b/INV4/pallet-inv4/src/multisig.rs @@ -5,11 +5,10 @@ //! Handles the core actions within an already established multisig. //! //! ### Core functionalities: -//! - Minting/Burning voting tokens. +//! - Minting/Burning voting tokens to existing and new members. //! - Handling proposal votes. //! - Dispatching approved proposals when both support and approval meet/exceed their minimum required thresholds. //! - Canceling proposals. -//! - Adding/Removing multisig members. use super::pallet::{self, *}; use crate::{ @@ -215,8 +214,7 @@ where // Get the voting token balance of the caller let voter_balance: BalanceOf = T::AssetsProvider::balance(core_id, &owner); - // If caller doesn't own the token, they have no voting power and his - // voting power is directly correlated with his token balance. + // If caller doesn't own the token, they have no voting power. ensure!(!voter_balance.is_zero(), Error::::NoPermission); // Get the multisig call data from the storage diff --git a/INV4/pallet-inv4/src/origin.rs b/INV4/pallet-inv4/src/origin.rs index f94553e1..8249a0ef 100644 --- a/INV4/pallet-inv4/src/origin.rs +++ b/INV4/pallet-inv4/src/origin.rs @@ -2,9 +2,11 @@ //! //! ## Overview //! -//! This module enhances security and fee handling for multisig operations, defines a custom origin [`INV4Origin`] and +//! This module introduces a custom origin [`INV4Origin`], enabling self-management for cores and //! includes the [`ensure_multisig`] function to guarantee calls genuinely come from the multisig account. -//! The origin also conviniently automates fee deductions associated with dispatched proposals directly from the multisig account. +//! This is an efficient approach considering that converting from CoreId to AccountId is a one-way operation, +//! so the origin brings the CoreId to dispatchable calls. +//! Converting to a `RawOrigin::Signed` origin for other calls is handled in the runtime. use crate::{ account_derivation::CoreAccountDerivation, @@ -40,7 +42,7 @@ where } } -/// Matches the origin to ensures the passed origin is indeed from the multisig itself. +/// Ensures the passed origin is a multisig, returning [`MultisigInternalOrigin`]. pub fn ensure_multisig( o: OuterOrigin, ) -> Result, BadOrigin> diff --git a/INV4/pallet-inv4/src/voting.rs b/INV4/pallet-inv4/src/voting.rs index dd671a4a..bd8786f9 100644 --- a/INV4/pallet-inv4/src/voting.rs +++ b/INV4/pallet-inv4/src/voting.rs @@ -47,7 +47,7 @@ pub struct Tally { } impl Tally { - /// Used manually building a `Tally`. + /// Allows for building a `Tally` manually. pub fn from_parts( ayes: Votes, nays: Votes, @@ -61,7 +61,7 @@ impl Tally { } } - /// Cheks if a vote is valid then adds the voters balance to his desired vote. + /// Check if a vote is valid and add the member's total voting token balance to the tally. pub fn process_vote( &mut self, account: T::AccountId, @@ -215,14 +215,14 @@ impl CustomPolling> for Pallet { /// Represents a proposal vote within a multisig context. /// -/// This is both the vote and it's strenght as in how many tokens are being voted. +/// This is both the vote and how many voting tokens it carries. #[derive(PartialEq, Eq, Clone, Copy, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub enum Vote { Aye(Votes), Nay(Votes), } -/// This type is used for checkign how many votes a voting option has. +/// Type alias for [`Vote`] with [`BalanceOf`]. pub type VoteRecord = Vote>; impl Pallet @@ -230,7 +230,7 @@ where Result, ::RuntimeOrigin>: From<::RuntimeOrigin>, { - /// Cheks the minimum support and approval required for passing a proposal on said core. + /// Returns the minimum support and required approval thresholds of a core. pub fn minimum_support_and_required_approval(core_id: T::CoreId) -> Option<(Perbill, Perbill)> { CoreStorage::::get(core_id).map(|core| (core.minimum_support, core.required_approval)) }