Allow calling proxy.proxy from smart contracts in moonbase (#2148)

* allow calling proxy.proxy from smart contracts in moonbase

* Remove redundant is_precompile check

* Rename proxy_force_type to proxyForceType

* Compile contracts

precompiles/proxy/Proxy.sol

@@ -51,8 +51,8 @@ interface Proxy {
     /// @custom:selector 14a5b5fa
     function removeProxies() external;
 
-    /// @dev Dispatch the given subcall (`call_to`, `call_data`) from an account that the sender
-    /// is authorised for through `add_proxy`
+    /// @dev Dispatch the given subcall (`callTo`, `callData`) from an account that the sender
+    /// is authorised for through `addProxy`
     /// @custom:selector 0d3cff86
     /// @param real The account that the proxy will make a call on behalf of
     /// @param callTo Recipient of the call to be made by the `real` account
@@ -63,14 +63,14 @@ interface Proxy {
         bytes memory callData
     ) external payable;
 
-    /// @dev Dispatch the given subcall (`call_to`, `call_data`) from an account that the sender
-    /// is authorised for through `add_proxy`
-    /// @custom:selector 4a36b2cd
+    /// @dev Dispatch the given subcall (`callTo`, `callData`) from an account that the sender
+    /// is authorised for through `addProxy`
+    /// @custom:selector 685b9d2f
     /// @param real The account that the proxy will make a call on behalf of
     /// @param forceProxyType Specify the exact proxy type to be used and checked for this call
     /// @param callTo Recipient of the call to be made by the `real` account
     /// @param callData Data of the call to be made by the `real` account
-    function proxy_force_type(
+    function proxyForceType(
         address real,
         ProxyType forceProxyType,
         address callTo,

precompiles/proxy/src/lib.rs

@@ -18,7 +18,7 @@
 #![feature(assert_matches)]
 
 use evm::ExitReason;
-use fp_evm::{Context, PrecompileFailure, PrecompileHandle, PrecompileSet, Transfer};
+use fp_evm::{Context, PrecompileFailure, PrecompileHandle, Transfer};
 use frame_support::dispatch::{Dispatchable, GetDispatchInfo, PostDispatchInfo};
 use pallet_evm::AddressMapping;
 use pallet_proxy::Call as ProxyCall;
@@ -32,7 +32,7 @@ use sp_core::H160;
 use sp_core::U256;
 use sp_runtime::{
 	codec::Decode,
-	traits::{ConstU32, Get, StaticLookup, Zero},
+	traits::{ConstU32, StaticLookup, Zero},
 };
 use sp_std::marker::PhantomData;
 
@@ -70,6 +70,38 @@ where
 	}
 }
 
+#[derive(Debug)]
+pub struct OnlyIsProxyAndProxy<Runtime>(PhantomData<Runtime>);
+
+impl<Runtime> SelectorFilter for OnlyIsProxyAndProxy<Runtime>
+where
+	Runtime: pallet_proxy::Config + pallet_evm::Config + frame_system::Config,
+	<<Runtime as pallet_proxy::Config>::RuntimeCall as Dispatchable>::RuntimeOrigin:
+		From<Option<Runtime::AccountId>>,
+	<Runtime as pallet_proxy::Config>::ProxyType: Decode + EvmProxyCallFilter,
+	<Runtime as frame_system::Config>::RuntimeCall:
+		Dispatchable<PostInfo = PostDispatchInfo> + GetDispatchInfo,
+	<<Runtime as frame_system::Config>::RuntimeCall as Dispatchable>::RuntimeOrigin:
+		From<Option<Runtime::AccountId>>,
+	<Runtime as frame_system::Config>::RuntimeCall: From<ProxyCall<Runtime>>,
+{
+	fn is_allowed(_caller: H160, selector: Option<u32>) -> bool {
+		match selector {
+			None => false,
+			Some(selector) => {
+				ProxyPrecompileCall::<Runtime>::is_proxy_selectors().contains(&selector)
+					|| ProxyPrecompileCall::<Runtime>::proxy_selectors().contains(&selector)
+					|| ProxyPrecompileCall::<Runtime>::proxy_force_type_selectors()
+						.contains(&selector)
+			}
+		}
+	}
+
+	fn description() -> String {
+		"Allowed for all callers only for selectors 'is_proxy', 'proxy', 'proxy_force_type'".into()
+	}
+}
+
 pub const CALL_DATA_LIMIT: u32 = 2u32.pow(16);
 
 type GetCallDataLimit = ConstU32<CALL_DATA_LIMIT>;
@@ -238,6 +270,7 @@ where
 	/// - `force_proxy_type`: Specify the exact proxy type to be used and checked for this call.
 	/// - `call_to`: Recipient of the call to be made by the `real` account.
 	/// - `call_data`: Data of the call to be made by the `real` account.
+	#[precompile::public("proxyForceType(address,uint8,address,bytes)")]
 	#[precompile::public("proxy_force_type(address,uint8,address,bytes)")]
 	#[precompile::payable]
 	fn proxy_force_type(
@@ -301,14 +334,6 @@ where
 		force_proxy_type: Option<<Runtime as pallet_proxy::Config>::ProxyType>,
 		evm_subcall: EvmSubCall,
 	) -> EvmResult {
-		// Proxied call can be dispatched by users only.
-		// We should forbid precompiles here because pre_check allows precompiles.
-		if <Runtime as pallet_evm::Config>::PrecompilesValue::get()
-			.is_precompile(handle.context().caller)
-		{
-			return Err(revert("Proxy.proxy not callable by precompiles"));
-		}
-
 		// Read proxy
 		let real_account_id = Runtime::AddressMapping::into_account_id(real.clone().into());
 		let who = Runtime::AddressMapping::into_account_id(handle.context().caller);

precompiles/proxy/src/mock.rs

@@ -24,7 +24,8 @@ use frame_support::{
 use pallet_evm::{EnsureAddressNever, EnsureAddressOrigin, SubstrateBlockHashMapping};
 use precompile_utils::{
 	precompile_set::{
-		AddressU64, CallableByContract, PrecompileAt, PrecompileSetBuilder, SubcallWithMaxNesting,
+		AddressU64, CallableByContract, CallableByPrecompile, OnlyFrom, PrecompileAt,
+		PrecompileSetBuilder, RevertPrecompile, SubcallWithMaxNesting,
 	},
 	testing::MockAccount,
 };
@@ -108,9 +109,12 @@ pub type Precompiles<R> = PrecompileSetBuilder<
 			ProxyPrecompile<R>,
 			(
 				SubcallWithMaxNesting<1>,
-				CallableByContract<crate::OnlyIsProxy<R>>,
+				CallableByContract<crate::OnlyIsProxyAndProxy<R>>,
+				// Batch is the only precompile allowed to call Proxy.
+				CallableByPrecompile<OnlyFrom<AddressU64<2>>>,
 			),
 		>,
+		RevertPrecompile<AddressU64<2>>,
 	),
 >;
 

precompiles/proxy/src/tests.rs

@@ -18,14 +18,17 @@ use crate::mock::{
 	AccountId, ExtBuilder, PCall, PrecompilesValue, ProxyType, Runtime, RuntimeCall, RuntimeEvent,
 	RuntimeOrigin,
 };
+use evm::{ExitReason, ExitSucceed};
 use frame_support::{assert_ok, dispatch::Dispatchable};
 use pallet_evm::Call as EvmCall;
 use pallet_proxy::{
 	Call as ProxyCall, Event as ProxyEvent, Pallet as ProxyPallet, ProxyDefinition,
 };
 use precompile_utils::precompile_set::AddressU64;
 use precompile_utils::{assert_event_emitted, assert_event_not_emitted, prelude::*, testing::*};
-use sp_core::{Get, H160, U256};
+use sp_core::{Get, H160, H256, U256};
+use std::cell::Cell;
+use std::rc::Rc;
 use std::str::from_utf8;
 
 #[test]
@@ -686,3 +689,102 @@ fn proxy_proxy_should_fail_if_called_by_precompile() {
 				.execute_reverts(|output| output == b"Function not callable by precompiles");
 		})
 }
+
+#[test]
+fn proxy_proxy_should_succeed_if_called_by_allowed_precompile() {
+	// "Not proxy" means that the security filter has passed, so the call to proxy.proxy would work
+	// if we had done a proxy.add_proxy before.
+	ExtBuilder::default()
+		.with_balances(vec![
+			(AddressU64::<1>::get().into(), 1000),
+			(Bob.into(), 1000),
+		])
+		.build()
+		.execute_with(|| {
+			PrecompilesValue::get()
+				.prepare_test(
+					// Address<2> allowed in mock.rs
+					AddressU64::<2>::get(),
+					Precompile1,
+					PCall::proxy {
+						real: Address(Alice.into()),
+						call_to: Address(Bob.into()),
+						call_data: BoundedBytes::from([]),
+					},
+				)
+				.execute_reverts(|output| output == b"Not proxy");
+		})
+}
+
+#[test]
+fn proxy_proxy_should_succeed_if_called_by_smart_contract() {
+	ExtBuilder::default()
+		.with_balances(vec![
+			(AddressU64::<1>::get().into(), 1000),
+			(Bob.into(), 1000),
+		])
+		.build()
+		.execute_with(|| {
+			// Set code to Alice address as it if was a smart contract.
+			pallet_evm::AccountCodes::<Runtime>::insert(H160::from(Alice), vec![10u8]);
+
+			// Bob allows Alice to make calls on his behalf
+			assert_ok!(RuntimeCall::Proxy(ProxyCall::add_proxy {
+				delegate: Alice.into(),
+				proxy_type: ProxyType::Any,
+				delay: 0,
+			})
+			.dispatch(RuntimeOrigin::signed(Bob.into())));
+
+			let inside = Rc::new(Cell::new(false));
+			let inside2 = inside.clone();
+
+			// The smart contract calls proxy.proxy to call address Charlie as if it was Bob
+			PrecompilesValue::get()
+				.prepare_test(
+					Alice,
+					Precompile1,
+					PCall::proxy {
+						real: Address(Bob.into()),
+						call_to: Address(Charlie.into()),
+						call_data: BoundedBytes::from([1]),
+					},
+				)
+				.with_subcall_handle(move |subcall| {
+					let Subcall {
+						address,
+						transfer,
+						input,
+						target_gas: _,
+						is_static,
+						context,
+					} = subcall;
+
+					assert_eq!(context.caller, Bob.into());
+					assert_eq!(address, Charlie.into());
+					assert_eq!(is_static, false);
+
+					assert!(transfer.is_none());
+
+					assert_eq!(context.address, Charlie.into());
+					assert_eq!(context.apparent_value, 0u8.into());
+
+					assert_eq!(&input, &[1]);
+
+					inside2.set(true);
+
+					SubcallOutput {
+						reason: ExitReason::Succeed(ExitSucceed::Returned),
+						output: b"TEST".to_vec(),
+						cost: 13,
+						logs: vec![log1(Bob, H256::repeat_byte(0x11), vec![])],
+					}
+				})
+				.execute_returns_encoded(());
+
+			// Ensure that the subcall was actually called.
+			// proxy.proxy does not propagate the return value, so we cannot check for the return
+			// value "TEST"
+			assert!(inside.get(), "subcall not called");
+		})
+}

runtime/moonbase/src/precompiles.rs

@@ -34,7 +34,7 @@ use pallet_evm_precompile_democracy::DemocracyPrecompile;
 use pallet_evm_precompile_modexp::Modexp;
 use pallet_evm_precompile_parachain_staking::ParachainStakingPrecompile;
 use pallet_evm_precompile_preimage::PreimagePrecompile;
-use pallet_evm_precompile_proxy::{OnlyIsProxy, ProxyPrecompile};
+use pallet_evm_precompile_proxy::{OnlyIsProxyAndProxy, ProxyPrecompile};
 use pallet_evm_precompile_randomness::RandomnessPrecompile;
 use pallet_evm_precompile_referenda::ReferendaPrecompile;
 use pallet_evm_precompile_relay_encoder::RelayEncoderPrecompile;
@@ -173,7 +173,7 @@ type MoonbasePrecompilesAt<R> = (
 		AddressU64<2059>,
 		ProxyPrecompile<R>,
 		(
-			CallableByContract<OnlyIsProxy<R>>,
+			CallableByContract<OnlyIsProxyAndProxy<R>>,
 			SubcallWithMaxNesting<0>,
 			// Batch is the only precompile allowed to call Proxy.
 			CallableByPrecompile<OnlyFrom<AddressU64<2056>>>,