Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix transfer claim #214

Draft
wants to merge 1 commit into
base: compatible
Choose a base branch
from

Conversation

MartinOndejka
Copy link
Collaborator

@MartinOndejka MartinOndejka commented Sep 18, 2024

Fixes 2 issues:

  1. When user tries to prove claim, it needs to provide as witness all the previously claimed transfers. Before there was no way of fetching that since it wasn't exposed in a statement.
  2. The action extension of transfers was calculated incorrectly.

This PR requires the change to the inner account VK, and forced state root update in outer account.

@MartinOndejka MartinOndejka self-assigned this Sep 18, 2024
@MartinOndejka MartinOndejka requested a review from L-as as a code owner September 18, 2024 13:09
@L-as
Copy link
Collaborator

L-as commented Sep 18, 2024

I'm not really sure this PR is worth reviewing for issues or bugs, or mistakes, or merging, because the design is outdated.

@@ -209,6 +209,11 @@ module Process_transfer = struct
(* We give it permission *)
; implicit_account_creation_fee =
Boolean.false_ (* Custom token account, can't be true *)
; events =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have actions already?

let before = List.map ~f:(value_to_actions TR.typ) before in
let after = List.map ~f:(value_to_actions TR.typ) after in
let%bind trans1 =
Action_state_extension.prove ~dummy:is_new ~source:pointer before
Action_state_extension.prove ~dummy:is_new
~source:Zkapp_account.Actions.empty_state_element before
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pretty sure this is somewhat wrong, somewhat right, but it isn't worth it I think

@L-as
Copy link
Collaborator

L-as commented Sep 18, 2024

can't you mock it without the changes if you set the permissions to Either?

@L-as L-as marked this pull request as draft January 2, 2025 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants