Ğ1 potentiellement bloquée : Fofo − ruleMembershipEnoughCerts

En vrai j’ai trouvé l’'explication il y a déjà 4h mais je voulais être sûr que c’était ça (donc vérifier par un test), avant d’en parler :slight_smile:

Merci à @gerard94, c’est à la lecture de son post que j’ai deviné l’origine du problème.

Il s’agit du bug #1394 qui n’a été que partiellement corrigé, voir ce sujet : Ancien membre de retour avec seulement 4 certifications

Après avoir lu le post de gerard, j’ai été vérifié dans l’historique git les commit de cgeek en lien avec ce ticket et j’ai effectivement constaté que le code de génération d’un bloc n’avait pas été modifié, seule la vérification d’un bloc avait été corrigée.

Or, si le bug #1394 s’est produit a l’époque, c’est nécessairement que le code de génération d’un bloc a généré un bloc faisant rentrer un membre avec 4 certifications, donc il y a forcément aussi un problème avec le code de génération d’un bloc.

Je n’avais pas vraiment regardé en détail ce bug et le correctif de @cgeek à l’époque, donc je n’avais pas tilté de point.

Donc convaincu qu’il restait un bug à débusquer dans le code de génération d’un bloc, je me suis plongé dans cette partie du code a la recherche de l’anomalie, et je l’ai trouvée :

On voit bien que si un même issuer se trouve dans links et newLinks, il sera comptabilisé deux fois :

async checkHaveEnoughLinks(target: string, newLinks: any): Promise<any> {
    const links = await this.dal.getValidLinksTo(target);
    let count = links.length;
    if (newLinks[target] && newLinks[target].length) {
      count += newLinks[target].length;
    }
    if (count < this.conf.sigQty) {
      throw (
        "Key " +
        target +
        " does not have enough links (" +
        count +
        "/" +
        this.conf.sigQty +
        ")"
      );
    }
  }

Corrigé ce code fût plutôt facile, le plus délicat fût de reproduire le bug via un test automatisé. Ce bug étant pour moi dans la continuité du #1394, j’ai adapté le test reproduisant ce dernier :

--- a/test/integration/fork-resolution/coming-back-with-less-than-sigqty.ts
+++ b/test/integration/fork-resolution/coming-back-with-less-than-sigqty.ts
@@ -81,17 +81,8 @@ describe('A member coming back with less than `sigQty` valid certs total', () =>
     await s1.commit({ time: now + 13 })
     await s1.commit({ time: now + 13 })
     const c1 = await cat.makeCert(toc) // <-- a renewal ==> this is what we want to observe
-    const join = await toc.makeMembership('IN');
-    
-    // Inject c1 & join in mempool
-    await cat.sendCert(c1)
-    await toc.sendMembership(join)
-
-    // Generate potential next bloc, must NOT include toc join (#1402)
-    const b_gen = s1.generateNext({ time: now + 13 })
-    assertEqual((await b_gen).joiners.length, 0);
-
-    // Try to force toc coming back, must be fail because toc not have enough certs (#1394)
+    const join = await toc.makeMembership('IN')
+    // toc is **NOT** coming back! not enough certs
     await assertThrows(s1.commit({
       time: now + 13,
       joiners: [join],
@@ -99,13 +90,13 @@ describe('A member coming back with less than `sigQty` valid certs total', () =>
     }), 'BLOCK_WASNT_COMMITTED')
     // BUT is coming back with 1 more cert
     const c2 = await tac.makeCert(toc)
-    const b2 = await s1.commit({
+    const b = await s1.commit({
       time: now + 13,
       joiners: [join],
       certifications: [c1, c2]
     })
-    assertEqual(b2.membersCount, 3) // <--- toc is welcome back :)
-    assertEqual(b2.number, 12)
+    assertEqual(b.membersCount, 3) // <--- toc is welcome back :)
+    assertEqual(b.number, 12)
   })

Voici comment reproduire le bug chez vous :

nvm use 10
git checkout 04d16320ae613ce1db30a510f6002cf96604dc2b
yarn
 ./node_modules/mocha/bin/_mocha --opts "" --timeout 20000 test/integration/fork-resolution/coming-back-with-less-than-sigqty.js

Vous devriez obtenir beaucoup d’infos sur la sortie standard, dont ceci a la fin :

    1) (t = 13) toc is NOT coming back with 1 cert only!


  3 passing (2s)
  1 failing*


  1) A member coming back with less than `sigQty` valid certs total (t = 13) toc is NOT coming back with 1 cert only!:

      AssertionError [ERR_ASSERTION]: 1 == 0
      + expected - actual

      -1
      +0
      
      at Object.assertEqual (test/integration/tools/test-framework.js:47:12)
      at test (test/integration/fork-resolution/coming-back-with-less-than-sigqty.js:84:26)
      at process._tickCallback (internal/process/next_tick.js:68:7)

C’est cette ligne qui échoue :

assertEqual((await b_gen).joiners.length, 0);

Ce qui signifie que le bloc généré contient un joiners alors qu’il ne devrait pas, CQFD.

Vous pouvez ensuite checkout la branche fix/1402 puis rejouer le même test et vérifier que cette fois-ci il est vert :

nvm use 10
git checkout fix/1402
yarn
 ./node_modules/mocha/bin/_mocha --opts "" --timeout 20000 test/integration/fork-resolution/coming-back-with-less-than-sigqty.js

La Ğ1 s’est débloquée car Fofo a obtenu 2 certifications supplémentaires émises au bloc 318048 et 318053, ce qui lui a permis d’avoir sufisamment de certifications pour que le bloc contenant son renouvellement soit accepté.

Si Fofo était resté à 4 certifications en attente pour lui, la Ğ1 serait restée bloquée et nous aurions été obliger de publier un correctif en urgence.

La situation étant résolue, et étant donné que je compte publier une nouvelle version 1.8 de Duniter incessamment sous peu, je pense que ce correctif attendra Duniter 1.8 :slight_smile:

8 Likes