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

[Docs]: Update Accounts section #1012

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

phklive
Copy link
Contributor

@phklive phklive commented Dec 11, 2024

In this PR I propose to update the Accounts section of the Miden documentation:

  • more explanative
  • more approachable for new developers
  • shortened
  • simplified
  • re-phrased
  • updated structure

@phklive phklive added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Dec 11, 2024
@phklive phklive self-assigned this Dec 11, 2024
@phklive phklive marked this pull request as ready for review December 11, 2024 13:32
@phklive phklive requested a review from Dominik1999 December 11, 2024 13:40
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. To me its clearer now.

I think, we can get rid of even more details:

  • the bits of an account ID. We can simply say, type and storage mode are encoded in the ID
  • that the Account Vault is represented as sparse Merkle tree

I left comments for those.

We can think about having deep dives for those implementation details in a different section.

docs/architecture/accounts.md Outdated Show resolved Hide resolved
docs/architecture/accounts.md Outdated Show resolved Hide resolved
docs/architecture/accounts.md Outdated Show resolved Hide resolved
docs/architecture/accounts.md Outdated Show resolved Hide resolved
docs/architecture/accounts.md Outdated Show resolved Hide resolved
docs/architecture/accounts.md Outdated Show resolved Hide resolved

# increments the nonce (anyone should be able to call that function)
push.1 exec.account::incr_nonce
Assets are stored in a sparse Merkle tree, allowing for efficient, cryptographically secure proofs of ownership and state:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to explain that they are stored in a sparse Merkle tree?

I know I also had it in my version, but maybe its better to stick to the protocol and not the implementation.

So we could say how big the vault is and that it can hold fungible and non-fungible assets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to something shorter and less implementation focused.

docs/architecture/accounts.md Show resolved Hide resolved
docs/architecture/accounts.md Show resolved Hide resolved
docs/architecture/accounts.md Outdated Show resolved Hide resolved

## Account design
In Miden, an `Account` represents an entity capable of holding assets, storing data, and executing custom code. Each `Account` is essentially a specialized smart contract providing a programmable interface for interacting with its state and managed assets.
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 add here the "Why" as well?

In Miden, we have accounts as entities because we want expressive smart contracts.

For payments only we could do the UTXO model alone.

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 feel like the documentation should explain what and not really why. I don't think that it's relevant for the user / builder to understand our choice. It's more something that we would put in a blog post imo.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, I can see that argument. For me, at the beginning of the chapter, especially in a design / yellow paper section, we can have one paragraph of the why.

@phklive phklive requested a review from Dominik1999 December 13, 2024 11:08
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.

2 participants