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

Updates with more info #1118

Merged

Conversation

Dominik1999
Copy link
Collaborator

I am adding some more info to the Transaction chapter. It explains a transaction as a process and provides a bit more context.

This is still a draft, but you can already comment. I will go over it a second time to also incorporate Bobbin's feedback.

@Dominik1999 Dominik1999 requested a review from phklive February 3, 2025 09:29
@phklive phklive added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Feb 5, 2025
Copy link
Contributor

@phklive phklive left a comment

Choose a reason for hiding this comment

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

I think that we added some valuable missing information but that we have also made it a lot lower level / complex to read.

Let's try to find a good middleground.

@Dominik1999 Dominik1999 force-pushed the dominik_update_transaction_chapter branch from c087a82 to 5561d68 Compare February 7, 2025 12:08
Copy link
Contributor

@phklive phklive left a comment

Choose a reason for hiding this comment

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

Getting closer.

Some examples might go a lil too low-level + we are talking about implementation detail types, e.g, TransactionArguments or BlockHeader, might be okay for now.

@bobbinth
Copy link
Contributor

bobbinth commented Feb 9, 2025

Thank you! I think we are moving in the right direction - though, we are not quite there yet. One aspect is that language/phrasing can be improved throughout - but for now I won't focus on this. More importantly, I think we are missing some important pieces and I would also re-shuffle things a bit. Here is how I'm thinking of the overall structure:

  1. We start with a short intro which explains some very basic things about the transaction (i.e., a transaction updates a state of one account but consumes/produces 0 or more notes) and motivation for the design. So, I wouldn't have "what's the purpose of ..." section - this content can just go after the first paragraph.
  2. After intro, I would go into the lifecycle - to explain how a transaction is executed. Some important concepts to bring up here:
    a. The sequence in which things happen (prologue, notes, tx script, epilogue).
    b. Accounts involved in a transaction, concept of native vs. foreign (state of native account can be updated, but state of foreign accounts can only be read).
    c. How notes work - i.e., notes can call methods of the account, create other notes, set transaction expiration etc.
    d. How transaction script works and why it is needed.
    e. The role of the nonce - i.e., it needs to be incremented if account state changes, we use it for transaction authentication.
    f. How a transaction uses a reference block and why it is needed (e.g., notes consumed in a transaction must have been created before the reference block). Also, maybe I'd bring up the concept of ephemeral notes here (i.e., not all notes need to be authenticated).
  3. Give an example of a simple transaction. I would probably just do consuming a P2ID note. Or maybe we can do 2 examples: consuming a P2ID note and issuing a P2ID note. I probably wouldn't go into the SWAP note (we could have a separate file section for this in the future). But I would get pretty detailed here - e.g., would use procedure names even (e.g., "note calls receive_assets procedure on an account).
  4. Next, I would go into different transaction types (local vs. network).
    a. For local transactions, I would bring up delegated proving.
    b. For network transactions, we could even explain the example from the deck of how handling shared state works.
  5. A section mentioning some limitations (e.g., at most 1K notes consumed/produced per transaction) and maybe performance data (a simple transaction takes about 90K cycles, signature verification is the dominant cost).

@Dominik1999
Copy link
Collaborator Author

Thanks @bobbinth. I changed the structure and mostly followed your approach. Let's not focus on the language now, but ensure we describe all protocol rules and provide sufficient examples to make it easy to follow.

Can you take a look, Bobbin, to see if we describe "Consuming a P2ID note" in the right depth?
In general, somewhere in the docs, we should also describe the current transaction kernel, including the memory layout, the procedures, and different contexts. But I am not sure if this makes sense before we have the Rust compiler.

@phklive can you take a stab at "Creating a P2ID note"?

Copy link
Contributor

@PhilippGackstatter PhilippGackstatter left a comment

Choose a reason for hiding this comment

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

Not a full review yet, but some initial comments.

Copy link
Contributor

@PhilippGackstatter PhilippGackstatter 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 to me. I like the example of the P2ID note creation/consumption to make it more concrete!

Left some comments inline.

Copy link
Contributor

@phklive phklive left a comment

Choose a reason for hiding this comment

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

I believe that we are starting to achieve a good state between high-level explanation and details.

One thing that we could do is to merge what is in the: **info** and **good to know** blocks directly into the text.

They seem out of place and as if it was information that we did not manage to fit.

@phklive
Copy link
Contributor

phklive commented Feb 13, 2025

Created an issue to incorporate the **Good to know** section into the text of Transaction.md

#1147

@phklive phklive merged commit 90681b3 into phklive-docs-update-transaction Feb 13, 2025
12 checks passed
@phklive phklive deleted the dominik_update_transaction_chapter branch February 13, 2025 08:57
Dominik1999 added a commit that referenced this pull request Feb 13, 2025
* Initiate structure for transaction

* Updated transaction section

* rename from plural to singular

* Removed conclusions + Removed header for 'What is X' and replaced in by direct text

* Updates with more info (#1118)

* updates to the chapter

* adding more clarity

* First pass on the updated doc

* changing the strucutre a bit

* changing the strucutre a bit

* Second wave of updates, improved wording, changed sentences, fixed formatting

* Fix typo

* after PHs review

* Fix formatting, improve parts, fix typos, ready for review 👀

* after Bobbin's review

* restructuring and adding more clarity

* Added example for creating note + closed all comments 🎙️

* Fix typos ‼️

* Specify the example for P2ID script and add RECIPIENT explanation ✅

* Remove image 📸

* Consolidate good to know

---------

Co-authored-by: Paul-Henry Kajfasz <kajfaszph@gmail.com>

---------

Co-authored-by: Dominik Schmid <35031754+Dominik1999@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog This PR does not require an entry in the `CHANGELOG.md` file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants