Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support comparing SDK objects across Env's #1360

Closed
leighmcculloch opened this issue Oct 3, 2024 · 0 comments · Fixed by #1436
Closed

Support comparing SDK objects across Env's #1360

leighmcculloch opened this issue Oct 3, 2024 · 0 comments · Fixed by #1436
Assignees

Comments

@leighmcculloch
Copy link
Member

leighmcculloch commented Oct 3, 2024

Feature request

For writing differential tests it is important that objects like a Vec, or Address, or any other SDK value, can be compared across Envs. This is because ideally when you are doing differential testing you can start both sets of test cases running in their own Envs so that things like Addresses, IDs, etc are all identical for easy comparison.

I'd like to be able to do this:

#[contract]
struct Token;
#[contractimpl]
impl Token {
    // ...
}

mod token {
    soroban_sdk::contractimport!(file = "token.wasm");
}

#[test]
fn test_differential() {
    let (auths1, events1, ledger1) = {
        let env = Env::default();
        let admin = Address::generate(&env);
        let id = env.register(Token, (&admin,));
        let token = TokenClient::new(&env, &id);

        let a = Address::generate(&env);
        let b = Address::generate(&env);
        token.mock_all_auths().mint(&a, &10);
        assert_eq!(token.balance(&a), 10);
        assert_eq!(token.balance(&b), 0);

        token.mock_all_auths().transfer(&a, &b, &2);
        (env.auths(), env.events().all(), env.to_ledger_snapshot())
    };
    let (auths2, events2, ledger2) = {
        let env = Env::default();
        let admin = Address::generate(&env);
        let id = env.register(token::WASM, (&admin,));
        let token = TokenClient::new(&env, &id);

        let a = Address::generate(&env);
        let b = Address::generate(&env);
        token.mock_all_auths().mint(&a, &10);
        assert_eq!(token.balance(&a), 10);
        assert_eq!(token.balance(&b), 0);

        token.mock_all_auths().transfer(&a, &b, &2);
        (env.auths(), env.events().all(), env.to_ledger_snapshot())
    };
    assert_eq!(auths1, auths2);
    assert_eq!(events1, events2);
    assert_eq!(ledger1, ledger2);
}

Today, if you run the above, it'll result in a panic when comparing the auths1 and auths2, and events1 and events2. The panic occurs because of the same env check here, and in similar places in other types:

impl Ord for Address {
fn cmp(&self, other: &Self) -> Ordering {
self.env.check_same_env(&other.env).unwrap_infallible();
let v = self
.env
.obj_cmp(self.obj.to_val(), other.obj.to_val())
.unwrap_infallible();
v.cmp(&0)

The check is reasonable, because the cmp functions of all the sdk types calls the env and asks the env to do the comparison.

When the cmp functions are called, if currently not building for wasm, the code could check if currently in the same env, and if not, convert the sdk type to it's XDR equivalent, and compare those.

What alternatives are there?

To expect people to do differential testing inside the same env, but that seems unrealistic. It would not be possible to just simply compare two auths or two events objects, becauses the addresses would be different.

github-merge-queue bot pushed a commit that referenced this issue Feb 12, 2025
### What
Updates every type that is a handle to a type stored in the host so that
it is comparable with itself via a conversion to an `ScVal` even when
the values are across different `Env`s.

### Why
To make it much easier to perform differential testing.

Today it is not possible to compare values of an SDK type, when the
values are a handles to values stored on different hosts. This is
because the comparison of SDK types is supported by calling through to
the host. It makes sense for comparison to work this way because the
value is on the host side.

However when writing tests it is helpful to be able to compare SDK types
even if they belong to different Envs. The main use case is differential
tests, where two sequences of invokes should be comparable.

In tests the efficiency of the comparison is not important, so
converting to ScVal is an okay way to fall back to do the comparison. In
some ways this is a bit of a hack, but it's the least disruptive way of
supporting the comparison, and avoids introducing a ton of code to the
Env that would otherwise be out of place.

Talking to @graydon we have quite solid testing around the consistency
of ScVal and Val values, so converting the Vals to ScVal to do the
comparison is safe.

Close #1360
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants