substrate pallet-sudo: add `CheckOnlySudo` signed extension

pallet-sudo: add `CheckOnlySudo` signed extension

Open koushiro opened this issue 1 year ago • 3 comments

`CheckOnlySudo` SignedExtension

The CheckOnlySudo is a signed extension for checking the signer of the transaction, and ensure the signed transactions are only valid if they are signed by root.

I think this signed extension is useful in some scenarios, such as in the initial stage of the substrate-based network, the runtime can only accept calls signed by sudo account.

Oct 14 '22 08:10 koushiro

I think this signed extension is useful in some scenarios, such as in the initial stage of the substrate-based network, the runtime can only accept calls signed by sudo account.

Can you be more specific? Why should this be required? If someone tries to send a transaction that isn't allowed yet, they will need to pay fees for this. So, they should be discouraged from sending any transactions at all ;)

Oct 14 '22 19:10 bkchr

Can you be more specific? Why should this be required? If someone tries to send a transaction that isn't allowed yet, they will need to pay fees for this. So, they should be discouraged from sending any transactions at all ;)

For my project, it didn't have any tokens in the early running stage, I needed the sudo account to set the balances later.

Oct 17 '22 03:10 koushiro

I found a similar implementation in culumus, maybe it can be replaced with CheckOnlySudo in a future PR.

Oct 17 '22 08:10 koushiro

@bkchr What do you think? Is this acceptable?

Oct 20 '22 03:10 koushiro

Is there anyone else who can review this PR?

Oct 25 '22 07:10 koushiro

This PR has been idle for a week, is there any progress?

Nov 02 '22 02:11 koushiro

I do not really like the idea of moving the business/validation logic from the runtime to the higher level (extension). This can be used as an alternative to ensure_sudo on the runtime level, which makes easier to make a mistake. For example it wont be covered by unit tests.

Nov 02 '22 13:11 muharem

I do not really like the idea of moving the business/validation logic from the runtime to the higher level (extension). This can be used as an alternative to ensure_sudo on the runtime level, which makes easier to make a mistake. For example it wont be covered by unit tests.

That is not what this is doing. The idea/problem here as @koushiro explained is in the initial phase of a chain without any tokens. The problem without this extension would be that people could spam the chain with transactions. With this extension we could reject these transactions before they enter the tx pool.

Nov 02 '22 14:11 bkchr

@koushiro would be nice to have some docs, telling also when this supposed to be used

Nov 02 '22 14:11 muharem

bot merge

Nov 03 '22 22:11 bkchr

Waiting for commit status.