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

[C4 Analysis] Add warnings in LSP0, LSP6 & LSP17 #632

Merged
merged 10 commits into from
Sep 21, 2023
Merged

[C4 Analysis] Add warnings in LSP0, LSP6 & LSP17 #632

merged 10 commits into from
Sep 21, 2023

Conversation

b00ste
Copy link
Member

@b00ste b00ste commented Sep 19, 2023

What does this PR introduce?

In this PR I add warnings and best practices for the LSP0, LSP6 & LSP17 documentation.

LSP0ERC725Account

Fallback

  • Warning that the 0x00000000 function selector is the only one that returns.

LSP6KeyManager

Permissions

  • Risk when using a single controller with SUPER permissions.
  • Risk when granting permissions to 3rd parties.

Relay Call

  • Risk of Relay Services to send your transaction with log gas price.
  • Risk of Relay Services to front-run your transaction .

LSP17ContractExtension

@Hugoo
Copy link
Contributor

Hugoo commented Sep 19, 2023

@b00ste b00ste force-pushed the DEV-7972 branch 2 times, most recently from 9df864a to 67f56cb Compare September 19, 2023 12:20
@b00ste b00ste changed the title [C4 Analysis] Add warnings in LSP6 [C4 Analysis] Add warnings in LSP6 & LSP17 Sep 19, 2023
@b00ste b00ste changed the title [C4 Analysis] Add warnings in LSP6 & LSP17 [C4 Analysis] Add warnings in LSP0, LSP6 & LSP17 Sep 19, 2023
Copy link
Member

@CJ42 CJ42 left a comment

Choose a reason for hiding this comment

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

Let's discuss where these should go. They might fit better maybe in the "Contracts" section instead of the "Standards" section (which is more for the High Level concepts of each standard)

@CJ42 CJ42 requested a review from skimaharvey as a code owner September 21, 2023 13:44
@Hugoo Hugoo self-requested a review as a code owner September 21, 2023 22:08
@Hugoo Hugoo merged commit 25b0d9a into main Sep 21, 2023
2 checks passed
@Hugoo Hugoo deleted the DEV-7972 branch September 21, 2023 22:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants