Precompile fixes (#1806)

* disable dispatch precompile * prevent smart contracts to call proxy precompile * Update precompiles/proxy/src/tests.rs Co-authored-by: Nisheeth Barthwal <[email protected]> * Fix typescript tests * more test fixes * Prettier * skip before too Co-authored-by: nanocryk <[email protected]> Co-authored-by: Nisheeth Barthwal <[email protected]>
moonbeam-foundation · Sep 12, 2022 · c7d8fc1 · c7d8fc1
1 parent d059a53
commit c7d8fc1
Show file tree

Hide file tree

Showing 10 changed files with 82 additions and 25 deletions.
diff --git a/precompiles/proxy/src/lib.rs b/precompiles/proxy/src/lib.rs
@@ -57,6 +57,14 @@ where
 	<Runtime as frame_system::Config>::Call: From<ProxyCall<Runtime>>,
 {
 	fn execute(handle: &mut impl PrecompileHandle) -> EvmResult<PrecompileOutput> {
+		handle.record_cost(RuntimeHelper::<Runtime>::db_read_gas_cost())?;
+		let caller_code = pallet_evm::Pallet::<Runtime>::account_codes(handle.context().caller);
+		// Check that caller is not a smart contract s.t. no code is inserted into
+		// pallet_evm::AccountCodes except if the caller is another precompile i.e. CallPermit
+		if !(caller_code.is_empty() || &caller_code == &[0x60, 0x00, 0x60, 0x00, 0xfd]) {
+			return Err(revert("Batch not callable by smart contracts"));
+		}
+
 		let selector = handle.read_selector()?;
 
 		handle.check_function_modifier(match selector {

diff --git a/precompiles/proxy/src/tests.rs b/precompiles/proxy/src/tests.rs
@@ -29,7 +29,7 @@ use pallet_proxy::{
 use precompile_utils::{
 	assert_event_emitted, assert_event_not_emitted, prelude::*, solidity, testing::*,
 };
-use sp_core::H160;
+use sp_core::{H160, U256};
 use std::str::from_utf8;
 
 #[test]
@@ -474,8 +474,6 @@ fn test_solidity_interface_has_all_function_selectors_documented_and_implemented
 	}
 }
 
-use sp_core::U256;
-
 #[test]
 fn test_nested_evm_bypass_proxy_should_allow_elevating_proxy_type() {
 	ExtBuilder::default()
@@ -527,3 +525,54 @@ fn test_nested_evm_bypass_proxy_should_allow_elevating_proxy_type() {
 			}));
 		})
 }
+
+#[test]
+fn fails_if_called_by_smart_contract() {
+	ExtBuilder::default()
+		.with_balances(vec![(Alice, 1000), (Bob, 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]);
+
+			let bob: H160 = Bob.into();
+			PrecompilesValue::get()
+				.prepare_test(
+					Alice,
+					Precompile,
+					EvmDataWriter::new_with_selector(Action::AddProxy)
+						.write::<Address>(bob.into())
+						.write::<u8>(ProxyType::Something as u8)
+						.write::<u32>(1)
+						.build(),
+				)
+				.execute_reverts(|output| output == b"Batch not callable by smart contracts");
+		})
+}
+
+#[test]
+fn succeed_if_called_by_precompile() {
+	ExtBuilder::default()
+		.with_balances(vec![(Alice, 1000), (Bob, 1000)])
+		.build()
+		.execute_with(|| {
+			// Set dummy code to Alice address as it if was a precompile.
+			pallet_evm::AccountCodes::<Runtime>::insert(
+				H160::from(Alice),
+				vec![0x60, 0x00, 0x60, 0x00, 0xfd],
+			);
+
+			let bob: H160 = Bob.into();
+			PrecompilesValue::get()
+				.prepare_test(
+					Alice,
+					Precompile,
+					EvmDataWriter::new_with_selector(Action::AddProxy)
+						.write::<Address>(bob.into())
+						.write::<u8>(ProxyType::Something as u8)
+						.write::<u32>(1)
+						.build(),
+				)
+				.execute_returns(vec![]);
+		})
+}
diff --git a/runtime/moonbase/src/precompiles.rs b/runtime/moonbase/src/precompiles.rs
@@ -30,7 +30,6 @@ use pallet_evm_precompile_call_permit::CallPermitPrecompile;
 use pallet_evm_precompile_collective::CollectivePrecompile;
 use pallet_evm_precompile_crowdloan_rewards::CrowdloanRewardsWrapper;
 use pallet_evm_precompile_democracy::DemocracyWrapper;
-use pallet_evm_precompile_dispatch::Dispatch;
 use pallet_evm_precompile_modexp::Modexp;
 use pallet_evm_precompile_parachain_staking::ParachainStakingWrapper;
 use pallet_evm_precompile_proxy::ProxyWrapper;
@@ -112,7 +111,7 @@ pub type MoonbasePrecompiles<R> = PrecompileSetBuilder<
 				PrecompileAt<AddressU64<9>, Blake2F, ForbidRecursion, AllowDelegateCall>,
 				// Non-Moonbeam specific nor Ethereum precompiles :
 				PrecompileAt<AddressU64<1024>, Sha3FIPS256>,
-				PrecompileAt<AddressU64<1025>, Dispatch<R>>,
+				// PrecompileAt<AddressU64<1025>, Dispatch<R>>,
 				PrecompileAt<AddressU64<1026>, ECRecoverPublicKey>,
 				// Moonbeam specific precompiles:
 				PrecompileAt<AddressU64<2048>, ParachainStakingWrapper<R>>,

diff --git a/runtime/moonbase/tests/integration_test.rs b/runtime/moonbase/tests/integration_test.rs
@@ -2837,8 +2837,8 @@ fn precompile_existence() {
 	ExtBuilder::default().build().execute_with(|| {
 		let precompiles = Precompiles::new();
 		let precompile_addresses: std::collections::BTreeSet<_> = vec![
-			1, 2, 3, 4, 5, 6, 7, 8, 9, 1024, 1025, 1026, 2048, 2049, 2050, 2051, 2052, 2053, 2054,
-			2055, 2056, 2057, 2058, 2059, 2060, 2061, 2062, 2063, 2064,
+			1, 2, 3, 4, 5, 6, 7, 8, 9, 1024, 1026, 2048, 2049, 2050, 2051, 2052, 2053, 2054, 2055,
+			2056, 2057, 2058, 2059, 2060, 2061, 2062, 2063, 2064,
 		]
 		.into_iter()
 		.map(H160::from_low_u64_be)

diff --git a/runtime/moonbeam/src/precompiles.rs b/runtime/moonbeam/src/precompiles.rs
@@ -30,7 +30,6 @@ use pallet_evm_precompile_call_permit::CallPermitPrecompile;
 use pallet_evm_precompile_collective::CollectivePrecompile;
 use pallet_evm_precompile_crowdloan_rewards::CrowdloanRewardsWrapper;
 use pallet_evm_precompile_democracy::DemocracyWrapper;
-use pallet_evm_precompile_dispatch::Dispatch;
 use pallet_evm_precompile_modexp::Modexp;
 use pallet_evm_precompile_parachain_staking::ParachainStakingWrapper;
 use pallet_evm_precompile_relay_encoder::RelayEncoderWrapper;
@@ -108,7 +107,7 @@ pub type MoonbeamPrecompiles<R> = PrecompileSetBuilder<
 				PrecompileAt<AddressU64<9>, Blake2F, ForbidRecursion, AllowDelegateCall>,
 				// Non-Moonbeam specific nor Ethereum precompiles :
 				PrecompileAt<AddressU64<1024>, Sha3FIPS256>,
-				PrecompileAt<AddressU64<1025>, Dispatch<R>>,
+				// PrecompileAt<AddressU64<1025>, Dispatch<R>>,
 				PrecompileAt<AddressU64<1026>, ECRecoverPublicKey>,
 				// Moonbeam specific precompiles:
 				PrecompileAt<AddressU64<2048>, ParachainStakingWrapper<R>>,

diff --git a/runtime/moonbeam/tests/integration_test.rs b/runtime/moonbeam/tests/integration_test.rs
@@ -2766,8 +2766,8 @@ fn precompile_existence() {
 	ExtBuilder::default().build().execute_with(|| {
 		let precompiles = Precompiles::new();
 		let precompile_addresses: std::collections::BTreeSet<_> = vec![
-			1, 2, 3, 4, 5, 6, 7, 8, 9, 1024, 1025, 1026, 2048, 2049, 2050, 2051, 2052, 2053, 2054,
-			2055, 2056, 2058, 2060, 2062, 2063, 2064,
+			1, 2, 3, 4, 5, 6, 7, 8, 9, 1024, 1026, 2048, 2049, 2050, 2051, 2052, 2053, 2054, 2055,
+			2056, 2058, 2060, 2062, 2063, 2064,
 		]
 		.into_iter()
 		.map(H160::from_low_u64_be)

diff --git a/runtime/moonriver/src/precompiles.rs b/runtime/moonriver/src/precompiles.rs
@@ -30,7 +30,6 @@ use pallet_evm_precompile_call_permit::CallPermitPrecompile;
 use pallet_evm_precompile_collective::CollectivePrecompile;
 use pallet_evm_precompile_crowdloan_rewards::CrowdloanRewardsWrapper;
 use pallet_evm_precompile_democracy::DemocracyWrapper;
-use pallet_evm_precompile_dispatch::Dispatch;
 use pallet_evm_precompile_modexp::Modexp;
 use pallet_evm_precompile_parachain_staking::ParachainStakingWrapper;
 use pallet_evm_precompile_relay_encoder::RelayEncoderWrapper;
@@ -108,7 +107,7 @@ pub type MoonriverPrecompiles<R> = PrecompileSetBuilder<
 				PrecompileAt<AddressU64<9>, Blake2F, ForbidRecursion, AllowDelegateCall>,
 				// Non-Moonbeam specific nor Ethereum precompiles :
 				PrecompileAt<AddressU64<1024>, Sha3FIPS256>,
-				PrecompileAt<AddressU64<1025>, Dispatch<R>>,
+				// PrecompileAt<AddressU64<1025>, Dispatch<R>>,
 				PrecompileAt<AddressU64<1026>, ECRecoverPublicKey>,
 				// Moonbeam specific precompiles:
 				PrecompileAt<AddressU64<2048>, ParachainStakingWrapper<R>>,

diff --git a/runtime/moonriver/tests/integration_test.rs b/runtime/moonriver/tests/integration_test.rs
@@ -2685,8 +2685,8 @@ fn precompile_existence() {
 	ExtBuilder::default().build().execute_with(|| {
 		let precompiles = Precompiles::new();
 		let precompile_addresses: std::collections::BTreeSet<_> = vec![
-			1, 2, 3, 4, 5, 6, 7, 8, 9, 1024, 1025, 1026, 2048, 2049, 2050, 2051, 2052, 2053, 2054,
-			2055, 2056, 2058, 2060, 2062, 2063, 2064,
+			1, 2, 3, 4, 5, 6, 7, 8, 9, 1024, 1026, 2048, 2049, 2050, 2051, 2052, 2053, 2054, 2055,
+			2056, 2058, 2060, 2062, 2063, 2064,
 		]
 		.into_iter()
 		.map(H160::from_low_u64_be)

diff --git a/tests/tests/test-contract/test-contract-delegate-call.ts b/tests/tests/test-contract/test-contract-delegate-call.ts
@@ -50,8 +50,8 @@ describeDevMoonbeam("DELEGATECALL for precompiles", (context) => {
   let proxyInterface: ethers.utils.Interface;
 
   const PRECOMPILE_PREFIXES = [
-    1, 2, 3, 4, 5, 6, 7, 8, 9, 1024, 1025, 1026, 2048, 2049, 2050, 2051, 2052, 2053, 2054, 2055,
-    2056, 2057, 2058, 2059,
+    1, 2, 3, 4, 5, 6, 7, 8, 9, 1024, 1026, 2048, 2049, 2050, 2051, 2052, 2053, 2054, 2055, 2056,
+    2057, 2058, 2059,
   ];
 
   // Ethereum precompile 1-9 are pure and allowed to be called through DELEGATECALL

diff --git a/tests/tests/test-proxy/test-proxy-leader-demo.ts b/tests/tests/test-proxy/test-proxy-leader-demo.ts
@@ -183,7 +183,8 @@ describeDevMoonbeam("Proxy Leader Demo - End Voting", (context) => {
     expectEVMResult(result.events, "Succeed");
   });
 
-  it("should be able to stop", async function () {
+  // TODO: rework this test, contract cannot call proxy precompile
+  it.skip("should be able to stop", async function () {
     expect(await leaderContract.methods.isVoting().call()).to.be.true;
 
     const { result } = await context.createBlock(
@@ -199,10 +200,12 @@ describeDevMoonbeam("Proxy Leader Demo - End Voting", (context) => {
   });
 });
 
+// TODO: rework this test, contract cannot call proxy precompile
 describeDevMoonbeam("Proxy Leader Demo - Winners", (context) => {
   let leaderContract: Contract;
 
   before("setup contract and voting results", async function () {
+    this.skip();
     leaderContract = await setupPoolWithParticipants(context);
 
     // start voting
@@ -266,15 +269,15 @@ describeDevMoonbeam("Proxy Leader Demo - Winners", (context) => {
     });
   });
 
-  it("should proxy charleth as governor", async function () {
+  it.skip("should proxy charleth as governor", async function () {
     expect(await leaderContract.methods.governor().call()).to.equal(CHARLETH_ADDRESS);
   });
 
-  it("should proxy dorothy as staker", async function () {
+  it.skip("should proxy dorothy as staker", async function () {
     expect(await leaderContract.methods.staker().call()).to.equal(DOROTHY_ADDRESS);
   });
 
-  it("should setup proxy types for contract address", async function () {
+  it.skip("should setup proxy types for contract address", async function () {
     const proxies = await context.polkadotApi.query.proxy.proxies(leaderContract.options.address);
     expect(proxies[0].toJSON()).to.deep.equal([
       {
@@ -290,7 +293,7 @@ describeDevMoonbeam("Proxy Leader Demo - Winners", (context) => {
     ]);
   });
 
-  it("should not allow baltathar to stake via proxy", async function () {
+  it.skip("should not allow baltathar to stake via proxy", async function () {
     const { result } = await context.createBlock(
       context.polkadotApi.tx.proxy
         .proxy(
@@ -304,7 +307,7 @@ describeDevMoonbeam("Proxy Leader Demo - Winners", (context) => {
     expect(result.error.name).to.equal("NotProxy");
   });
 
-  it("should allow dorothy to stake via proxy", async function () {
+  it.skip("should allow dorothy to stake via proxy", async function () {
     const { result } = await context.createBlock(
       context.polkadotApi.tx.proxy
         .proxy(
@@ -334,7 +337,7 @@ describeDevMoonbeam("Proxy Leader Demo - Winners", (context) => {
     ]);
   });
 
-  it("should not allow dorothy to vote via proxy", async function () {
+  it.skip("should not allow dorothy to vote via proxy", async function () {
     const { result } = await context.createBlock(
       context.polkadotApi.tx.proxy
         .proxy(
@@ -350,7 +353,7 @@ describeDevMoonbeam("Proxy Leader Demo - Winners", (context) => {
     expect(result.error.name).to.equal("NotProxy");
   });
 
-  it("should allow charleth to vote via proxy", async function () {
+  it.skip("should allow charleth to vote via proxy", async function () {
     const { result } = await context.createBlock(
       context.polkadotApi.tx.proxy
         .proxy(