-
Notifications
You must be signed in to change notification settings - Fork 318
Support real-pays-fee through three proxy levels #2679
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devnet-ready
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,14 @@ type RuntimeOriginOf<T> = <T as frame_system::Config>::RuntimeOrigin; | |
| type AccountIdOf<T> = <T as frame_system::Config>::AccountId; | ||
| type LookupOf<T> = <T as frame_system::Config>::Lookup; | ||
|
|
||
| const MAX_REAL_PAYS_FEE_PROXY_DEPTH: u8 = 3; | ||
|
|
||
| enum FeePayerResolution<AccountId> { | ||
| Payer(AccountId), | ||
| SignerPays, | ||
| NotApplicable, | ||
| } | ||
|
|
||
| #[freeze_struct("f003cde1f9da4a90")] | ||
| #[derive(Encode, Decode, DecodeWithMemTracking, Clone, Eq, PartialEq, TypeInfo)] | ||
| #[scale_info(skip_type_params(T))] | ||
|
|
@@ -85,58 +93,60 @@ where | |
|
|
||
| /// Determine who should pay the transaction fee for a proxy call. | ||
| /// | ||
| /// Follows the RealPaysFee chain up to 2 levels deep: | ||
| /// Follows the RealPaysFee chain up to three proxy levels deep: | ||
| /// - Case 1: `proxy(real=A, call)` → A pays if `RealPaysFee<A, signer>` | ||
| /// - Case 2: `proxy(real=B, proxy(real=A, call))` → A pays if both | ||
| /// `RealPaysFee<B, signer>` and `RealPaysFee<A, B>` are set; B pays if only the former. | ||
| /// - Case 3: `proxy(real=B, batch([proxy(real=A, ..), ..]))` → A pays if | ||
| /// `RealPaysFee<B, signer>`, all batch items are proxy calls with the same real A, | ||
| /// and `RealPaysFee<A, B>` is set; B pays if only the first condition holds. | ||
| /// - Case 4: `proxy(real=C, proxy(real=B, batch([proxy(real=A, ..), ..])))` | ||
| /// → A pays if all three `RealPaysFee` relationships are set and the batch is homogeneous. | ||
| /// | ||
| /// Returns `None` if the signer should pay (no RealPaysFee opt-in). | ||
| fn extract_real_fee_payer( | ||
| call: &RuntimeCallOf<T>, | ||
| origin: &RuntimeOriginOf<T>, | ||
| ) -> Option<AccountIdOf<T>> { | ||
| let signer = origin.as_system_origin_signer()?; | ||
| let (outer_real, delegate, inner_call) = Self::extract_proxy_parts(call, signer)?; | ||
|
|
||
| // Check if the outer real account has opted in to pay for the delegate. | ||
| if !pallet_proxy::Pallet::<T>::is_real_pays_fee(&outer_real, &delegate) { | ||
| return None; | ||
| match Self::resolve_real_fee_payer(call, signer, MAX_REAL_PAYS_FEE_PROXY_DEPTH) { | ||
| FeePayerResolution::Payer(payer) => Some(payer), | ||
| FeePayerResolution::SignerPays | FeePayerResolution::NotApplicable => None, | ||
| } | ||
| } | ||
|
|
||
| // outer_real pays. Try to push the fee deeper into nested proxy structures. | ||
| let inner_call: &RuntimeCallOf<T> = (*inner_call).as_ref().into_ref(); | ||
| fn resolve_real_fee_payer( | ||
| call: &RuntimeCallOf<T>, | ||
| delegate: &AccountIdOf<T>, | ||
| remaining_proxy_depth: u8, | ||
| ) -> FeePayerResolution<AccountIdOf<T>> { | ||
| let Some((real, _, inner_call)) = Self::extract_proxy_parts(call, delegate) else { | ||
| return FeePayerResolution::NotApplicable; | ||
| }; | ||
|
|
||
| // Case 2: inner call is another proxy call. | ||
| if let Some(inner_payer) = Self::extract_inner_proxy_payer(inner_call, &outer_real) { | ||
| return Some(inner_payer); | ||
| if !pallet_proxy::Pallet::<T>::is_real_pays_fee(&real, delegate) { | ||
| return FeePayerResolution::NotApplicable; | ||
| } | ||
|
|
||
| // Case 3: inner call is a batch of proxy calls with the same real. | ||
| if let Some(batch_payer) = Self::extract_batch_proxy_payer(inner_call, &outer_real) { | ||
| return Some(batch_payer); | ||
| if remaining_proxy_depth <= 1 { | ||
| return FeePayerResolution::Payer(real); | ||
| } | ||
|
|
||
| // Case 1: simple proxy, outer_real pays. | ||
| Some(outer_real) | ||
| } | ||
| let inner_call: &RuntimeCallOf<T> = (*inner_call).as_ref().into_ref(); | ||
|
|
||
| /// Check if an inner call is a proxy call where the inner real has opted in to pay. | ||
| /// `outer_real` is used as the implicit delegate for `proxy` calls. | ||
| fn extract_inner_proxy_payer( | ||
| inner_call: &RuntimeCallOf<T>, | ||
| outer_real: &AccountIdOf<T>, | ||
| ) -> Option<AccountIdOf<T>> { | ||
| let (inner_real, inner_delegate, _call) = | ||
| Self::extract_proxy_parts(inner_call, outer_real)?; | ||
| match Self::resolve_real_fee_payer(inner_call, &real, remaining_proxy_depth - 1) { | ||
| FeePayerResolution::Payer(payer) => return FeePayerResolution::Payer(payer), | ||
| FeePayerResolution::SignerPays => return FeePayerResolution::SignerPays, | ||
| FeePayerResolution::NotApplicable => {} | ||
| } | ||
|
|
||
| if pallet_proxy::Pallet::<T>::is_real_pays_fee(&inner_real, &inner_delegate) { | ||
| Some(inner_real) | ||
| } else { | ||
| None | ||
| match Self::extract_batch_proxy_payer(inner_call, &real, remaining_proxy_depth - 1) { | ||
| FeePayerResolution::Payer(payer) => return FeePayerResolution::Payer(payer), | ||
| FeePayerResolution::SignerPays => return FeePayerResolution::SignerPays, | ||
| FeePayerResolution::NotApplicable => {} | ||
| } | ||
|
|
||
| FeePayerResolution::Payer(real) | ||
| } | ||
|
|
||
| /// Check if an inner call is a batch where ALL items are proxy calls with the same real | ||
|
|
@@ -145,42 +155,64 @@ where | |
| fn extract_batch_proxy_payer( | ||
| inner_call: &RuntimeCallOf<T>, | ||
| outer_real: &AccountIdOf<T>, | ||
| ) -> Option<AccountIdOf<T>> { | ||
| let calls: &Vec<<T as pallet_utility::Config>::RuntimeCall> = | ||
| match inner_call.is_sub_type()? { | ||
| pallet_utility::Call::batch { calls } | ||
| | pallet_utility::Call::batch_all { calls } | ||
| | pallet_utility::Call::force_batch { calls } => calls, | ||
| _ => return None, | ||
| }; | ||
| remaining_proxy_depth: u8, | ||
| ) -> FeePayerResolution<AccountIdOf<T>> { | ||
| let Some(utility_call) = inner_call.is_sub_type() else { | ||
| return FeePayerResolution::NotApplicable; | ||
| }; | ||
| let calls: &Vec<<T as pallet_utility::Config>::RuntimeCall> = match utility_call { | ||
| pallet_utility::Call::batch { calls } | ||
| | pallet_utility::Call::batch_all { calls } | ||
| | pallet_utility::Call::force_batch { calls } => calls, | ||
| _ => return FeePayerResolution::NotApplicable, | ||
| }; | ||
|
|
||
| if calls.is_empty() { | ||
| return None; | ||
| return FeePayerResolution::NotApplicable; | ||
| } | ||
|
|
||
| let mut common_real: Option<AccountIdOf<T>> = None; | ||
| let mut first_proxy_call: Option<&RuntimeCallOf<T>> = None; | ||
|
|
||
| for call in calls.iter() { | ||
| let call_ref: &RuntimeCallOf<T> = call.into_ref(); | ||
| let (inner_real, inner_delegate, _) = Self::extract_proxy_parts(call_ref, outer_real)?; | ||
| let Some((inner_real, inner_delegate, _)) = | ||
| Self::extract_proxy_parts(call_ref, outer_real) | ||
| else { | ||
| return FeePayerResolution::NotApplicable; | ||
| }; | ||
|
|
||
| match &common_real { | ||
| None => { | ||
| // Check RealPaysFee once on the first item and memoize. For `proxy` | ||
| // calls the delegate is always `outer_real`, so a single read covers | ||
| // the entire batch; for `proxy_announced` it uses the explicit delegate. | ||
| if !pallet_proxy::Pallet::<T>::is_real_pays_fee(&inner_real, &inner_delegate) { | ||
| return None; | ||
| return FeePayerResolution::NotApplicable; | ||
| } | ||
| first_proxy_call = Some(call_ref); | ||
| common_real = Some(inner_real); | ||
| } | ||
| // All items must share the same real account. | ||
| Some(existing) if *existing != inner_real => return None, | ||
| // Mixed real accounts intentionally leave the original signer paying fees. | ||
| Some(existing) if *existing != inner_real => return FeePayerResolution::SignerPays, | ||
| _ => {} | ||
| } | ||
| } | ||
|
|
||
| common_real | ||
| let Some(common_real) = common_real else { | ||
| return FeePayerResolution::NotApplicable; | ||
| }; | ||
| if remaining_proxy_depth > 1 | ||
| && let Some(first_call) = first_proxy_call | ||
| { | ||
| match Self::resolve_real_fee_payer(first_call, outer_real, remaining_proxy_depth) { | ||
| FeePayerResolution::Payer(payer) => return FeePayerResolution::Payer(payer), | ||
| FeePayerResolution::SignerPays => return FeePayerResolution::SignerPays, | ||
| FeePayerResolution::NotApplicable => {} | ||
| } | ||
| } | ||
|
|
||
| FeePayerResolution::Payer(common_real) | ||
|
Comment on lines
+202
to
+215
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is there recursion here? The intended supported shapes were: For the batch case, the lookup should only inspect the immediate batch items, verify they are all proxy calls with the same real, and verify RealPaysFee<real, delegate> is set. Batch lookup should stop there. If that check fails, fee resolution should fall back to the nearest already-proven payer. Recursing from the batch lookup changes the original model and makes the payer selection harder to reason about. |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -200,11 +232,11 @@ where | |
| type Pre = Pre<T>; | ||
|
|
||
| fn weight(&self, call: &RuntimeCallOf<T>) -> Weight { | ||
| // Account for up to 3 storage reads in the worst-case fee payer resolution | ||
| // (outer is_real_pays_fee + inner/batch is_real_pays_fee + margin). | ||
| // Account for up to four storage reads in the worst-case fee payer resolution | ||
| // (three proxy hops + margin). | ||
| self.inner | ||
| .weight(call) | ||
| .saturating_add(T::DbWeight::get().reads(3)) | ||
| .saturating_add(T::DbWeight::get().reads(4)) | ||
| } | ||
|
|
||
| fn validate( | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t think we need
FeePayerResolutionhere.Under the original model, this is just
Option<AccountId>.Some(account)means this call shape proved that account can pay.Nonemeans this lookup did not prove a payer, so the caller should keep the nearest already-proven payer.The extra
SignerPaysstate changes that model. It lets a deeper failed/ambiguous lookup erase an outer real that was already proven to have opted in, which is not the behavior we want. The signer should only pay when the outermost lookup fails to prove any real payer.