Il n’y avait pas d’erreur de logique à ce niveau, et pas plus ailleurs dans le protocole. Je m’explique.
L’idée générale c’était que pour la Ğ1 l’adhésion expire au bout d’un an (BR_G93), tandis que la révocation implicite se fait au bout de 2 ans (BR_G96). Pour Ğ1-Test c’est plus court, mais on parle en mois tout de même.
Or le temps blockchain n’avance pas de plusieurs mois en 2 blocs. Donc le conflit est impossible avec ces paramétrages :
expires_on = MedianTime + msValidity
revokes_on = MedianTime + msValidity*2
Il n’y a pas besoin de rajouter une condition pour éviter que l’opération se déroule « en même temps », les actions se dérouleront à plusieurs milliers de blocs d’intervalle.
Pourtant, tu me diras, manifestement il y a conflit lors de la synchro. Mais là aussi j’ai bien fait attention à ce que BR_G93 ne se déclenche qu’une fois :
If `MS.expired_on == null OR MS.expired_on == 0` Then
C’est-à-dire que la règle BR_G93 n’agit que si le membre n’a pas déjà son adhésion expirée.
Mais alors pourquoi passe-t-on quand même dans cette condition au même bloc que la révocation implicite (au bloc 79.000+) alors que gerard94 est censé voir son adhésion expirée 40.000 blocs plus tôt ?! Dit autrement, tout se passe comme si l’expiration de l’adhésion de gerard94 au bloc 39312 n’avait pas été enregistrée. Et en effet, c’est bien ça le soucis : la synchro rapide omettait de déclencher cette mise à jour.
D’où le fait que le bug ne se reproduisais pas dans les tests, puisque précisément il n’y avait pas de bug à ce niveau hors « synchro rapide ». Les nœuds ne pouvaient pas planter, il avaient eu 40.000 occasions de bien faire expirer l’adhésion.
Bon et donc quel était le problème ? Dans la synchro rapide, ce code là :
for (const entry of index) {
if (entry.op === 'CREATE' && (entry.expires_on || entry.revokes_on)) {
sync_expires.push(entry.expires_on || entry.revokes_on);
}
}
Aurait plutôt dû être comme ça :
for (const entry of index) {
if (entry.expires_on) {
sync_expires.push(entry.expires_on)
}
if (entry.revokes_on) {
sync_expires.push(entry.revokes_on)
}
}
Traduction :
- déjà le
entry.op === 'CREATE'
est complètement hors sujet (@elois l’a bien vu) - et aussi on a besoin de refaire les calculs d’expiration à la fois au moment
entry.expires_on
mais aussi au momententry.revokes_on
, pas juste l’un ou l’autre.
Mais ce n’est pas le cœur du bug. C’est plutôt ce code là qui est censé déclencher le recalcul (en lien avec le code précédant) :
let currentNextExpiring = sync_nextExpiring
sync_nextExpiring = sync_expires.reduce((max, value) => max ? Math.min(max, value) : value, sync_nextExpiring)
const nextExpiringChanged = currentNextExpiring !== sync_nextExpiring
Or ce code de déclenche JAMAIS le recalcul, car nextExpiringChanged
vaut toujours false
. Correction, en initialisant avec une très grosse date :
let currentNextExpiring = sync_nextExpiring
sync_nextExpiring = sync_expires.reduce((max, value) => max ? Math.min(max, value) : value, 9007199254740991); // Far far away date
const nextExpiringChanged = currentNextExpiring !== sync_nextExpiring
Et du coup là, en effet, on recalcule bien plus souvent.
Bon après, le changement que tu as fait dans le protocole @elois ne fait aucun mal, et même il contourne le bug. Mais c’est plutôt ce code-là qu’il faudrait mettre.
Je vais pousser la modif sur dev
.