Bug transferUd bad value

Networks ĞDev
1

Je pense qu’il y a un bug avec l’extrinsic universalDividend.transferUd sur la Gdev.

Lorsque je transfer 1 DU, je transfer bien 1 DU.
Mais si je transfer 5 DU, je transfer 2.027 DU.

Je ne vois aucun pattern qui pourrait expliquer une erreur dû aux milli-tokens, ou de conversion.
Je vous laisse essayer de votre côté pour confirmer le problème.

Exemple:

image

image
image192×346 35.1 KB

image
image2001×757 79.8 KB

→

https://duniter-portal.axiom-team.fr/#/explorer/query/0x4044c82a969782b5423941bf04d2aec9bd3c9d2b3707cc6114a32cf30c15b3e8

image
image574×244 16.2 KB

18446744073709551 / 9099406734269001 = 2,0272469

Je n’ai pas encore investigué Duniter sur une chaine locale.
Préférez-vous que j’ouvre un ticket ?

2

C’est encore un coup de la saturation :

do_transfer_ud fait value.saturating_mul(ud_amount) / 1_000u32.into()

or (5000*9099406734269001).bit_length() == 66 donc la valeur est tronquée avant la division.

3 Likes
3

Ne serait il pas possible de throw une erreur lors d’un débordement comme ça ? Même dans la pratique on ne devrait pas déborder, ça me parait bancale de tronquer silencieusement au lieux de throw quelque chose comme DispatchError::Arithmetic(Overflow).

Je ne connais pas assez bien l’archi pour savoir où placer ce check exactement, mais je pense qu’il faudrait.

3 Likes
4

Merci @poka pour ce rapport d’erreur et pour la suggestion. En effet, on peut utiliser ensure_mul ici car l’appel est faillible.

Je viens de créer un ticket : transfer_ud: use checked_mul instead of saturating_mul (#304) · Issues · nodes / rust / Duniter v2S · GitLab

Nous (les développeurs qui travaillent avec Substrate) avons l’habitude d’utiliser saturating_* au lieu de ensure_* pour deux raisons :

  • Certains aspects du runtime doivent être infaillibles (tout ce qui concerne l’exécution obligatoire).
  • Dans la pratique, tous les projets Substrate utilisent u128 comme type de solde, donc la saturation n’est jamais déclenchée.
4 Likes