P2ID script and test

[Dominik1999]

I want to create a P2ID MASM script, see #12

I had to create a test in miden-tx/src/test.rs that creates two Accounts and a Note. The Note is created using the P2ID script. The ID that is set in the script is the AccountID of the target_account.

Unfortunately, we don't have an account procedure for account::add_asset yet. So I created an artificial procedure that is being called. Only the target_account can successfully consume the note and execute the note script.

I will squash all commits into one single commit after the review.

I will add the script to miden-lib after the initial review.

[frisitano]

Looks good so far! A few minor comments left inline.

[bobbinth]

Looks great! Thank you! I left some comments inline. Some of them can be applied now, but some would need to be addressed after we implement #22.

Another comment (not for this PR, but would should create an issue for this): it would be good to set up structure for standard note scripts - something similar to how we handle miden library and SatKernel.

One approach would be to put note scripts under miden-lib/asm/scripts directory. This would also mean that the current content of miden-lib/asm would need to be moved under miden-lib/asm/miden or something like that.

We would also need to modify the build script to compile these scripts and save them into miden-lib/assets directory. One issue here is that we don't have a way to serialize ProgramAst into files. This should be fairly easy to do and the extension we can use is something like .masb (Miden Assembly binary). This is something we'll need to implement in Miden VM - so, let's create an issue for this.

Lastly, we may want to provide some convenience interface from miden-lib crate to create notes for standard scripts. We could have an enum to identify each script. For example:

pub enum Script {
    P2ID { target: AccountId },
    P2IDR {
        target: AccountId,
        recall_height: u32,
    },
}

And then we could have a function which could look something like this:

pub fn create_note(
    script: Script,
    assets: Vec<Asset>,
    sender: AccountId,
    tag: Option<Felt>,
) -> Result<Note, SomeError>

[bobbinth]

I wonder if we should just create a separate method for handling this case - e.g., something like:

pub fn mock_inputs_with_account(
    account: Acount,
    consumed_notes: Vec<Note>,
) -> (Account, BlockHeader, ChainMmr, Vec<Note>)

An then mock_inputs() would create an account and then just call this function.

[bobbinth]

Would be good to use the same commenting style as we use in other places (e.g., see here). For example, it could start something like this:

# Deposits all assets of this note into an account. Fails if the current account ID does 
# not match the target account ID.
#
# Input:  [target_account_id, ...]
# Output: [...]
being
    # get the current account ID and make sure it is the same as the target account ID
    exec.account::get_id
    assert_eq
    # => [...]

[bobbinth]

Is there a reason to use 1000000000 rather than just 0?

[bobbinth]

We shouldn't be calling add_asset directly here as the call to add_asset should be done from within account code (if it is done from the note, it won't pass the access check). So, what we need is a procedure on the target account - something like receive_asset. This procedure could look as follows:

# Input:  [ASSET, ...]
# Output: [...]
export.recieve_asset
    # call account::add_asset from here
end

Ideally, this procedure would be a part of basic wallet and then we'd be able to call it from here. For now, we can probably just load the asset onto the stack and then drop it using dropw procedure. We can update this later when basic wallet interface is implemented.

Another thing, we would need to use call instruction to invoke the account procedure. Calling convention for such procedures is still an open question (see #179). This is also something to figure out as we work through the basic wallet interface.

Lastly, computing the end pointer and then incrementing current pointer within the loop would be more efficient because add.1 just takes a single VM cycle.

[bobbinth]

Once everything is implemented, we probably want to confirm that transaction result contains the expected data. Maybe leave a TODO here to revisit this later?

[frisitano]

Heads up that the account vault has been implemented in masm - see here. I believe this should unblock this PR.

[Dominik1999]

Closing in favor of #244