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-L1-L04 Lost Funds in Messenger Contracts #637

Merged
merged 2 commits into from Jul 20, 2023

Conversation

zimpha
Copy link
Member

@zimpha zimpha commented Jul 11, 2023

Purpose or design rationale of this PR

This PR fix the bug (L-04 Lost Funds in Messenger Contracts) reported by OpenZeppelin. The following are the details:

The ScrollMessengerBase contract has a receive function that does not have any code implemented. There is no use case for the messenger contracts to receive any ETH outside of the payable functions. Instead, users might accidentally send ETH to it. Hence, consider removing the receive function.

The receive function is used for initialization. Adding an onlyOwner modifier to make sure no other user can send funds to the contract.

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:

  • build: Changes that affect the build system or external dependencies (example scopes: yarn, eslint, typescript)
  • ci: Changes to our CI configuration files and scripts (example scopes: vercel, github, cypress)
  • docs: Documentation-only changes
  • feat: A new feature
  • fix: A bug fix
  • perf: A code change that improves performance
  • refactor: A code change that doesn't fix a bug, or add a feature, or improves performance
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
  • test: Adding missing tests or correcting existing tests

Deployment tag versioning

Has tag in common/version.go been updated?

  • 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 Jul 11, 2023
@zimpha zimpha self-assigned this Jul 11, 2023
@github-actions
Copy link

github-actions bot commented Jul 11, 2023

LCOV of commit d369ccb during Contracts #1126

Summary coverage rate:
  lines......: 51.3% (894 of 1741 lines)
  functions..: 68.1% (203 of 298 functions)
  branches...: no data found

Files changed coverage rate: n/a

@HAOYUatHZ HAOYUatHZ merged commit 25e4b6c into develop Jul 20, 2023
3 checks passed
@HAOYUatHZ HAOYUatHZ deleted the fix/lost_funds_in_messenger_contracts branch July 20, 2023 09:08
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

3 participants