safe math for contracts

[goastler]

fixes prosopo/captcha-private#504

[pr-explainer-bot]

Pull Request Review

Hey there! 👋 I've summarized the previous results for you. Here's a markdown document for your pull request review:

Changes

1. Added use common::Math; in line 6.
2. Modified the calculation of env_max_user_history_age_seconds in line 28.
3. Modified the calculation of env_max_user_history_age_blocks in lines 38-43.
4. Modified the calculation of age_threshold in line 52.
5. Modified the calculation of summary.score in lines 88-92.
6. Modified the calculation of before in line 104.
7. Modified the calculation of next in line 138.

Suggestions

1. In line 28, instead of using multiple wrapping_mul calls, it would be more readable to use the mul method from the Math module.
2. In lines 38-43, instead of using Math::add_panic and Math::div_panic separately, it would be more concise to use the add_div_panic method from the Math module.
3. In lines 88-92, instead of using separate variables for total and correct, it would be clearer to calculate correct directly in the calculation of summary.score.

Bugs

The Math module in common/src/lib.rs could potentially have math errors, such as overflow or division by zero. It would be good to review the implementation of the methods in this module to ensure they handle such cases properly.

Improvements

In the get_max_user_history_age_blocks function in captcha/src/lib.rs, the calculation of env_max_user_history_age_blocks can be refactored for better readability. Instead of using Math::add_panic and Math::div_panic separately, we can use the add_div_panic method from the Math module. Here's the refactored code snippet:

pub fn get_max_user_history_age_blocks(&self) -> u32 {
    let env_max_user_history_age_blocks: u32 = Math::add_div_panic(
        self.get_max_user_history_age_seconds(),
        self.get_block_time() as u32,
        1,
    );
    env_max_user_history_age_blocks
}

Rating

Code Rating: 7/10

Criteria:

• Readability: The code is generally readable and follows Rust conventions.
• Performance: The code seems efficient as it uses checked operations to avoid overflow.
• Security: The code does not raise any immediate concerns, but it would be good to ensure proper error handling and validation of inputs in the calling code.

That's it! Feel free to make any necessary changes based on the feedback. Good luck with your pull request! 🚀