forked from WiiQare/SmartContract
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Code optimizations #3
Comments
Most of the optimizations were done, with the exception of the status as enum, it will stay string to account for future transitions between parties. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Summary
This issue references minor code comments or gas optimizations that do not represent a vulnerability or security risk.
Detail
Remove redundant Voucher struct
The
Voucher
struct is defined both inWiiQareVoucherV1
andSharedStructs
. All throughout the code onlySharedStructs.Voucher
is used. RemoveWiiQareVoucherV1.Voucher
to avoid mistakes in the future that could arise with inconsistencies betweenWiiQareVoucherV1.Voucher
andSharedStructs.Voucher
.audit-wiiqare-1/contracts/WiiQareVoucherV1.sol
Lines 23 to 30 in 0243110
Fix typo in mintVoucherEvent
Event names the variable
nftVoucer
instead ofnftVoucher
.audit-wiiqare-1/contracts/WiiQareVoucherV1.sol
Line 43 in 0243110
Remove unused / useless code
The constructor sets
_voucherID = 0;
which is redundant asuint256
variables start with their default values of0
.audit-wiiqare-1/contracts/WiiQareVoucherV1.sol
Line 56 in 0243110
Remove unused internal functions
_decrementVoucherID()
and_resetVoucherID()
.audit-wiiqare-1/contracts/WiiQareVoucherV1.sol
Lines 172 to 185 in 0243110
Voucher struct may cost more gas than necessary
The struct uses
uint256
andstring
that take at least a full word in storage. Strings will take more than a word if they are more than 31 bytes long, consider replacing with bytes32 or uint256 where appropriate.By looking at the tests, I understand that
status
takes a limited number of values (claimed
orunclaimed
) and could be replaced with anenum
(taking 8 bits of storage). This could be tightly packed into a slot ifvalue
is converted touint248
:This will save 20k gas on minting any new voucher but slightly increase the cost of reading
value
whenstatus
is not read (see https://docs.soliditylang.org/en/latest/internals/layout_in_storage.html).audit-wiiqare-1/contracts/WiiQareSharedStructs.sol
Lines 5 to 12 in 0243110
The text was updated successfully, but these errors were encountered: