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

Expanded access control section from cairo #141

Closed
wants to merge 8 commits into from

Conversation

ggrieco-tob
Copy link
Member

No description provided.

@montyly
Copy link
Member

montyly commented Nov 3, 2022

@ggrieco-tob : can you take a look at the comments above?

@smonicas : can you take a look at the PR?

@ggrieco-tob
Copy link
Member Author

@montyly this is ready for another review


It is still possible to interact with contracts directly. But from the perspective of the contract, the caller's address will be 0. Since 0 is also the default value for uninitialized storage, it's possible to accidentally construct access control checks that fail open instead of properly restricting access to only the intended users.
* If a contract is called from another L2 contract, [access control should be implemented using `get_caller_address()`](https://www.cairo-lang.org/docs/hello_starknet/user_auth.html#getting-the-caller-address).
Copy link
Member

Choose a reason for hiding this comment

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

can we explain why in one sentence?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done, please take a look

@ggrieco-tob
Copy link
Member Author

is there anything blocking this merge?

@montyly
Copy link
Member

montyly commented Mar 22, 2023

@smonicas or @technovision99 : can you take a look at the PR?

@montyly montyly requested a review from smonicas March 22, 2023 09:14
@technovision99
Copy link
Contributor

lgtm, after regenesis we should link here instead imo: https://docs.starknet.io/documentation/architecture_and_concepts/L1-L2_Communication/messaging-mechanism/

@ggrieco-tob
Copy link
Member Author

@technovision99 let's document that in an issue so we don't forget

@montyly
Copy link
Member

montyly commented Feb 21, 2024

replaced by #338

@montyly montyly closed this Feb 21, 2024
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.

3 participants