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 
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 
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 
Oui pareil dans mes TU je ne set pas tout les champs du bloc 
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 
).
1 Like
@cgeek ça y ai les tests passent. Et mes tests manuels sur g1-test sont concluants
Du coup tu peut enfin merger ma MR et on vas enfin pouvoir tester la passage en V12 sur la G1-test 
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 ?
Pour moi oui. On pourra faire de même avec Verify a block signature with DuniterPy.
Je propose de le faire à l’occasion de l’annonce de la 1.7.20 pour la mise en production sur la Ğ1-Test.
Ça permettra aux forgeurs de comprendre la raison de ce changement de protocole, pour ensuite l’appliquer à la Ğ1.
Maintenant que Silkaj est basé sur DuniterPy 0.56, qui implémente la vérification de blocs, je vais pouvoir publier la commande de vérification des blocs en masse.
Je fais un aparté, mais j’ai pas bien compris la solution à ce problème :
Quelle a été la solution retenue ?
Le problème était spécifique aux tests et était du au fait que le champ issuer du bloc n’étais pas défini dans les TU, donc il n’y a plus rien a contourner, on utilise la clé publique du trousseau la place (c’est la même valeur dans tout les cas).
À ce que j’ai compris, le nœud va toujours générer des blocs valides devant la vérification ?
Il va utiliser la bibliothèque non buggée ?
Non il génère des blocs comme avant (pour ne pas perdre en perf), mais maintenant il vérifie la validité de la signature si la pow est valide, ce qu’il ne faisait pas avant. Ainsi, si la signature est invalide le bloc ne sera même pas envoyé au réseau, et la pow va continuer comme si le bloc trouvé n’avais pas le bon hash.
2 Likes
Ok, merci, ça répond à ma question.
Du coup, quelle était la problématique pour ne pas pouvoir utiliser une bibliothèque performante (en langage compilé) et non buggé ?
C’est pas un problème récurrent, mais quand même. Ça a lieu tous les x fois dans la fourchette 50 − 80 000 blocs selon les chiffres ci-dessus.
la problématique c’est qu’il était hors de question pour moi de me lancer dans du binding C ou C++, ça aurait rendu la contribution infiniment plus compliquée et chronophage a effectuée, je n’y serait peut être même pas arrivé, le binding entre node et C/C++ est vraiment très très compliqué notamment pour ne pas casser les builds des différents artifacts ça peut etre un vrai casse-tête.
S’il n’y avait pas le problème de VM windows pas a jours, j’aurais fait un binding Rust, infiniment plus simple a faire et plus safe, je suis toujours ok pour migrer la crypto de Duniter en Rust le jours ou le problème avec la VM windows sera réglé.
Ok, merci pour l’explication 
1 Like
Discussion rendue publique pour raison d’archivage, le bug en question a été corrigé dans Duniter 1.7.21.