Duniter utilise une ancienne version buggée de TweetNaCl: que faire?

Oui je n’ai pas compris non plus pourquoi ce test échoue soudainement, mais ce n’est rien d’important car ça concerne un très vieux test. Je pense que c’est un problème d’isolation des tests entre eux.

1 Like

C’est bon j’ai terminé ma review, il n’y a qu’un seul point à prendre en compte de mon côté.

J’ai aussi ajouté un commit suite au rebase.

Il me reste à regarder pour le job « integration » qui ne passe plus, mais cela vient plutôt des mes commits pour le bug#1396.

1 Like

Merci pour la review :slight_smile:

Oui j’ai vu, je suis ok avec ta remarque, je ferai ça quand je peut (peut-être ce soir sinon dans la semaine) :sweat_smile:

Ok, aussi j’ai effectué un nouveau rebase pour corriger l’anomalie sur le job integration.

Autrement dit j’ai fait un git push --force-with-lease et donc ta branche locale sera écrasée par la nouvelle quand tu tireras les modifications.

1 Like

J’ai oublié de mentionné hier que la relecture de cette MR a été l’occasion pour moi de ressentir un sentiment très positif que je n’avais pas eu depuis longtemps.

Celui de ne plus être le seul « gros développeur » sur le cœur et savoir que je peux presque partir l’esprit tranquille. Bon, je sais bien que tu vas rester sur Dunitrust, pas de problème :wink: mais ça fait plaisir quand même.

4 Likes

En vrai j’étais très motivé pour oxyder progressivement Duniter plutôt que de faire une autre implémentation from scrath. Mais les soucis avec la VM windows m’ont démotivé à l’époque :confused:

Pour rappel, avec Neon, le Rust et le NodeJS se marient très très bien, y compris avec mingw et la version desktop fonctionnais nickel sous linux. Le seul bloquage est la livraison sous windows qui demande une VM a jours.

Si ce point était débloqué ça pourrait me remotiver a oxyder Duniter. Par exemple je saurais très facilement remplacer naclb et scryptb par une crate Rust, ainsi le problème de performance soulevé par tweetnacl-js ne se poserai plus :slight_smile:

1 Like

Oui, après il y a d’autres problématiques à oxyder Duniter. Le principal soucis que je vois c’est le débogage : soit tu débogues le TypeScript, soit le Rust, mais les deux en même temps je ne suis pas sûr que ce soit possible. Dans tous les cas ça complexifie ce processus.

Ensuite ça impose la forme de Duniter à l’implémentation Rust sous-jacente au début. Bien sûr, petit à petit Duniter lui-même peut s’adapter pour adopter la forme imposée par l’implémentation Rust, mais au début c’est assez contraignant.

Par contre ça apporte d’autres avantages dont la suppression de l’effet tunnel que connaît actuellement Dunitrust (du fait des mécaniques manquantes pour tourner déjà en tant que nœud à plein temps), mais aussi la couverture fonctionnelle offerte par les TF que possède Duniter.

Je t’avoue que moi-même ça me motiverait bien cette histoire.

4 Likes

@cgeek bon ça fait 4 soirs que je me prends un mur avec mocha, pour une raison que j’ignore le check de la sig de la preuve dans un worker bloque, l’appel a la fonction verify ne retourne jamais, la fonction est appelée puis…le programme bloque et ne rend jamais la main, on est alors obligé de le tuer.

Plus précisément :

mocha sur le seul fichier test/fast/prover/prover-pow-1-cluster.js -> bloque.
débogueur vscode sur le seul fichier test/fast/prover/prover-pow-1-cluster.js -> bloque.
mocha all tests -> tous les tests dans test/fast/prover/prover-pow-1-cluster.js échouent sauf le premier.

Je ne peux pas déboguer cette partie avec vscode, quand je place un breakpoint sur la ligne app/modules/prover/lib/proof.ts:216 le debuggeur ne l’atteint jamais (à mon avis c’est le côté multi-thread qui n’est pas géré comme il faut par le déboguer de vscode).

De plus, si je commente les lignes 215 à 220 inclus, tout refonctionne normalement, tous les tests passent.

Enfin, si j’appelle la fonction verify puis que je force a true la valeur de sigOk, comme ceci :

if (found) {
  let sigOk = verifyFunc(raw, sig);
  sigOk = true;
  if (!sigOk) {
    found = false;
  }
}

Alors le programme bloque et ne rend jamais la main. Ce n’est donc pas que la fonction verify renverrait false, la fonction verify ne rend pas la main.

Plus troublant encore, le problème est spécifique aux tests, quand je déploie mon noeud de dev sur la g1-test j’arrive à trouver des blocs, il ne bloque pas à l’appel de la fonction verify :

2020-01-23T23:04:32+01:00 - info: ENGINE c#0#3 HAS FOUND A PROOF #000241A6331FEB976CCB9F508F07B26AB30BD34A4F32B7E59BDFC6B0AD75DEAD
2020-01-23T23:04:32+01:00 - info: Matched 3 zeros 000241A6331FEB976CCB9F508F07B26AB30BD34A4F32B7E59BDFC6B0AD75DEAD with Nonce = 10099900001233 for block#506074 by 42jMJt
2020-01-23T23:04:32+01:00 - info: Done: #506074, 000241A6331FEB976CCB9F508F07B26AB30BD34A4F32B7E59BDFC6B0AD75DEAD in 5.57s (~9856 tests, ~1769.80 tests/s, using 8 cores, CPU 60%)
2020-01-23T23:04:32+01:00 - info: FOUND proof-of-work with 3 leading zeros followed by [0-6]!
2020-01-23T23:04:32+01:00 - info: SIDE Block #506074-000241A6 added to the blockchain in 0 ms
2020-01-23T23:04:32+01:00 - info: Block resolution: 1 potential blocks after current#506073...
2020-01-23T23:04:32+01:00 - info: Block #506074 added to the blockchain in 14 ms
2020-01-23T23:04:32+01:00 - trace: PoW loops = 4
2020-01-23T23:04:32+01:00 - info: Block resolution: 0 potential blocks after current#506074...
2020-01-23T23:04:32+01:00 - warn: Waiting 0ms before starting to compute next block...
2020-01-23T23:04:32+01:00 - trace: PoW loops = 5
2020-01-23T23:04:32+01:00 - info: Generating proof-of-work with 7 leading zeros followed by [0-9A-D]... (CPU usage set to 60%) for block#506075 42jMJt
2020-01-23T23:04:39+01:00 - info: [42jMJtb8] ⬇ PEER 42jMJtb8 506044-0
2020-01-23T23:04:39+01:00 - info: [42jMJtb8] ✔ PEER 42jMJtb8 506044-0
2020-01-23T23:04:39+01:00 - info: [42jMJtb8] ⬇ PEER 42jMJtb8 506044-0
2020-01-23T23:04:55+01:00 - info: Matched 3 zeros 00013C808392D83FF1DAE344E319691D9BD5581EAEACFFBBCE167B17982391DF with Nonce = 10099900005970 for block#506075 by 42jMJt
2020-01-23T23:04:55+01:00 - info: Matched 3 zeros 00013C808392D83FF1DAE344E319691D9BD5581EAEACFFBBCE167B17982391DF with Nonce = 10099900005970 for block#506075 by 42jMJt
2020-01-23T23:04:59+01:00 - info: Matched 3 zeros 000BF259F6D78321A6E5DDF555CC0605E691479AEA623F00E849738E942464A2 with Nonce = 10099900006616 for block#506075 by 42jMJt
2020-01-23T23:04:59+01:00 - info: Matched 3 zeros 000BF259F6D78321A6E5DDF555CC0605E691479AEA623F00E849738E942464A2 with Nonce = 10099900006616 for block#506075 by 42jMJt
2020-01-23T23:04:59+01:00 - info: Matched 3 zeros 000BF259F6D78321A6E5DDF555CC0605E6

Oui la diff est très faible sur g1-test en ce moment ça m’arrange bien pour mes tests :stuck_out_tongue:


Ce comportement bizarre des tests auto. est au delà de mes compétences, ça fait 4 jours que j’essaye et je n’ai aucune piste ni aucune explication :confused:
Il semblerait que le NodeJs reste hélas un écosystème qui m’échappe, c’est pas faute d’essayer pourtant, mais je ne bloque jamais autant et aussi longtemps en Java ou en Rust :laughing:


Je vois bien une solution de contournement, j’aimerais savoir ce que tu en penses :

On pourrait laisser proof.ts inchangé mais seulement utilisé la fonction verify non buggée en validation locale des blocs v12. Ainsi, si le noeud génère par malchance un bloc avec sig invalide (1 chance sur 2^64), il va refuser son propre bloc, est ce que ça te semble ok comme solution ?

Je vais jeter un œil, déjà. Lors de la relecture de la MR une chose m’a fait tiqué dans proof.ts donc je vais vérifier ça.

1 Like

Rien à voir avec ce que je pensais.

C’est parce que la variable block.issuer est undefined à l’exécution. Si tu remplaces block.issuer par pair.pub dans proof.ts ça fonctionne bien.

3 Likes

@cgeek je viens de tester, et en effet utiliser pair.pub fonctionne mais le mystère reste entier pour moi :

  • Comment voit tu que block.issuer est undefined a l’execution ? (graçe au débogueur de webstorm ?)
  • Pourquoi l’appel de la fonction verify avec un param undefined fait bloqué le worker dans les tests ?
  • Pourquoi le problème ne se produit que dans les tests ?

La j’ai un truc qui marche sans comprendre pourquoi ça marche :slight_smile:

Quand le débogueur n’est pas disponible, il reste le débogage à coup de console.log. C’est là que j’ai pu voir l’état des variables au moment du verify().

Parce que la fonction verify() plante ensuite quand il faut vérifier la signature par rapport à la clé publique, qui est “undefined”.

Le test en question est un test unitaire, il envoie un bloc sans émetteur. Mais c’est aussi bien, car a priori c’est bien la clé publique associée au trousseau local qui signe. Mettre pair.pub est la solution la plus naturelle pour proof.ts à mon sens.

On pourrait contourner en laissant block.issuer et en modifiant le test, mais pour moi ce n’est pas justifié.

2 Likes

J’ai essayé de rajouter des console.log mais il ne s’affichaient pas dans ma console, il y a une option particulière a donner a mocha pour qu’il printe les console.log ?

Pourquoi l’erreur ne remonte pas ? C’est avec ces erreurs “silencieuses” que j’ai le plus de mal :confused: En Java j’aurais eu une NPE explicite et en Rust ça aurait été détecté a la compilation. Je trouve perso que ces erreurs “'silencieuses” font que NodeJs est plus difficile a utiliser.

Merci, c’est l’info qui me manquais pour comprendre :slight_smile:
Oui pareil dans mes TU je ne set pas tout les champs du bloc :wink:

Non aucun, le stdout des sous-processus sont bindés à celui du principal.

Je ne sais pas, c’est le multi-processus qui fait ça. Je suis bien d’accord que là-dessus Node complique la tâche. En même temps à la base il n’est pas du tout fait pour du calcul intensif.

1 Like

@cgeek Concernant la CI, le test PoW Cluster should answer within 50ms for a basic PoW réussi chez moi mais plante avec le runner redshift.
C’est le temps de vérification de la signature qui rend l’opération légèrement plus longue, sur mon poste de dev ça reste en dessous de 50ms mais le runner redshift n’est pas assez puissant pour ça.

Est ce qu’on rajoute un délai ?

Oui ce sont des cas de tests sensibles à la machine, c’est un peu nul. On peut mettre un peu plus long c’est pas gênant.

1 Like

ok je tente avec 100ms on vas voir si ça passe (long a tester vu que faut faire tourner l’IC a chaque fois :stuck_out_tongue:).

1 Like

@cgeek ça y ai les tests passent. Et mes tests manuels sur g1-test sont concluants :slight_smile:

Du coup tu peut enfin merger ma MR et on vas enfin pouvoir tester la passage en V12 sur la G1-test :smiley:

4 Likes

MR acceptée.

J’ai également mergé la branche 1.7 dans dev.

A noter que désormais je suis assez fan de Git Flow et que j’ai décidé de suivre cette méthode pour Duniter. Donc toute MR sera effectuée en --no-ff, c’est-à-dire qu’un commit de merge est présent et que l’on conserve “la branche” dans l’historique de commits même après sa suppression.

De cette façon on retrace mieux l’histoire du code.

J’ai modifié les paramètres GitLab du dépôt Duniter à cette fin.

2 Likes

Étant donné que le correctif est mergé, que ce bug ne permet pas d’exploit connu, et que Duniter avec correctif va être livré très prochainement, peut-on clore ce sujet et le rendre public a des fin d’archivage public ?