À quoi servent les pending membership?

Protocols Ğ1v2 protocol
21

Euh, ces palettes ne connaissent pas les certifs, tu veux parler de duniter-wot ?

Il faut en effet supprimer les certifs forgeron et choisir un délai après révocation (même mécanisme que pour la révocation principale). Mais le délai devrait être plus court ici.

Par contre si le forgeron revient il faut s’assurer qu’il a changé ses clés de session. Avec une table de clés bannies ? (et donc une expiration des bannissements au bout de N années…)

22

En fait non, je me suis mal exprimé, on garde l’implémentation dans la pallet wot, mais la fonction qu’elle appelle sera dans une autre pallet.

// cette partie reste dans la pallet wot
if receiver_received_count < T::MinCertForMembership::get()
// cette partie disparaît, pas besoin de vérifier si l'identité est membre pour transmettre
// le message qu'elle est passée en dessous du seuil de certifications
&& pallet_membership::Pallet::<T, I>::is_member(&receiver)
// cette partie est déplacée dans les pallets qui vont gérer les adhésions
if let Err(e) = <pallet_membership::Pallet<T, I>>::force_expire_membership(receiver) {

Donc en gros, ça fait

if receiver_received_count < MinCertForMembership
    try_remove_membership()

Pour l’instance wot, ce sera la pallet identity qui essaiera de retirer l’adhésion (mais si elle n’est déjà plus là il n’y a rien à faire), pour l’instance smith_wot, ce sera la pallet authoriy_members.

23

Les idty_id sont des u32 (32 bits), pas des clés publiques (32 octets). Donc l’impact de la duplication est moindre. Mais effectivement, ça fait de la duplication peu utile.

24

Dans un premier temps ça peut être fait comme ça, et dans un 2ème temps : on peut carrément faire sauter la pallet duniter-wot.

Ce n’est pas que je veux tout faire sauter ! Mais ça permet de découpler la partie WoT de la partie autorités, vu que cette idée de Smith WoT ne te satisfait pas vraiment et que cet avis est partagé par d’autres (moi le 1er). La solution d’Eloïs avait le mérite de fonctionner tout de suite, mais si l’on peut faire plus adapté (et surtout moins complexe), ça peut valoir le coup.

A mon avis, authority_members pourrait être auto-suffisante pour gérer la problématique d’entrée/sortie des Smith, avec des règles plus spécifiques que celles de la WoT classique. Il y aura sûrement un lien avec la pallet identity, mais pas beaucoup plus.

Actuellement je ne suis pas serein sur les tests, je trouve que la combinatoire est trop grande avec tous les liens qui existent au niveau des pallet de WoT, qui en plus est dupliquée avec les Smith. Je pense que stratégiquement il vaudrait mieux séparer tout ça.

1 Like
25

Actuellement il y a des palettes très génériques (membership, identity, cert) et une palette “colle”, duniter-wot. Qui ferait son travail ?

Idée pour éviter les problèmes de combinaisons imprévues : implémenter une machine à états dans duniter-wot, qui décrit entièrement tous les états possibles d’une identité, et toutes les transitions entre états (aucune transition partielle). Mais ça risque d’être une usine à gaz monolithique.

2 Likes
26

Si l’on part des deux hypothèses suivantes :

  • les Smith sont gérés indépendamment via la pallet authority-members
  • le concept d’adhésion de la WoT saute

Alors la palette duniter-wot ne sert plus qu’à “coller” identiy, certification ainsi qu’un peu de distance, ce que ces palettes pourraient faire en interne sur leurs parties respectives car il resterait :

  • Constantes :
    • appliquer FirstIssuableOn : concept de certification
    • appliquer MinCertForMembership : concept de certification
    • appliquer MinCertForCreateIdtyRight : concept de certification
    • appliquer IsDistanceOk : concept de distance
  • Fonctions :
    • check_create_identity : checks de certifications
    • check_validate_identity : check réductible à l’appel de distance.requestDistanceEvaluation
    • check_cert_allowed : check de certifications
    • on_idty_change.IdentityEvent::(Created|Removed) : check de certifications
    • on_new_cert : event de certifications
    • on_removed_cert : event de certifications

En fait, en l’écrivant, je me rends compte que 90% du code restant sont des traitements liés aux certifications. Quant à la vérification de distance, c’est un appel direct à la pallet de distance, il n’y a pas de code à déplacer, juste à laisser les palettes identity et distance se parler.

2 Likes
27

@HugoTrentesaux pour clarifier ma position : je ne compte pas refactorer quoi que ce soit pour l’instant. Si tu veux tu peux déjà t’occuper des PendingMembership comme tu souhaitais le faire. Tu peux y aller, nous ne nous marcherons pas dessus.

Eventuellement je verrai pour un petit POC de modification de la palette authority-members afin d’isoler la feature “Smith”, condition préalable au démantèlement des adhésions et de la palette duniter-wot. Sauf si à un moment tu me dis que finalement tu a bien envie de le faire.

1 Like
28

J’ai du mal à comprendre concrètement. La proposition est-elle :

  • coupler fortement toutes ces palettes ensemble, en distribuant le code de duniter-wot dans chacune d’elles ?
  • fusionner duniter-wot et identity ? (ce qui créerait des dépendances improbables, genre de certifications à distance)
  • n’utiliser que des fonctions génériques dans les Config et quelques providers/handlers dans runtime-common pour éviter d’avoir des fonctions ad hoc dédiées aux spécificités de la WoT et à la glu inter-palettes ?

Les deux premières solutions me font peur et je trouverais ça très dommage, la dernière me plaît mais semble plus difficile.

29

La proposition ne peut pas être ni la 1ère ni la 2ème: car duniter-wot ne ferait plus que de la certification, donc seule la palette certification serait impactée.

Mais je ne suis pas sûr que ça ressemble exactement à la 3ème proposition non plus, je ne vois pas de quelle “glue” il y aurait besoin :

  1. identity.create_identity a besoin de la palette certification, aujourd’hui abstrait par duniter-wot via CheckIdtyCallAllowed::check_create_identity
  2. identity.confirm_identity : aucun lien avec une autre palette
  3. certification.add_cert : a besoin de vérifier l’existence des identités, aujourd’hui abstrait par duniter-wot via CheckCertAllowed::check_cert_allowed
  4. distance.request_distance_evaluation : a besoin de vérifier l’existence et le statut des identités (+ je rajouterai : vérifier aussi le nombre de certifications)

Le workflow final pour devenir membre prend 4 étapes avec un workflow sans adhésions (contre 6 auparavant : il fallait demander + confirmer l’adhésion), et dedans les seuls “liens” à faire sont que identity et certification se causent, et que distance puisse avoir accès à identity et certification.

On peut parler de “glue”, je ne sais pas exactement quelle forme donner à celle-ci, mais j’ai l’impression qu’un simple couplage fort (trait Config qui implémente identity ou certification selon le cas) serait tout à fait acceptable et logique. Passer par des types comme CheckIdtyCallAllowed (comme aujourd’hui) me paraît un peu overkill, mais pourquoi pas.

1 Like
30

Pas si évident, notamment pour les tests unitaires de la palette. Passer par un handler/provider est plus souple.

1 Like
31

J’ai beaucoup avancé dans mon refac dans la MR !215, le code est devenu plus propre et compréhensible.

Et au vu de ton post Refonte de la Smith WoT et de l’intérêt que tu portes à la question, je me demande si je ne ferais pas mieux d’arrêter là dans le ticket #156 et de ne pas :

  • retirer le call revoke_membership
  • retirer l’instance smith de la pallet membership
  • apporter des modification à la pallet authority_members
  • permettre le changement de clé pour les forgerons

Comme ça j’ai fait la simplification que j’estimais indispensable et les bases sont propres pour les évolutions que tu proposes. Parmi ces nouvelles bases :

  • une distinction claire pour les statuts d’identité :
enum IdtyStatus {
    Unconfirmed,
    Unvalidated,
    Member,
    NotMember,
    Revoked,
}

  • une distinction entre la révocation d’identité et le retrait d’identité avec une cause précisée dans le message d’erreur (cf RevocationReason et RemovalReason)

  • des deadlines pour faire les actions bien définies

/// Period during which the owner can confirm the new identity.
// something like 2 days but this should be done quickly as the first certifier is helping
type ConfirmPeriod: Get<Self::BlockNumber>;
/// Period before which the identity has to be validated (become member).
// this is the 2 month period in v1
type ValidationPeriod: Get<Self::BlockNumber>;
/// Period before which an identity who lost membership is automatically revoked.
// this is the 1 year period in v1
type AutorevocationPeriod: Get<Self::BlockNumber>;
/// Period after which a revoked identity is removed and the keys are freed.
type DeletionPeriod: Get<Self::BlockNumber>;

  • un grand ménage dans les IdtyEvent qui passe de 5 à 2 cas

  • des messages d’erreurs plus précis partout (certifications, toile, membership, distance…)

enum MembershipRemovalReason {
    // reach end of life
    Expired,
    // was explicitly revoked
    Revoked,
    // received certs count passed below threshold
    NotEnoughCerts,
    // system reasons (consumers, authority members, or root)
    System,
}

  • le retrait de “validate_identity” qui n’a pas de raison d’être vu que “claim_membership” existe

  • et évidemment le retrait de pending* et request_membership

Il me reste un peu de ménage à faire notamment sur les live tests, mais sinon c’est regardable en l’état.

3 Likes
32

Je vais faire une relecture d’ici demain soir.

En effet ça ne vaut peut-être pas le coup de refactorer certaines parties touchant aux memberships et à authority_members. Ou au moins fais une pause le temps de voir concrètement la MR que je vais proposer, et voir si celle-ci convient et les impacts que tu pourras en tirer pour poursuivre ton refacto.

1 Like
33

Je voyais plutôt ça dans l’autre sens, tu bases ta branche sur la mienne. D’abord refac membership, ensuite refonte smith wot.

34

Oui c’est ça : je dis juste qu’avant d’aller plus loin, il faut soit jeter ce que je vais faire, soit le merger sur ta branche.