Fix the stack management for account procedures

[hackaugusto]

AFAIU, the current code is organized as follows:

1. miden lib is used to expose easy to use functions to users, e.g.:

miden-base/miden-lib/asm/miden/account.masm

Lines 87 to 96 in dbe83c9

	#! Sets an item in the account storage. Panics if the index is out of bounds.
	#!
	#! Stack: [index, V']
	#! Output: [R', V]
	#!
	#! - index is the index of the item to set.
	#! - V' is the value to set.
	#! - V is the previous value of the item.
	#! - R' is the new storage root.
	export.set_item

And internally this function manages the stack, such that the user won't lose its state:

miden-base/miden-lib/asm/miden/account.masm

Lines 96 to 101 in dbe83c9

	export.set_item
	push.0 movdn.5 push.0 movdn.5 push.0 movdn.5
	# => [index, V', 0, 0, 0]
	syscall.set_account_item
	# => [R', V]

2. the kernel library expects padded stack, since it can't return more than 16 elements, and it must overwrite some of the elements in the stack:

miden-base/miden-lib/asm/kernels/transaction/api.masm

Lines 163 to 170 in dbe83c9

	#! Stack: [index, V', 0, 0, 0]
	#! Output: [R', V]
	#!
	#! - index is the index of the item to set.
	#! - V' is the value to set.
	#! - V is the previous value of the item.
	#! - R' is the new storage root.
	export.set_account_item

The above means the user would exec.set_item and have a nice to use API.

However, for security, set_account_item must authenticate the origin of the call, e.g. to prevent a rogue note from changing the account's code. This means the function can not be execed, and it must be called instead.

Now, because the call can return at most 16 elements in the stack, the code above that was suppose to provide a nice API is unusable, because it is pushing 3 elements to the stack which are never cleaned up:

miden-base/miden-lib/asm/miden/account.masm

Lines 97 to 102 in dbe83c9

	push.0 movdn.5 push.0 movdn.5 push.0 movdn.5
	# => [index, V', 0, 0, 0]
	syscall.set_account_item
	# => [R', V]
	end

[hackaugusto]

To provide the above interface, one thing that could be done, is to use the same strategy as #701, and have the accounts procedures available to dyncall.

The user would initialize the memory with the callback addresses in the beginning of the transaction script, and then would exec the code in miden/account.masm, which would in turn use a dyncall to transition to the authenticated account code. Alternatively, the user would not even need to do the initialization himself if we have a flag in memory and perform lazily initialization from the miden lib.

[bobbinth]

I think a couple of things may be conflated here. A few comments:

However, for security, set_account_item must authenticate the origin of the call, e.g. to prevent a rogue note from changing the account's code. This means the function can not be execed, and it must be called instead.

We need to use syscall rather than call to invoke kernel procedures - though, the effect on the stack would be the same in both cases.

Now, because the call can return at most 16 elements in the stack, the code above that was suppose to provide a nice API is unusable, because it is pushing 3 elements to the stack which are never cleaned up:

I'm not sure I follow this part. The referenced code should not leave any extra items on the stack (if it does, there is a bug somewhere). The call sequence looks something like this:

1. The user runs exec.account::set_item in miden/accounts.masm. This is the procedure which has the user-friendly interface (i.e., the user does not need to worry about padding the stack).
2. Internally, account::set_item does the stack padding and makes a syscall to set_account_item defined in api.masm.
3. Then, set_account_item performance procedure authentication and executes account::set_item defined in miden/kernels/tx/account.masm.
   a. As a side note, putting the kernels modules into the miden library was probably not a good decision - they should be in a separate library which is imported by the api.masm instead of the miden library.

Flow between items 2 and 3 has been the source of quite a bit of confusion, and we should probably refactor it (as is described in #685) - but all of this is hidden from the user because exec.account::set_item from the first step should work like any other function.

[hackaugusto]

The user runs exec.account::set_item in miden/accounts.masm. This is the procedure which has the user-friendly interface (i.e., the user does not need to worry about padding the stack).

This only works from inside the account context, as in set_item is not a nice to use function for transaction scripts, for the account code, otherwise the authentication would fail. So there is another call done by the tx script to the code that does set_item. This is the one I'm referring to.

[bobbinth]

This only works from inside the account context, as in set_item is not a nice to use function for transaction scripts, for the account code, otherwise the authentication would fail. So there is another call done by the tx script to the code that does set_item. This is the one I'm referring to.

hmmm - account::set_item should only be available from account context. Trying to execute it from transaction or note script should be an error. We list here which kernel procedure are allowed to be called from which context.

[bobbinth]

Or I guess you mean we have a procedure on the account which internally invokes account::set_item, and this procedure is called from a transaction script via the call instruction. In this case yes, the caller is responsible for padding the stack and this is also one of the motivations for defining a consistent ABI described in #685.

[hackaugusto]

hmmm - account::set_item should only be available from account context. Trying to execute it from transaction or note script should be an error. We list here which kernel procedure are allowed to be called from which context.

Yes. We seem to agree on how it works. We don't seem to agree on how useful it is.

Basically what I'm trying to say is this only works as a nice to use function if you're writing the account code. And there is very little use for that. The vast majority of the code will be note scripts or tx scripts. At least in my eyes, the most common case should be the easiest. So that would be making the so-called nice to use functions usable from the tx script.

Account code is more complicated, we can't really hide the padding of the stack, because the function exposed by the account are called, and they have to handle these conventions anyways. If they have to be aware of the convention when exposing the account's API, it doesn't seem that much extra to assume the author can call the kernel using the same convention.

[bobbinth]

Basically what I'm trying to say is this only works as a nice to use function if you're writing the account code. And there is very little use for that. The vast majority of the code will be note scripts or tx scripts.

I am not sure about the last point. Writing account procedures which internally call kernel procedures would be fairly common too. I can't say whether it will be more or less common then writing note/tx scripts - but in my mind they are roughly comparable in importance.

I agree that there isn't much we can do with the procedures exposed by the account interface itself because they must be call-ed. And #685 attempts to define a standard way to call them. But I don't think just because we can't make these procedures as "easy-to-use" as regular procedures, we should make other procedures more complicated.

So, basically, my thinking is:

• Invoking kernel procedures from within account procedures could work the way it does now (where the user does not need to worry about padding the stack).
• Invoking account procedures from note/tx scripts would be more complicated and the user would need to pad the stack, but the methodology for padding should be standardized to have a "uniform ABI".

[Fumuran]

@bobbinth I think that everything you described was done by ABI PR. The procedures from the miden::account could be executed only in the account context: we can enter it by calling the contracts procedures (wallet, faucets, auth) or by calling the procedures from the user's account. In all these cases stack should be padded before the call.

I think that this issue is not relevant anymore.