Omniscia rain protocol Audit
TierwiseCombine Manual Review Findings
TierwiseCombine Manual Review Findings
TCE-01M: Potential Nullification of Upcoming Tiers
Type | Severity | Location |
---|---|---|
Logical Fault | TierwiseCombine.sol:L48-L64 |
Description:
The selectLte
function will nullify any upcoming tiers to "never-held" in case they do not match the blockNumber_
argument.
Example:
contracts/tier/libraries/TierwiseCombine.sol
48/// Given a list of reports, selects the best tier in a tierwise fashion.49/// The "best" criteria can be configured by `logic_` and `mode_`.50/// Logic can be "every" or "any", which means that the reports for a given51/// tier must either all or any be less than or equal to the reference52/// `blockNumber_`.53/// Mode can be "min", "max", "first" which selects between all the block54/// numbers for a given tier that meet the lte criteria.55/// @param reports_ The list of reports to select over.56/// @param blockNumber_ The block number that tier blocks must be lte.57/// @param logic_ `LOGIC_EVERY` or `LOGIC_ANY`.58/// @param mode_ `MODE_MIN`, `MODE_MAX` or `MODE_FIRST`.59function selectLte(60 uint256[] memory reports_,61 uint256 blockNumber_,62 uint256 logic_,63 uint256 mode_64) internal pure returns (uint256) {65 unchecked {66 uint256 ret_;67 uint256 block_;68 bool anyLte_;69 uint256 length_ = reports_.length;70 for (uint256 tier_ = 1; tier_ <= 8; tier_++) {71 uint256 accumulator_;72 // Nothing lte the reference block for this tier yet.73 anyLte_ = false;74
75 // Initialize the accumulator for this tier.76 if (mode_ == MODE_MIN) {77 accumulator_ = TierConstants.NEVER_REPORT;78 } else {79 accumulator_ = 0;80 }81
82 // Filter all the blocks at the current tier from all the83 // reports against the reference tier and each other.84 for (uint256 i_ = 0; i_ < length_; i_++) {85 block_ = TierReport.tierBlock(reports_[i_], tier_);86
87 if (block_ <= blockNumber_) {88 // Min and max need to compare current value against89 // the accumulator.90 if (mode_ == MODE_MIN) {91 accumulator_ = block_.min(accumulator_);92 } else if (mode_ == MODE_MAX) {93 accumulator_ = block_.max(accumulator_);94 } else if (mode_ == MODE_FIRST && !anyLte_) {95 accumulator_ = block_;96 }97 anyLte_ = true;98 } else if (logic_ == LOGIC_EVERY) {99 // Can short circuit for an "every" check.100 accumulator_ = TierConstants.NEVER_REPORT;101 break;102 }103 }104 if (!anyLte_) {105 accumulator_ = TierConstants.NEVER_REPORT;106 }107 ret_ = TierReport.updateBlockAtTier(108 ret_,109 tier_ - 1,110 accumulator_111 );112 }113 return ret_;114 }115}
Recommendation:
We advise this trait of the system to be evaluated and potentially omitted as it can lead to unintended tier wipes from a valid user.
Alleviation:
The Rain Protocol team stated that this is a core functionality of their system and instead opted to expand the documentation of the function to warn that care should be utilized due to its status overriding nature. As a result, we consider this exhibit adequately dealt with.