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

Add access permission to accounts #12003

Merged
merged 25 commits into from
Jan 13, 2023
Merged

Conversation

mrmr1993
Copy link
Member

@mrmr1993 mrmr1993 commented Oct 18, 2022

This PR adds an access permission to accounts, which prohibits zkApp accounts from being accessed if the authorization is incorrect. This ensures that an 'empty update' to e.g. a token zkApp can't be used to pass unauthorized sub-calls.

Testing is still TODO; opening this to see if CI finds any glaring errors in existing tests.

Checklist:

  • Modified the current draft of release notes with details on what is completed or incomplete within this project
  • Document code purpose, how to use it
    • Mention expected invariants, implicit constraints
  • Tests were added for the new behavior
    • Document test purpose, significance of failures
    • Test names should reflect their purpose
  • All tests pass (CI will check this if you didn't)
  • Serialized types are in stable-versioned modules
  • Does this close issues? List them
    Fixes 2-pass transaction application model #12548

@mrmr1993 mrmr1993 added the ci-build-me Add this label to trigger a circle+buildkite build for this branch label Oct 18, 2022
@mrmr1993 mrmr1993 force-pushed the feature/add-access-permission branch from 3f24c25 to a832cfd Compare October 20, 2022 15:55
@mrmr1993 mrmr1993 force-pushed the feature/add-access-permission branch from a832cfd to 66dae59 Compare October 20, 2022 16:38
@mrmr1993 mrmr1993 marked this pull request as ready for review October 20, 2022 18:44
@mrmr1993 mrmr1993 requested review from a team and imeckler as code owners October 20, 2022 18:44
Copy link
Contributor

@mitschabaude mitschabaude left a comment

Choose a reason for hiding this comment

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

looks great!

@mrmr1993 mrmr1993 self-assigned this Oct 31, 2022
@mrmr1993
Copy link
Member Author

!ci-build-me

@mrmr1993
Copy link
Member Author

mrmr1993 commented Dec 2, 2022

!ci-build-me

@mrmr1993 mrmr1993 requested a review from a team as a code owner December 15, 2022 18:22
@mrmr1993
Copy link
Member Author

!ci-build-me

@mrmr1993
Copy link
Member Author

!ci-build-me

@mrmr1993
Copy link
Member Author

!ci-build-me

@mrmr1993
Copy link
Member Author

!ci-build-me

@mrmr1993
Copy link
Member Author

!ci-build-me

@mrmr1993
Copy link
Member Author

mrmr1993 commented Jan 3, 2023

!ci-build-me

| `Set_delegate ->
Boolean.true_
| `Access ->
failwith
Copy link
Member

Choose a reason for hiding this comment

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

why put a crasher here? seems unsafe?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's no sensible default value for access, so this ensures that we call it correctly everywhere.

@mrmr1993
Copy link
Member Author

mrmr1993 commented Jan 4, 2023

!ci-build-me

@mrmr1993
Copy link
Member Author

!ci-build-me

@mrmr1993 mrmr1993 merged commit a3661e4 into develop Jan 13, 2023
@mrmr1993 mrmr1993 deleted the feature/add-access-permission branch January 13, 2023 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-build-me Add this label to trigger a circle+buildkite build for this branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants