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

Update ERC-6492: add audit report and changes prompted by the audit #267

Closed
wants to merge 9 commits into from

Conversation

Ivshti
Copy link
Contributor

@Ivshti Ivshti commented Feb 18, 2024

Summary

ERC-6492 was recently audited and a few non-critical issues were found, and as such it is strongly advised to change the reference implementation.

This PR does not change the ERC text itself in any way except adding a link to the audit. The ERC is final.

@eip-review-bot
Copy link
Collaborator

eip-review-bot commented Feb 18, 2024

File ERCS/erc-6492.md

Requires 2 more reviewers from @axic, @gcolvin, @lightclient, @SamWilsn

Copy link

The commit 369d39b (as a parent of 93664c8) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci label Feb 18, 2024
@github-actions github-actions bot removed the w-ci label Feb 19, 2024
Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

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

This'll need to wait for a Call for Input: ethcatherders/EIPIP#321

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no copyright license information in this file. Please add a LICENSE file to your assets folder with the license this file is provided under. We generally prefer CC0-1.0, but would also accept MIT, Apache, or similar.

@@ -243,6 +262,8 @@ However, deploying a contract requires a `CALL` rather than a `STATICCALL`, whic

Furthermore, it is likely that this ERC will be more frequently used for off-chain validation, as in many cases, validating a signature on-chain presumes the wallet has been already deployed.

**The reference implementation in this ERC has been audited:** look for `ERC6492-Hunter-Security-Audit-Report-v1.0.pdf` in the assets directory.
Copy link
Contributor

@SamWilsn SamWilsn Feb 21, 2024

Choose a reason for hiding this comment

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

Suggested change
**The reference implementation in this ERC has been audited:** look for `ERC6492-Hunter-Security-Audit-Report-v1.0.pdf` in the assets directory.
**The reference implementation in this ERC has been audited:** look for [`ERC6492-Hunter-Security-Audit-Report-v1.0.pdf`](../assets/eip-6492/ERC6492-Hunter-Security-Audit-Report-v1.0.pdf) in the assets directory.

@SamWilsn
Copy link
Contributor

SamWilsn commented Apr 4, 2024

As discussed in the previously mentioned Call for Input, I am closing this PR without merging.

@SamWilsn SamWilsn closed this Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants