Distance

Je me sens obligé de demander/proposer cette fonctionnalité avant d’aller plus loin.

Si j’ai bien compris, nous évitons les calls automatiques dans duniter pour s’assurer de pouvoir taxer quelqu’un si ce call échoue, et ainsi éviter le spam.

Mais ne serait-il pas possible de créer un nouvel extrinsic distance.requestDistanceEvaluationThenValidate(idtyIndex), qui ferait la même chose requestDistanceEvaluation, mais qui programmerait également l’extrinsic validateIdentity grâce au call schedule en root (car appelable uniquement via root), à la condition que distance.identityDistanceStatus(idtyIndex) soit validé ?

Ou bien, que cet extrinsic validateIdentity soit appelé par l’oracle directement au moment de la publication si l’identité a le status confirmedByOwner, étant donné que cet extrinsic est appelable par n’importe qui ?

Ainsi en cas d’échec, c’est celui qui appelle requestDistanceEvaluationThenValidate qui trinque pour tout.

Je sais que techniquement c’est possible, mais les questions sont:

  • Est-ce idiomatique de substrate et son oracle ?
  • Si oui pensez-vous pouvoir implémenter ça pour le prochain runtime ?

Je préfère demander maintenant pour m’assurer de ne pas rentrer dans des mécaniques de validation d’identité côté client qui deviendraient obsolète avant même la mise en prod.


Dans l’hypothèse ou ceci n’est que fantaisie, le seul élément bloquant pour moi côté client reste que requestDistanceEvaluation ne soit pas appelable pour n’importe qui (requestDistanceEvaluation(idtyIndex)), de manière à la batcher avec la certification numéro minCertForMembership (élément qui sera probablement renommé vue que vous avez décidé de faire sauter la palette membership ^^).

2 Likes

Dans le fond, c’est exactement ce que je souhaite aussi : que distance.requestDistanceEvaluation soit la seule étape qui permet de passer du statut de futur membre à membre, ou encore de décaler le moment où le statut de membre peut être perdu (sauf à perdre une certification qui ferait passer en dessous des 5 requises, bien sûr).

Au regard du code, identity.validate_identity ne fait que vérifier :

  1. Le nombre de certifications est suffisant
  2. La règle de distance est respectée

En termes d’écritures en cas de succès, et sous l’hypothèse du retrait des adhésions, et bah je dirais : 1 seule écriture supplémentaire pour :

  • le passage de l’identité au statut Validated
  • son champ first_eligible_ud

Je manque d’expertise sur les poids pour trancher, mais j’ai du mal à voir comment nous pourrions être contre le comportement que tu proposes.

2 Likes

C’est faisable d’autoriser d’autres comptes à demander l’évaluation, on peut enregistrer le account_id du compte demandeur plutôt que celui de l’identité candidate. En cas d’échec, le compte demandeur sera slashé.

Cette possibilité amène une question : pour la future implémentation de l’antispam, est-ce que la limite s’applique à la fréquence de demandes/échecs (demandes ou échecs, c’est encore une autre question) de la part d’un compte demandeur ? d’une identité demandeuse (seuls les membres pourraient demander, autres que les identités candidates) ? pour l’identité candidate ?

Pour la fonction deux-en-un distance+validation, je verrais ça dans duniter-wot.

Il n’y a pas d’autres hooks qui sont appelés quand on devient membre, pour mettre à jour N par exemple, le membership…? Le vrai poids serait donc plus élevé. Mais comme il y a déjà assez d’antispams pour toutes les étapes préalables à devenir membre (difficile de devenir membre tous les 10 blocs) ça ne me choque pas que le poids de cette validation (qui vient d’un inhérent, pas d’un extrinsic) soit gratuit.

2 Likes

En effet j’ai loupé la mise à jour de N dans mon analyse, il faudrait compter une écriture supplémentaire.

Rien de très coûteux ceci dit, et encore moins au regard du calcul de distance qui le précède. Si quelqu’un devient membre, c’est plutôt une chose à fêter qu’à faire payer :slight_smile:

Tout à fait d’accord, le fonctionnement actuel est trop lourd pour l’utilisateur, il faut le simplifier, et ça ne vaut pas la peine d’implémenter dans une UI des choses qui vont devenir obsolètes (par contre, je le fais dans Ğcli parce que ça facilite les tests).

Tu fais référence à #113 ou à autre chose ? C’est bien de décrire un scénario d’attaque avant de se lancer dans des réflexions antispam. Dans l’idée de substrate, la seule chose qui entre en jeu contre le spam c’est de la monnaie, nous on a en plus le concept d’identité. Si on limite certains appels aux identité, ça permet de voir qui spamme et de faire intervenir de la modération humaine. Si celle-ci échoue, on peut ajouter un statut “blacklisted” à l’identité problématique.

Moi aussi, je rajouterais un hook appelé après une évaluation positive de la distance qui déclencherait la validation de l’adhésion. D’abord la refacto des pending membership qui devrait clarifier la question de la “validation” d’identité, et je pourrai faire dans la foulée.

Qu’appelles-tu N ??

:thinking: visiblement vous vous comprenez, mais pas moi :joy:

oui, le scénario étant simplement une demande de distance très fréquente de la part d’un compte membre ou non, valide ou en échec.

Le nombre de membres, pour la formule du DU.

On peut limiter la demande de distance aux comptes membres, ce qui n’est pas gênant puisque en pratique, elle pourra être faite par le cinquième certificateur (éventuellement sur option d’un résultat positif donné par un tiers).

Il n’y a pas besoin de le mettre à jour manuellement, c’est donné par le trait MembersCount dont voici l’implémentation :

impl<T: Config<I>, I: 'static> MembersCount for Pallet<T, I> {
    fn members_count() -> u32 {
        Membership::<T, I>::count()
    }
}

Nous échangions sous l’hypothèse du retrait des adhésions.

1 Like

Ce serait pas mal de ramener ce temps d’évaluation à 5 minutes comme dans Duniter v1. Par contre il faudra trouver un remplaçant pour on_new_session. Ça pourrait être un on_init avec un modulo, tout simplement.

Je réfléchis à ça (dans #159) et il y a deux cas pour lesquels on peut souhaiter une évaluation de la distance : claim_membership (qui remplace totalement validate_identity depuis !215) et renew_membership qui est l’autre moment auquel on est amené à demander une évaluation de la distance. Heureusement on peut entièrement le déduire du statut de l’identité :

  • Unvalidated → claim_membership → Member
  • Member → renew_membership → Member
  • NotMember → claim_membership → Member

Et si on a un autre statut (Unconfirmed, Revoked), le call ne fonctionnera juste pas.

Par contre, si on laisse la possibilité de l’appeler pour quelqu’un d’autre, ça permet de raviver le compte d’un membre inactif, ce qu’on ne souhaite pas. Et pour l’antispam sur request_distance_evaluation qui fonctionne actuellement en reserve / slash, un membre très riche pourrait demander l’évaluation de la distance pour toute la toile à chaque session sans rien perdre puisque les distances sont valides.

2 Likes

La solution pourrait être celle-ci : on ne peut appeler request_distance_evaluation que pour :

  • soi même
  • une identité qui est unconfirmed

Et après une évaluation de distance positive, la pallet wot regarde le statut de l’identité et si c’est unconfirmed, elle appelle claim_membership. Sinon elle regarde si le compte qui a demandé l’évaluation correspond à l’identité Ça devrait toujours fonctionner, mais si ça ne fonctionne pas, c’est pas grave, il suffit de le faire manuellement.

Comme ça on a un onboarding correct et une sécurité correcte.

[edit] :arrow_right: Draft: automatically claim membership (!219) · Merge requests · nodes / rust / Duniter v2S · GitLab

Au niveau des benchmarks, comment se passe l’étalonnage ? J’imagine que c’est la pire situation qui est retenue, n’est-ce pas ?

@bgallois a mis à jour la documentation pour écrire les benchmarks : docs/dev/weights-benchmarking.md · master · nodes / rust / Duniter v2S · GitLab. On y trouve ceci :

How to Write Benchmarks

Calls

Ensure that any extrinsic call is benchmarked using the most computationally intensive path, i.e., the worst-case scenario.

Hooks

Benchmark each hook to determine the weight consumed by it; hence, it is essential to benchmark all possible paths.

Handlers and Internal Functions

When designing handlers and internal functions, it is advisable to avoid having them return weight for the following reasons:

  1. Simplified Benchmarking: Writing benchmarks for hooks or calls where handlers and internal functions are utilized becomes more straightforward.
  2. Reduced Benchmarking Complexity: By directly measuring execution and overhead in a single pass, the number of benchmarks is minimized.
  3. Enhanced Readability: Understanding that weight accounting occurs at the outermost level improves the overall readability of the code.

One notable exception is the internal functions called in hooks like on_idle or on_initialize that can be easier to benchmark separately when the hook contains numerous branching.

Donc dans le cas des calls c’est bien la pire situation qui est retenue et elle sert à déterminer a priori si on inclut la call ou pas, mais dans le cas des hook ou des handlers qu’on va de toute manière inclure, on peut mesurer chaque chemin. L’idée est qu’ils vont être appelés quoi qu’il en soit et dans un contexte où il reste pas mal de poids : on_initialize (le poids du bloc), ou on_idle (le poids restant connu à l’avance), et donc on peut appeler la fonction et voir ce qu’il reste comme poids après.

Elois avait ouvert des tickets d’optimisation sur des “non mandatory inherents”. Je pense que l’idée était que plutôt d’utiliser des rouages internes, utiliser à la place des inherents, donc des calls non signés, non propagés, et ajoutés optionnellement par l’auteur du bloc. Mais je pense qu’on est encore très loin de devoir se poser ces questions d’optimisation.

Il faut regarder les poids pour se faire des idées d’ordre de grandeur pour éviter de passer de l’énergie à optimiser des aspects négligeables. Il y a un boulot de visibilisation des poids à faire dans la #128 pour qu’on puisse lire ça sur une page web plutôt que d’avoir à fouiller les valeurs dans le code.

1 Like

Si en une session il y a beaucoup d’évaluations, elles passeront toutes en un seul bloc qui peut donc être un peu lourd. On pourrait étaler les identités à traiter sur plusieurs blocs, au prix de compliquer un peu le storage.

Cependant réduire la période d’évaluation réduira d’autant le poids de l’inhérent puisqu’il y aura moins d’identités en moyenne donc autant faire ça d’abord.

2 Likes

Désolé pour ma non-réactivité à ce qui s’est passé sur GitLab ces derniers temps. J’ai enfin des vacances sans examens à la rentrée donc je vais pouvoir lire les notifs.

Je vais commencer par inclure l’oracle dans l’exécutable duniter, en sous-commande, pour simplifier la compilation, la livraison et l’installation. #140

Si c’est possible ce serait mieux de pouvoir le lancer en même temps que Duniter. À voir les avantages/inconvénients de coupler les deux à l’exécution (thread) : en cas de plantage on perd les résultats, mais la communication est plus rapide et propre (ni fichier ni RPC).

2 Likes

Le paramètre max_depth de 5 pas n’est pas strictement nécessaire dans le runtime car seul l’oracle l’utilise.

Cependant si on veut le modifier il faut que les calculateurs pensent à mettre à jour l’oracle ou à changer l’argument à la main. Grâce à la règle de la médiane, avec un ensemble constant de calculateurs la nouvelle valeur devrait entrer en vigueur dès que plus de la moitié a mis à jour. Mais si l’ensemble varie, les résultats seront un peu aléatoires pendant la transition.

Faut-il ajouter ce paramètre dans le runtime pour faciliter sa mise à jour par les mêmes processus de décision que les autres paramètres, même s’il apparaît comme du code mort dans le runtime ?

Il n’y aura pas de code mort, car le paramètre est affiché à la génération du Genesis dans les logs.

Oui je suis pour, il y a déjà des TODO dans le code.

edit : oops, tu as dit “dans le Runtime”. Mais peu importe, je ne sais même pas si tu auras un warning, après tout ce sera une constante exposée.

Avec toutes les macros je pense qu’un paramètre inutilisé n’est jamais du code mort selon le compilateur (puisqu’il est exposé dans des APIs).

C’est justement un problème : il n’y a pas de moyen systématique de vérifier qu’un paramètre ou une constante runtime est effectivement utilisé. Il faut juste y penser.

1 Like

Un inconvénient d’intégrer du code subxt dans Duniter :

La compilation dépend du fichier metadata, qui dépend d’une compilation préalable de Duniter.

En cas de changement des metadata, il faudra donc d’abord compiler Duniter sans la feature distance-oracle pour mettre à jour le fichier et pouvoir le compiler avec la feature.

Si d’autres fonctionnalités subxt sont ajoutées, tout ce code devrait être désactivable avec une feature unique.

Peut-être que cela risque de casser la CI en cas de changement de l’API de distance ?

1 Like

Ce fichier metadata.scale m’a toujours gêné dans sa génération. Il y aurait sûrement moyen de le générer à la compilation ? Un peu comme le Runtime.

2 Likes

Effectivement, j’essaye de rebase !226 et ça pose problème.

Il faudrait avoir un mode alternatif pour que subxt aille chercher ce qu’il lui faut directement dans le code plutôt que dans les metadata.scale, ce serait plus pratique.