Follow-up on #1352

[serban300]

An alternative solution to using macros like declare_bridge_reject_obsolete_grandpa_header().

The current PR implements this approach only for the grandpa headers validation. If the feedback is positive, we can extend this to parachains and messages.

It looks like we don't have enough slots in SignedPayload to have a SignedExtension implementation for each validator, so I created an aggregated validator that implements SignedExtension.

Starting from here we could variate the approach. For example we could have a macro that generates the structures that implement RejectObsoleteGrandpaHeader and the SignedExtension implementation that aggregates all validations. I think we could also use impl_for_tuples as suggested here.

Posting this PR to gather feedback on the general idea. I might need to add some comments and do some polishing.

[svyatonik]

So the main goal imo here is to avoid implementing anything in the runtime code (or at least minimize the amount of required code). That's because we'll have at least 5 runtimes (soon) with such extensions and it'll be hard to maintain it. If you'd find solution for that, it would be perfect :)

[serban300]

So the main goal imo here is to avoid implementing anything in the runtime code (or at least minimize the amount of required code). That's because we'll have at least 5 runtimes (soon) with such extensions and it'll be hard to maintain it. If you'd find solution for that, it would be perfect :)

I pushed a new version that minimizes the amount of code implemented in the runtime and also leads to using a single macro instead of 3. Sorry, I force-pushed, I got lost in all the changes.

I couldn't leverage impl_trait_for_tuples. We can implement FilterCall for tuples, but I don't know if it would help.

Update related to last checkbox - I was thinking that possibly using impl_for_tuples would help. So something like:

trait FilterCall<Call> {
  fn is_valid(call: &Call) -> Option<bool>;
}

// implement `FilterCall` for Pallet<T, I> and for tuples, so we could use something like `(WithWestendGrandpa, WithRialtoGrandpa)`

type SignedExtension = SignedExtension<(WithWestendGrandpa, WithRialtoGrandpa)>;

Could you expand on this please ? We can implement FilterCall for tuples, but I'm not sure how to get from FilterCall to SignedExtension.

[svyatonik]

I pushed a new version that minimizes the amount of code implemented in the runtime and also leads to using a single macro instead of 3. Sorry, I force-pushed, I got lost in all the changes.

That's fine :) Seems great, thanks a lot! I'll take another look later. Yeah - I have also failed to avoid using macro at all, but your version looks much better to me!

I couldn't leverage impl_trait_for_tuples. We can implement FilterCall for tuples, but I don't know if it would help.

There's frame_support::traits::Contains, similar to FilterCall :) I wonder if we can use it instead of our own trait?

Could you expand on this please ? We can implement FilterCall for tuples, but I'm not sure how to get from FilterCall to SignedExtension.

I was thinking about something like that:

// every pallet implements `Contains<Call>` or some similar trait:
impl<T, I> Contains<Call> for pallet_bridge_parachains::Pallet<T, I> {
 fn contains(call: &Call) -> bool {
  // do validation here 
}
}

// Cotnains is implemented for tuples, so given that we have impl Contains<Call> for our three pallets, we may then make a tuple e.g. `(pallet_bridge_parachains::Pallet<Runtime, WithRialtoParachainsInstance>, pallet_bridge_grandpa::Pallet<Runtime, WithRialtoGrandpaInstance>)` and it would also implement a Contains<Call>. So we can use something like that:

struct OurGreatExtension<C, F: Contains<Call>>;

impl SignedExtensions for OurGreatExtension<C, F: Contains<Call>> {
  fn do-verification-or-how-it-is-called(call: &Call) -> bool {
    if F::contains(call) {
     return transaction-is-outdated-error;
   }
  }
}

// and finally use something like that in the runtime:

type SignedExtension = (
...
OurGreatExtension<Call, (pallet_bridge_parachains::Pallet<Runtime, WithRialtoParachainsInstance>, pallet_bridge_grandpa::Pallet<Runtime, WithRialtoGrandpaInstance>)>
...
);

I have attempted that, but back then I have failed to climb down from runtime Call to pallet level call and deconstruct the call in the Contains<Call> implementation. But nvm - removing macro at all costs is not the goal as I said :)

[serban300]

There's frame_support::traits::Contains, similar to FilterCall :) I wonder if we can use it instead of our own trait?

The problem with Contains is that the check only returns a bool. We return different Errors right now: InvalidTransaction::Stale, InvalidTransaction::Call, etc. which we wouldn't be able to do with a bool.

I was thinking about something like that:

// every pallet implements `Contains<Call>` or some similar trait:
impl<T, I> Contains<Call> for pallet_bridge_parachains::Pallet<T, I> {
 fn contains(call: &Call) -> bool {
  // do validation here 
}
}

// Cotnains is implemented for tuples, so given that we have impl Contains<Call> for our three pallets, we may then make a tuple e.g. `(pallet_bridge_parachains::Pallet<Runtime, WithRialtoParachainsInstance>, pallet_bridge_grandpa::Pallet<Runtime, WithRialtoGrandpaInstance>)` and it would also implement a Contains<Call>. So we can use something like that:

struct OurGreatExtension<C, F: Contains<Call>>;

impl SignedExtensions for OurGreatExtension<C, F: Contains<Call>> {
  fn do-verification-or-how-it-is-called(call: &Call) -> bool {
    if F::contains(call) {
     return transaction-is-outdated-error;
   }
  }
}

// and finally use something like that in the runtime:

type SignedExtension = (
...
OurGreatExtension<Call, (pallet_bridge_parachains::Pallet<Runtime, WithRialtoParachainsInstance>, pallet_bridge_grandpa::Pallet<Runtime, WithRialtoGrandpaInstance>)>
...
);

Oh, this can really work. I can give it a try.

[serban300]

I was thinking about something like that:

// every pallet implements `Contains<Call>` or some similar trait:
impl<T, I> Contains<Call> for pallet_bridge_parachains::Pallet<T, I> {
 fn contains(call: &Call) -> bool {
  // do validation here 
}
}

// Cotnains is implemented for tuples, so given that we have impl Contains<Call> for our three pallets, we may then make a tuple e.g. `(pallet_bridge_parachains::Pallet<Runtime, WithRialtoParachainsInstance>, pallet_bridge_grandpa::Pallet<Runtime, WithRialtoGrandpaInstance>)` and it would also implement a Contains<Call>. So we can use something like that:

struct OurGreatExtension<C, F: Contains<Call>>;

impl SignedExtensions for OurGreatExtension<C, F: Contains<Call>> {
  fn do-verification-or-how-it-is-called(call: &Call) -> bool {
    if F::contains(call) {
     return transaction-is-outdated-error;
   }
  }
}

// and finally use something like that in the runtime:

type SignedExtension = (
...
OurGreatExtension<Call, (pallet_bridge_parachains::Pallet<Runtime, WithRialtoParachainsInstance>, pallet_bridge_grandpa::Pallet<Runtime, WithRialtoGrandpaInstance>)>
...
);

Oh, this can really work. I can give it a try.

I did some small experiments and now the problem is that C and F would need to implement all the constraints specified by SignedExtension (Codec + Debug + Sync + Send + Clone + Eq + PartialEq + StaticTypeInfo) and this complicates things. Considering this, personally I think that having the macro is easier.

[svyatonik]

Looks much better than my version, thank you! :)