Code optimizations

[cducrest]

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 in WiiQareVoucherV1 and SharedStructs. All throughout the code only SharedStructs.Voucher is used. Remove WiiQareVoucherV1.Voucher to avoid mistakes in the future that could arise with inconsistencies between WiiQareVoucherV1.Voucher and SharedStructs.Voucher.

audit-wiiqare-1/contracts/WiiQareVoucherV1.sol

Lines 23 to 30 in 0243110

	struct Voucher {
	uint256 value;
	string currencySymbol;
	string ownerID;
	string providerID;
	string beneficiaryID;
	string status;
	}

Fix typo in mintVoucherEvent

Event names the variable nftVoucer instead of nftVoucher.

audit-wiiqare-1/contracts/WiiQareVoucherV1.sol

Line 43 in 0243110

event mintVoucherEvent(uint256 voucherID, SharedStructs.Voucher nftVoucer);

Remove unused / useless code

The constructor sets _voucherID = 0; which is redundant as uint256 variables start with their default values of 0.

audit-wiiqare-1/contracts/WiiQareVoucherV1.sol

Line 56 in 0243110

_voucherID = 0;

Remove unused internal functions _decrementVoucherID() and _resetVoucherID().

audit-wiiqare-1/contracts/WiiQareVoucherV1.sol

Lines 172 to 185 in 0243110

	function _decrementVoucherID() internal {
	uint256 value = _voucherID;
	require(value > 0, "Counter: decrement overflow");
	unchecked {
	_voucherID = _voucherID - 1;
	}
	}
	/**
	* Sets the voucherID to 0
	*/
	function _resetVoucherID() internal onlyOwner {
	_voucherID = 0;
	}

Voucher struct may cost more gas than necessary

The struct uses uint256 and string 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 or unclaimed) and could be replaced with an enum (taking 8 bits of storage). This could be tightly packed into a slot if value is converted to uint248:

library SharedStructs {
    enum Status {
        claimed,
        unclaimed
    }
    struct Voucher {
        uint248 value;
        Status status;
        string currencySymbol;
        string ownerID;
        string providerID;
        string beneficiaryID;
    }
}

This will save 20k gas on minting any new voucher but slightly increase the cost of reading value when status 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

	struct Voucher {
	uint256 value;
	string currencySymbol;
	string ownerID;
	string providerID;
	string beneficiaryID;
	string status;
	}

[liviuvlad-innovatorspark]

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.
Gas optimization was done in this pr