Stdlibs test returning error when rebased to next

[0xkanekiken]

On rebasing from the latest next branch, we see some issues when trying to execute some miden program which seems to be working earlier with no hiccups.

@Al-Kindi-0 highlighted this issue when he was running fri_fold2_ext2 test.
Steps to reproduce the error:

1. Go to the following commit in Al's miden fork
2. If you run cargo test fri_fold2_ext2, the program will get executed, but the final stack will not match as the expected stack is not what is written in the test yet. But if we uncomment the part and rerun the test, the test fails even before and returns the following error:

running 1 test
thread 'stdlib::crypto::fri::fri_fold2_ext2' panicked at 'called `Result::unwrap()` on an `Err` value: NotU32Value(BaseElement(6666087519483434683))', miden/tests/integration/helpers/mod.rs:189:36
stack backtrace:

3. We observed a weird thing when we tried to add some redundant code(which doesn't affect the program) in the proc, e.g., push.3 drop, the program was still failing. When we inlined this procedure and used it directly in test program, the test compiled.

The following is working:

use.std::crypto::fri
 begin
          exec.fri::preprocess
          dup                         #[add_i,add_i,num_q,d,g,t_d,add_f,...]
          movdn.6                     #[add_i,num_q,d,g,t_d,add_f,add_i,...]
          mem_storew                  #[num_q,d,g,t_d,...]
          push.0.0.0
          adv_loadw                   #[0,p,e1,e0,d,g,t_d,...]
          drop                        #[p,e1,e0,d,g,t_d,...]
          dup                         #[p,p,e1,e0,d,g,t_d,...]
          movup.5                     #[g,p,p,e1,e0,d,t_d,...]
          swap                        #[p,g,p,e1,e0,d,t_d,...]
          exp.u32 
          push.4
          drop
 end

whereas the following is not:

 use.std::crypto::fri
 begin
        exec.fri::preprocess
        exec.fri::verify_fri
 end

export.verify_fri
    dup                         #[add_i,add_i,num_q,d,g,t_d,add_f,...]
    movdn.6                     #[add_i,num_q,d,g,t_d,add_f,add_i,...]
    mem_storew                  #[num_q,d,g,t_d,...]
    push.0.0.0
    adv_loadw                   #[0,p,e1,e0,d,g,t_d,...]
    drop                        #[p,e1,e0,d,g,t_d,...]
    dup                         #[p,p,e1,e0,d,g,t_d,...]
    movup.5                     #[g,p,p,e1,e0,d,t_d,...]
    swap                        #[p,g,p,e1,e0,d,t_d,...]
    exp.u32                #[poe,p,e1,e0,d,t_d,...]
    push.4
    drop        
end

[bobbinth]

Quick question: what is the int value of BaseElement(6666087519483434683)?

[0xkanekiken]

Quick question: what is the int value of BaseElement(6666087519483434683)?

15205196862421381961 is the int value

[bobbinth]

This is not a u32 value, right? And so the error is correct?

[0xkanekiken]

Agreed but the following is passing:

export.verify_fri
        dup                         #[add_i,add_i,num_q,d,g,t_d,add_f,...]
        movdn.6                     #[add_i,num_q,d,g,t_d,add_f,add_i,...]
        mem_storew                  #[num_q,d,g,t_d,...]
        push.0.0.0
        adv_loadw                   #[0,p,e1,e0,d,g,t_d,...]
        drop                        #[p,e1,e0,d,g,t_d,...]
        dup                         #[p,p,e1,e0,d,g,t_d,...]
        movup.5                     #[g,p,p,e1,e0,d,t_d,...]
        swap                        #[p,g,p,e1,e0,d,t_d,...]
        exp.u32                #[poe,p,e1,e0,d,t_d,...]
end

And on adding push.2 and drop to the above proc after exp.u32, it is failing with a NotU32Value error.

export.verify_fri
        dup                         #[add_i,add_i,num_q,d,g,t_d,add_f,...]
        movdn.6                     #[add_i,num_q,d,g,t_d,add_f,add_i,...]
        mem_storew                  #[num_q,d,g,t_d,...]
        push.0.0.0
        adv_loadw                   #[0,p,e1,e0,d,g,t_d,...]
        drop                        #[p,e1,e0,d,g,t_d,...]
        dup                         #[p,p,e1,e0,d,g,t_d,...]
        movup.5                     #[g,p,p,e1,e0,d,t_d,...]
        swap                        #[p,g,p,e1,e0,d,t_d,...]
        exp.u32                #[poe,p,e1,e0,d,t_d,...]
        push.2
        drop
end

We should have this error in the first case as well

[bobbinth]

Yep, that's definitely a bug. Could we create a small test which triggers this?

[0xkanekiken]

Sure, I'll try to narrow it down to a small test.

[itzmeanjan]

I was trying to rebase my PR #495 , bringing latest work from next (upstream) branch and I'm seeings errors like below, when running ec_ext5 standard library test cases

cargo test -p miden ec_ext5

...

failures:

---- stdlib::math::ec_ext5::test_ec_ext5_gen_multiplication stdout ----
thread 'stdlib::math::ec_ext5::test_ec_ext5_gen_multiplication' panicked at 'called `Result::unwrap()` on an `Err` value: NotBinaryValue(BaseElement(658699090402360355))', miden/tests/integration/helpers/mod.rs:189:36
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    stdlib::math::ec_ext5::test_ec_ext5_gen_multiplication

Note
It seems that the way field element is expected, due to presence of if.true ... end code block ahead, can't be found on stack. Meaning it's supposed to be {0, 1} as result of previous line of instruction, but some change has made its way into Miden VM implementation s.t. it's no more the case which is exactly why tests are failing. Error is correct, but due to some bug in field element representation, value expected to be {0, 1} is not anymore {0, 1}, which is why executor is throwing such error.

How to reproduce this error ?

git clone https://github.com/itzmeanjan/miden.git

pushd miden

git remote add upstream https://github.com/maticnetwork/miden.git
git checkout next
git pull upstream next
git checkout ec-ext5-mulgen
git merge next
cargo test -p miden ec_ext5

popd

[tohrnii]

@itzmeanjan your issue seems to be unrelated. We made a change to mem_store and loc_store instructions recently (#500) to also drop the element from stack so the subsequent drop instruction is no longer required. I did this change to your PR for all the occurrences of loc_store and the tests seems to be passing. Can you please check if this fixes the issue for you or if it still persists?

[0xkanekiken]

Closed by #555

[itzmeanjan]

@itzmeanjan your issue seems to be unrelated. We made a change to mem_store and loc_store instructions recently (#500) to also drop the element from stack so the subsequent drop instruction is no longer required. I did this change to your PR for all the occurrences of loc_store and the tests seems to be passing. Can you please check if this fixes the issue for you or if it still persists?

Thanks, I didn't know that.