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(contracts): OZ-N03 Inconsistent Usage of msg.sender #906

Merged
merged 3 commits into from Oct 4, 2023

Conversation

zimpha
Copy link
Member

@zimpha zimpha commented Sep 1, 2023

Purpose or design rationale of this PR

This PR partially fixed the issue reported by Openzepplin (N-03 Inconsistent Usage of msg.sender) during ScrollOwner and Rate Limiter audit. The following are the details:

The ScrollOwner contract inherits from OpenZeppelin's AccessControl contract through its inheritance chain. This contract uses the _msgSender function.

Likewise, the ETHRateLimiter contract inherits from OpenZeppelin's Ownable contract. This contract also uses the _msgSender function.

The _msgSender function from OpenZeppelin's Context contract is used to allow the usage of metadata execution.

However, the ScrollOwner and ETHRateLimiter contracts directly use msg.sender instead of the _msgSender function. Even though there is no security risk per se, having some methods use msg.sender while others use the _msgSender function can be confusing.

Consider standardizing the usage of the _msgSender function throughout the codebase to increase clarity and consistency.

PR title

Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:

  • fix: A bug fix

Deployment tag versioning

Has tag in common/version.go been updated or have you added bump-version tag to this PR?

  • No, this PR doesn't involve a new deployment, git tag, docker image tag
  • Yes

Breaking change label

Does this PR have the breaking-change label?

  • No, this PR is not a breaking change
  • Yes

@zimpha zimpha added the bug Something isn't working label Sep 1, 2023
@zimpha zimpha self-assigned this Sep 1, 2023
@github-actions
Copy link

github-actions bot commented Sep 1, 2023

LCOV of commit 4fd8082 during Contracts #88

Summary coverage rate:
  lines......: 51.8% (1024 of 1978 lines)
  functions..: 65.2% (227 of 348 functions)
  branches...: no data found

Files changed coverage rate: n/a

icemelon
icemelon previously approved these changes Sep 19, 2023
@icemelon icemelon merged commit 55e0b11 into develop Oct 4, 2023
4 checks passed
@icemelon icemelon deleted the fix/OZ-N03_inconsistent_usage_of_msgsender branch October 4, 2023 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants