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 TWAMM.sol #62

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Update TWAMM.sol #62

wants to merge 2 commits into from

Conversation

0xScratch
Copy link

Well, this PR made 2 types of changes and these changes feels like valid as far as I am concerned. If not then do let me know and tell the reason which will be another great learning experience. Anyways let's dive into the changes made:

1. Struct Packing: As we know, that solidity use slots and the usage of it really affects the gas (means more the slots more gas used by the contract). I have noticed that rest of the structs were perfectly packed but not this one. Earlier it was using 4 slots and now decreased to 3.

2. Changed bool [variable-name] = false; to bool [variable-name];: Not really sure about this change (looks like it's been done on purpose) but while I was going through the repository, there were several examples of unsigned integers not initialized to any value, thus leaving their value as 0 by default. And, same concept applies to this boolean variable. When not initialized, it's default value gonna be false

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.

2 participants