Oneshot accounts
Add lighter accounts that can be consumed only once.
Je n'ai pas encore fait les tests unitaires car la config de mock est assez longue à faire. Mais les tests d'intégration sont là.
Est-ce que l'ordre des deposit_event
importe ? Si non, le code des calls peut être un peu factorisé sans coût.
Merge request reports
Activity
requested review from @librelois
- Resolved by Éloïs
- Resolved by Éloïs
- Resolved by Éloïs
- Resolved by Éloïs
Est-ce que l'ordre des deposit_event importe ? Si non, le code des calls peut être un peu factorisé sans coût.
Non on s'en fou, va y factorise le code :)
Je n'ai pas encore fait les tests unitaires car la config de mock est assez longue à faire
Si tu peut prendre le temps de le faire dans cette PR ce serait bien
Il faudrait aussi un test end2end, car j'ai peur qu'on est des soucis lors de la consommation d'un oneshot account du fait que le compte n'existe pas du point de vue de la pallet sytem !
added 1 commit
- e775eae8 - feat(duniter-account): finalized PoC oneshot account
- Resolved by Éloïs
- Resolved by Éloïs
- Resolved by Éloïs
- Resolved by Éloïs
@tuxmain évite de rajouter le suffixe
1
dans tout ce qui apparais au monde extérieur, comme les nom des call et de leurs paramètres :)On le fait uniquement dons le code interne.
- Resolved by Éloïs
@tuxmain pour les call qui consomment un oneshot account, il faut empecher substrate de créer un compte via l'incrément du nonce. Pour ce faire, je propose de créer notre propre wrapper
DuniterCheckNonce<Inner: SignedExtension>(Inner)
et dans l'impl du traitSignedExtension
réécrire uniquement le comportement de la méthodepre_dispath
dans le cas d'un call qui consomme un oneshot account. Il faut bien sur être transparent en appelant la méthode de Inner dans tout les autres cas.Cherche les occurences à
CheckNonce
dans la codebase et tu verra ou est ce que c'est injecté pour chaque runtimeEdited by Éloïs
added 18 commits
-
e775eae8...8bc5e17f - 16 commits from branch
master
- 984d95d8 - PoC about oneshot accounts
- 5b6686a1 - feat(duniter-account): finalized PoC oneshot account
-
e775eae8...8bc5e17f - 16 commits from branch
added 1 commit
- 0406fec8 - feat(duniter-account): finalized PoC oneshot account
added 74 commits
-
0406fec8...33630d52 - 72 commits from branch
master
- 6d4f4bfb - PoC about oneshot accounts
- 2a6e392b - feat(duniter-account): finalized PoC oneshot account
-
0406fec8...33630d52 - 72 commits from branch
@librelois Je crois qu'il ne reste qu'à faire prendre les frais sur le oneshot account plutôt que sur le compte normal.
Juste pour confirmer : il faut utiliser un
OnChargeTransaction
custom, c'est ça ?Il faut aussi décider, pour
consume_oneshot_account_two_dests
, si on prend les frais sur un destinataire, ou également sur les deux, ou avec un ratio défini par l'utilisateur. Je dirais que sur un seul c'est plus pratique pour l'utilisateur et plus rapide à calculer.added RN-runtime label
- Resolved by Éloïs
@tuxmain il faut créer une implémentation du trait
pallet_transaction_payment::OnChargeTransaction
dans la palletoneshot_account
qui doit prentre un génériqueInner
et apellerInner
dans tous les cas sauf si le call est censé être émis par un oneshot account. Si le call est consé être émis par un oneshot account il faut prélever les fauis au oneshot account, pas au destinataire. Il ne faut jamais prelever des frais à un compte qui n'a pas signé pour ça, ce serait une grave faille de sécurité.Edited by Éloïs
- Resolved by Éloïs
@tuxmain aussi je pense qu'il faut déplacer tout ça dans une nouvelle pallet
oneshot_account
, et bouger également le CheckNonce dedans. Oui c'est plus dur mais ça me semble être un découplage essentiel. La fonctionnalité "onethot account" que nous ajoutons ici n'est pas spécifique à duniter, donc ça doit être dans sa propre pallet, pallet qu'on pourra d'ailleurs extraire dans son propre dépôt à l'avenir quand elle sera mature.
added 8 commits
-
2a6e392b...a674fd7b - 6 commits from branch
master
- 0f5c53f7 - PoC about oneshot accounts
- b96d8380 - feat(duniter-account): finalized PoC oneshot account
-
2a6e392b...a674fd7b - 6 commits from branch
added 7 commits
-
b96d8380...4415b0f5 - 5 commits from branch
master
- 64d1a812 - PoC about oneshot accounts
- b866eb8c - feat(duniter-account): finalized PoC oneshot account
-
b96d8380...4415b0f5 - 5 commits from branch
@librelois J'ai implémenté
OnChargeTransaction
, danswithdraw_fee
je prélèvefee
si possible en respectantExistentialDeposit
, sinon le compte est supprimé.On devrait pouvoir ne pas respecter le
ExistentialDeposit
si le compte est consommé vers un compte normal existant, mais si le call échoue alors on aura un compte oneshot< ExistentialDeposit
... Ou alors il faut que le call lui-même en cas d'échec supprime le compte s'il est< ExistentialDeposit
?- Resolved by Éloïs
- Resolved by Éloïs
- Resolved by Éloïs
- Resolved by Éloïs
- Resolved by Éloïs
- Resolved by Éloïs
added 1 commit
- a106114d - feat(duniter-account): finalized PoC oneshot account
added 1 commit
- ead18738 - feat(duniter-account): finalized PoC oneshot account
added 1 commit
- eb91ce7f - feat(duniter-account): finalized PoC oneshot account
@librelois Le test d'intégration passe, mais les tests concombre échouent et dans PolkadotJS,
createOneshotAccount
donne l'erreur :signAndSend: error: RpcError: 1002: Verification Error: Runtime error: Execution failed: ApiError( FailedToConvertParameter { function: "validate_transaction", parameter: "tx", error: Error { cause: None, desc: "Could not decode `Call`, variant doesn't exist" } } )
(et si je change la variante du MultiAddress, c'est "could not decode [autre chose]".
@tuxmain je viens de tester sur ta branche et les tests cucumber fonctionne très bien une fois corrigés, je te laisse trouver tout selu comment les corriger, un indice toutefois: il faut parfois tenir compte des frais.
En revanche, polkadotjs fait effectivement n’importe-quoi, en plus dans les blocs qui contiennent un extrinsic avec un call oneshot account, polkadotjs indique que c'est un
system.killPrefix
, je pense que c'est parce qu'il garde en cache les metadata de la ğdev, mais je ne sais pas comment vérifier ça ni comment le forcer à update ses metadata. En tout les cas on ne va pas perdre du temps à essayer de fixer polkadotjs, il faut plutôt via cucumber ou/et tester en ligne de commande :)Edited by Éloïsadded 1 commit
- 9024a09f - feat(duniter-account): finalized PoC oneshot account
changed milestone to %runtime-400
- Resolved by Éloïs
- Resolved by Éloïs
@tuxmain peut tu stp faire le nettoyage dans tes changements pour qu'on puisse y voir clair, la c'est galère a review, notamment supprime tout les changements dans la pallet duniter-account, sauf ceux dont tu à strictement besoin.
Je te recommande de faire un reset soft pour review toi même tout tes changements et ne commiter que ce qui est vraiment indispensable pour cette MR :)
Aussi, il faut bouger CheckNonce dans la pallet oneshot, et bien sur ajouter la généricité nécessaire pour que ça fonctionne. Inspire toi de comment est écrit CheckNonce dans frame_system, il faut constamment s'inspirer du code de substrate lui-même.
EDIT: @tuxmain aussi, désormais tout nouveaux call ou hook devrait être benchmarker dans la MR qui l'ajoute, je vais donc te demander d'ajouter les benchmark tests pour les 3 calls de la pallet oneshot, tu peut t'inspirer de comment j'ai fait pour la pallet universal dividend dans ce commit: !73 (a0b05d23)
Si tu rencontre des difficultés, n’hésite pas à me demander une visio ou une session de peer programming, j'ai conscience que ce que je te demande est difficile, mais c'est la réalité de ce qu'il faut faire pour une contribution complète :)
Edited by Éloïs
added 1 commit
- cd1b9021 - feat(oneshot-account): Pallet oneshot-account
added 16 commits
-
a52f16e4...ae65f879 - 15 commits from branch
master
- 39c4dfad - feat(oneshot-account): Pallet oneshot-account
-
a52f16e4...ae65f879 - 15 commits from branch
added 5 commits
-
39c4dfad...4bf87cec - 4 commits from branch
master
- 166fee3a - feat(oneshot-account): Pallet oneshot-account
-
39c4dfad...4bf87cec - 4 commits from branch
added 1 commit
- 76e7c186 - fix: metadata should comply subxt & polkadotjs expectations
@tuxmain après analyse cette MR n'est pas du tout terminée, les modules mock, test et benchmarking ne sont pas déclarés dans le lib.rs donc même pas compilés, quand je les ajoutent rien ne compile.
Merci d'inclure réellement les tests et benchmarking et de les faire fonctionner.