Improve secondary key permissions. (#1679)

* Split secondary key permissions out of KeyRecords.

* Fix JS integration tests.

* Make sure to remove permissions when removing a secondary key.

* Fix remove key permissions.

* Use BTreeMap for pallet/extrinsic permissions.

* Tried to fix TS integration tests.

* Fix js integration tests.

* Add integration test for contract permissions.

* Check secondary key call permissions in SignedExtension.

* Add origin call filter to enforce call permissions.

* Fix expected error.

* Update MultiSig to support newer permissions.

* Fix call whitelist.

* Whitelist identity.accept_primary_key

* Remove origin_call_filter.

* Remove extra clone.

* Change back to nextest.

* Regen TS types.

* cargo fmt

* Fix merge issues.

* Add storage migrations for split key permissions.

---------

Co-authored-by: Adam Dossa <[email protected]>

integration/Cargo.toml

@@ -27,4 +27,4 @@ sp-keyring = "29.0"
 sp-weights = "25.0"
 
 polymesh-api = { version = "3.7.0", features = ["download_metadata"] }
-polymesh-api-tester = { version = "0.6.1", features = ["download_metadata"] }
+polymesh-api-tester = { version = "0.6.1", default-features = false, features = ["download_metadata", "v7"] }

integration/src/lib.rs

@@ -5,8 +5,10 @@ pub use polymesh_api_tester::*;
 use std::collections::{BTreeMap, BTreeSet};
 
 use polymesh_api::types::polymesh_primitives::{
-    identity_id::PortfolioId, secondary_key::PalletPermissions, subset::SubsetRestriction,
-    DispatchableName, PalletName,
+    identity_id::PortfolioId,
+    secondary_key::{ExtrinsicPermissions, PalletPermissions},
+    subset::SubsetRestriction,
+    ExtrinsicName, PalletName,
 };
 use polymesh_api::*;
 
@@ -147,42 +149,33 @@ impl PalletPermissionsBuilder {
         pallet.add(&extrinsic.to_string());
     }
 
-    fn build_entries(&self) -> BTreeSet<PalletPermissions> {
+    fn build_entries(&self) -> BTreeMap<PalletName, PalletPermissions> {
         if let Some(entries) = &self.entries {
             entries
                 .iter()
                 .map(|(pallet, extrinsics)| {
-                    let dispatchable_names = match extrinsics.build() {
+                    let extrinsics = match extrinsics.build() {
                         SubsetRestriction::Whole => SubsetRestriction::Whole,
                         SubsetRestriction::These(names) => SubsetRestriction::These(
-                            names
-                                .into_iter()
-                                .map(|n| DispatchableName(n.as_bytes().into()))
-                                .collect(),
+                            names.into_iter().map(|n| ExtrinsicName(n.into())).collect(),
                         ),
                         SubsetRestriction::Except(names) => SubsetRestriction::Except(
-                            names
-                                .into_iter()
-                                .map(|n| DispatchableName(n.as_bytes().into()))
-                                .collect(),
+                            names.into_iter().map(|n| ExtrinsicName(n.into())).collect(),
                         ),
                     };
-                    PalletPermissions {
-                        pallet_name: PalletName(pallet.as_bytes().into()),
-                        dispatchable_names,
-                    }
+                    (PalletName(pallet.into()), PalletPermissions { extrinsics })
                 })
                 .collect()
         } else {
             Default::default()
         }
     }
 
-    pub fn build(&self) -> SubsetRestriction<PalletPermissions> {
+    pub fn build(&self) -> ExtrinsicPermissions {
         match &self.mode {
-            RestrictionMode::Whole => SubsetRestriction::Whole,
-            RestrictionMode::These => SubsetRestriction::These(self.build_entries()),
-            RestrictionMode::Except => SubsetRestriction::Except(self.build_entries()),
+            RestrictionMode::Whole => ExtrinsicPermissions::Whole,
+            RestrictionMode::These => ExtrinsicPermissions::These(self.build_entries()),
+            RestrictionMode::Except => ExtrinsicPermissions::Except(self.build_entries()),
         }
     }
 }
@@ -360,18 +353,30 @@ impl IntegrationUser for User {
     ) -> Result<()> {
         let permissions = permissions.into();
         let sk = self.get_sk(sk)?.account();
-        let record = self
+        let asset = self
             .api
             .query()
             .identity()
-            .key_records(sk)
+            .key_asset_permissions(sk)
             .await?
-            .ok_or_else(|| anyhow!("Missing KeyRecords"))?;
-        let key_permissions = match record {
-            KeyRecord::SecondaryKey(_, perms) => Some(perms),
-            _ => None,
-        };
-        assert_eq!(Some(permissions), key_permissions);
+            .ok_or_else(|| anyhow!("Missing asset permissions"))?;
+        assert_eq!(permissions.asset, asset);
+        let portfolio = self
+            .api
+            .query()
+            .identity()
+            .key_portfolio_permissions(sk)
+            .await?
+            .ok_or_else(|| anyhow!("Missing portfolio permissions"))?;
+        assert_eq!(permissions.portfolio, portfolio);
+        let extrinsic = self
+            .api
+            .query()
+            .identity()
+            .key_extrinsic_permissions(sk)
+            .await?
+            .ok_or_else(|| anyhow!("Missing extrinsic permissions"))?;
+        assert_eq!(permissions.extrinsic, extrinsic);
         Ok(())
     }
 

integration/tests/multisig_permissions.rs

@@ -307,18 +307,30 @@ impl MuliSigState {
         permissions: impl Into<Permissions> + Send,
     ) -> Result<()> {
         let permissions = permissions.into();
-        let record = self
+        let asset = self
             .api
             .query()
             .identity()
-            .key_records(self.account)
+            .key_asset_permissions(self.account)
             .await?
-            .ok_or_else(|| anyhow!("Missing KeyRecords"))?;
-        let key_permissions = match record {
-            KeyRecord::SecondaryKey(_, perms) => Some(perms),
-            _ => None,
-        };
-        assert_eq!(Some(permissions), key_permissions);
+            .ok_or_else(|| anyhow!("Missing asset permissions"))?;
+        assert_eq!(permissions.asset, asset);
+        let portfolio = self
+            .api
+            .query()
+            .identity()
+            .key_portfolio_permissions(self.account)
+            .await?
+            .ok_or_else(|| anyhow!("Missing portfolio permissions"))?;
+        assert_eq!(permissions.portfolio, portfolio);
+        let extrinsic = self
+            .api
+            .query()
+            .identity()
+            .key_extrinsic_permissions(self.account)
+            .await?
+            .ok_or_else(|| anyhow!("Missing extrinsic permissions"))?;
+        assert_eq!(permissions.extrinsic, extrinsic);
         Ok(())
     }
 }

pallets/common/src/traits/permissions.rs

@@ -13,7 +13,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program. If not, see <http://www.gnu.org/licenses/>.
 
-use polymesh_primitives::{DispatchableName, IdentityId, PalletName, SecondaryKey};
+use polymesh_primitives::{ExtrinsicName, IdentityId, PalletName, SecondaryKey};
 
 /// Permissions module configuration trait.
 pub trait Config: frame_system::Config {
@@ -44,6 +44,6 @@ pub trait CheckAccountCallPermissions<AccountId> {
     fn check_account_call_permissions(
         who: &AccountId,
         pallet_name: impl FnOnce() -> PalletName,
-        function_name: impl FnOnce() -> DispatchableName,
+        function_name: impl FnOnce() -> ExtrinsicName,
     ) -> Option<AccountCallPermissionsData<AccountId>>;
 }

pallets/contracts/src/benchmarking.rs

@@ -37,10 +37,10 @@ use polymesh_primitives::asset::AssetID;
 use polymesh_primitives::identity::limits::{
     MAX_ASSETS, MAX_EXTRINSICS, MAX_PALLETS, MAX_PORTFOLIOS,
 };
-use polymesh_primitives::secondary_key::DispatchableNames;
+use polymesh_primitives::secondary_key::ExtrinsicNames;
 use polymesh_primitives::{
-    AssetPermissions, Balance, DispatchableName, ExtrinsicPermissions, PalletName,
-    PalletPermissions, Permissions, PortfolioId, PortfolioNumber, PortfolioPermissions, Ticker,
+    AssetPermissions, Balance, ExtrinsicName, ExtrinsicPermissions, PalletName, PalletPermissions,
+    Permissions, PortfolioId, PortfolioNumber, PortfolioPermissions,
 };
 
 use crate::chain_extension::*;
@@ -164,11 +164,14 @@ fn secondary_key_permission(
     let portfolio = PortfolioPermissions::elems(
         (0..n_portfolios).map(|did| PortfolioId::user_portfolio(did.into(), PortfolioNumber(0))),
     );
-    let dispatchable_names =
-        DispatchableNames::elems((0..n_extrinsics).map(|e| DispatchableName(Ticker::generate(e))));
-    let extrinsic = ExtrinsicPermissions::elems((0..n_pallets).map(|p| PalletPermissions {
-        pallet_name: PalletName(Ticker::generate(p)),
-        dispatchable_names: dispatchable_names.clone(),
+    let extrinsics = ExtrinsicNames::elems((0..n_extrinsics).map(|e| ExtrinsicName::generate(e)));
+    let extrinsic = ExtrinsicPermissions::these((0..n_pallets).map(|p| {
+        (
+            PalletName::generate(p),
+            PalletPermissions {
+                extrinsics: extrinsics.clone(),
+            },
+        )
     }));
     Permissions {
         asset,

pallets/external-agents/src/benchmarking.rs

@@ -18,8 +18,7 @@ use frame_benchmarking::benchmarks;
 use polymesh_common_utilities::benchs::{create_and_issue_sample_asset, user, AccountIdOf, User};
 use polymesh_common_utilities::traits::asset::Config as Asset;
 use polymesh_common_utilities::TestUtilsFn;
-use polymesh_primitives::{AuthorizationData, ExtrinsicPermissions, PalletPermissions};
-use scale_info::prelude::format;
+use polymesh_primitives::{AuthorizationData, ExtrinsicPermissions, PalletName, PalletPermissions};
 use sp_std::prelude::*;
 
 pub(crate) const SEED: u32 = 0;
@@ -32,9 +31,8 @@ fn setup<T: Asset + TestUtilsFn<AccountIdOf<T>>>() -> (User<T>, AssetID) {
 }
 
 fn perms(n: u32) -> ExtrinsicPermissions {
-    ExtrinsicPermissions::elems(
-        (0..=n as u64)
-            .map(|w| PalletPermissions::entire_pallet(format!("Pallet{}", w).as_bytes().into())),
+    ExtrinsicPermissions::these(
+        (0..=n as u64).map(|w| (PalletName::generate(w), PalletPermissions::whole())),
     )
 }
 

pallets/external-agents/src/lib.rs

@@ -551,30 +551,29 @@ impl<T: Config> Module<T> {
     fn agent_permissions(asset_id: &AssetID, agent: IdentityId) -> ExtrinsicPermissions {
         let pallet = |p: &str| PalletPermissions::entire_pallet(p.into());
         let in_pallet = |p: &str, dns| PalletPermissions::new(p.into(), dns);
-        fn elems<T: Ord, const N: usize>(elems: [T; N]) -> SubsetRestriction<T> {
-            SubsetRestriction::elems(elems)
-        }
         match GroupOfAgent::get(asset_id, agent) {
             None => ExtrinsicPermissions::empty(),
             Some(AgentGroup::Full) => ExtrinsicPermissions::default(),
             Some(AgentGroup::Custom(ag_id)) => {
                 GroupPermissions::get(asset_id, ag_id).unwrap_or_else(ExtrinsicPermissions::empty)
             }
             // Anything but extrinsics in this pallet.
-            Some(AgentGroup::ExceptMeta) => SubsetRestriction::except(pallet("ExternalAgents")),
+            Some(AgentGroup::ExceptMeta) => {
+                ExtrinsicPermissions::except([pallet("ExternalAgents")])
+            }
             // Pallets `CorporateAction`, `CorporateBallot`, and `CapitalDistribution`.
-            Some(AgentGroup::PolymeshV1CAA) => elems([
+            Some(AgentGroup::PolymeshV1CAA) => ExtrinsicPermissions::these([
                 pallet("CorporateAction"),
                 pallet("CorporateBallot"),
                 pallet("CapitalDistribution"),
             ]),
-            Some(AgentGroup::PolymeshV1PIA) => elems([
+            Some(AgentGroup::PolymeshV1PIA) => ExtrinsicPermissions::these([
                 // All in `Sto` except `Sto::invest`.
                 in_pallet("Sto", SubsetRestriction::except("invest".into())),
                 // Asset::{issue, redeem, controller_transfer}.
                 in_pallet(
                     "Asset",
-                    elems([
+                    SubsetRestriction::elems([
                         "issue".into(),
                         "redeem".into(),
                         "controller_transfer".into(),

pallets/external-agents/src/migrations.rs

@@ -5,7 +5,7 @@ use super::*;
 
 mod v0 {
     use super::*;
-    use polymesh_primitives::Ticker;
+    use polymesh_primitives::{v6, Ticker};
 
     decl_storage! {
         trait Store for Module<T: Config> as ExternalAgents {
@@ -27,7 +27,7 @@ mod v0 {
 
             // This storage changed the Ticker key to AssetID.
             pub GroupPermissions get(fn permissions):
-                double_map hasher(blake2_128_concat) Ticker, hasher(twox_64_concat) AGId => Option<ExtrinsicPermissions>;
+                double_map hasher(blake2_128_concat) Ticker, hasher(twox_64_concat) AGId => Option<v6::ExtrinsicPermissions>;
         }
 
     }
@@ -92,7 +92,7 @@ pub(crate) fn migrate_to_v1<T: Config>() {
         let asset_id = ticker_to_asset_id
             .entry(ticker)
             .or_insert(AssetID::from(ticker));
-        GroupPermissions::insert(asset_id, ag_id, ext_perms);
+        GroupPermissions::insert(asset_id, ag_id, ExtrinsicPermissions::from(ext_perms));
     });
     log::info!("{:?} items migrated", count);
 }

pallets/identity/src/benchmarking.rs

@@ -17,7 +17,6 @@ use crate::*;
 
 use frame_benchmarking::{account, benchmarks};
 use frame_system::RawOrigin;
-use scale_info::prelude::format;
 use sp_core::H512;
 use sp_std::prelude::*;
 
@@ -29,11 +28,11 @@ use polymesh_primitives::asset::AssetID;
 use polymesh_primitives::identity::limits::{
     MAX_ASSETS, MAX_EXTRINSICS, MAX_PALLETS, MAX_PORTFOLIOS, MAX_SECONDARY_KEYS,
 };
-use polymesh_primitives::secondary_key::DispatchableNames;
+use polymesh_primitives::secondary_key::ExtrinsicNames;
 use polymesh_primitives::{
-    AssetPermissions, AuthorizationData, Claim, CountryCode, DispatchableName,
-    ExtrinsicPermissions, PalletName, PalletPermissions, Permissions, PortfolioId, PortfolioNumber,
-    PortfolioPermissions, Scope, SecondaryKey, Signatory,
+    AssetPermissions, AuthorizationData, Claim, CountryCode, ExtrinsicName, ExtrinsicPermissions,
+    PalletName, PalletPermissions, Permissions, PortfolioId, PortfolioNumber, PortfolioPermissions,
+    Scope, SecondaryKey, Signatory,
 };
 
 const SEED: u32 = 0;
@@ -311,17 +310,16 @@ benchmarks! {
                 PortfolioId::user_portfolio(did.into(), PortfolioNumber(0))
             })
         );
-        let dispatchable_names = DispatchableNames::elems(
+        let extrinsics = ExtrinsicNames::elems(
             (0..e as u64).map(|e| {
-                DispatchableName(format!("Calls{}", e).as_bytes().into())
+                ExtrinsicName::generate(e)
             })
         );
-        let extrinsic = ExtrinsicPermissions::elems(
+        let extrinsic = ExtrinsicPermissions::these(
             (0..l as u64).map(|p| {
-                PalletPermissions {
-                    pallet_name: PalletName(format!("Pallet{}", p).as_bytes().into()),
-                    dispatchable_names: dispatchable_names.clone(),
-                }
+                (PalletName::generate(p), PalletPermissions {
+                    extrinsics: extrinsics.clone(),
+                })
             })
         );