Inconsistent Comment and Condition in Boundary Check for Scalar Market Creation

[hats-bug-reporter[bot]]

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

Description: Description

The function createScalarMarket is responsible for creating a scalar market and contains a boundary check on the upperBound parameter, with the intention of ensuring it does not conflict with reserved values in the system. The check enforces that upperBound must be strictly less than type(uint256).max - 2. However, the accompanying comment indicates that Reality reserves values for INVALID and UNRESOLVED_ANSWER, suggesting that the maximum allowed value should be type(uint256).max - 3. This inconsistency between the condition and the comment creates ambiguity regarding the intended constraint. If the check aims to exclude only the last two reserved values, the condition should allow for equality (<=). Alternatively, if the current condition is correct, the comment should be updated to reflect that the maximum allowable value is three units less than uint256.max. This ambiguity could lead to misunderstandings or incorrect assumptions during further development or use of the contract.

Attachments

1. Proof of Concept (PoC)

function createScalarMarket(CreateMarketParams calldata params) external returns (address) {
    require(params.upperBound > params.lowerBound, "upperBound must be higher than lowerBound");
    // values reserved by Reality for INVALID and UNRESOLVED_ANSWER.
@>      require(params.upperBound < type(uint256).max - 2, "upperBound must be less than uint256.max - 2"); 
    require(params.outcomes.length == 2, "Outcomes count must be 2");

    string[] memory encodedQuestions = new string[](1);
    encodedQuestions[0] = encodeRealityQuestionWithoutOutcomes(params.marketName, params.category, params.lang);

    return createMarket(
        params,
        params.marketName,
        InternalMarketConfig({
            encodedQuestions: encodedQuestions,
            outcomeSlotCount: 3, // additional outcome for Invalid Result.      
            templateId: REALITY_UINT_TEMPLATE
        })
    );
}

2. Revised Code

Review the logic and intention behind the boundary check on upperBound to ensure clarity and consistency. Either update the comment to accurately describe the enforced constraint or adjust the condition to align with the comment. This will improve code readability and prevent potential misinterpretations of the boundary restrictions for scalar market creation.

[clesaege]

Yeah, the code did reserve the last 3 slots, but even type(uint256).max - 2 is way too high to make any sense (we'd end up around 1E77 and dividing by that would lead to 0, and a market with a maximum value of 1E77 would be obviously wrong and not pass curation).

As per contest rules are excluded:

• Issues about code/project quality which do not lead to exploitable vulnerabilities.
• Any issue that is only theoretical but can't happen in practice.

[Giannis443]

Hey @clesaege, thank you for your reply. It is up to users' creativity and foresight when they create a market, as they may envision outcomes and values that make sense within their specific context. For instance, while type(uint256).max - 2 may seem excessively high in many cases, there are potential scenarios where such a high value could be relevant.

One example could be predicting the number of observable planets by the end of 2030. Given that this number isn't finite and considering that technological advancements could drastically improve our observation capabilities in the coming years, the rate at which we discover new planets could increase exponentially. Under these circumstances, a value around 1E77 could be feasible.

Another potential example can be a market predicting the maximum storage capacity of a single computer chip. Technology has been advancing rapidly and future advancements could significantly increase the limits.

Under these circumstances, a value around 1E77 is feasible.

[clesaege]

Such a high value would be likely to cause issues in other part of the code and here being able to set a max value to 115792089237316195423570985008687907853269984665640564039457584007913129639933 instead of 115792089237316195423570985008687907853269984665640564039457584007913129639932 would provide little interest.