Désolé si je te fais répéter, mais pour être bien clair : Dunitrust ne vérifiera pas la signature de ces blocs ? Et quid de la vérification du reste des règles du protocole v10 et v11 ?
2 Likes
Au contraire, tu a raison, tout changement de protocole doit être parfaitement clair, je formaliserai ça dans la RFC v12 (et j’en profiterai pour déplacer au préalable la RFC v12 actuelle en v13).
Dans un premier temps non, si une liste exhaustive des blocs v10 et v11 invalide est faite alors je l’inscrirai en dur dans Dunitrust et il vérifiera alors tout les blocs v10 et v11 qui ne sont pas dans cette liste.
Toutes les autres règles seront toujours vérifiées, il s’agit de ne skipper que la vérification de la signature et que pour les blocs dont le champ version est <= 11 
1 Like
D’après le billet détaillé sur le bug, ce dernier ne présente pas de risque de sécurité, on peut donc en parler publiquement sans crainte :
However, beyond the wrongness of the resulting x-coordinate3, and because this error happens at the end of computations steps there is no risk it could lead to greater damages like for instance revealing bits of user’s secret.
Cadeau :
[15144, 31202, 85448, 87566, 90830, 109327, 189835, 199172, 221274, 253582]
Ça offre une nouvelle commande à Silkaj : verify. La vérification a été plutôt rapide sur un cœur.
Même punition sur ĞTest :
[24316, 62067, 62551, 93288, 173118, 183706, 196196, 246027, 247211, 263207, 307038, 328741, 335914, 377316, 395714, 396024, 407913, 422366, 496751]
Donc, rien de régulier entre les deux monnaies ni entre les numéros de blocs au sein de la même monnaie.
1 Like
Merci @Moul, a noter que (et je l’ai indiqué dans la RFC v12) ont a potentiellement le même problème avec les transactions signées par Duniter, je pense notamment aux transactions émises par remuniter.
Et je sais pour avoir eu le problème qu’il existe au moins 1 transaction invalide dans la blockchain g1 
1 Like
C’est valable pour tout ce qui est signé, notamment identités, certifications, adhésions. Et vu les volumes, il est tout à fait possible que ce soit déjà arrivé sur un document de WoT.
Les documents signés par cette bibliothèque boguée intégrée dans Duniter sont les blocs et potentiellement les transactions de Remuniter. Je sais pas quelle bibliothèque Remuniter utilise pour signer ses transactions.
La signature des autres documents devraient être valides, étant donné que les clients utilisent libsodium (j’ai pas vérifié pour Césium).
2 Likes
Ah oui pas faux désolé, j’ai parlé trop vite.
Cesium Android utilise la libsodium (la lib C).
Pour Cesium desktop et web, je dois vérifier. Le nom du fichier JS est nacl_factory, mais je ne sais plus si cela provenait de TweetNacl ou nacl-js.
Je regarde dès que j’ai machine sous la main.
2 Likes
OK, donc pour la version web, Cesium utilise js-nacl v1.1.0
Dans lea prochaine version, je vais faire la mise à jour en version > 1.3.0 qui est basé sur libsodium 1.0.11.
2 Likes
@cgeek @Moul @Inso voici la MR: https://git.duniter.org/nodes/typescript/duniter/merge_requests/1283
J’ai découpé les changements en une quinzaine de petits commit pour plus de lisibilité
D’abord 3 commits pour rétablir la CI sur la branche 1.7 (la CI n’avais pas tournée depuis plus de 6 mois sur 1.7 et ne fonctionnais plus).
Puis 11 commits pour le dev a proprement parlé, j’ai pris l’initiative de renommer certaines constantes pour plus de clarté.
Pour reviewer, je vous invite a regarder les changements commit par commit et à lire les nom des commits, ça vous aidera a comprendre les changements plus rapidement.
J’ai également rajouté un test d’intégration pour tester les sauts de version du protocole (test/integration/protocol-version-jump.ts), ce qui m’a parmi de constater un bug dans la formule suivante :
// More than 70% of the computing network converted? Let's go to next version.
if (Math.floor(nbNoncesWithNextVersionCode / uniqIssuersInFrame.length) > 0.6) {
return constants.BLOCK_NEW_GENERATED_VERSION
}
La présence de Math.floor impliquait que le saut de version ne pouvait se faire que si l’on avait 100% des issuers de la fenêtre courante qui étaient prêt, ce qu’on a du avoir pour le passage en V11 du coup on ne s’en ai pas aperçu 
@cgeek peut tu reviewer cette MR quand tu aura un peu de temps ? N’hésite pas a me faire part de tes retours
7 Likes
Merci Eloïs, je regarde dès que je trouve un créneau. Il faut bien me laisser une semaine je dirais.
Oui je me rappelle de ce point. Il y avait bien des tests pourtant, mais ceux-ci simulaient un accord à 100% sans que je m’en sois rendu compte. Heureusement c’est passé quand même, mais je crois que ça s’est fait un petit peu dans la douleur (fork).
2 Likes
Humble review faite pour ma part.
Beau boulot !
1 Like
Merci je vais essayer de traiter tes retours dans le Week-End (possiblement dimanche) 
@Moul c’est bon j’ai traité tous les retours de ta review, peut tu passer tes remarques en résolues ou apporter des précisions si elles en le sont pas ?
@cgeek en traitant les retours de moul j’ai un test qui est passé au rouge alors qu’il était vert lors de mon dev initial. J’ai rejoué ce test sans mon dev (branche test-ci), et le test est rouge quand même, il n’a donc a priori pas de lien avec mon dev et est devenu rouge pour une autre raison inconnue.
Il s’agit du test test/integration/protocol/v0.5-transactions.js. Il échoue à la ligne 41 :
await cat.sendMoney(51, tac);
Après analyse en debug, il s’avère que cat n’a aucune source de monnaie. La cause : aucun DU n’a été créé dans les 3 blocs commités. J’ai l’intuition qu’il y a un lien avec le « temps » du premier DU mais je n’ai rien trouvé après 1h d’investigation, peut tu regarder ce test ? Merci
@cgeek je vois que tu a corrigé le problème avec le test v0.5-transactions.js, du coup je vais me rebaser sur 1.7 dés que tu y aura rétabli la CI
1 Like
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
Oui j’ai vu, je suis ok avec ta remarque, je ferai ça quand je peut (peut-être ce soir sinon dans la semaine)
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