MultiSigWallet Code Style Findings

MSW-01C: Data Location Optimization

Description:

The linked functions are declared as external and possess a memory argument.

Example:

Copy
115function initialize(address[] memory _signers) 
116    external 
117    validNumberOfSigners(_signers.length)
118{
119    require(signers[0] == msg.sender);
120    require(signers.length == 1, "Contract already initialized");
121
122    for (uint i=0; i<_signers.length; i++) {
123        require(_signers[i] != address(0), "Not a valid signer");
124    }
125    signers = _signers;
126}

Recommendation:

We advise the memory argument to be set as calldata greatly optimizing the gas cost of the functions.

Alleviation:

The data locations were optimized to calldata correctly.

MSW-02C: Inefficient Loop Limit Evaluation

Description:

The linked for loops and statements access the length member of the signers array repeatedly from storage instead of temporarily storing it in memory.

Example:

Copy
230uint required = signers.length / 2 + 1;
231for (uint i=0; i<signers.length; i++) {
232    if (confirmations[transactionId][signers[i]])
233        count += 1;
234    if (count == required)
235        return true;
236}

Recommendation:

We advise an in-memory variable to be declared that stores the result of signers.length as it is not meant to change during the iterations and will result in a gas decrease for the relevant functions.

Alleviation:

The length of the array is now properly cached in the for loops linked by the exhibit.

MSW-03C: Inexistent Error Messages

Description:

The linked require checks possess no error message explicitly defined.

Example:

Copy
48modifier onlyWallet() {
49    require(msg.sender == address(this));
50    _;
51}

Recommendation:

We advise one to be set to aid in the debugging and readability of the codebase.

Alleviation:

Error messages were appropriately defined for all require invocations that were missing them.

MSW-04C: Redundant Variable Visibility

Description:

The two constant variables of the contract are declared as public redundantly generating bytecode for their getter functions.

Example:

Copy
21uint constant public MAX_SIGNER_COUNT = 50;
22uint constant public TRANSACTION_EXPIRATION_LIMIT = 30 days;

Recommendation:

We advise them to be set as private or internal as they bear no use outside of the contract context they are in.

Alleviation:

The two variables were properly set to private.

MSW-05C: Variable Tight-Packing

Description:

The Transaction struct can be optimized by re-ordering the elements declared within it.

Example:

Copy
36struct Transaction {
37    address destination;
38    uint value;
39    bytes data;
40    bool executed;
41    uint createdAt;
42}

Recommendation:

Solidity contains a mechanism known as tight-packing which will store two variables that fit in a single 256-bit variable under the same slot in storage, thus optimizing their gas cost. In the Transaction struct, the destination variable can be tight-packed with the executed variable as the former is 160-bits and the latter is 8-bits which total less than 256-bits. We advise it to be done so.

Alleviation:

The destination member was relocated to be tightly packed with the executed boolean.