Pallets instanciables et vérifications d'adhésion

Duniter v2 utilise des pallets instanciables comme :

  • la pallet wot
  • la pallet membership
  • la pallet certifications

Les deux instances de ces pallets sont :

  • toile principale
  • toile forgeron

Cela veut dire qu’il y a :

// une seule pallet identity (pas instanciable)
Identity: pallet_identity::{Pallet, Call, Config<T>, Storage, Event<T>} = 41,

// Web Of Trust
Wot: pallet_duniter_wot::<Instance1>::{Pallet} = 40,
Membership: pallet_membership::<Instance1>::{Pallet, Call, Config<T>, Storage, Event<T>} = 42,
Cert: pallet_certification::<Instance1>::{Pallet, Call, Config<T>, Storage, Event<T>} = 43,

// une seule pallet distance (pas instanciable)
Distance: pallet_distance::{Pallet, Call, Storage, Inherent} = 44,

// Smith Sub-Wot
SmithSubWot: pallet_duniter_wot::<Instance2>::{Pallet} = 50,
SmithMembership: pallet_membership::<Instance2>::{Pallet, Call, Config<T>, Storage, Event<T>} = 52,
SmithCert: pallet_certification::<Instance2>::{Pallet, Call, Config<T>, Storage, Event<T>} = 53,

C’est pratique, parce que ça permet de découper en petit composants testables et réutilisables des fonctionnalités très similaires.

Mais jusque là, les traits comme CheckCertAllowed et CheckMembershipCallAllowed étaient définis sans prendre en compte les instances, avec I: 'static :

impl<T: Config<I>, I: 'static> /* [...] */ for Pallet<T, I>

Et les seules vérifications faites l’étaient sur une pallet non instanciable (en l’occurence pallet_identity).

Cela empêche par exemple :

  • de vérifier que l’émetteur d’une certification est bien membre
  • de vérifier que le destinataire d’une certification a bien une adhésion ou une demande d’adhésion en cours

Pour contourner ça, elois a utilisé le statut d’identité (Demande d'adhésion et statut d'identité), ce qui fonctionnait à peu près pour la toile principale, mais :

  • causait un comportement étrange pour la toile forgeron (Becoming smith (request_membership, claim_membership))
  • limitait les possibilités de perte de statut membre (expiration d’adhésion, passage sous le seuil de certifications, ou évaluation négative du critère de distance)

Je pense que elois n’avait pas totalement conscience de ça et que c’était la raison secrète qui le poussait à vouloir Proposition de supprimer la notion d'identité désactivée mais non révoquée et la notion d'adhésion.


La bonne manière de faire (selon moi) serait de donner les instances Cert et Membership à Wot (et SmithCert et SmithMembership à SmithSubWot) pour pouvoir implémenter les traits sur des instances, et non pas de manière statique.

Comme ça représente quand même un certain chantier qui peut me prendre quelques semaines à finaliser (implémentation, relecture, tests…), je pense que je vais le faire après le lancement du réseau de test tant attendu. À voir si ça vaudra la peine de déployer dessus (sûrement des migrations complexes à écrire) ou s’il vaudra mieux rebooter un nouveau réseau de test.

Et je vais me contenter d’un fix simple pour #129 en supprimant simplement les métadonnées d’adhésion.

2 Likes

Je n’arrive pas à comprendre le problème. Je viens de décommenter le code lié au ticket #136 sur ce commit, et corriger une petite coquille (un && au lieu d’un ||).

Et les deux tests de fixme_tests.rs échouent comme attendu. :man_shrugging:

Je suis vraiment mal réveillé aujourd’hui, oublie.

Néanmoins je ne vois toujours pas le soucis, les WoT impliquées sont bien séparées, je ne vois pas en quoi tu ne pourrais pas faire les vérifications.

1 Like

Les wot sont bien séparées, mais elles n’ont pas “conscience” des instances de certification et membership auxquelles elles sont liées. J’avoue que je n’ai pas creusé à fond la question, je pense que c’est mieux de régler ça après le lancement du réseau de test. C’est vrai qu’on pourrait quand même faire les vérifications en implémentant les traits Check*CallAllowed sur les pallets certification et membership et en les fournissant sous forme de tuple. Mais de toute façons les implémentations devraient prendre en compte l’instance I pour explorer les données rattachées à cette instance, donc je trouve ça plus simple de tout relier en utilisant la pallet wot.

En gros, si on émet une certification forgeron, vers une identité A, il faut vérifier :

  • que A est bien membre de la toile principale (son adhesion n’a pas expiré par exemple)
  • que A a une adhésion en cours à la toile forgeron ou est membre de la toile forgeron

Ces vérifications ne doivent pas se faire uniquement de manière statique en stockant dans Idty son statut, mais bien auprès de chaque instance des pallets.

On peut omettre certaines vérifications si on fait en sorte qu’elles se réalisent toujours, mais dans se cas, il faut s’assurer de la cohérence du storage.

Exemple : si A perd son statut de membre sur la toile principale, il doit aussi perdre son statut de membre de la toile forgeron. Dans l’implémentation d’elois, il n’y avait pas ce problème parce l’identité (et toutes les données liées) était immédiatement supprimées (Proposition de supprimer la notion d'identité désactivée mais non révoquée et la notion d'adhésion). Mais c’était une trop grosse modification par rapport au protocole Duniter v1 et en plus très “punitive”, au sens où il fallait recommencer une identité de zéro, changer de pseudo, obtenir les certifications…
Donc j’ai autorisé la perte d’adhésion sans suppression d’identité, mais en introduisant des “bugs”. Maintenant il faut travailler à corriger ces “bugs” pour se rapprocher davantage du comportement v1 qui est bien plus souple et logique sur ce point.

3 Likes