chore: use saturating sub with max_ttl computation

[mootz12]

What

Use saturating_sub when computing max_ttl.

Why

Add another defense layer to potential misconfigurations, and improve consistency with math usage in the repository.

Closes #1791

Known limitations

None

[Copilot]

Pull request overview

This PR makes Storage::max_ttl() more defensive and consistent by using saturating arithmetic when computing the maximum TTL from ledger values.

Changes:

• Switch max_ttl computation from max - seq to max.saturating_sub(seq) to avoid underflow in edge/misconfigured cases.

[leighmcculloch]

This is fine but the host guarantees that max ttl is greater than the current seq, so in practice this will have no impact.

Will this introduce instructions into the wasm guest side that are redundant? I think so. Although maybe it won't be meaningful difference since the minus will already be doing a check when compiled with checks on.

What's the motivation for doing this?

If we keep it the way it is we can add a comment explaining why the sub is safe.

[mootz12]

Will this introduce instructions into the wasm guest side that are redundant?

Actually, this reduces size. Seems the WASM code for saturating_sub is denser than a regular subtraction with overflow-checks = true. Turning overflow checks off, the regular subtraction is smaller.

The test:

#![no_std]
use soroban_sdk::{contract, contractimpl, Env, Symbol};

#[contract]
pub struct Contract;

#[contractimpl]
impl Contract {
    pub fn put(e: Env, key: Symbol, val: Symbol) {
        e.storage().persistent().set(&key, &val);
        let max_ttl = e.storage().max_ttl();
        e.storage().persistent().extend_ttl(&key, max_ttl, max_ttl);
    }

    pub fn get(e: Env, key: Symbol) -> Option<Symbol> {
        let entry = e.storage().persistent().get(&key);
        let max_ttl = e.storage().max_ttl();
        if entry.is_some() {
            e.storage().persistent().extend_ttl(&key, max_ttl, max_ttl);
        }
        entry
    }

    pub fn del(e: Env, key: Symbol) {
        e.storage().persistent().remove(&key)
    }
}

Results

• regular sub, overflow checks = 830 bytes
• saturating sub, overflow checks = 803 bytes
• regular sub, no overflow checks = 789 bytes

What's the motivation for doing this?

The host does prevent this from occurring. The SDK uses safe math functions in most locations outside of indexes/counters. This just brings this up to that standard.