From 8f8297e9debe79139e3a41be8b54a5fcd603f783 Mon Sep 17 00:00:00 2001
From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com>
Date: Wed, 6 Mar 2024 01:49:18 +0100
Subject: [PATCH] Permissioned contract deployment (#3377)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Closes: #3196
---------
Co-authored-by: Alexander Theißen <alex.theissen@me.com>
Co-authored-by: PG Herveou <pgherveou@parity.io>
---
.../contracts-rococo/src/contracts.rs | 3 +
prdoc/pr_3377.prdoc | 14 ++
substrate/bin/node/runtime/src/lib.rs | 2 +
.../src/parachain/contracts_config.rs | 4 +-
substrate/frame/contracts/src/lib.rs | 35 ++++-
substrate/frame/contracts/src/tests.rs | 131 +++++++++++++++++-
6 files changed, 182 insertions(+), 7 deletions(-)
create mode 100644 prdoc/pr_3377.prdoc
diff --git a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/contracts.rs b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/contracts.rs
index 681b95ce6a5..171ac6a9528 100644
--- a/cumulus/parachains/runtimes/contracts/contracts-rococo/src/contracts.rs
+++ b/cumulus/parachains/runtimes/contracts/contracts-rococo/src/contracts.rs
@@ -21,6 +21,7 @@ use frame_support::{
parameter_types,
traits::{ConstBool, ConstU32, Nothing},
};
+use frame_system::EnsureSigned;
use pallet_contracts::{
weights::SubstrateWeight, Config, DebugInfo, DefaultAddressGenerator, Frame, Schedule,
};
@@ -65,6 +66,8 @@ impl Config for Runtime {
type MaxCodeLen = ConstU32<{ 123 * 1024 }>;
type MaxStorageKeyLen = ConstU32<128>;
type UnsafeUnstableInterface = ConstBool<true>;
+ type UploadOrigin = EnsureSigned<Self::AccountId>;
+ type InstantiateOrigin = EnsureSigned<Self::AccountId>;
type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>;
type MaxDelegateDependencies = ConstU32<32>;
type CodeHashLockupDepositPercent = CodeHashLockupDepositPercent;
diff --git a/prdoc/pr_3377.prdoc b/prdoc/pr_3377.prdoc
new file mode 100644
index 00000000000..8e5b3935512
--- /dev/null
+++ b/prdoc/pr_3377.prdoc
@@ -0,0 +1,14 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Permissioned contract deployment
+
+doc:
+ - audience: Runtime Dev
+ description: |
+ This PR introduces two new config types that specify the origins allowed to
+ upload and instantiate contract code. However, this check is not enforced when
+ a contract instantiates another contract.
+
+crates:
+- name: pallet-contracts
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index f4615802515..18ce13cff80 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/bin/node/runtime/src/lib.rs
@@ -1365,6 +1365,8 @@ impl pallet_contracts::Config for Runtime {
type MaxCodeLen = ConstU32<{ 123 * 1024 }>;
type MaxStorageKeyLen = ConstU32<128>;
type UnsafeUnstableInterface = ConstBool<false>;
+ type UploadOrigin = EnsureSigned<Self::AccountId>;
+ type InstantiateOrigin = EnsureSigned<Self::AccountId>;
type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>;
type RuntimeHoldReason = RuntimeHoldReason;
#[cfg(not(feature = "runtime-benchmarks"))]
diff --git a/substrate/frame/contracts/mock-network/src/parachain/contracts_config.rs b/substrate/frame/contracts/mock-network/src/parachain/contracts_config.rs
index 3f26c6f372e..3c06131dd60 100644
--- a/substrate/frame/contracts/mock-network/src/parachain/contracts_config.rs
+++ b/substrate/frame/contracts/mock-network/src/parachain/contracts_config.rs
@@ -25,7 +25,7 @@ use frame_support::{
traits::{ConstBool, ConstU32, Contains, Randomness},
weights::Weight,
};
-use frame_system::pallet_prelude::BlockNumberFor;
+use frame_system::{pallet_prelude::BlockNumberFor, EnsureSigned};
use pallet_xcm::BalanceOf;
use sp_runtime::{traits::Convert, Perbill};
@@ -90,6 +90,8 @@ impl pallet_contracts::Config for Runtime {
type Schedule = Schedule;
type Time = super::Timestamp;
type UnsafeUnstableInterface = ConstBool<true>;
+ type UploadOrigin = EnsureSigned<Self::AccountId>;
+ type InstantiateOrigin = EnsureSigned<Self::AccountId>;
type WeightInfo = ();
type WeightPrice = Self;
type Debug = ();
diff --git a/substrate/frame/contracts/src/lib.rs b/substrate/frame/contracts/src/lib.rs
index 32d789a458f..f941f152ffe 100644
--- a/substrate/frame/contracts/src/lib.rs
+++ b/substrate/frame/contracts/src/lib.rs
@@ -379,6 +379,24 @@ pub mod pallet {
#[pallet::constant]
type MaxDebugBufferLen: Get<u32>;
+ /// Origin allowed to upload code.
+ ///
+ /// By default, it is safe to set this to `EnsureSigned`, allowing anyone to upload contract
+ /// code.
+ type UploadOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Self::AccountId>;
+
+ /// Origin allowed to instantiate code.
+ ///
+ /// # Note
+ ///
+ /// This is not enforced when a contract instantiates another contract. The
+ /// [`Self::UploadOrigin`] should make sure that no code is deployed that does unwanted
+ /// instantiations.
+ ///
+ /// By default, it is safe to set this to `EnsureSigned`, allowing anyone to instantiate
+ /// contract code.
+ type InstantiateOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Self::AccountId>;
+
/// Overarching hold reason.
type RuntimeHoldReason: From<HoldReason>;
@@ -636,7 +654,7 @@ pub mod pallet {
determinism: Determinism,
) -> DispatchResult {
Migration::<T>::ensure_migrated()?;
- let origin = ensure_signed(origin)?;
+ let origin = T::UploadOrigin::ensure_origin(origin)?;
Self::bare_upload_code(origin, code, storage_deposit_limit.map(Into::into), determinism)
.map(|_| ())
}
@@ -785,11 +803,17 @@ pub mod pallet {
salt: Vec<u8>,
) -> DispatchResultWithPostInfo {
Migration::<T>::ensure_migrated()?;
- let origin = ensure_signed(origin)?;
+
+ // These two origins will usually be the same; however, we treat them as separate since
+ // it is possible for the `Success` value of `UploadOrigin` and `InstantiateOrigin` to
+ // differ.
+ let upload_origin = T::UploadOrigin::ensure_origin(origin.clone())?;
+ let instantiate_origin = T::InstantiateOrigin::ensure_origin(origin)?;
+
let code_len = code.len() as u32;
let (module, upload_deposit) = Self::try_upload_code(
- origin.clone(),
+ upload_origin,
code,
storage_deposit_limit.clone().map(Into::into),
Determinism::Enforced,
@@ -803,7 +827,7 @@ pub mod pallet {
let data_len = data.len() as u32;
let salt_len = salt.len() as u32;
let common = CommonInput {
- origin: Origin::from_account_id(origin),
+ origin: Origin::from_account_id(instantiate_origin),
value,
data,
gas_limit,
@@ -844,10 +868,11 @@ pub mod pallet {
salt: Vec<u8>,
) -> DispatchResultWithPostInfo {
Migration::<T>::ensure_migrated()?;
+ let origin = T::InstantiateOrigin::ensure_origin(origin)?;
let data_len = data.len() as u32;
let salt_len = salt.len() as u32;
let common = CommonInput {
- origin: Origin::from_runtime_origin(origin)?,
+ origin: Origin::from_account_id(origin),
value,
data,
gas_limit,
diff --git a/substrate/frame/contracts/src/tests.rs b/substrate/frame/contracts/src/tests.rs
index 2d1c5b315a3..2f8f6d4a437 100644
--- a/substrate/frame/contracts/src/tests.rs
+++ b/substrate/frame/contracts/src/tests.rs
@@ -45,6 +45,7 @@ use frame_support::{
assert_err, assert_err_ignore_postinfo, assert_err_with_weight, assert_noop, assert_ok,
derive_impl,
dispatch::{DispatchErrorWithPostInfo, PostDispatchInfo},
+ pallet_prelude::EnsureOrigin,
parameter_types,
storage::child,
traits::{
@@ -434,6 +435,33 @@ impl Contains<RuntimeCall> for TestFilter {
}
}
+parameter_types! {
+ pub static UploadAccount: Option<<Test as frame_system::Config>::AccountId> = None;
+ pub static InstantiateAccount: Option<<Test as frame_system::Config>::AccountId> = None;
+}
+
+pub struct EnsureAccount<T, A>(sp_std::marker::PhantomData<(T, A)>);
+impl<T: Config, A: sp_core::Get<Option<crate::AccountIdOf<T>>>>
+ EnsureOrigin<<T as frame_system::Config>::RuntimeOrigin> for EnsureAccount<T, A>
+where
+ <T as frame_system::Config>::AccountId: From<AccountId32>,
+{
+ type Success = T::AccountId;
+
+ fn try_origin(o: T::RuntimeOrigin) -> Result<Self::Success, T::RuntimeOrigin> {
+ let who = <frame_system::EnsureSigned<_> as EnsureOrigin<_>>::try_origin(o.clone())?;
+ if matches!(A::get(), Some(a) if who != a) {
+ return Err(o)
+ }
+
+ Ok(who)
+ }
+
+ #[cfg(feature = "runtime-benchmarks")]
+ fn try_successful_origin() -> Result<T::RuntimeOrigin, ()> {
+ Err(())
+ }
+}
parameter_types! {
pub static UnstableInterface: bool = true;
}
@@ -458,6 +486,8 @@ impl Config for Test {
type MaxCodeLen = ConstU32<{ 123 * 1024 }>;
type MaxStorageKeyLen = ConstU32<128>;
type UnsafeUnstableInterface = UnstableInterface;
+ type UploadOrigin = EnsureAccount<Self, UploadAccount>;
+ type InstantiateOrigin = EnsureAccount<Self, InstantiateAccount>;
type MaxDebugBufferLen = ConstU32<{ 2 * 1024 * 1024 }>;
type RuntimeHoldReason = RuntimeHoldReason;
type Migrations = crate::migration::codegen::BenchMigrations;
@@ -5924,7 +5954,106 @@ fn root_cannot_instantiate() {
vec![],
vec![],
),
- DispatchError::RootNotAllowed
+ DispatchError::BadOrigin
+ );
+ });
+}
+
+#[test]
+fn only_upload_origin_can_upload() {
+ let (wasm, _) = compile_module::<Test>("dummy").unwrap();
+ UploadAccount::set(Some(ALICE));
+ ExtBuilder::default().build().execute_with(|| {
+ let _ = Balances::set_balance(&ALICE, 1_000_000);
+ let _ = Balances::set_balance(&BOB, 1_000_000);
+
+ assert_err!(
+ Contracts::upload_code(
+ RuntimeOrigin::root(),
+ wasm.clone(),
+ None,
+ Determinism::Enforced,
+ ),
+ DispatchError::BadOrigin
+ );
+
+ assert_err!(
+ Contracts::upload_code(
+ RuntimeOrigin::signed(BOB),
+ wasm.clone(),
+ None,
+ Determinism::Enforced,
+ ),
+ DispatchError::BadOrigin
+ );
+
+ // Only alice is allowed to upload contract code.
+ assert_ok!(Contracts::upload_code(
+ RuntimeOrigin::signed(ALICE),
+ wasm.clone(),
+ None,
+ Determinism::Enforced,
+ ));
+ });
+}
+
+#[test]
+fn only_instantiation_origin_can_instantiate() {
+ let (code, code_hash) = compile_module::<Test>("dummy").unwrap();
+ InstantiateAccount::set(Some(ALICE));
+ ExtBuilder::default().build().execute_with(|| {
+ let _ = Balances::set_balance(&ALICE, 1_000_000);
+ let _ = Balances::set_balance(&BOB, 1_000_000);
+
+ assert_err_ignore_postinfo!(
+ Contracts::instantiate_with_code(
+ RuntimeOrigin::root(),
+ 0,
+ GAS_LIMIT,
+ None,
+ code.clone(),
+ vec![],
+ vec![],
+ ),
+ DispatchError::BadOrigin
+ );
+
+ assert_err_ignore_postinfo!(
+ Contracts::instantiate_with_code(
+ RuntimeOrigin::signed(BOB),
+ 0,
+ GAS_LIMIT,
+ None,
+ code.clone(),
+ vec![],
+ vec![],
+ ),
+ DispatchError::BadOrigin
+ );
+
+ // Only Alice can instantiate
+ assert_ok!(Contracts::instantiate_with_code(
+ RuntimeOrigin::signed(ALICE),
+ 0,
+ GAS_LIMIT,
+ None,
+ code,
+ vec![],
+ vec![],
+ ),);
+
+ // Bob cannot instantiate with either `instantiate_with_code` or `instantiate`.
+ assert_err_ignore_postinfo!(
+ Contracts::instantiate(
+ RuntimeOrigin::signed(BOB),
+ 0,
+ GAS_LIMIT,
+ None,
+ code_hash,
+ vec![],
+ vec![],
+ ),
+ DispatchError::BadOrigin
);
});
}
--
GitLab