Gros Fork sur la Ğ1 !

1

Le bloc #89111 contient une transaction qui n’est pas acceptée par les nœuds en 1.6.16 pour une raison qui m’échappe encore. @cgeek une idée ??

Nous devons réagir vite et choisir entre :

  1. Tous downgrader a la 1.6.14
  2. Tous upgrader sur la 1.6.16
   "transactions": [
       {
         "version": 10,
         "currency": "g1",
         "locktime": 0,
         "hash": "FAC2C83C1E3E611EE17132C794048DCF675D303A5988DE496EDCA15E0603C18A",
         "blockstamp": "89107-0000019BE9E5704BC179736908D8C091068B41E4245D396F03BC540C12369C53",
         "blockstampTime": 0,
         "issuers": [
           "FdrUEyKDokpfnDzCPVxkKj239NSTPFdnBD11eF6B4g4e"
         ],
         "inputs": [
           "1001:0:D:FdrUEyKDokpfnDzCPVxkKj239NSTPFdnBD11eF6B4g4e:54878",
           "1001:0:D:FdrUEyKDokpfnDzCPVxkKj239NSTPFdnBD11eF6B4g4e:55154",
           "40000:0:T:353C4385F4EDBE5370691DC0E5B887B74B8F3D12518A0FFF06C6AD3B49FCA472:0",
           "40003:0:T:14D9EB4504CE2A0224DD5F2121CA8A19A4F2948D6ABE4B83D81310DEE958BAAE:0"
         ],
         "outputs": [
           "81283:0:SIG(Dgnyj2rpsLvWw7Za6gASwkAVmYjEU4aE9zFGLjfDDfCM)",
           "722:0:SIG(FdrUEyKDokpfnDzCPVxkKj239NSTPFdnBD11eF6B4g4e)"
         ],
         "unlocks": [
           "0:SIG(0)",
           "1:SIG(0)",
           "2:SIG(0)",
           "3:SIG(0)"
         ],
         "signatures": [
           "aPQPIWNFIfaJrooA5lZHugnqVSJTKOpk4VaXUQaVexkhHNMAHlMfj+jqqFGCZzj0meWiMypxy1lXQMKrcQJuBQ=="
         ],
         "comment": ""
       }
     ],

Il est flagrant que le fork est du a une incompatibilité entre versions :

tmp1
tmp11204×745 103 KB

1 Like
2

Alors sur le fork 1.6.16 la transaction a été écrite avec 2 transactions de change par @Pafzedog au bloc #89109 . Ce que je ne comprend pas c’est que cette nouvelle feature (la possibilité d’avoir des tx chainés dans le même bloc) ne devait s’appliquer qu’a partir du 1er Mars !

je vais investiguer :thinking:

3

Par chance, la branche qui l’a emportée est une branche compatible avec l’ensemble des versions, avec la version de bruno, qui contient les 2 tx de change mais pas la tx finale, donc pas de chainage de tx.

Ceci étant j’ai revérifier dans le code et le timestamp d’application du chainage des tx est bien 1519862400 soit au 1er mars, je ne comprend donc pas comment @Pafzedog a fait pour générer un bloc avec des tx chainée, le mystère reste entier !

4

Soit c’est la fonction getMaxTransactionDepth() qui n’a pas fait son boulot, soit ce code est erroné :

const max = getMaxTransactionDepth(sindex)
//
const allowedMax = block.medianTime > 1519862400 ? CommonConstants.BLOCK_MAX_TX_CHAINING_DEPTH : 1
if (max > allowedMax) {
  throw "The maximum transaction chaining length per block is " + CommonConstants.BLOCK_MAX_TX_CHAINING_DEPTH
}

En y réfléchissant un peu, là, j’ai comme un doute sur la valeur par défaut 1.

Bon par contre je ne pourrais pas regarder plus loin ce soir, toutefois la 1.6.16 étant minoritaire les forks seront résolus à l’ancienne.

5

Je suis redescendu en version du coup, trop d’instabilités sur la 1.6.16 :slight_smile:

6

Exact le problème viens de là, la fonction getMaxTransactionDepth() retourne zéro pour les tx classiques, c’est donc zéro la profondeur par défaut, mystère résolu, je vais pusher un correctif :slight_smile:

2 Likes
7

8 messages ont été déplacés vers un nouveau sujet : Stabilité des pre-releases

8

Un message a été intégré dans un sujet existant : [duniter-ts] Sauter la 1.7.0 ?

9

@cgeek Bon en passant la profondeur max a zéro le test d’inté transactions-chaining.ts ne passe plus, je viens d’y passer 30 min et je ne comprend pas ton test, j’ai l’impression que tu tente de chainer deux transactions mais avec un temps en 2016, j’ai changer le temps pour après le 1er mars 2018 mais le test échoue toujours :sweat:

Au tout cas je viens de tester mon correctif sur g1-test, et les transactions sont accepter sans problème par mon noeud, il arrive même a les écrire : http://g1-test.duniter.fr/#/app/block/125756/00043F2630BECD0292772F8E7C6E10B6B037A410C2D9DA0F8FC69D850F8FA62E

1 Like
10

Hé bien oui en fait c’est logique que le test ne passe plus, mais tout cela manque certainement d’explications. Qu’à cela ne tienne, les voici.

En fait le code actuel du test aurait dû échouer si j’avais correctement codé la fonction qui interdisait le chaînage de transactions : en effet dans ce test, alors que le DU est de 1200 unités, on réalise deux transferts chaînés :

  • un de 1040 unités
  • un de 160 unités
// Current state
let current = await s1.get('/blockchain/current');
(await s1.get('/tx/sources/DKpQPUL4ckzXYdnDRvCRKAm1gNvSdmAXnTrJZ7LvM5Qo')).should.have.property('sources').length(1);
let tx1 = await toc.prepareITX(1040, tic); // Rest = 1200 - 1040 = 160
let tx2 = await toc.prepareUTX(tx1, ['SIG(0)'], [{ qty: 160, base: 0, lock: 'SIG(' + tic.pub + ')' }], {
  comment: 'also take the remaining 160 units',
  blockstamp: [current.number, current.hash].join('-'),
  theseOutputsStart: 1
});
const tmp = CommonConstants.TRANSACTION_MAX_TRIES;
CommonConstants.TRANSACTION_MAX_TRIES = 2;
await unit.shouldNotFail(toc.sendTX(tx1)); // <--- ICI : j'envoie 1040
await unit.shouldNotFail(toc.sendTX(tx2)); // <--- LA : j'envoie 160 chaînés aux 1040
(await s1.get('/tx/sources/DKpQPUL4ckzXYdnDRvCRKAm1gNvSdmAXnTrJZ7LvM5Qo')).should.have.property('sources').length(1);
(await s1.get('/tx/sources/DNann1Lh55eZMEDXeYt59bzHbA3NJR46DeQYCS2qQdLV')).should.have.property('sources').length(1);
await s1.commit({ time: now + 7210 }); // TX1 + TX2 commited
(await s1.get('/tx/sources/DKpQPUL4ckzXYdnDRvCRKAm1gNvSdmAXnTrJZ7LvM5Qo')).should.have.property('sources').length(0);
(await s1.get('/tx/sources/DNann1Lh55eZMEDXeYt59bzHbA3NJR46DeQYCS2qQdLV')).should.have.property('sources').length(3); // The UD + 1040 + 160 units sent by toc

Et donc lors du commit, on s’attend à tort à ce que les deux transactions passent simultanément, et donc qu’il n’existe plus de source pour la clé “DKpQPUL4ckzXYdnDRvCRKAm1gNvSdmAXnTrJZ7LvM5Qo” qui aurait tout consommé (la totalité de son DU de 1200).

Pourquoi à tort ? Car étant donné la date inscrite (que l’on ne voit pas dans l’extrait de code, mais qui est 2016), le chaînage ne devrait pas être autorisé en un seul commit ! Or là, on vérifie qu’il est autorisé ! (qu’il n’existe plus de sources pour “DK[…]” après un seul commit). C’est n’importe quoi.

Donc effectivement @elois , quand tu corriges le code et met la valeur 0, le test échoue. Précisément car le test vérifie qu’on peut chaîner des transactions, or le code correct ne le permet pas avec une telle date !

Bref le test était erroné. Et donc maintenant que tu as mis le bon code, c’est le test qui ne passe plus. C’est tout à fait normal.

Et donc je viens de pusher une modification pour le test sur ta branche corrective @elois, qui vérifie qu’il n’y a pas chaînage. Et les tests passent ! :slight_smile:

2 Likes
11

Mercin @cgeek, du coup je viens de merger la branche qui corrige le ticket 1260, et gitlab a automatiquement fermer le ticket correspondant, c’est bien foutu :slight_smile:

2 Likes