Implémentation des quotas

Une palette de plus, mais comme elle est découplée de l’identité on pourra facilement ajouter le partage de quota avec des comptes portefeuille.

Cette solution peut être efficace si le partage de quota est fixe (on déplace 20% du quota de son compte membre à un compte portefeuille) car il suffit d’ajouter une entrée dans la map pour le nouveau compte avec quota, et mettre à jour le quota du compte membre (il faut donc ajouter un champ pour le quota maximum du compte).

Par contre elle l’est moins si on veut un quota fluide entre plusieurs comptes (chaque compte portefeuille autorisé peut piocher dans le quota de l’identité). Dans ce cas-là il faut stocker le quota dans l’identité et transformer cette map en AccountId -> IdtyId.

Dans ce dernier cas on peut anticiper et faire un truc plus générique qui permettra plus de délégation (autoriser non seulement le partage de quota, mais aussi certains calls au nom du compte principal). Cependant, le quota étant très particulier (pas un call, et il faut vérifier l’état de membre), ce n’est pas forcément pertinent (introduit des traitements différents, alors que pour les calls le traitement sera plus uniforme).

Je veux dire que je ne sais pas si ça influence uniquement le calcul des frais, ou aussi les règles de remplissage du bloc et le benchmark.

À mon avis, il vaut mieux éviter de faire une pallet de plus, et la pallet duniter-account ne faisant quasiment rien, ça me paraît être le bon endroit pour implémenter les quotas (qu’on veut dans l’idée garantir pour tous les comptes qui déclarent leur identité).

D’un point de vue sémantique, ma proposition linked_idty n’a aucun rapport avec les quotas, c’est juste une manière de dire qu’un compte est rattaché à une identité (unique).

Par contre, effectivement, IdtyValue n’est pas forcément le bon endroit pour ranger une information de quotas. Ce serait mieux de le mettre dans IdtyData.

Effectivement, changer le weight c’est pour rendre compte de ce qui est réellement consommé : https://docs.substrate.io/build/tx-weights-fees/#post-dispatch-weight-correction

Il y a un choix que je n’ai pas bien compris dans l’implémentation des oneshot account. Pourquoi wrapper CheckNonce et OnChargeTransaction plutôt que de se contenter de configurer respectivement SignedExtra et pallet_transaction_payment au niveau du runtime ?

Pour moi, les implémentations suivantes devraient être propres au runtime.

impl CheckNonce
{
    // [...]
    fn additional_signed(&self) {
        self.0.additional_signed()
    }

    fn pre_dispatch(/* [...] */) {
        if let Some(
            crate::Call::consume_oneshot_account { .. }
            | crate::Call::consume_oneshot_account_with_remaining { .. },
        ) = call.is_sub_type()
        {
            Ok(())
        } else {
            self.0.pre_dispatch(/* [...] */)
        }
    }

    fn validate(&self, /* [...] */) {
        self.0.validate(/* [...] */)
    }
}
impl OnChargeTransaction
{
  // [...]
  fn withdraw_fee(/* [...] */) {
        if let Some(
            Call::consume_oneshot_account { .. }
            | Call::consume_oneshot_account_with_remaining { .. },
        ) = call.is_sub_type()
        {
          // [...] (d'ailleurs il reste un TODO dedans)
        } else {
            T::InnerOnChargeTransaction::withdraw_fee(/* [...] */)
        }
    }
}

Si on ajoute une autre pallet qui configure davantage OnChargeTransaction selon le même schéma, il faudra choisir dans quel ordre on wrappe, ce qui n’a pas de sens je trouve :

// état actuel
impl pallet_transaction_payment::Config for Runtime {
    type OnChargeTransaction = OneshotAccount; // pourquoi ??
}
impl pallet_oneshot_account::Config for Runtime {
    type InnerOnChargeTransaction = CurrencyAdapter<Balances, HandleFees>;
}
// si on customise davantage, en wrappant dans cet ordre
// OnChargeTransaction → OneshotAccount → Other → CurrencyAdapter
impl pallet_other::Config for Runtime {
    type InnerOnChargeTransaction = CurrencyAdapter<Balances, HandleFees>; 
}
impl pallet_oneshot_account::Config for Runtime {
    type InnerOnChargeTransaction = Other;
}
impl pallet_transaction_payment::Config for Runtime {
    type OnChargeTransaction = OneshotAccount;
}
// on pourrait inverser OneshotAccount et Other 
1 Like

Ce devait être Élois qui m’avait dit de faire comme ça, dans un esprit de modularité.

Mais en effet il sera plus pratique d’éviter d’imbriquer des wrappers quand il y en aura plus.

1 Like

Ok, merci pour la réponse.

Je me rends compte qu’avant d’implémenter des quotas, il y a pas mal de tests à ajouter pour vérifier le comportement des frais, notamment que tout est viré vers la trésorerie plutôt que de disparaître. Et le tip doit être versé au validateur s’il est défini à la trésorerie aussi, c’est plus simple.

1 Like

Un risque avec cette implémentation des quotas est que si le weight est ajusté et que par conséquent le fee est revu à la baisse, correct_and_deposit_fee risque de rembourser en Ğ1 des frais qui ont été payés avec des quotas et par conséquent créer de la monnaie de nulle part.

Pour éviter ça, je pense qu’il faudrait éviter de mélanger les registres “poids, frais, monnaie, quotas” et utiliser la pallet asset-tx-payment à la place de la pallet transaction-payment. Donc on aurait effectivement une monnaie poids non transférable régie par la règle des quotas.

C’est un peu “départ à la case zéro” en matière de conception pour les frais, et ça implique d’assez gros changements dans le code. Mais j’ai peur qu’on se prenne les pieds dans le tapis si on ne le fait pas. Dans tous les cas il faut ajouter plein de tests pour les différents cas de figure de frais pour bien comprendre le fonctionnement (cf Comportement actuel des frais pour un exemple mystérieux).

<transaction_payment::CurrencyAdapter as OnChargeTransaction>::correct_and_deposit_fee prend already_withdrawn en argument donc il gère déjà le remboursement en fonction de ce qui a réellement été payé.

Il suffit de ne pas payer avec les quotas, mais de réduire les frais directement, de sorte à retourner ce qui a été payé hors quota dans withdraw_fee.

Si on ne wrappe pas les OnChargeTransaction on risque d’avoir plus de bugs (pas d’isolation entre les comportements des différentes palettes puisque tout est mélangé dans une fonction commune). Du coup ça peut valoir le coup de créer une macro qui génère cette imbrication mais avec une syntaxe en liste ordonnée (par ordre d’exécution) plutôt que d’indiquer sur chaque palette celle qui vient après.

1 Like

On peut avoir une fonction qui est juste un match et qui appelle les OnChargeTransaction des pallets.

// comme ça, mais pour toutes les pallets qui s'occupent de onchargetransaction
if let Some(
    Call::consume_oneshot_account { .. }
     | Call::consume_oneshot_account_with_remaining { .. },
) = call.is_sub_type()
1 Like

La difficulté que j’identifie est que l’on doit décider de PaysFee (Pays::Yes / Pays::No) en ayant connaissance des frais d’extrinsics, et donc on ne peut pas le faire au niveau du retour DispatchResultWithPostInfo du Call (un call n’a connaissance que de son poids maximal, pas de la conversion de ce poids en frais ni du tip associé). Donc on doit de toute façon payer des frais (générer un imbalance) et le prendre en charge de différentes manières.

Donc si je résume la manière dont on va insérer les quotas dans le paiement des frais :

  • on configure la pallet pallet_transaction_payment
  • on définit son type OnChargeTransaction
  • avec quelque chose qui implémente le trait OnChargeTransaction

c’est-à-dire

pub trait OnChargeTransaction<T: Config> {
	type Balance: frame_support::traits::tokens::Balance;
	type LiquidityInfo: Default;
	fn withdraw_fee(
		who: &T::AccountId,
		call: &T::RuntimeCall,
		dispatch_info: &DispatchInfoOf<T::RuntimeCall>,
		fee: Self::Balance,
		tip: Self::Balance,
	) -> Result<Self::LiquidityInfo, TransactionValidityError>;
	fn correct_and_deposit_fee(
		who: &T::AccountId,
		dispatch_info: &DispatchInfoOf<T::RuntimeCall>,
		post_info: &PostDispatchInfoOf<T::RuntimeCall>,
		corrected_fee: Self::Balance,
		tip: Self::Balance,
		already_withdrawn: Self::LiquidityInfo,
	) -> Result<(), TransactionValidityError>;
}
  • la fonction withdraw_fee distingue plusieurs cas particuliers
  • le cas des calls consume_oneshot_account (déjà traité hormis le TODO dans le code)
  • le cas des calls pour lesquels on ne veut pas pouvoir appliquer de quotas, dans ce cas on utilise l’implémentation par défaut du CurrencyAdapter
  • le cas des calls pour lesquels on veut pouvoir appliquer des quotas
  • dans ce cas withdraw_fees applique le comportement discuté dans les posts au dessus, ce qui peut aboutir à deux cas
  • le cas où les frais sont payés en Ğ1, withdraw est appelé sur le type Currency
  • le cas où les frais sont payés en Ğ1, on met en oeuvre la mécanique des quotas

Une implémentation possible serait de définir un autre token (fongible) mais non transférable pour les quotas. À voir si c’est moins d’efforts que “à la main” vu que toute la logique est déjà implémentée.

Dans tous les cas, le résultat de withdraw_fees est collecté dans le pre_dispatch. S’il s’agit de InitialPayment::Native(already_withdrawn) (non nul), c’est au niveau du post_dispatch que correct_and_deposit_fee est appelé et rembourse ou dépose en trésorerie.


Note : de cette manière, il ne peut pas y avoir de remboursement de quotas et le montant maximal est toujours prélevé, il vaudrait donc mieux utiliser une implémentation à base de tokens si on veut que les quotas soient remboursés au même titre que des frais une fois le poids réel du call connu.

Je pense que je me trompe d’endroit pour insérer cette gestion des quotas. Ce serait plutôt au niveau de correct_and_deposit_fee. En très résumé :

  • les frais maximum sont prélevés avec withdraw_fee
  • le call est exécuté, le poids d’exécution mesuré
  • si des quotas sont disponibles, ils sont décrémentés et les frais sont remboursés entièrement (sauf tip)
  • sinon les frais sont remboursés partiellement en fonction du poids réel et le reste est viré à la trésorerie

Problème : ça ma paraît être une mauvaise idée d’exécuter des traitements potentiellement compliqués (comme une lecture en db) dans un post_dispatch

Peut-être qu’il faudrait plutôt un call “reimburse_fees_using_quotas” appelé comme hook d’un paiement de frais. Ce serait une implémentation plus fidèle à ce que c’est vraiment : un “patch grossier pour exonérer les frais” plutôt que un “mécanisme essentiel profond”.

1 Like

Encore une autre idée pour découpler la question des quotas de la question des frais :

  • le comportement des frais est laissé tel quel
  • dans correct_and_deposit_fee on ajoute les frais payés par le compte à une queue de remboursement limitée en taille
    refund_queue: BoundedVecDeque<(AccountId, Fee), MAX_QUEUED_REFUNDS>
  • dans on_idle on traite les remboursements

De cette manière :

  • on est sûr de ne pas toucher un aspect structurant de substrate que sont les frais d’extrinsic
  • on découple le paiement des frais de leur remboursement
  • en cas de saturation de blockchain, la queue se sature et les remboursements sont différés
  • on n’a pas à trop se soucier du surcoût associé aux quotas parce qu’il “recycle” une ressource blockchain non utilisée
  • on fournit à l’utilisateur notre engagement de toujours “utiliser gratuitement une monnaie libre gérée comme un commun numérique”

(ok le dernier point c’est de la déco, mais il faut garder en tête la raison pour laquelle on ajoute ces quotas)

4 Likes

Solution pour le moins élégante !

Ça me plaît mieux que les propositions précédentes qui semblaient très complexes.

Par contre, si plusieurs blocs d’affilée sont saturés (même par des transactions hors-quota), la queue sera saturée et n’aura pas l’occasion d’être traitée. Les transactions admissibles au quota émises pendant cette période ne seront jamais remboursées. (certes c’est très improbable)
Pour partiellement corriger ça, peut ajouter une préférence (déterministe ou probabiliste) pour les transactions éligibles au quota dans le client.

Les quotas sont une manière de ne pas faire payer des frais aux utilisateurs quand la ressource est abondante. Mais quand elle devient rare, c’est une solution systémique qu’il faut trouver comme :

  • augmenter la puissance totale de la blockchain en changeant de machine de référence
  • développer des solutions de type “layer 2” pour limiter les actions en blockchain principale
  • optimiser le coût de base des opérations (on a un peu de marge de ce côté aussi, il ne faut pas l’oublier)

En fait c’est un autre sujet donc je n’essaye pas de le traiter ici.

J’ai pas mal avancé dans l’implémentation, même si je me heurte à la difficulté des types paramétriques partagés entre des pallets et un type du runtime. Je ne sais toujours pas si c’est une bonne idée d’être parti sur un champ quotas dans IdtyData.

En gros pour l’instant :

  • dans correct_and_deposit_fee on vérifie seulement si account_data.linked_idty est défini pour ajouter à la queue
  • dans on_idle on appelle process_refund_queue
  • on y appelle spend_quotas qui cherche l’identité dans le storage et vérifie son statut avant d’appeler update_quotas et do_spend_quotas sur les IdtyData
  • ensuite, tout le traitement “quotas” est fait dans IdtyData qui est un type du runtime fourni à la pallet identity comme type paramétrique car il partage des informations avec la pallet universal dividend
  • c’est ensuite try_refund appelé dans process_refund_queue qui retirera les sous à la trésorerie et les mettra sur le compte (non implémenté)

Et la difficulté, c’est de faire en sorte que tout le monde utilise le même type Balance sans mettre des dépendances dans tous les sens avec le trait Currency :

  • IdtyData (type du runtime) dans son champ quotas
  • le trait IdtyQuotasLinker de la pallet identity qui permet à la pallet account de déléguer spend_quotas
  • le trait Quotas de la pallet identity implémenté par IdtyData qui gère update_quotas et do_spend_quotas

Comme IdtyData est un type du runtime, il connaît les types concrets et ne peut pas utiliser des types paramétriques (sinon il y a un cycle de dépendance entre le type runtime et le type idtydata), mais comme il prend des arguments venant de fonctions des pallets, il y a une incompatibilité.

Donc soit on fait une conversion à un moment donné (Balance / BlockNumber → u64 / u32) puis (u64 / u32 → Balance / BlockNumber), soit on déplace le champs quotas vers IdtyValue, mais ça implique quand même d’ajouter une dépendance de la pallet identity à la pallet balance ou un type paramétrique qui implémente le trait Currency.

Bref, voilà les réflexions que je dois avoir en ce moment :no_mouth:


[edit] je pars sur la conversion, c’est ce qui me paraît le plus naturel et ça ne devrait pas poser de problèmes (à tester)

2 Likes

Je ne sais pas si ça peut t’aider, mais dans une vidéo de l’équipe Substrate que j’ai vu il y a quelques temps, il y avait une partie sur “Share functionality between pallets” où l’intervenant revient sur les 2 principaux modes selon lui :

  • couplage fort (par héritage = dépendance direct du trait Config d’une autre pallet)
  • couplage souple (ajout d’un type qui sera implémenté à la définition du Runtime)

Le fait qu’IdtyData soit un type du Runtime (je comprends : pas un type d’une palette) ça signifie que c’est encore autre chose, n’est-ce pas ? Y a-t-il une contrainte qui explique qu’il soit défini comme ça ?

Je suis moins à l’aise avec les explications par vidéo qu’avec la documentation écrite et celle-ci : https://docs.substrate.io/build/pallet-coupling/ me convient bien.

On y retrouve bien les deux types de couplage sous le nom “Tightly coupled pallets” / “Loosely coupled pallets”.

Il faudrait que j’illustre le schéma de couplage des pallets dans Duniter-v2s qui m’a l’air cohérent. Pour l’instant duniter-account et fortement couplé avec balances provide_randomness et pallet_treasury (pallets/duniter-account/src/lib.rs#L52-L56). Je propose d’ajouter des couplages faibles par l’intermédiaire des types IdtyId, InnerOnChargeTransaction, Currency, IdtyLinker. Le type IdtyLinker implémente le trait IdtyQuotasLinker (noms à revoir) qui permet de demander un traitement externe des quotas.

Pour l’instant j’ai fait en sorte que ce soit la pallet identity qui implémente la gestion des quotas parce que notre notion de quotas est fortement liée à la notion d’identité. Et cette pallet n’a pas besoin d’être liée fortement à la pallet universal-dividend parce que la seule donnée qu’elle partage avec elle est dans IdtyData : le first_eligible_ud, d’où le couplage faible à ce type (pallets/identity/src/lib.rs#L79).

Maintenant la question : « pourquoi IdtyData est un type du runtime (et donc pas un type d’une pallet) ? ». Ça rentre dans “couplage souple”, mais au lieu d’utiliser un type implémenté par une pallet (par exemple le type AccountData (runtime/common/src/pallets_config.rs#L70), on utilise un type implémenté par le runtime (configuré runtime/common/src/pallets_config.rs#L445, défini runtime/common/src/entities.rs#L75-L79).

J’imagine que l’idée est de permettre à plusieurs pallets d’ajouter des données liées à une identité dans les IdtyData sans avoir à modifier la pallet identity elle-même (les IdtyValue). Et ça tombe bien parce que c’est ce dont j’ai besoin pour les quotas. Est-ce qu’on a d’autre exemples de ça dans le runtime polkadot ? Il faudrait que je l’étudie plus en détail.

Je pense que c’est un reste du travail d’elois pour avoir beaucoup de modularité, et ça aide pas mal au niveau des tests. Est-ce que c’est un peu “sur-ingéniéré” ? Je n’ai pas encore assez de recul là dessus, je verrai en avançant dans l’implémentation, mais pour l’instant le code que j’écris m’a l’air lisible et à sa place, on verra à la relecture :slight_smile:

5 Likes

En écrivant les tests, je me rends compte que ce serait effectivement plus simple de regrouper les fonctionnalités des quotas dans une pallet.

Du coup la solution que je retiens est de conserver le linked_idty dans duniter-account (c’est bien sa place) et de partir sur une pallet quotas avec une map IdtyId(last_use, amount).

Je laisse ma branche actuelle hugo-quotas-2 de la MR !183 en l’état, et je repars sur une branche hugo-quotas avec une pallet quotas. C’est un peu de travail de réécriture sur quelque chose qui pourrait marcher en l’état, mais c’est plus propre et ça m’embêterait de partir sur une solution trop crade.

2 Likes

C’est bon, les quotas ont leur pallet ! Maintenant que c’est fait, il m’apparaît évident que c’est la bonne approche. J’ai pu développer facilement des tests unitaires, ce qui était trop compliqué avant, et les tests d’intégration fonctionnent toujours. Et en bonus, ça va m’aider à comprendre le point toujours mystérieux (Comportement actuel des frais) puisque dans l’opération, le montant des frais est resté de 2, mais le montant du remboursement est passé à 1 :thinking:

bonus

J’aurais pu implémenter ça plus rapidement si je n’avais pas posé quelques demies journées de congés pour cause de grasse matinée car couché tard et tonne de boulot militant pour la BASE et la lutte contre l’A69.

6 Likes

Une question intéressante de spécifications liée aux quotas a été soulevée dans les discussions sur la MR.

À partir de quand a-t-on droit aux quotas ? Quand l’identité est :

  • créée ?
  • confirmée ?
  • validée ?

Est-ce qu’une identité qui perd le statut de membre perd aussi le droit au quotas (= remboursement des frais de transaction) ?