Overflow, underflow and div by zero?

[viscid oriole]

Hello!

I had some questions about how to avoid these cases. I read in some places that adding overflow-checks=true was enough.
In other places I read that .checked_add(), checked_sub, .checked_div and .checked_mul was needed.

So I created a repo in order to experiment all these combinations.
You can check it out here: https://github.com/esteblock/overflow-soroban

My findings?
Even without any of the techniques above (so just +, - *, /), the contract went to panic for the undesired cases. Why? I don't know. What extra security provide the overflow-checks=true, .checked_add(), checked_sub, .checked_div and .checked_mul? Don't know

Does anyone know? Thanks!

[buoyant kestrel]

hold on.. are you saying that this should not panic because you're using a checked_ fn?

fn checked_increment_should_panic_overflow() {
    let env = Env::default();
    let contract_id = env.register_contract(None, IncrementContract);
    let client = IncrementContractClient::new(&env, &contract_id);

    
    assert_eq!(client.increment(&1), 1);
    let max_value: i128 = i128::MAX;
    client.checked_increment(&max_value);
}

...\overflow-soroban\increment_overflow_checks\src\test.rs

pub fn checked_increment(env: Env, n: i128) -> i128 {
   // (...)
   // Increment the count.
   count = count.checked_add(n).unwrap();
   // (...)
}

[verbal lily]

unwrapping the option from checked_add will panic if something if the option is the None variant. You would need to check for that yourself if you want to be safe.

[verbal lily]

You are running the contracts from test while setting the release profile.

You should use the dev profile instead : https://doc.rust-lang.org/cargo/reference/profiles.html#dev, which by default looks like ```toml
[profile.dev]
opt-level = 0
debug = true
split-debuginfo = '...' # Platform-specific.
debug-assertions = true
overflow-checks = true
lto = false
panic = 'unwind'
incremental = true
codegen-units = 256
rpath = false

so you have to explicitly set `overflow-checks` to false:
```toml
[profile.dev]
overflow-checks = false

[buoyant kestrel]

yep, most likely the .unwrap() causing the panic, but wanted to make sure on what his assumptions were (expected). Also, if that's the cause for the panic the err message is pretty precise:

Debug events (newest first):
   0: "Debug escalating error '' to panic"
   1: "Debug contract call invocation resulted in error Status(UnknownError(0))"
   2: "Debug caught panic 'called `Option::unwrap()` on a `None` value' from contract function 'Symbol(obj#2)'"

while a dev profile works, you can also just cargo t --release 🙂

[viscid oriole]

No.
I am saying that that should panic and it indeed does.
What is strange to me is that with just "+" it does also panics...

So my question is something like... is that checked fn is already implemented?

[viscid oriole]

Oh I did not know that.
Will try this

[viscid oriole]

Thanks! Will check that also!

[viscid oriole]

Very good feedback! Thank you very much!
I upgraded my code and now it does overflow when overflow_checks=false; and without any checked_fn.... as expected.

I hope this thread can help any other curious like me.

[buoyant kestrel]

I upgraded my code
... so, you updated your code to check for Option<> on checked_fn ?