Skip to content
Snippets Groups Projects

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.

Edited by Pascal Engélibert

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Éloïs
  • Éloïs
  • É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 :smile:

    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

    Compare with previous version

  • Éloïs
  • Éloïs
  • Éloïs
  • É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 trait SignedExtension réécrire uniquement le comportement de la méthode pre_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 runtime :smile:

      Edited by Éloïs
  • added 18 commits

    Compare with previous version

  • added 1 commit

    • 0406fec8 - feat(duniter-account): finalized PoC oneshot account

    Compare with previous version

  • added 74 commits

    Compare with previous version

  • @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 pallet oneshot_account qui doit prentre un générique Inner et apeller Inner 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

    Compare with previous version

  • added 7 commits

    Compare with previous version

  • @librelois J'ai implémenté OnChargeTransaction, dans withdraw_fee je prélève fee si possible en respectant ExistentialDeposit, 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

      @tuxmain keep it simple, le paiement des frais ne doit pas causer la destruction du compte, c'est comme ça que fonctionne déjà tout les call de substrate, donc on fait pareil.

  • Éloïs resolved all threads

    resolved all threads

  • Éloïs
  • Éloïs
  • Éloïs
  • Éloïs
  • Éloïs
  • Éloïs
  • added 1 commit

    • a106114d - feat(duniter-account): finalized PoC oneshot account

    Compare with previous version

  • added 3 commits

    • 57ac5589 - 1 commit from branch master
    • 8ae64c4b - PoC about oneshot accounts
    • 09a91c43 - feat(duniter-account): finalized PoC oneshot account

    Compare with previous version

  • added 1 commit

    • ead18738 - feat(duniter-account): finalized PoC oneshot account

    Compare with previous version

  • added 1 commit

    • eb91ce7f - feat(duniter-account): finalized PoC oneshot account

    Compare with previous version

  • @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ïs
  • added 1 commit

    • 9024a09f - feat(duniter-account): finalized PoC oneshot account

    Compare with previous version

  • Éloïs changed milestone to %runtime-400

    changed milestone to %runtime-400

  • É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

    Compare with previous version

  • added 2 commits

    • dedaa23e - feat(oneshot-account): Pallet oneshot-account
    • ccf38862 - feat(oneshot-account): benchmarking

    Compare with previous version

  • added 1 commit

    • a52f16e4 - feat(oneshot-account): benchmarking

    Compare with previous version

  • added 16 commits

    Compare with previous version

  • added 5 commits

    Compare with previous version

  • Pascal Engélibert marked this merge request as ready

    marked this merge request as ready

  • Éloïs added 1 commit

    added 1 commit

    • 76e7c186 - fix: metadata should comply subxt & polkadotjs expectations

    Compare with previous version

  • @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.

  • added 1 commit

    • 04c788a0 - fix(oneshot-account): use benchmarking

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading