Omniscia Alliance Block Audit

SplitPayment Code Style Findings

SplitPayment Code Style Findings

SPT-01C: Potential Conditional Optimization

Description:

The current execution of splitTransfer will evaluate the fromThis_ boolean on each iteration of the shareholders inefficiently.

Example:

contracts/payment/SplitPayment.sol
59function splitTransfer(
60 address token_,
61 address from_,
62 uint256 totalAmount_,
63 Share[] memory shares_
64) internal {
65 unchecked {
66 bool fromThis_ = from_ == address(this);
67 for (uint256 i_ = 0; i_ < shares_.length; i_++) {
68 // FixedPointMath uses checked math so disallows overflow.
69 // Integrity checks disallow `0` shares.
70 // If there is any dust due to rounding error in the
71 // calculations it will favour the sender by simply never
72 // sending the rounding error to any recipient.
73 uint256 amount_ = totalAmount_.fixedPointMul(shares_[i_].share);
74 if (fromThis_) {
75 IERC20(token_).safeTransfer(shares_[i_].recipient, amount_);
76 } else {
77 IERC20(token_).safeTransferFrom(
78 from_,
79 shares_[i_].recipient,
80 amount_
81 );
82 }
83 }
84 }
85}

Recommendation:

We advise the code to instead evaluate the fromThis_ boolean clause directly in a single if statement that splits the execution to two separate for loops. While this will increase the one-time deployment cost of the contract due to an increase in bytecode size, it will significantly optimize the gas cost of its execution and will additionally render the fromThis_ variable declaration redundant.

Alleviation:

The code was split as advised, ensuring that the if conditional is evaluated at the upper level and that each branch of the if structure contains a for loop within that is optimal and executes a single statement.