BwareBridge Code Style Findings

BBE-01C: Inconsistent Proxy Gap

Description:

All contracts have been adjusted to at-most possess 50 storage slots which are offset via a __gap array declared at the end of the contract, however, Initializable does not conform to this pattern.

Example:

Copy
85uint256[50] private __gap;

Recommendation:

We advise the pattern to be applied to the rest of hte contracts as well by setting the size of the __gap to the corresponding number instead of 50. To note, the two bool variables of Initializable are tight-packed into a single slot which is usually ill-advised for proxy-based contracts so we advise potential padding to be added between them as well.

Alleviation:

The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.

BBE-02C: Loop Iterator Optimization

Description:

The iterator of the for loop within __MultiSig_init_unchained is utilized as is once and utilized incremented by one twice.

Example:

Copy
775for (uint256 i = 0; i < owners_len; i++) {
776    address ith_owner = owners[i];
777    require(ith_owner != address(0), "Empty-address owner provided");
778    require(_get_owner_index[ith_owner] == 0, "Duplicate owner provided");
779
780    _get_owner_index[ith_owner] = i + 1;
781    _get_owner_addr[i + 1] = ith_owner;
782}

Recommendation:

We advise the loop to be adjusted to for (uint256 i = 1; i <= owners_len; i++) and the respective statements to be adjusted, reducing the gas cost of the function.

Alleviation:

The codebase was updated exactly according to our recommendation thereby alleviating this exhibit.

BBE-03C: Operation Consistency

Description:

The addition of a new entry to the bitmap is done so via an OR (|) operation whereas a removal from the bitmap is done via a simple subtraction (-) instead of a XOR operation (^).

Example:

Copy
883if (_order_state.order_hash == revoked_order && (_order_state.bitmap_confirm & uint128(o_bit) > 0)) {
884    _order_state.yet_confirmed++;
885    _order_state.bitmap_confirm -= uint128(o_bit);
886
887    emit Revoke(msg.sender, revoked_order);
888}

Recommendation:

We advise either style to be adopted to ensure the codebase remains legible and is consistent. All rudimentary operations utilize the same gas and as such, it is entirely up to the Bware team to decide which style should be adopted.

Alleviation:

The XOR and assignment operation is now correctly utilized (^=) in the specified statement.

BBE-04C: Redundant Visibility Specifiers

Description:

The linked constant variable declarations are set as public generating a getter function for them redundantly.

Example:

Copy
997bytes32 public constant PERMIT_TYPEHASH = keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");
998bytes32 public constant REDEEM_TYPEHASH = keccak256("Redeem(address authorizer,address to,uint256 volume,uint256 chainId,uint256 txHash)");
999bytes32 public constant DOMAIN_TYPEHASH = keccak256("EIP712Domain(string name,uint256 chainId,address verifyingContract)");

Recommendation:

We advise them to be set as internal or private given that a getter for them should not exist and they should simply be replicated as constant variables wherever they are necesssary.

Alleviation:

The development team has acknowledged this exhibit but decided to not apply its remediation in the current version of the codebase citing time constraints.