Address `TransactionRequest` design decisions

[igamigo]

Should we bind TransactionRequest to a specific Account?

(Reference: #822)

Currently, a transaction can get executed in the Client by any Account: the API takes the TransactionRequest and a specific AccountId. However, TransactionRequest is not fully sender-agnostic today. “Own notes”, if the request object contains them, effectively embed the sender because certain note types need the sender account ID to be computed. Concretely, swaps derive payback note details inside the swap via NoteInputs, which requires the sender ID; faucet mints cannot prevriously declare specific NoteAssets because the final assets depend on which faucet actually executes. By contrast, non-own outputs only need a NoteRecipient to be complete (OutputNote::Full). The current API doesn’t let us build “everything except the sender,” so client code would need to end up duplicating some code from miden-base.

We could attempt to make it fully agnostic if we move sender resolution into an explicit binding step. Instead of “own notes,” the request would carry output templates. There are two kinds: (a) external outputs, where a NoteRecipient is enough and the note is already full; and (b) own outputs, where we store a template describing what will be emitted via the sender’s AccountInterface, and we only materialize it once the sender is known. To make this ergonomic, we introduce a small trait that can be implemented by P2ID, swap, etc., so we can construct notes without duplicating miden-base logic. An attempt was done in #811.

Alternatively, we could decide to bind TransactionRequest to a specific account ID. In this sense, the object will already be consistent with all other behaviors.

Or we could explore something in the middle where the request can be "bound" to a specific account ID (eg, by calling a specific bind() step, maybe on the builder). Here, sender-dependent validation happens: resolve own-note templates, compute swap payback “future notes,” check faucet constraints, etc. This could be optional and there could be two different request types to have a more explicit separation and the client APIs could support both.

Remove "leaky" fields

(note: there was an issue/discussion miden-base about this that I could not find after a quick search)

The expected_output_recipients field exists to support public note creation, because the VM host needs to unhash all recipients related to the public notes that were created during the transaction execution. There are a number of alternatives that we could do to avoid this, like fetching output notes-related data from DataStore.

Input notes as InputNote

We could decide to track all inputs as InputNote instead of NoteId. InputNote already describes whether the note gets used as authenticated or unauthenticated so we can merge input_notes and unauthenticated_input_notes into a single inputs: Vec<InputNote>.
There are some implications here. For example, if we do this, we can no longer express the transaction request as a function of something that only the executor knows (if a server somehow sends a TransactionRequest for a user to execute, the server is expected to know the details about the notes themselves).

Revisit foreign_accounts

This should be fairly straightforward. Once lazy loading lands, we should be able to remove the foreign accounts description from TransactionRequest. We could also decide to keep it only for private accounts, since there is otherwise no other external way to communicate a specific private account state. This is also somewhat related to the account state endpoints refactors, since the client will need to be able to sync specific accounts, and store relevant data so that it can be fetched on transaction runtime.

Mutators on TransactionRequest

#1254 adds a mutable getter for the advice map that a user may want to set for the transaction to use. This is currently the only way to mutate a TransactionRequest object, which breaks a bit with the builder convention of having a static object be created by its builder (ie, TransactionRequestBuilder). Is this enough/OK in relation to being able to mutate a request? Are there any more general alternatives that would make sense here? Or we could make mutators even more specific as suggested here.

---

Some of these are more concrete to address than others, so we can make separate issues as we see fit to track each part accordingly.

[mmagician]

Revisit foreign_accounts

This should be fairly straightforward. Once lazy loading lands, we should be able to remove the foreign accounts description from TransactionRequest. We could also decide to keep it only for private accounts, since there is otherwise no other external way to communicate a specific private account state. This is also somewhat related to the account state endpoints refactors, since the client will need to be able to sync specific accounts, and store relevant data so that it can be fetched on transaction runtime.

So the idea is to remove foreign_accounts from the TransactionRequestBuilder completely? This would be great, as it's currently a DevEx pain to manually specify foreign accounts when making tx's (cc @partylikeits1983, we ran into this recently for AggLayer testing).

How would this work exactly? IIUC, we don't know which foreign accounts to fetch from the network until we actually execute the transaction. So this would need to involve some kind of trial execution, catching DataStoreError::AccountNotFound(account_id) and then automatically fetching this from the network (unless cached already?), but I think this already starts to describe a different issue.

[bobbinth]

How would this work exactly? IIUC, we don't know which foreign accounts to fetch from the network until we actually execute the transaction. So this would need to involve some kind of trial execution, catching DataStoreError::AccountNotFound(account_id) and then automatically fetching this from the network (unless cached already?), but I think this already starts to describe a different issue.

This could happen lazily:

1. A transaction starts executing.
2. At some point the code makes an FPI call.
3. The host makes a request to the DataStore to fetch the account.
   a. If the account is cached in the data store, it returns the requested data.
   b. If not, we make an RPC call to the node to get the account (via the GetAccount request). Once the request comes back, we cache the account.

The main downside here is additional latency - though, I'm hoping in most cases, the roundtrip shouldn't add more than 100ms. But then again, if a transaction requires many FPI calls, these would add up.

So the idea is to remove foreign_accounts from the TransactionRequestBuilder completely?

I would keep them, but would treat them as optional. That is, if foreign accounts are not specified, they are fetched "lazily" as i described above. But if they are, we could pre-load them before the transaction starts executing to reduce latency.