Optimize Offer structure (gas usage)

[pauliax]

Description

As the gas inefficiencies are also in the scope, I would like to suggest packing variables in struct more efficiently.
OffersBase.sol has an Offer struct:

    struct Offer {
        uint64 expiresAt;
        address bidder;
        uint16 offerCut;
        uint128 total;
        uint128 unsuccessfulFee;
    }

From https://solidity.readthedocs.io/en/v0.4.21/miscellaneous.html
"Statically-sized variables (everything except mapping and dynamically-sized array types) are laid out contiguously in storage starting from position 0. Multiple items that need less than 32 bytes are packed into a single storage slot if possible",
"Finally, in order to allow the EVM to optimize for this, ensure that you try to order your storage variables and struct members such that they can be packed tightly. "

Also, you should take into consideration that smaller data types like uint128 or uint64 are not always more efficient than uint (which defaults to uint256):
"When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size."

Impact

Low

Fix

    struct Offer {
        uint128 total;
        uint128 unsuccessfulFee;
        uint64 expiresAt;
        uint16 offerCut;
        address bidder;
    }

or you could try to find a more efficient way to store this structure.

Ethereum address

0x09Cf79Bdf8F68739979C8c825C103A7538Bd4f4b

[sunsetlover]

But isn't it already packed optimally in two 256 bit words in their code? How can you make it smaller?

Also, be careful in changing the order. When an offer is updated with more ether, total and expiresAt are both updated. In the way they currently have it, this only changes one 256 bit word (5000 gas), but in your fix, its changing both 256 bit words (costing 10,000 gas).

Edit: actually I’m not actually sure how to update both of those variables at the same time without updating the entire struct for that tokenID (at least with solidity specifically). You could create two mappings: one holds the first word and the other holds the second word though paying the extra gas for clarity may just be worth it.

[hwrdtm]

Thanks @pauliax for your feedback! We will take it into consideration.

[arthcmr]

Thanks for your participation, @pauliax! Our team has reviewed your submission, and have decided to reject the issue.