didiermis commented 7 months ago

Title of changes made: [Fix] Avoiding arithmetic overflows in key scaling operations

Overview

The primary goal of the changes is to mitigate potential arithmetic overflows while performing some vital scaling operations in the "Afloat" and "Fund-Admin" pallets. The purpose is to ensure the safety of mathematical computations and maintain data integrity across the systems.

This PR adds some "safe" arithmetic functions to add, multiply amounts or perform other arithmetic operations without risking any overflow. Whenever such functions are called, they check if the given operation would result in an overflow. If true, it returns an error before executing the operation.

The changes were mainly targeting the "create_tax_credit()" in the "Afloat" Pallet and "calculate_drawdown_total_amount()" and "calculate_revenue_total_amount()" in the "Fund-Admin" Pallet.

Tickets

Fix #31

Implementation notes

The implementation involves two significant changes:

Addition of the safe arithmetic functions, "safe_add()" and "safe_multiply_offer()". These functions perform the mathematical operation and check for a potential overflow. If the operation would result in an overflow, they short-circuit and return an ArithmeticOverflow error.
Integration of the newly
created safe arithmetic functions into the existing logic. The safe functions are called before performing mathematical calculations in "create_tax_credit()" in "Afloat" Pallet and "calculate_drawdown_total_amount()" and "calculate_revenue_total_amount()" in the "Fund-Admin" Pallet. If an overflow is detected, the functions will return an error, stopping the operation and preserving the integrity of the system.

Interesting/controversial decisions

The changes performed might have some tradeoffs in terms of performance since we are checking for potential arithmetic overflow for each operation. However, these checks are essential to maintain the integrity of the system and avoid any data corruption. Additionally, the checks for overflow should have minimum performance impact since they are straightforward and fast to compute.

Test coverage

The changes include testing of the newly added functionality. In the "Afloat" pallets, we tested the potential overflow using the "replicate_overflow_for_start_take_sell_order()" function, and in the "Fund-Admin" pallets, we used the "replicate_overflow_for_a_drawdown_submission()" and "replicate_overflow_for_a_revenue_submission()". These tests can effectively detect and prevent any arithmetic overflow during the operations.

Loose ends

This PR doesn't cover every potential overflow scenario. Other parts of the codebase may also benefit from the addition of similar safe arithmetic operations.