Bug: firstEligibleUd is not incremented for some validated identities

Nodes Duniter-v2s
1

J’ai l’impression que les identités migrés ne créés pas leurs DU.

par exemple: 5D2HHRj6zCEL9iY3CBpKy1mBMtpXUJCFDNvevBqVQAWdzqSS

Il est bien membre, mais j’ai l’impression qu’il ne créé pas de DU, est-ce que je me trompe ?

1 Like
2

J’ai l’impression que moi non plus je ne crée pas de DU.

1 Like
3

Le DU doit maintenant être réclamé manuellement, via l’extrinsic universalDividend.claimUds. C’est pour tous les comptes pareil.

4

Je sais bien, j’ai implémenté dans gecko la vue solde avec DU non réclamés depuis 1 semaine.

firstEligibleUd ne bouge plus j’ai l’impression, c’est censé tomber à quelle heure le DU sur la GD2 ?

5

J’ai réclamé mon DU tout à l’heure, maintenant mon firstEligibleUd vaut 15, ce qui est bien le nombre de jours de la blockchain.

Il me semble que c’est à l’heure du premier bloc, donc vers 01:49.

Du coup j’ai refait un claim, c’est passé à 16 et j’ai bien gagné des G1.

6

oui pareil, mais par contre pas pour ChristCosmic ni zombie, qui ont changé de ownerkey.

21:

{
  data: {
    firstEligibleUd: 0
  }
  nextCreatableIdentityOn: 0
  oldOwnerKey: [
    5GAT6CJW8yVKwUuQc7sM5Kk9GZVTpbZYk9PfjNXtvnNgAJZ1
    166,121
  ]
  ownerKey: 5D2HHRj6zCEL9iY3CBpKy1mBMtpXUJCFDNvevBqVQAWdzqSS
  removableOn: 0
  status: Validated
}
7

Ah oui, dans le code je ne vois rien qui migre firstEligibleUd vers la nouvelle clé lors d’un changeOwnerKey (c’est stocké dans une table différente des identités, indexé par clé publique et non par identité).

3 Likes
8

Ton analyse est fausse @tuxmain, MembersStorage n’est pas une map à part mais une «vue» sur le storage des identités, il n’y a donc rien à migrer.

D’ailleurs le vrai bug n’a absolument rien à voir avec le changement de clé associée à une identité, c’est pourquoi @Maaltir est également impacté.

Je sais d’où vient le bug, mais je ne vais pas le dire tout de suite, car j’aimerais bien que @tuxmain arrive à comprendre pourquoi son analyse est fausse et à trouver la vraie cause, c’est un bon exercice pour progresser.

En tous les cas, je coderais test et fix dès que je trouverais le temps :wink:

3 Likes
9

Pauvre @tuxmain qui ne va plus oser diag quoi que ce soit :sweat_smile:

10

En y regardant de plus prêt, toutes les identités créés et certifiés après le genesis ont un firstEligibleUd à 0.

Je ne suis même sûr qu’il ai bougé une seule fois pour une identité non genesis depuis la GD2.
Ca a donc probablement avoir avec le validateIdentity mechanism je dirais.

1 Like
11

C’est pas le but recherché, au contraire, j’ai besoin que d’autres personnes que moi soit capables de diagnostiquer les bugs dans duniter v2, et pour ça forcément il faut essayer, c’est en faisant des erreurs qu’on apprend :slight_smile:

Ici, l’erreur de @tuxmain a été de considérer que MembersStorage était un storage item, alors que ce n’ait qu’un type associé qui permet d’accéder à des données, ce n’est pas un storage item en lui-même.

J’ai employé le terme « vue » car c’est la notion analoque la plus proche en DB relationnelle, et comme tuxmain a parlé de « table » j’ai pensé que ça lui parlerait.

De mon côté j’ai codé un test qui me confirme que le bug correspond bien a ce que j’ai identifié, et je confirme que ça n’a absolument aucun rapport avec changeOwnerKey.

Tu brûles, bien joué, analyser le storage est un très bon réflexe, c’est comme ça que j’ai fait ce midi quand j’ai découvert ce fils.

1 Like
12

Je ne comprends pas.

Dans validate_identity, l’événement MembershipAcquired est émis avant d’insérer l’identité dans Identities. Un handler initialise first_eligible_ud lors de cet événement, seulement s’il trouve l’identité dans Identities. Mais il me semble que l’événement ne peut pas être lu avant la fin de la fonction, donc l’identité doit bien avoir été insérée avant que le handler passe, donc ça devrait être bon.

Et c’est quand même perturbant qu’une fonction commençant par can_ ait des side effects.

13

Oui c’est exact

L’evenement n’ait jamais “lu” par la blockchain, il est lu uniquement par le monde extérieur et les tests. Certains évènements (et c’est le cas ici) sont couplés à une sorte de callback pour pouvoir notifier le runtime, mais la callback est bien exécuté immédiatement, pas à la fin du call.

Justement non, et c’est là tout le problème.

Oui, ça ne devrait pas, le problème c’est qu’on ne peut pas claim le membership dans le handler du identity event, car si ça échoue il faut rollback la validation de l’identité. Donc on n’a nécessairement besoin d’une callback faillible avec side effect quelque part.
Si tu veux essayer de refacto cette partie plus clairement, n’hésite pas, les tests sont là pour garantir que tu ne casseras pas tout :slight_smile:

2 Likes
14

C’est à cause de cette ligne, c’est bien ça ?

T::OnEvent::on_event(&sp_membership::Event::MembershipAcquired(idty_id, metadata));
15

Cette ligne appelle la callback, mais ce n’est pas la cause du bug, il n’y a pas besoin de la modifier pour le fix (que j’ai déjà codé).

16

Je n’avais pas pris le temps de réactiver les sanity tests sur la 2ème itération de la gdev, si je les avais réactivés j’aurais été informé de ce bug dès le 1er nouveau membre post-genesis.

Je viens de remettre en place la scheduled pipeline, tous les jours à 9h am.

2 Likes
17

Quant au bug, il est testé et corrigé, et j’ai releasé le runtime 303 qui contient le fix, puis j’ai corrigé le storage.

Tout devrait être rentré dans l’ordre, @poka et @Maaltir vous confirmez ?

Je n’ai toujours pas lu d’explication satisfaisante du bug, donc je ne sais pas si quelqu’un d’autre que moi l’a réellement compris: le bug était causé par un problème d’ordre des instructions de lecture/écriture qui causais « l’effacement » d’une écriture précédente.

Dans l’exécution du call validate_identity il y a deux bout de codes différents qui modifient l’identité, un dans le call lui-même, qui passe l’identité à l’état « validé », et un autre dans la pallet universal dividend, qui initialise la valeur du champ firstEligibleUd.

Le problème, c’est que le 2ème bout de code était appelé au milieu de l’exécution du 1er, en gros on avait quelque chose du genre:

  1. Lecture de l’identité et stockage dans une variable locale idty_value
  2. Appel de la callback can_validate_identity, qui fait des contrôles, réclame le membership, et initialise le champ firstEligibleUd de l’identité.
  3. Modification de la variable locale idty_value (notamment passage au statut « validé »)
  4. Écriture de idty_value dans le storage.

Il y a deux écritures sur l’identité, aux étapes 2 et 4. le problème, c’est que l’étape 4 « écrase » les données écrites lors de l’étape 2.

Voila l’explication que j’aurais aimé que l’un de vous me donne, bon je ne vous ai laissé que 6h, mais c’était plus que suffisant :stuck_out_tongue:

EDIT: le fix que j’ai choisi: déplacer l’initialisation du champ firstEligibleUd à la fin du call, dans la callback de l’event sur l’identité.

Vous pouvez lire le test et le fix dans les 3 derniers commit de la branche elois-fix-idty-post-genesis:

1 Like
18

Je confirme que le storage a bien été corrigé pour tous les comptes membres de mon coffre !

Merci pour les explications, j’ai pu comprendre quel était l’origine du problème.

1 Like
19

Je confirme j’ai plein de gdev sur mon compte. :star_struck: