Further generalize `TransactionRequest`

[igamigo]

After #237 is merged, TransactionRequest will contain a TransactionScript. This has the following consequences:

1. Generating this script requires quite a bit of knowledge about the target account (e.g., we need to know which authentication scheme the account uses).
2. TransactionScript object also includes advice inputs required to execute the script. Currently, this includes the private key associated with the authentication scheme of the account. Though, this is something we need to change regardless (i.e., but adding an Authenticator component to the client).

Both of these make it difficult (if not impossible) to have a serialized version of transaction request. A potentially better alternative would be to have some kind of TransactionScriptTemplate - which would define what the transaction script should do, without saying how it should do it. How exactly this template should look like should be a separate issue/discussion, but to give one example:

We may want to indicate that the transaction should send some asset to some recipient. The template for this could look like:

pub enum TransactionScriptTemplate {
   SendAsset { asset: Asset, recipient Digest }
}

Based on this info, the client would be able to build a transaction script that calls send_asset on the account and then authenticates the transaction according to the account's authentication scheme.

_Originally posted by @bobbinth in https://github.com/0xPolygonMiden/miden-client/pull/237#discussion_r1544859824_

After this is done, we should look into having a file definition that can be easily deserialized into a TransactionRequest as suggested in #203

[mFragaBA]

should we merge this and #398 into a single issue?

[bobbinth]

should we merge this and #398 into a single issue?

I'd keep 3 separate issues for these as we probably will address them in 3 separate PRs.

[bobbinth]

A couple of additional things left over from #398:

• Potentially rename expected_partial_notes to expected_future_notes.
• Consider also adding a MerkleStore to transaction request (this would require a similar change to TransactionArgs in miden-base).

[mFragaBA]

A couple of additional things left over from #398:

* Potentially rename `expected_partial_notes` to `expected_future_notes`.

* Consider also adding a `MerkleStore` to transaction request (this would require a similar change to `TransactionArgs` in `miden-base`).

I took a quick look and the chankes seem to be pretty straightforward although correct me if I'm wrong.

On the miden base side, in the ToTransactionKernelInputs implementation:

• the TransactionArgs within PreparedTransaction (or we might add another member to PreparedTransaction) should contain the merkle store
• instead of using AdviceInputs::default we use AdviceInputs::with_merkle_store

On the client side:

• we include the merkle store in transaction request (perhaps we can switch to more of a builder structure as we talked)
• we can replicate tests similar to the ones in the vm

[bobbinth]

Yes, this shouldn't be too complicated.

• the TransactionArgs within PreparedTransaction (or we might add another member to PreparedTransaction) should contain the merkle store
• instead of using AdviceInputs::default we use AdviceInputs::with_merkle_store

I think we could probably just replace AdviceMap with AdviceInputs in the TransactionArgs (since AdviceInputs contain advice map and Merkle store). It could look something like this:

pub struct TransactionArgs {
    tx_script: Option<TransactionScript>,
    note_args: BTreeMap<NoteId, Word>,
    advice_inputs: AdviceInputs,
}

[igamigo]

@bobbinth a couple of questions regarding generalizing TransactionScript. I think the original comment is a little bit stale, but would we just want to solve the problem for authentication methods? Right now the transaction request looks like this:

pub struct TransactionRequest {
    /// ID of the account against which the transactions is to be executed.
    account_id: AccountId,
    // Notes to be consumed by the transaction that are not authenticated.
    unauthenticated_input_notes: Vec<Note>,
    /// Notes to be consumed by the transaction together with their (optional) arguments. This
    /// has to include both authenticated and unauthenticated notes.
    input_notes: BTreeMap<NoteId, Option<NoteArgs>>,
    /// A list of notes expected to be generated by the transactions.
    expected_output_notes: Vec<Note>,
    /// A list of note details of notes we expect to be created as part of future transactions.
    expected_partial_notes: Vec<NoteDetails>,
    /// Optional transaction script (together with its arguments).
    tx_script: Option<TransactionScript>,
    /// Initial state of the `AdviceMap` that provides data during runtime.
    advice_map: AdviceMap,
}

We also have TransactionTemplate which builds into a TransactionRequest:

pub enum TransactionTemplate {
    /// Consume the specified notes against an account.
    ConsumeNotes(AccountId, Vec<NoteId>),
    /// Mint fungible assets using a faucet account and creates a note with the specified
    /// type that can be consumed by the target Account ID
    MintFungibleAsset(FungibleAsset, AccountId, NoteType),
    /// Creates a pay-to-id note with the specified type directed to a specific account
    PayToId(PaymentTransactionData, NoteType),
    /// Creates a pay-to-id note directed to a specific account, specifying a block height after
    /// which the note can be recalled
    PayToIdWithRecall(PaymentTransactionData, u32, NoteType),
    /// Creates a swap note offering a specific asset in exchange for another specific asset
    Swap(SwapTransactionData, NoteType),
}

We shouldn't need to introduce a new TransactionScriptTemplate for deciding on the authentication method, right? We could just use the data from the TransactionTemplate and then choose to call the correct proc (ie, just replacing an extra field based on what the client knows about the account). Or do we want to authenticate any transaction (ie, with custom scripts) by injecting a call to the authentication procedure before ending the script anyway?

[bobbinth]

Introducing TransactionScriptTemplate may not be the right way to do it, but I'm also not sure the current TransactionTemplate is the right approach here (maybe it is).

The use case we are trying to support is some external entity (e.g., an exchange operator) building a transaction request for the user, and then the user building and executing the implied transaction. The main challenge here is that this entity may not know much about the account which will try to execute the transaction (in fact, it may not even know the ID of the account either - but let's put it aside for now), and so, it cannot assume what kind of authentication the account supports or, more generally, what procedures the account exposes.

To give some examples of what the requests might look like:

1. Execute a transaction which consumes authenticated notes (note1_id, args1) and (note2_id, args2).
2. Execute a transaction which consumed unauthenticated note (note3, args3).
3. Execute a transaction which produces notes (note4) and (partial_note5).

The first two use cases we already support, but we don't support the 3rd one. This is because for this use case we need to build a specific transaction script, and the entity that needs to build the transaction request cannot build this script because it doesn't know much about the account.

On the other hand, TransactionTemplate is too specific to support all of the above use cases (e.g., there is no way to specify that we want to consume an unauthenticated note via transaction template; we can add that, of course, but I think that's going in the wrong direction).

In my mind, the chain is still:

TransactionTemplate -> TransactionRequest -> TransactionResult -> ProvenTransaction

In the above, TransactionRequest is "more general" than TransactionTemplate - i.e., transaction template can represent a subset of transactions of as compared to what transaction request can represent.

---

I actually think we are pretty close to what we need with the current TransactionRequest. Maybe the changes we need to make are as follows:

• Remove tx_script.
• Add a field to specify the notes the transaction should produce via its transaction script.

For example, the struct could look like so:

pub struct TransactionRequest {
    /// ID of the account against which the transactions is to be executed.
    account_id: AccountId,
    // Notes to be consumed by the transaction that are not authenticated.
    unauthenticated_input_notes: Vec<Note>,
    /// Notes to be consumed by the transaction together with their (optional) arguments. This
    /// has to include both authenticated and unauthenticated notes.
    input_notes: BTreeMap<NoteId, Option<NoteArgs>>,
    /// Notes to be generated by the transaction's tx_script.
    native_output_notes: Vec<PartialNote>,
    /// A list of notes expected to be generated by the transactions.
    expected_output_notes: Vec<Note>,
    /// A list of note details of notes we expect to be created as part of future transactions.
    expected_partial_notes: Vec<NoteDetails>,
    /// Initial state of the `AdviceMap` that provides data during runtime.
    advice_map: AdviceMap,
}

The idea is that the client would then take native_output_notes (don't love the name - so, suggestions are welcome) and would build a transaction script which would produce these notes according to the specifics of the account the client manages. This also implies that construction of TransactionScript would happen inside Client::new_transaction() rather than inside Client::build_transaction_request().

---

A quick comment about account ID. At some point, we may want to consider removing account_id from TransactionRequest to make it even more general. That is, whoever builds the transaction request would not specify the account that needs to execute it, and then it would be up to the user to decide which account to apply the request to. This would require refactoring Client::new_transaction() method to look something like this:

pub fn new_transaction(
    &mut self,
    account_id: AccountId,
    transaction_request: TransactionRequest,
) -> Result<TransactionResult, ClientError> 

But I don't think we should tackle this as a part of this issue.

[igamigo]

I see, got it. One question:

The first two use cases we already support, but we don't support the 3rd one. This is because for this use case we need to build a specific transaction script, and the entity that needs to build the transaction request cannot build this script because it doesn't know much about the account.

When we mention that the entity "doesn't know much about the account", we mean that we don't know how the account is authenticated, right? If this is the case, then we are still not really supporting 1 and 2 because they do call a hardcoded auth_tx::auth_tx_rpo_falcon512.

I agree on the rest of the topics, it's about what I had in mind.

[igamigo]

One other comment regarding this: While native_output_notes tells us what notes the script is meant to create, it doesn't tell us how. Are we to assume based on the account's interface? eg, if the account_id is a faucet's ID, we call distribute, otherwise call send_asset

[bobbinth]

When we mention that the entity "doesn't know much about the account", we mean that we don't know how the account is authenticated, right? If this is the case, then we are still not really supporting 1 and 2 because they do call a hardcoded auth_tx::auth_tx_rpo_falcon512.

Ah yes! This is correct.

While native_output_notes tells us what notes the script is meant to create, it doesn't tell us how. Are we to assume based on the account's interface? eg, if the account_id is a faucet's ID, we call distribute, otherwise call send_asset.

Yes, there will need to be some logic in the client to figure out how to interpret the request. The logic could be relatively simple for now - for example:

• If the account implements the "basic wallet" interface, use send_asset (as an aside, it might make sense to rename this to something like send_note in miden-base since this doesn't have to send assets any more).
• If the account is a faucet, use distribute (assuming we are trying to distribute the native asset of the faucet).
• Otherwise, return an error.

This logic an be encapsulated into a component which could be extended in the future (we could even make it "pluggable" like we do with the store, authenticator etc. down the road).

[tomyrd]

I have another question regarding the building of the transaction script. How would we handle custom transaction scripts that don't follow the common structure? Would we receive a String that we append to the whole tx_script at the end/beginning?

[bobbinth]

I guess there should be a mechanism for specifying a wholly custom transaction script - though, I haven't thought of one. In such a case, this script would be used directly (rather than appended to another script). So, basically, there could be two cases:

• Specify native output notes, and in this case the client builds the script.
• Specify a custom script, and in this case this script will be used for the transaction.

[tomyrd]

I just pushed an initial implementation of the script building in the client. It's not finished but we can use it as a starting point for further discussion if needed.

[bobbinth]

Closed by #438.