Unnecessary usage of modifier

Github username: -- Twitter username: -- Submission hash (on-chain): 0xa12d42f476f20830a264cbc23a927d34e2ef13fd1e0364ee588147dbb15ca3be Severity: low

Description: Description

The modifier previousBondMustNotBeatMaxPrevious is used in notifyOfArbitrationRequest where it checks whether the function parameter max_previous greater than question’s current bond. However the parameter max_previous isn’t used within the function. Making the modifier unnecessary.

Furthermore, the modifier is used in submitAnswer functions which again checks whether the provided function parameter max_previous is greater than question’s current bond, and isn’t used within the functions. However is every submitAnswer function alongside previousBondMustNotBeatMaxPrevious modifier, there is the bondMustDoubleAndMatchMinimum modifier which performs even stricter checks, requiring the msg.value to be at least two times greater than the current bond, or at least in par with the min_bond in case the current bond is zero.

In conclusion, no matter what value is provided in the parameter max_previous , it will be an additional protection against change in bond increases. The msg.value is the best way to revert a submitAnswer , by providing as match as a user wants to pay, and in case the current question’s bond increases before the user’s transaction gets included, in case the msg.value is not enough, the function will be reverting.

Attachments

Proof of Concept (PoC)

function notifyOfArbitrationRequest(bytes32 question_id, address requester, uint256 max_previous) 
    onlyArbitrator(question_id)
    stateOpen(question_id)
    previousBondMustNotBeatMaxPrevious(question_id, max_previous)
external {
    require(questions[question_id].finalize_ts > UNANSWERED, "Question must already have an answer when arbitration is requested");
    questions[question_id].is_pending_arbitration = true;
    emit LogNotifyOfArbitrationRequest(question_id, requester);
}
...
function submitAnswer(bytes32 question_id, bytes32 answer, uint256 max_previous) 
    stateOpen(question_id)
    bondMustDoubleAndMatchMinimum(question_id)
    previousBondMustNotBeatMaxPrevious(question_id, max_previous)
external payable {
    _addAnswerToHistory(question_id, answer, msg.sender, msg.value, false);
    _updateCurrentAnswer(question_id, answer);
}

Revised Code\ The modifier previousBondMustNotBeatMaxPrevious is unnecessary and can be removed from the following functions, alongside the parameter max_previous : submitAnswer , submitAnswerFor , submitAnswerCommitment and notifyOfArbitrationRequest .

hats-finance / SeeR-PM-0x899bc13919880db76edf4ccd72bdfa5dfa666fb7

Unnecessary usage of modifier #9