Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transaction docs copy edit #589

Merged
merged 7 commits into from
Apr 15, 2024
Merged

Transaction docs copy edit #589

merged 7 commits into from
Apr 15, 2024

Conversation

kmurphypolygon
Copy link
Contributor

@kmurphypolygon kmurphypolygon commented Apr 10, 2024

Docs in the transaction section now copy edited.

Copy link
Collaborator

@Dominik1999 Dominik1999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! That was the last piece.

I left some small comments.

docs/architecture/transactions/overview.md Outdated Show resolved Hide resolved
docs/architecture/transactions/overview.md Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is important to add information in the overview.md chapter about how to read the following chapters. Or even better, lay the inner logic out to the reader.

We could have a short paragraph explaining that there are two transaction modes and that any transaction execution requires the transaction kernel, which is context-sensitive and exposes the following procedures.

Or we could add a small sub table of contents?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We try to avoid manual tables of contents as much as possible because they tend to get forgotten and quickly go out of date. However, I do agree there needs to be some sort of summary of the content in this section in relation to the chapters.
Uploading Screenshot 2024-04-11 at 16.47.58.png…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment makes more sense if "modes" is a word choice and not a specific technical term, which I had assumed it was.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have now moved the previous "modes" section back in which might help resolve this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this part is not solved yet, or I am missing it.

In my opinion, we need to add something like this to the overview section. At the moment, to the reader it might not be clear what all the subchapter are about.

Miden transactions are programs that run in the Miden VM. That is why, in Miden, there is a proof for every transaction.

The Miden TransactionExecutor prepares, executes, and proves transactions (see [execution]). The executor compiles the TransactionKernel and the user-defined note and transaction scripts into one single executable program for the Miden VM (see [kernel]). Users can write their scripts using [kernel procedures] and [contests].

Maybe you find an even better formulation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you think this needs its own chapter rather than putting under the overview chapter?

Copy link
Contributor Author

@kmurphypolygon kmurphypolygon Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it in the overview section previously? If so, then I have misunderstood that the meaning of the word "modes" is a more general term rather than a specific technical term. We can move it back, no probs, but perhaps use a better word here, like types?

Copy link
Contributor Author

@kmurphypolygon kmurphypolygon Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have moved this section back in, the overview is quite long now tho.. actually it's probably ok

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think, after you rebase, those docs-not-included should be gone

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did and they weren't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something my side probably, not sure.


The kernel program always starts executing in a root context. Thus, the prologue sets the memory for the root context. To move execution into a different context, we can invoke a procedure using the `call` or `dyncall` instruction. In fact, any time we invoke a procedure using the `call` instruction, the procedure is executed in a new context.
Miden assembly program execution can span multiple isolated contexts. An execution context defines its own memory space which is inaccessible from other execution contexts. Note scripts cannot directly write to account data which should only be possible if the account exposes relevant functions.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you maybe add how Miden assembly programs fit into here? The transaction kernel is written in Miden assembly.


The kernel program always starts execution from a root context. Thus, the prologue sets the memory for the root context. To move execution into a different context, we can invoke a procedure using the `call` or `dyncall` instruction. In fact, any time we invoke a procedure using the `call` instruction, the procedure is executed in a new context.

While executing in a note, account, or tx script context, we can execute some procedures in the kernel context, which is where all necessary information was stored during the prologue. You can switch to the kernel context via the `syscall` instruction. The set of procedures invoked via the `syscall` instruction is limited by the [transaction kernel API](https://github.com/0xPolygonMiden/miden-base/blob/main/miden-lib/asm/kernels/transaction/api.masm). Once the procedure call via `syscall` returns, the execution moves back to the note, account, or tx script from which it was invoked.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we rephrase this section a bit? There are many "we can ..." or "you can ..." which I think we don't use in the rest of the docs.

I know, I have written those, but there was no proper review yet and now that I see it, I think it doesn't really fit.

Copy link
Contributor Author

@kmurphypolygon kmurphypolygon Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for this is that there was a lot of passive tense, which ideally we should avoid where we can and go for active voice instead. "Can" is a bit of a cheating way to deal with it so I'll have another look but by and large avoid the passive. I kept a lot of it in in the end but sometimes there was too much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try now, after next commit

@@ -1,38 +1,48 @@
# Transaction Execution
Transactions are being executed by the Miden Transaction Executor. Transaction execution results in a `ExecutedTransaction` object and consists of the following steps:
The Miden transaction executor is the component that executes transactions.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how yet, but we could also try embedding this explanation here (https://crates.io/crates/miden-tx) into this chapter.

My explanation has more information, but I like how concise the explanation on crate.io is. Also, when we show this code, it might be easier for some readers to follow.

What do you think?

------8<------

The first requirement is to have a DataStore implementation. DataStore objects are responsible to load the data needed by the transactions executor, specially the account's code, the reference block data, and the note's inputs.

let store = DataStore:new();

Once a store is available, a TransactionExecutor object can be used to execute a transaction. Consuming a zero or more notes, and possibly calling some of the account's code.

let executor = TransactionExecutor::new(store);
let executed_transaction = executor.execute_transaction(account_id, block_ref, note_ids, tx_args);

With the transaction execution done, it is then possible to create a proof:

let prover = TransactionProver::new(ProvingOptions::default());
let proven_transaction = prover.prove_transaction(executed_transaction);

And to verify a proof:

let verifier = TransactionVerifier::new(SECURITY_LEVEL);
verifier.verify(proven_transaction);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is more of an API doc so I would add it separately in an API section of the docs, not in the architecture section ... let me know and i can do that

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, indeed. Let's keep this as an architecture section


## The Epilogue
The Epilogue finalizes the transaction. It
The Epilogue finalizes the transaction. It does the following:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you lower case Epilogue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just do it these in future directly and i'll accept

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, these are easy to miss ..

| `get_assets` | `[dest_ptr]` | `[num_assets, dest_ptr]` | note | <details><summary>View</summary>Writes the assets of the currently executing note into memory starting at the specified address. dest_ptr is the memory address to write the assets. num_assets is the number of assets in the currently executing note.</details> |
| `get_inputs` | `[dest_ptr]` | `[dest_ptr]` | note | <details><summary>View</summary>Writes the inputs of the currently executed note into memory starting at the specified address. dest_ptr is the memory address to write the inputs.</details> |
| `get_sender` | `[]` | `[sender]` | note | <details><summary>View</summary>Returns the sender of the note currently being processed. Panics if a note is not being processed. sender is the sender of the note currently being processed.</details> |
| `get_assets` | `[dest_ptr]` | `[num_assets, dest_ptr]` | note | Writes the assets of the currently executing note into memory starting at the specified address `dest_ptr `. is the memory address to write the assets. `num_assets` is the number of assets in the currently executing note. |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this HTML code <details><summary>View</summary> allows to collapse and expand the text to make the table less clinched.

Shouldn't we keep that?

Copy link
Contributor Author

@kmurphypolygon kmurphypolygon Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, we don't use that across the board ... normally we would do API docs in a different way - not with tables - maybe even importing them with a plugin directly from code ..

a (not very) quick fix for this is to make the far column bulleted lists

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did bulleted lists for now but there are better ways but it might mean building a plugin ...

Copy link
Collaborator

@Dominik1999 Dominik1999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot! I left a couple of more comments, I think when those are resolved we can merge

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this part is not solved yet, or I am missing it.

In my opinion, we need to add something like this to the overview section. At the moment, to the reader it might not be clear what all the subchapter are about.

Miden transactions are programs that run in the Miden VM. That is why, in Miden, there is a proof for every transaction.

The Miden TransactionExecutor prepares, executes, and proves transactions (see [execution]). The executor compiles the TransactionKernel and the user-defined note and transaction scripts into one single executable program for the Miden VM (see [kernel]). Users can write their scripts using [kernel procedures] and [contests].

Maybe you find an even better formulation.

@@ -1,37 +1,70 @@
# Transactions overview

Transactions in Miden can be understood as facilitating account state changes. Asset transfers between accounts are done by executing transactions. They take a single account and some [notes](../notes.md) as input and output the same account at a new state together with some other notes.
Transactions in Miden facilitate account state changes and asset transfers.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me it is clearer, when you only say

Transactions in Miden facilitate single account state changes. Two transactions are needed to transfer assets between accounts.

This is one of the most important paradigms in Miden. In EVM chains, a transaction involves two accounts and asset transfers are facilitated by one single transaction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's clear, added

Network transactions are useful for two reasons:

1. Clients may not have sufficient resources to generate zk-proofs.
2. Executing many transactions against the same public account by different clients is challenging as the account state changes after every transaction. In this case, the Miden Node / Operator acts as a "synchronizer" as they can execute transactions sequentially and feed the output of the previous transaction into the subsequent one.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you lower-case Miden node

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like I missed this bit ... clarifying.

@@ -1,38 +1,48 @@
# Transaction Execution
Transactions are being executed by the Miden Transaction Executor. Transaction execution results in a `ExecutedTransaction` object and consists of the following steps:
The Miden transaction executor is the component that executes transactions.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, indeed. Let's keep this as an architecture section

Transactions are being executed by the Miden Transaction Executor. Transaction execution results in a `ExecutedTransaction` object and consists of the following steps:
The Miden transaction executor is the component that executes transactions.

Transaction execution consists of the following steps and results in a `ExecutedTransaction` object:

1. Fetch the data required to execute a transaction from the data store.
2. Compile the transaction into an executable [MASM](https://0xpolygonmiden.github.io/miden-vm/user_docs/assembly/main.html) program using the transaction compiler.
3. Execute the transaction program and create an `ExecutedTransaction` object.
4. Prove the `ExecutedTransaction` using the Transaction Prover.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am unsure if we should keep Transaction Prover capitalized or if we should make it lower case as transaction executor at the first sentence of this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, lowering, i miss them quite often ..

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we need a transition to the next chapter, the kernel?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's chat about the flow in this doc because i need to understand it better, and to answer your question, maybe yes

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, let's have a short call. I will ping you after lunch

1. The [prologue](#prologue) prepares the transaction for processing by parsing the transaction data and setting up the root context.
2. Note processing executes the note processing loop which consumes each `InputNote` and invokes the note script of each note.
3. Transaction script processing executes the optional transaction script.
4. The epilogue finalizes the transaction by computing the created notes commitment, the final account hash, asserting asset invariant conditions, and asserting the nonce rules are upheld.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we link to the prologue above, can you also link to the epilogue?

The transaction kernel program receives two type of inputs, public inputs via the `operand_stack` and secret inputs via the `advice_provider`. The stack holds the global inputs. They serve as a commitment to the data being provided via the advice provider. The advice stack holds data of the last known block, account and input note data. The details are layed out in the next paragraph.
## Input

The transaction kernel program receives two type of inputs, public inputs via the `operand_stack` and secret inputs via the `advice_provider`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is it two types or two type?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch


_Note: One needs to provide the note data to compute the Nullifier, e.g. the [note script](https://0xpolygonmiden.github.io/miden-base/architecture/notes.html#script) and the [serial number](https://0xpolygonmiden.github.io/miden-base/architecture/notes.html#serial-number). So one needs to know the note data to execute the prologue of a transaction. This is how the [note recipient](https://0xpolygonmiden.github.io/miden-base/architecture/notes.html#note-recipient) defines the set of users who can consume a specific note. The executing account needs to provide the pre-image data to the recipient at the time of execution._
As the account data is read from the advice provider, the account hash is computed. If the account is new then the global initial account hash is updated and the new account is validated. If the account already exists then the computed account hash is asserted against the account hash provided via global inputs. It is also asserted that the account id matches the account id provided via the global inputs (`operand_stack`).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: is it provided via the global inputs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not necessary


## The Note Processing
If there are input notes they are being consumed in a loop. For every note, the [MAST root](https://0xpolygonmiden.github.io/miden-vm/design/programs.html) of the note script is being loaded onto the stack. Then, by calling a [`dyncall`](https://0xpolygonmiden.github.io/miden-vm/user_docs/assembly/code_organization.html?highlight=dyncall#dynamic-procedure-invocation) the note script is being executed in a new context to prevent unwanted memory access.
Input note processing involves each note reading the data from the advice provider and storing it at the appropriate memory addresses.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reads to me as if the note would be reading data

... each note reading the data from the advice provider ...

It is more like the kernel reads the data from each note (which is provided via the advice provider). Basically, the kernel reads all data from the advice provider (and global inputs) and stores it into memory. All data means, all note data, and account data, and blockchain data.

Copy link
Collaborator

@Dominik1999 Dominik1999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, let's merge!

!!! tip "Key to diagram"
- The [transaction executor](execution.md) prepares, executes, and proves transactions.
- The executor compiles the [transaction kernel](kernel.md) plus user-defined notes and transaction scripts into a single executable program for the Miden VM.
- Users rite scripts using [kernel procedures](procedures.md) and [contexts](contexts.md).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: write not rite

@kmurphypolygon kmurphypolygon merged commit 3fec95b into main Apr 15, 2024
9 checks passed
@kmurphypolygon kmurphypolygon deleted the docs/txs branch April 15, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants