`checked` variants of `I256` arithmetic functions

[ozgunozerk]

What problem does your feature solve?

Right now, we don't have checked_sub, checked_mul, etc. for I256 custom type. This hinders the developers to write additional math libraries that may require the checked versions.

In our case, we are writing a fixed_point_math library, and a WAD equivalent for Soroban. However, especially the error-handling part becomes a bit inconsistent due to lack of checked variants.

Here is a code piece:

pub(crate) fn checked_mul_div_floor(env: &Env, x: &I256, y: &I256, z: &I256) -> Option<I256> {
    let zero = I256::from_i32(env, 0);
    let r = x.mul(y);

    // we handle `DivisionByZero` case, and return `None` as expected by the `checked` variant
    if *z == zero {
        return None;
    }

    if (r < zero && *z > zero) || (r > zero && *z < zero) {
        // ceil is taken by default for a negative result

        // if `rem_euclid` fails, this will panic, but it should have returned `None` instead...
        let remainder = r.rem_euclid(z);
        let one = I256::from_i32(env, 1);

        // similarly, if `div` or `sub` fails, this will panic, but it should have returned `None` instead...
        Some(r.div(z).sub(if remainder > zero { &one } else { &zero }))
    } else {
        // floor is taken by default for a positive or zero result
        Some(r.div(z))
    }
}

What would you like to see?

checked variants of all the arithmetic functions

What alternatives are there?

I saw that, behind the scenes, env.i256_{operation} is used. For example:

    pub fn mul(&self, other: &I256) -> I256 {
        let val = self.env.i256_mul(self.val, other.val).unwrap_infallible();
        I256 {
            env: self.env.clone(),
            val,
        }
    }

However, it is quite unintuitive, and even impossible to use the underlying i256_{operation} functions as alternative.

Impossible, due to:

• these functions are private, not public

Unintuitive, due to:

• we have to import i256Val type
• it is returning Result, and we have to map the Result into Option
• it is not named as checked, which creates confusion
• the required argument type I256Val, is reachable only through self.to_val_type(), which adds additional friction

[leighmcculloch]

+1 The 256-bit math host fns support checked, so we should expose checked_ fns in the sdk.

https://github.com/stellar/rs-soroban-env/blob/97546e04908701935dcf81d36bb0a69ac761430e/soroban-env-common/env.json#L443-L698

[leighmcculloch]

Actually it looks like the 256-bit math host fns do not return errors, and the docs are incorrect.

For example, the docs for u256_add say:

Performs checked integer addition. Computes lhs + rhs, returning ScError if overflow occurred.

The error is not returned to the guest, the host traps when the error occurs. Typically host fns docs will say, "Traps if...," with this type of error handling. Opened stellar/rs-soroban-env#1629 to address docs.

So to add the checked fns the SDK will either need to:
1️⃣ Perform the check in the guest side, meaning the check happens twice once in the guest and once in the host.
2️⃣ Add new host fns that return the failure as an error val type.

The SDK could do 1️⃣, and later be optimized with 2️⃣. However, doing 1️⃣ would mean that there are two implementations of all the checks that must be consistent and some checked fns, such as pow, would be non-trivial.

[leighmcculloch]

I got an agent to write impls for these to see what the complexity would look like, and in general I don't think we should be duplicating this amount of complexity guest side:

• Add checked arithmetic methods to U256 and I256 #1665

So I think we should pursue 2️⃣ from my comment above if these are needed.

To do 2️⃣, a conversation should be started in:

• https://github.com/orgs/stellar/discussions/categories/core-advancement-proposals

[ozgunozerk]

I think 2️⃣ would be a great solution! Will start a conversation there!

[ozgunozerk]

The CAP has been opened, and some discussion was there. How should we proceed on this one? @leighmcculloch

[leighmcculloch]

The CAP has been opened

I see it:

• `checked` variants of `I256` arithmetic functions stellar-protocol#1834

@anupsdf @dmkozh @sisuresh ☝🏻