Extend SignedExtension to include Origin specification

[tomusdrw]

Currently the Origin has a special meaning in the code and signed and unsigned transactions follow different paths in the validation and execution phase.

We could attempt to unify them and encapsulate all the logic in Extra. So that SignedExtensions would be allowed to return additional Origin information that would then be passed to dispatch.

That would make ValidateUnsigned and all _unsigned functions of SignedExtension obsolete, since both signed and unsigned extrinsics would follow the same path.

Possible simplifications in:

1. im-online - we introduce another SignedExtra which would check the signature inside the payload (not the signature of the transaction, since it's an unsigned transaction) and provide a special Origin containing the AuthorityId that signed that payload.
2. claims - details below
3. Any module using ValidateUnsigned trait or SignedExtension::validate_unsigned.
4. Clean and elegant srml::executive validation and dispatch.

Original convo (from Gav):

we could, i guess, extend SignedExtension API to include Origin-specification.
though it's a fairly fundamental change and i'm not sure how aggregation could work
but if we did it, then claims module would introduce a new Origin::EthereumClaimant(EthereumAddress)
and return that from its SignedExtension's validate (or pre_dispatch)
but there's a lot of interplay that would need to be figured out.
signed and unsigned are currently handled very differently.

[bkchr]

I don't think that we should merge validate_unsigned and validate. The separation actually gives us the ability to deny any unsigned transaction for that we don't have a signed extension registered. This is important, as the programmer can easily forget to include a SignedExtension.

[tomusdrw]

I imagine that we would disallow all transactions by default and SignedExtensions validate function would return the origin only if it "whitelists" a particular transaction. So that means that the most basic SignedExtension would just return a Origin::Signed(x) (after checking the signature) and that would get aggregated with other extensions. Unsigned would only be pass through if one of the extensions returns Origin::None (which means it does authorize the call).

I still think we would need one Extension that is able to just delegate validation to something similar to ValidateUnsigned. Having every pallet expose a separate extension that needs to be added to the list of Extra seems like an overkill.

[bkchr]

Hmm, currently the signature checking is part of the extrinsics itself. For a moment let's say we do it the way you propose. What do we do when two different SignedExtension's return two different Origin's? Which Origin is the correct one?

As described in the issue you linked today, my idea was to stick to the current model of validate and validate_unsigned. validate would still return the transaction validity, but validate_unsigned would return something different:

enum UnsignedResult {
    Valid(TransactionValidity),
    Invalid(TransactionValidityError),
    None
}

By default validate_unsigned returns UnsignedResult::None. If all SignedExtension's return None, the transaction is rejected. If at least one returns Valid and no returns Invalid, the transaction is accepted. If we do it that way, we will not merge both methods, however we will have a very easy system to validate unsigned transaction without a footgun (automatically accepting unsigned transactions). I also think that we could provide some default implementation of a SignedExtension that you could easily integrate in your pallet to validate an unsigned transaction.

[tomusdrw]

Hmm, currently the signature checking is part of the extrinsics itself. For a moment let's say we do it the way you propose. What do we do when two different SignedExtension's return two different Origin's? Which Origin is the correct one?

Yeah, that's the hard part about aggregation mentioned earlier. I think the best would be to implement it in a way that BOTH origins are valid (see Gav comment on claims, where a signed extension can have it's own Origin (eth signature) that augments the overarching one).

As described in the issue you linked today, my idea was to stick to the current model of validate and validate_unsigned. validate would still return the transaction validity, but validate_unsigned would return something different:

I like the idea, looks really clean. I was also thinking about letting the validate functions compose / merge the result on their own:

enum ValidityResult {
    Valid(TransactionValidity),
    Invalid(TransactionValidityError),
    None
}

type Origin: Origin + Into<RuntimeOrigin>;

fn validate_signed(&self) -> ValidityResult { ValidityResult::None }
fn validate_unsigned(&self) -> ValidityResult { ValidityResult::None }

// a working name, I don't have an idea yet.
fn validate(&self, origin: Self::Origin, previous: ValidityResult) -> (Self::Origin, ValidityResult) {
   // a default implementation would just work as it works currently (i.e. merging validity results).
   (origin, if origin.is_signed()  { self.validate_signed() } else { self.validate_unsigned() }.merge(previous))
}

A custom implementation would be able to inspect Origin returned by the previous extension and do something about it. For instance we can have an UnsignedTransaction that contains signature internally, and that get's converted to a regular signed dispatch, but just doesn't pay the fees.

But it's not really well thought out, so beware of dragons.

[arrudagates]

A custom implementation would be able to inspect Origin returned by the previous extension and do something about it. For instance we can have an UnsignedTransaction that contains signature internally, and that get's converted to a regular signed dispatch, but just doesn't pay the fees.

This issue has been stale since early 2020, has there been any progress on this? Or maybe a better way to do what I quoted?