Oracles Code Style Findings

ORA-01C: Inefficient Block Number Comparison

Description:

The latter conditional of the isMerkleRootVoting function is inefficient as the case whereby lastRewardBlockNumber is greater than block.number is impossible due to lastRewardBlockNumber being set to the current block.number by the oracle itself.

Example:

Copy
138uint256 lastRewardBlockNumber = rewardEthToken.lastUpdateBlockNumber();
139return merkleDistributor.lastUpdateBlockNumber() < lastRewardBlockNumber && lastRewardBlockNumber < block.number;

Recommendation:

We advise the comparison to be changed to an inequality one instead, better illustrating its purpose which is guarding against a reward and merkle root vote to be processed in a single block.

Alleviation:

The comparison was adjusted to an inequality one according to our recommendation.

ORA-02C: Multiple Top-Level Declarations

Description:

The Oracles contract contains two extra top-level interface declarations.

Example:

Copy
17interface IAccessControlUpgradeable {
18    /**
19     * @dev See {AccessControlUpgradeable-getRoleMemberCount}.
20     */
21    function getRoleMemberCount(bytes32 role) external view returns (uint256);
22
23    /**
24     * @dev See {AccessControlUpgradeable-getRoleMember}.
25     */
26    function getRoleMember(bytes32 role, uint256 index) external view returns (address);
27}
28
29interface IPrevOracles {
30    /**
31    * @dev Function for retrieving current rewards nonce.
32    */
33    function currentNonce() external view returns (uint256);
34}

Recommendation:

We advise them to be declared in their dedicated contracts to ensure standard-compliant code structure.

Alleviation:

The top level declarations have been omitted from the codebase and a new IOraclesV1 file was created and is now imported in their place.

ORA-03C: Redundant Visibility Specifier

Description:

The linked variable is meant to be used as an internally accessible constant and has no use outside of the contract as it represents a static value.

Example:

Copy
46bytes32 public constant ORACLE_ROLE = keccak256("ORACLE_ROLE");

Recommendation:

We advise it to be set to either internal or private to reduce the bytecode size of the contract.

Alleviation:

The Stakewise team stated that they prefer to retain the current visibility in place to ensure non-technically attuned persons can still read the status of users in the system when using basic tools such as Etherscan.

ORA-04C: Undocumented Consortium Level

Description:

The consortium level needed to be achieved for a particular vote is greater-than 66.66~% of the total oracles, as indicated by dividing both members of the inequality by 3 resulting in a 2/3 multiplier for the signatures.

Example:

Copy
284require(
285    signatures.length.mul(3) > getRoleMemberCount(ORACLE_ROLE).mul(2),
286    "Oracles: invalid number of signatures"
287);

Recommendation:

We advise this trait to be properly documented, potentially in a dedicated pure function, as currently value literals are directly used that can be ambiguous.

Alleviation:

The consortium calculation is now properly performed by an internal function better illustrating its purpose and optimizing the codebase.