Skip to content

Independent testing of inner_test_validate_owner#172

Open
mariaKt wants to merge 7 commits intoproofsfrom
mk/specs-modifications
Open

Independent testing of inner_test_validate_owner#172
mariaKt wants to merge 7 commits intoproofsfrom
mk/specs-modifications

Conversation

@mariaKt
Copy link

@mariaKt mariaKt commented Mar 9, 2026

inner_test_validate_owner is called from multiple tests, specifically the *multisig ones. This PR checks that this is not the source of any errors through two independent tests (multisig and no multisig).

Refactored version

Note that inner_test_validate_owner is designed to accept all parameters needed to computed the expected Result, as well as a Result parameter (utilized to provide the Result value already computed by another operation in our tests). During its execution, it computes the expected Result and asserts for equality with the actual Result. In order to be able to test it, I provided another, refactored version of it, that replicates it exactly but only computes the expected Result .

Proof correctness

I checked the split nodes of the proof for test_validate_owner_multisig to make sure that we explore the paths that we should. As expected, there are several branches for owner check, multisig checks, unsigned_exists result, signers_count result, multisig initialized, and many for the unsigned_exists closures. However, I found no branching for the signers_count closures.

I tested the exact same closures structure with the following small test, which passes

struct Wrapper {
    tag: u8,
    a: [[u8; 32]; 3],
}

fn main() {
    let c = Wrapper {
        tag: 42,
        a: [[1u8; 32], [2u8; 32], [3u8; 32]],
    };
    let d = Wrapper {
        tag: 84,
        a: [[1u8; 32], [3u8; 32], [5u8; 32]],
    };

    let equal_count = c.a
        .iter()
        .filter_map(|first| {
            d.a.iter().find(|second| {
                first == *second
            })
        })
        .count();

    assert!(equal_count == 2);

}

, so I am not concerned. My explanation is that this is due to the path conditions that are present when the signers_count is reached. Execution reaches this point only after evaluation of unsigned_exists, that needs to evaluate the same condition for the pairs of keys-signers. So, the path conditions placed to make the computation of unsigned_exists are enough to continue evaluation without branching. Please let me know what you think.

@mariaKt mariaKt force-pushed the mk/specs-modifications branch 7 times, most recently from 779ffd9 to 426ac72 Compare March 9, 2026 22:42
@mariaKt mariaKt force-pushed the mk/specs-modifications branch 2 times, most recently from 6f8ebfe to 0d0f053 Compare March 10, 2026 00:06
@mariaKt mariaKt force-pushed the mk/specs-modifications branch from 0d0f053 to ba5acae Compare March 10, 2026 00:31
@mariaKt mariaKt marked this pull request as ready for review March 11, 2026 22:00
@mariaKt mariaKt requested review from Stevengre and dkcumming March 11, 2026 22:10
// )?;

// result

Copy link
Author

@mariaKt mariaKt Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to eventually use the commented out version of this test (which calls the refactored function that only computes the result and then the one that uses it and calls inner_test_validate_owner, the one that is used by all the tests). This way, we would have a test that includes exactly the function called from the other tests.

Argument for: We should have a proof for this function if we plan to find a way to replace this call with a spec or a summary. I would like to have a discussion about this though, as I am not sure what is doable at this stage where we still modify the semantics.

Argument against: This changes the proof's time from ~10.5h to ~28.8h.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using lemmas (for expected_validate_owner_result and inner_test_validate_owner), we can get the full test to execute in about 15min.

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 this pull request may close these issues.

2 participants