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

Permissioned contract deployment #3377

Merged
merged 15 commits into from
Mar 6, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 14 additions & 0 deletions prdoc/pr_3377.prdoc
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions substrate/bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1357,6 +1357,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"))]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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 = ();
Expand Down
35 changes: 30 additions & 5 deletions substrate/frame/contracts/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,24 @@ pub mod pallet {
#[pallet::constant]
type MaxDebugBufferLen: Get<u32>;

/// Origin allowed to upload code.
Szegoo marked this conversation as resolved.
Show resolved Hide resolved
///
/// 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>;

Expand Down Expand Up @@ -618,7 +636,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(|_| ())
}
Expand Down Expand Up @@ -767,11 +785,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,
Expand All @@ -785,7 +809,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,
Expand Down Expand Up @@ -826,10 +850,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,
Expand Down
131 changes: 130 additions & 1 deletion substrate/frame/contracts/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -453,6 +454,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;
}
Expand All @@ -477,6 +505,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;
Expand Down Expand Up @@ -5942,7 +5972,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
);
});
}
Expand Down
Loading