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(contract): gas saving on var initialization #16

Merged
merged 1 commit into from
Jul 6, 2021

Conversation

PierrickGT
Copy link
Contributor

@@ -156,7 +156,7 @@ contract ATokenYieldSource is ERC20Upgradeable, IProtocolYieldSource, AssetManag
/// @param shares Amount of shares
/// @return Number of tokens
function _sharesToToken(uint256 shares) internal view returns (uint256) {
uint256 tokens = 0;
uint256 tokens;
Copy link
Contributor

Choose a reason for hiding this comment

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

How much is the gas saving? Assignment is more readable.

Copy link
Contributor Author

@PierrickGT PierrickGT Jun 30, 2021

Choose a reason for hiding this comment

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

If we look at supplyTokenTo that calls _tokenToShares, on the main branch it consumes on average 157,718 units of gas and on this branch, it consumes on average 157,718 units of gas, so it doesn't seem to change a thing. 😅
I agree that assignment is more readable.

@PierrickGT PierrickGT force-pushed the fixes/tensors-101 branch from b911ce9 to 95bd885 Compare July 6, 2021 08:33
@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.63% when pulling 95bd885 on fixes/tensors-101 into faa315e on fixes/c4-audit.

@PierrickGT PierrickGT merged commit 89535da into fixes/c4-audit Jul 6, 2021
@PierrickGT PierrickGT deleted the fixes/tensors-101 branch July 6, 2021 08:39
PierrickGT added a commit that referenced this pull request Jul 6, 2021
fix(contract): gas saving on var initialization
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants