Omniscia rain protocol Audit

TierOps Code Style Findings

TierOps Code Style Findings

TOP-01C: Potential Misuse of Select LTE Instruction

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:

contracts/vm/ops/TierOps.sol
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, taking
94// 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; // & 00000011
98 uint256 reportsLength_ = operand_ & 0x1F; // & 00011111
99
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

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:

contracts/vm/ops/TierOps.sol
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.