Add contract creation allow lists to EVM domains #3350

teor2345 · 2025-01-15T03:07:28Z

How to review this PR

This PR is the second in a series of PRs which add "private EVM" functionality to subspace. The accompanying spec PR is subspace/protocol-specs#55 and user documentation is TODO.

The first PR in the series is #3359, it moves some test code around, and makes minor fixes.

The third PR in the series is #3360, it applies these changes and a storage migration to the Taurus runtime. (This storage isn't currently deployed on mainnet, so a migration isn't needed there.)

What it does

This PR adds a pallet which filters ethereum contract creation using an allow list. The allow list can be updated by the domain sudo account.

By default, all accounts can create contracts, to maintain compatibility with existing EVM domains. But the chainspec can be configured with an initial allow list of accounts. For our "private" EVM, that will be the domain sudo.

Close #3344.

This PR doesn't do a dynamic check for the domain sudo inside the pallet or runtime. We decided that isn't needed, because we can add the sudo account in the chainspec, or they can add themselves to the list using the (sudo) call provided by the pallet.

Part of #3353.

Other Changes

This PR makes it easier to add further code which:

has domain runtime type specific configurations (for example: EVM or Auto ID)
does recursion through utility pallet batches or call wrappers
extracts pallet calls from the RuntimeCall type (using MaybeInto*Call traits)
tests using ethereum transactions or EVM runtimes, via the EvmOnchainStateApi runtime API, and contract address calculation function

It also simplifies some existing code which was doing those things.

Bug Fixes

Fixes some transfer prevention extension code, which recursed through pallet-utility calls to use an iterator loop instead. The calls are checked in the same order, but references to the "Call stack" are stored on the heap instead. This avoids stack overflow for deeply nested calls.

Fixes some bugs in test-only code which made it harder to use.

TODOs

Add contract creation tests (see test matrix in comments)
Split the PR into:
- Test refactors & minor bug fixes
- Private EVM production, test code (Add contract creation allow lists to EVM domains #3350), and documentation (Document how to use Private EVM in the specs repo #3358)
- Taurus version bump and migration

Code contributor checklist:

I have read, understood and followed contributing guide

vedhavyas

Make sense overall. Left questions on penalizing malicious operator including these txns in bundle

domains/runtime/evm/src/lib.rs

domains/pallets/evm_nonce_tracker/src/lib.rs

domains/runtime/evm/src/lib.rs

crates/pallet-domains/src/domain_registry.rs

vedhavyas · 2025-01-17T08:29:41Z

crates/pallet-domains/src/tests.rs

@@ -633,6 +634,7 @@ fn test_bundle_format_verification() {
            bundle_slot_probability: (1, 1),
            operator_allow_list: OperatorAllowList::Anyone,
            initial_balances: Default::default(),
+            initial_evm_contract_creation_allow_list: None,
        };


maybe add another test with some address included ?

I'd like to do this after the API is stable, so I don't have to rework the tests multiple times.

Test matrix:

Instantiate domain with allow list, using all allow list combinations below

Instantiate domain without allow list (existing tests)

Update allow list using call, with all allow list combinations below

Migration v2 to v3 storage format (Taurus only)

Test contract allow/deny for all combinations of:

Anyone vs Empty vs 1 Account vs Multiple Accounts (First vs not First)

Signer vs Not Signer vs Unsigned

Contract vs Non-Contract vs Deeply nested (contract vs non-contract)

Ethereum (self-contained) vs EVM (signed extension) calls

~~legacy vs vs eip1559 vs eip2930~~

domains/runtime/evm/src/lib.rs

domains/pallets/evm_nonce_tracker/src/lib.rs

domains/runtime/evm/src/lib.rs

crates/pallet-domains/src/runtime_registry.rs

crates/pallet-domains/src/domain_registry.rs

domains/runtime/evm/src/lib.rs

vedhavyas · 2025-01-20T08:12:24Z

crates/pallet-domains/src/lib.rs

@@ -161,7 +162,7 @@ pub type BlockTreeNodeFor<T> = crate::block_tree::BlockTreeNode<
 >;

 /// The current storage version.
-const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);


Is this due to Taurus being on storage version 2 already ?

Yes, in this branch: https://github.com/autonomys/subspace/tree/taurus-runtime-upgrade

Here's the exact change:
https://github.com/autonomys/subspace/pull/3342/files#diff-9f80450addc12cd3b681275c6acb663003edc27c2d8eeeb1f26147df9f8e5bbeR164

subspace/crates/pallet-domains/src/lib.rs

Line 164 in a19ba3d

const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);

if that is the case, we can manually bump right before we do taurus upgrade no ?

crates/subspace-runtime/src/lib.rs

teor2345

I've finished about half the tests, see the checklist in the comments

teor2345 · 2025-01-20T08:23:07Z

crates/pallet-domains/src/lib.rs

@@ -161,7 +162,7 @@ pub type BlockTreeNodeFor<T> = crate::block_tree::BlockTreeNode<
 >;

 /// The current storage version.
-const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);


Yes, in this branch: https://github.com/autonomys/subspace/tree/taurus-runtime-upgrade

Here's the exact change:
https://github.com/autonomys/subspace/pull/3342/files#diff-9f80450addc12cd3b681275c6acb663003edc27c2d8eeeb1f26147df9f8e5bbeR164

subspace/crates/pallet-domains/src/lib.rs

Line 164 in a19ba3d

const STORAGE_VERSION: StorageVersion = StorageVersion::new(2);

crates/subspace-runtime/src/lib.rs

teor2345 · 2025-01-21T00:53:10Z

crates/pallet-domains/src/tests.rs

@@ -633,6 +634,7 @@ fn test_bundle_format_verification() {
            bundle_slot_probability: (1, 1),
            operator_allow_list: OperatorAllowList::Anyone,
            initial_balances: Default::default(),
+            initial_evm_contract_creation_allow_list: None,
        };


Test matrix:

Instantiate domain with allow list, using all allow list combinations below

Instantiate domain without allow list (existing tests)

Update allow list using call, with all allow list combinations below

Migration v2 to v3 storage format (Taurus only)

Test contract allow/deny for all combinations of:

Anyone vs Empty vs 1 Account vs Multiple Accounts (First vs not First)

Signer vs Not Signer vs Unsigned

Contract vs Non-Contract vs Deeply nested (contract vs non-contract)

Ethereum (self-contained) vs EVM (signed extension) calls

~~legacy vs vs eip1559 vs eip2930~~

vedhavyas

Left some questions.

there are a lot of changes between the commits that is very confusing. Not a blocker for this PR

vedhavyas · 2025-01-31T08:16:07Z

domains/pallets/evm_nonce_tracker/src/lib.rs

+            origin: OriginFor<T>,
+            contract_creation_allowed_by: PermissionedActionAllowedBy<T::AccountId>,
+        ) -> DispatchResult {
+            ensure_root(origin)?;


This is fine to expect as sudo but ideally a domain owner should be able to update this allowlist instead of sudo so that Consensus sudo is not always required and is also a longer process.

Currently domain owner can update the XDM allow list that is initiated from the Consensus chain so I see no reason why the same domain owner should not be able to update this list as to who can create contracts as well.
Thoughts @dariolina

Apologies for not bringing this earlier.

vedhavyas · 2025-01-31T08:17:42Z

domains/runtime/evm/src/lib.rs

+/// Rejects contracts that can't be created under the current allow list.
+/// Returns false if the call is a contract call, and the account is *not* allowed to call it.
+/// Otherwise, returns true.
+pub fn is_create_contract_allowed(call: &RuntimeCall, signer: &AccountId) -> bool {


this fn can be simplied but no string opinion if it increases readability

vedhavyas · 2025-01-31T08:21:28Z

domains/pallets/evm_nonce_tracker/src/lib.rs

        ContractCreationAllowedBy::<T>::get()
            .map(|allowed_by| allowed_by.is_allowed(signer))
-            .unwrap_or_default()
+            .unwrap_or(true)


shouldn't this be false instead ?

vedhavyas · 2025-01-31T08:24:23Z

domains/pallets/evm_nonce_tracker/src/lib.rs

+        // Unlike domain instantiation, no storage value means "anyone can create contracts".
+        ContractCreationAllowedBy::<T>::get()
+            .map(|allowed_by| allowed_by.is_anyone_allowed())
+            .unwrap_or(true)


same here. Shouldn't this be false ?

vedhavyas · 2025-01-31T08:30:43Z

domains/client/domain-operator/src/tests.rs

@@ -207,6 +207,774 @@ pub fn generate_evm_account_list(
    }
 }

+#[tokio::test(flavor = "multi_thread")]
+async fn test_evm_domain_create_contracts_with_allow_list() {


this is a very big test. Can we split it to test multiple cases in different tests ?

vedhavyas · 2025-01-31T08:31:23Z

domains/client/domain-operator/src/tests.rs

+    produce_blocks!(ferdie, alice, 3).await.unwrap();
+
+    // add domain to consensus chain allowlist
+    ferdie


Why do we need to open a channel between consensus and domain for this tests ?

vedhavyas · 2025-01-31T08:33:21Z

crates/pallet-domains/src/lib.rs

@@ -161,7 +162,7 @@ pub type BlockTreeNodeFor<T> = crate::block_tree::BlockTreeNode<
 >;

 /// The current storage version.
-const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);


if that is the case, we can manually bump right before we do taurus upgrade no ?

vedhavyas · 2025-01-31T08:34:04Z

crates/subspace-runtime/src/lib.rs

@@ -121,7 +121,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
    spec_name: Cow::Borrowed("subspace"),
    impl_name: Cow::Borrowed("subspace"),
    authoring_version: 0,
-    spec_version: 2,
+    // TODO: before deploying the next runtime version to mainnet or taurus, bump this version
+    spec_version: 11,


even this one, let not bump it.
when wwe create a taurus upgrade branch, we can bump it then.
TODO might be noise here

teor2345 added enhancement New feature or request execution Subspace execution labels Jan 15, 2025

teor2345 self-assigned this Jan 15, 2025

teor2345 requested review from NingLin-P, vedhavyas and nazar-pc as code owners January 15, 2025 03:07

This comment was marked as resolved.

Sign in to view

teor2345 force-pushed the private-evm branch from 377d0da to 6a47075 Compare January 17, 2025 02:28

This comment was marked as resolved.

Sign in to view

teor2345 requested a review from NingLin-P January 17, 2025 02:32

teor2345 force-pushed the private-evm branch from 6a47075 to eabc2ac Compare January 17, 2025 05:17

This comment was marked as resolved.

Sign in to view

teor2345 mentioned this pull request Jan 17, 2025

Unify copies of GenesisDomainParams across different crates #3352

Open

This comment was marked as resolved.

Sign in to view

vedhavyas reviewed Jan 17, 2025

View reviewed changes

NingLin-P reviewed Jan 17, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

teor2345 force-pushed the private-evm branch from eabc2ac to 7497b5c Compare January 20, 2025 07:18

vedhavyas reviewed Jan 20, 2025

View reviewed changes

teor2345 force-pushed the private-evm branch from 7497b5c to b29c6f3 Compare January 21, 2025 02:51

teor2345 commented Jan 21, 2025

View reviewed changes

teor2345 force-pushed the private-evm branch from b29c6f3 to 4a7f2bf Compare January 21, 2025 03:03

This comment was marked as resolved.

Sign in to view

teor2345 force-pushed the private-evm branch from a94e6b3 to c5c1880 Compare January 23, 2025 02:31

teor2345 added the bug Something isn't working label Jan 24, 2025

teor2345 force-pushed the private-evm branch from 7cee104 to 0a796cc Compare January 27, 2025 05:51

teor2345 mentioned this pull request Jan 28, 2025

Fix custom transaction error code clash, and make ethereum tests easier to write #3359

Merged

1 task

teor2345 force-pushed the private-evm branch from 7a8d562 to 09ba296 Compare January 28, 2025 02:44

teor2345 changed the base branch from main to private-evm-prep January 28, 2025 02:45

This comment was marked as outdated.

Sign in to view

teor2345 force-pushed the private-evm branch 2 times, most recently from b5c3e27 to d50d942 Compare January 28, 2025 03:23

This was referenced Jan 28, 2025

Taurus storage migration for "Private EVM" #3360

Draft

Add Private EVM spec subspace/protocol-specs#55

Merged

teor2345 added 13 commits January 29, 2025 07:51

Add a contract creation filter to pallet-evm-nonce-tracker

5e9925a

Add a contract creation filter to the EVM domain

d187d5c

Allow contract creation to be set per domain instance

f53f640

Add a pallet-utility Call iterator and use it in existing code

d8da496

Add error code for contract creation not allowed

f3edd09

Reject unsigned contract calls unless anyone is allowed

a10e930

Rename pallet_evm_nonce_tracker to pallet-evm-tracker

3fac8e3

Abstract signed extension checks via traits

8c27104

Move contract create hooks to pallet-evm-tracker

bd03483

Add tests for contract allow lists

1b0889c

Add tests for contract create filtering

3f15d2d

Avoid a test panic that's easy to trigger

2caf9a4

Mainnet: Bump storage version, and sync spec version with Taurus

c013f78

teor2345 force-pushed the private-evm branch from d50d942 to c013f78 Compare January 28, 2025 21:52

Base automatically changed from private-evm-prep to main January 30, 2025 09:50

teor2345 marked this pull request as ready for review January 30, 2025 20:11

teor2345 enabled auto-merge January 30, 2025 20:12

teor2345 requested review from NingLin-P, vedhavyas and dariolina January 30, 2025 20:12

teor2345 disabled auto-merge January 30, 2025 20:14

vedhavyas reviewed Jan 31, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add contract creation allow lists to EVM domains #3350

Add contract creation allow lists to EVM domains #3350

teor2345 commented Jan 15, 2025 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

vedhavyas left a comment

vedhavyas Jan 17, 2025

teor2345 Jan 20, 2025

teor2345 Jan 21, 2025 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

vedhavyas Jan 20, 2025

teor2345 Jan 20, 2025

vedhavyas Jan 31, 2025

teor2345 left a comment

teor2345 Jan 20, 2025

teor2345 Jan 21, 2025 •

edited

Loading

This comment was marked as resolved.

This comment was marked as outdated.

vedhavyas left a comment

vedhavyas Jan 31, 2025

vedhavyas Jan 31, 2025

vedhavyas Jan 31, 2025

vedhavyas Jan 31, 2025

vedhavyas Jan 31, 2025

vedhavyas Jan 31, 2025

vedhavyas Jan 31, 2025

vedhavyas Jan 31, 2025

Add contract creation allow lists to EVM domains #3350

Are you sure you want to change the base?

Add contract creation allow lists to EVM domains #3350

Conversation

teor2345 commented Jan 15, 2025 • edited Loading

How to review this PR

What it does

Other Changes

Bug Fixes

TODOs

Code contributor checklist:

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

vedhavyas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teor2345 Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teor2345 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teor2345 Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as outdated.

vedhavyas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teor2345 commented Jan 15, 2025 •

edited

Loading

teor2345 Jan 21, 2025 •

edited

Loading

teor2345 Jan 21, 2025 •

edited

Loading