Omniscia rain protocol Audit
TierOps Code Style Findings
TierOps Code Style Findings
TOP-01C: Potential Misuse of Select LTE Instruction
Type | Severity | Location |
---|---|---|
Language Specific | TierOps.sol:L97, L107 |
Description:
The SELECT_LTE
opcode will attempt to invoke the selectLte
function of TierwiseCombine
, however, if the arguments to it are off by a single digit in the reportsLength_
and the selection mode is set as MODE_MAX
(1
) it will lead to a zero-value entry being assigned to the reports_
array and thus yielding the maximum tier improperly due to the zero-value based uninitialized stack.
Example:
91// Stacks the result of a `selectLte` combinator.92// All `selectLte` share the same stack and argument handling.93// In the future these may be combined into a single opcode, taking94// the `logic_` and `mode_` from the `operand_` high bits.95else if (opcode_ == SELECT_LTE) {96 uint256 logic_ = operand_ >> 7;97 uint256 mode_ = (operand_ >> 5) & 0x3; // & 0000001198 uint256 reportsLength_ = operand_ & 0x1F; // & 0001111199
100 // Need one more than reports length to include block number.101 state_.stackIndex -= reportsLength_ + 1;102 baseIndex_ = state_.stackIndex;103 uint256 cursor_ = baseIndex_;104
105 uint256[] memory reports_ = new uint256[](reportsLength_);106 for (uint256 a_ = 0; a_ < reportsLength_; a_++) {107 reports_[a_] = state_.stack[cursor_];108 cursor_++;109 }110 uint256 blockNumber_ = state_.stack[cursor_];111
112 state_.stack[baseIndex_] = TierwiseCombine.selectLte(113 reports_,114 blockNumber_,115 logic_,116 mode_117 );118 state_.stackIndex++;119}
Recommendation:
We advise callers of the operation to be sufficiently warned of this potential side effect in the documentation of the function.
Alleviation:
After deliberation with the Rain Protocol team, we concluded that this particular issue stems from a core mechanic of how the stack operates and can be applicable to other operations as well. Given that it is a developer-oriented logic check, we collectively concluded that adequate developer education and deploy-time protective checks are sufficient to prevent this type of issue from arising in production code of other developers. As a combination of the aforementioned, we consider this exhibit nullified.
TOP-02C: Unoptimized Stack Index Adjustment
Type | Severity | Location |
---|---|---|
Gas Optimization | TierOps.sol:L39, L44, L61, L69, L79, L89, L101, L118 |
Description:
The stackIndex
is adjusted twice within the applyOp
function's execution branches redundantly as the first subtraction can be offset by a single unit.
Example:
38if (opcode_ == REPORT) {39 state_.stackIndex -= 2;40 baseIndex_ = state_.stackIndex;41 state_.stack[baseIndex_] = ITier(42 address(uint160(state_.stack[baseIndex_]))43 ).report(address(uint160(state_.stack[baseIndex_ + 1])));44 state_.stackIndex++;45}
Recommendation:
We advise this to be done so to optimize the gas cost of the function.
Alleviation:
The Rain Protocol team has stated that they plan to address this in an upcoming pull request. Given that this is a finding planned to be fixed, we consider this exhibit acknowledged.