Deploy origin for pallet-contracts

[Szegoo]

In some cases, it could be useful to have permissioned contract deployment on a chain.

Probably the easiest way to enable this would be to add a new type to the config, e.g.:

type DeployOrigin: EnsureOrigin<Self::RuntimeOrigin>;

[bkchr]

@Szegoo I assume you would work on this?

CC @athei

[Szegoo]

@Szegoo I assume you would work on this?

Yes, will open a PR soon. Just wanted to confirm this is sensible first.

[athei]

Yes this makes sense to me.

[Szegoo]

Actually, this is not as straightforward as I thought. The contracts api assumes that the origin is an account, which might not be the case if we want to do something as I suggested in the issue description.

Maybe have something like this instead:

type DeployOrigin: Contains<T::AccountId>;

And then for example if we wanted to allow only members from a specific collective to deploy a contract we could have the following in the runtime config:

type DeployOrigin = TechnicalCommitteeMembershipInstance;

This is possible since the pallet_membership implements the Contains trait.

@bkchr @athei thoughts?

[bkchr]

The contracts api assumes that the origin is an account, which might not be the case if we want to do something as I suggested in the issue description.

Can you explain what you mean?

I don't really see why we need to use Contains here and not EnsureOrigin.

[athei]

Yes, a contract assumes that any other contract has an AccountId. But you can can construct a Origin::Signed from that AccountId and test it against your initially suggested

type DeployOrigin: EnsureOrigin<Self::RuntimeOrigin>;

[xlc]

You could add a signed extension to do whatever origin validation required. No need to modify the pallet.

[athei]

That would only work if you care about contracts deployed by a signed account but not by other contracts. Meaning you can't stop contracts from instantiating other contracts with that approach.

[Szegoo]

I don't really see why we need to use Contains here and not EnsureOrigin.

For each contract we store CodeInfo, which contains an owner property of type AccountId. If we used EnsureOrigin the origin might not be associated with an AccountId. E.g. if we were to set the DeployOrigin to root what account would be the owner of the deployed code?

[athei]

E.g. if we were to set the DeployOrigin to root what account would be the owner of the deployed code?

Contracts do not have owners. At least not as a concept of pallet-contracts. What you are referring to is probably which Origin does a contract constitute. As mentioned above it would be Origin::Signed(contract_account_id). You could match that to your EnsureOrigin filter. That test would rightfully fail of you set the DeployOrigin to only allow Origin::Root. But the filter could also for certain Origin::Signed to deploy with no problem. I really don't see the problem here.

You need to start getting more specific if you need further help.

[Szegoo]

As mentioned above it would be Origin::Signed(contract_account_id)

I get that. What I was referring to is this.

I am definitely not deep into the contracts pallet, but AFAIU the deployed code has an owner: link. What I was asking was who would be the owner of the uploaded code if the origin is not signed(e.g. root)?

[bkchr]

E.g. if we were to set the DeployOrigin to root what account would be the owner of the deployed code?

Then you can use EnsureRootWithSuccess and pass some accountid that represents the root account. We already have done this somewhere, but I can right now not find it.

[athei]

I am definitely not deep into the contracts pallet, but AFAIU the deployed code has an owner: link. What I was asking was who would be the owner of the uploaded code if the origin is not signed(e.g. root)?

Okay yes I get it now. I thought you were talking about instantiating a contract. But you mean uploading new code. Which of the two you want to make permissioned? Only the uploading of new code?

[Szegoo]

Then you can use EnsureRootWithSuccess and pass some accountid that represents the root account. We already have done this somewhere, but I can right now not find it.

Yeah, this seems exactly like what would be needed. Since the contracts pallet assumes in code deployment and instantiation that an account id associated with the origin, by making this slight adjustment to my initial suggestion we can make it work:

type DeployOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Self::AccountId>;

Which of the two you want to make permissioned? Only the uploading of new code?

I think restricting both would make the most sense.

[athei]

type DeployOrigin: EnsureOrigin<Self::RuntimeOrigin, Success = Self::AccountId>;

Yes, but there is a shorthand for this as Basti mentioned: EnsureRootWithSuccess.

Which of the two you want to make permissioned? Only the uploading of new code?

I think restricting both would make the most sense.

I think two config knobs would make sense then. You might want to allow everyone to instantiate existing code but make the upload restricted.

[pgherveou]

Seeing this a bit late,
but why can't we just leverage frame_system::BaseCallFilter here?

Zeigeist is doing something like this

https://github.com/zeitgeistpm/zeitgeist/blob/22992bc631ec321eec906be6dfe07b6bc3d2dfc5/runtime/zeitgeist/src/lib.rs?plain=1#L148-L157

[athei]

When discussing the issue here I thought the goal would be to also prevent contract from instantiating. Without this requirement the call filter would suffice AFAIK.