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

Fixed banking.sol and Enhanced the modifier #308

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

0xScratch
Copy link
Contributor

🛠️ Fixes Issue

  1. Invalid modifier: The modifier name used in the code is 'onlyonwner' where it's used 'onlyowner' within the functions. This will just create a damn error. Looks like someone pasted this code without even compiling it down...
  2. Gas consumption could be enhanced: That single is modifier is used frequently, thus taking more gas. Need to find a way we can resolve this...
  3. Some Typos and grammatical errors down in the comments: heading self-defines the issue. won't be a big deal

👨‍💻 Changes proposed

  1. changed the name of modifier onlyonwner to onlyOwner in all sections of code wherever it is used.
  2. Enhanced the gas optimizations using an internal function. Refer this -> https://gist.github.com/grGred/9bab8b9bad0cd42fc23d4e31e7347144#use-internal-view-functions-in-modifiers-to-save-bytecode (Must refer the links provided inside)
    Note: This type of enhancement is always advised whenever a modifier is used several times within the code. If it is used just once or twice, no need of any change.
  3. Fixed typing and grammatical mistakes.

✔️ Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

📄 Note to reviewers

Well, this was a pure buggy code, like a single typo mistake just made the contract go in vain. It's advised that these smart contracts must be checked thoroughly before merging them in the repo. Try pasting them in remix or some IDE before letting them be presented within this repository.
Although, I am going to check and go through the whole codebase..Maybe making some enhancements if possible!

@github-actions
Copy link

github-actions bot commented Oct 9, 2023

Our team will soon review your PR. Thanks @Aryan9592 :)

@github-actions github-actions bot added the SWOC23 label Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant