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

test and benchmark infrastructure should panic on runs where stack is not initialized. #120

Open
Sword-Smith opened this issue Aug 15, 2024 · 0 comments
Labels
good first issue Good for newcomers

Comments

@Sword-Smith
Copy link
Contributor

Sword-Smith commented Aug 15, 2024

I recently wrote this implementation of Closure

    impl Closure for NoUnrolling {
        fn rust_shadow(&self, _stack: &mut Vec<BFieldElement>) {
            todo!()
        }

        fn pseudorandom_initial_state(
            &self,
            _seed: [u8; 32],
            _bench_case: Option<BenchmarkCase>,
        ) -> Vec<BFieldElement> {
            vec![]
        }
    }

I got a nasty panic deep inside of Triton VM, attempt to subtract with overflow. The reason being that I did not initialize the stack with 16 words as the testing framework requires you to do since it gives you full control over the initial state of the VM.

So pseudorandom_initial_state should have been implemented like so:

    fn pseudorandom_initial_state(
            &self,
            _seed: [u8; 32],
            _bench_case: Option<BenchmarkCase>,
        ) -> Vec<BFieldElement> {
            self.init_stack_for_isolated_run()
        }

The benchmarking and testing infrastructure should have caught this error. An assert that you initialize the stack with at least NUM_OP_STACK_REGISTERS (16) should be in place before the VM's stack is set. This should be done for both benchmarks and for tests.

There's no reason to implement the fix for any function that contains the word "deprecated" though, as this is test/benchmark infrastructure we are slowly phasing out.

@Sword-Smith Sword-Smith added the good first issue Good for newcomers label Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

1 participant