search

0xPolygonMiden / miden-base

Core components of the Polygon Miden rollup
MIT License
67 stars 40 forks source link

Rework tests to prevent root context calls and remove kernel workarounds #851

Closed phklive closed 5 days ago

phklive commented 2 weeks ago

Feature description

The testing framework in Miden-base executes code directly from the kernel root context. Which should not be possible during "normal" execution of the virtual machine in the context of the Miden rollup.

This method makes some of the checks fail and forces us to use workarounds in the kernel code. One example is the following code:

https://github.com/0xPolygonMiden/miden-base/blob/e5f51018f59018391f40708653d253c751c249fc/miden-lib/asm/kernels/transaction/lib/account.masm#L543-L563

https://github.com/0xPolygonMiden/miden-base/blob/e5f51018f59018391f40708653d253c751c249fc/miden-lib/asm/kernels/transaction/lib/account.masm#L507-L525

https://github.com/0xPolygonMiden/miden-base/blob/e5f51018f59018391f40708653d253c751c249fc/miden-tx/src/host/account_procs.rs#L92-L96

These pieces of code check if the caller is equal to [0,0,0,0] which would only happen if the call is being made directly from the kernel context and not from a user context, which should be the case in a general circumstances.

We need to remove this workaround by:

  1. Reworking how we test code in Miden-base, without making calls from root context
  2. Making sure that the caller is not equal to [0,0,0,0] the call having been made from a valid user context
  3. Remove the conditional code from account.masm here and here and account_procs.rs

Why is this feature needed?

We need to make sure that we remove workarounds as soon as possible to keep the cleanest codebase possible.

igamigo commented 1 week ago

I think I got some of this working but not sure if there is any alternative: Basically I'm exposing more calls through the account interface and calling instead of execing. And then of course assembling/running the code or transaction with the new account code. Does that sound OK or is there a better approach? Here's a rough draft, most tests pass except 2 or 3 for which the interface is not correctly set up I think.

bobbinth commented 1 week ago

I think this approach works (assuming we can make all tests work like that).

igamigo commented 5 days ago

Closed by #857