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

WIP: Kernel developers tutorial for DAL #257

Closed
wants to merge 9 commits into from

Conversation

lthms
Copy link
Collaborator

@lthms lthms commented Jan 11, 2024

No description provided.

@lthms lthms requested a review from a team as a code owner January 11, 2024 00:45
Copy link

vercel bot commented Jan 11, 2024

@lthms is attempting to deploy a commit to the Trili Tech Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Collaborator

@rafoo rafoo left a comment

Choose a reason for hiding this comment

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

I've done a pass over the whole article but did not try the code.

docs/tutorials/build-files-archive-with-dal.md Outdated Show resolved Hide resolved
## The Big Picture

Before diving into the code, we will first take a moment to understand how our
files archive will work. It can be broken down into 6 steps:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should it be "file archive" or "files archive"?


As a reminder, the kernel of a Smart Rollup is a WASM program. You need to
install the `wasm32-unknown-unknwon` target with rustup. The `proto-alpha`
feature is necessary to get access to the functions specific to the DAL.
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 understand why the kernel needs to be aware that the DAL even exists. I thought it only had to know about the reveal channel and would be agnostic on the data-availability solution.

docs/tutorials/build-files-archive-with-dal.md Outdated Show resolved Hide resolved
docs/tutorials/build-files-archive-with-dal.md Outdated Show resolved Hide resolved
that by modifying the `run` function as follows.


```rust
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why isn't this a diff like Task 2?

for page_index in 1..num_pages {
let _result = host.reveal_dal_page(
target_level as i32,
index,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe rename this index parameter to slot_index to avoid confusing it with the page index.

Copy link
Collaborator

Choose a reason for hiding this comment

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

2. Second, we try to fetch the contents of the first page. If 0 bytes are
written by `reveal_dal_page`, then we know the targeted slot has not been
attested for this block, and we have nothing left to do.
3. Otherwise, we read as many page as necessary to get the full slot.
Copy link
Collaborator

Choose a reason for hiding this comment

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

You always read all the pages don't you?

docs/tutorials/build-files-archive-with-dal.md Outdated Show resolved Hide resolved
our archive. Or, we could deal with the fact that for now, it is not possible
for a consumer of the file to know it’s original size (we could fix that by
modifying the script we use to publish a file to prefix it with its size).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe also mention some of the limitations of the archive: it handles at most one file per level and the file size cannot exceed the DAL slot size.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link

vercel bot commented Jan 11, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs-staging ✅ Ready (Inspect) Visit Preview Jan 11, 2024 1:29pm

Copy link
Collaborator

@timothymcmackin timothymcmackin left a comment

Choose a reason for hiding this comment

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

  • Are there any prerequisites? Obviously it needs Rust; does it need any other things, like Clang and LLVM like the existing Smart Rollup tutorial?
  • I needed to run rustup target add wasm32-unknown-unknown before building -- this is mentioned but make it a prereq or an explicit step; I missed it the first time.
  • How do I install the proto-alpha feature mentioned after the Cargo.toml file? Is it enough to have it in the features field in Cargo.toml?
  • Weeklynet faucet doesn't seem to work with temple browser extension; I get a message that says "Parameters invalid;" is that a known issue with Weeklynet? I can connect by scanning the qr code in the mobile app and the faucet tried to send test, but it says "An error occurred" and the console shows a 500 with no further info.
  • Instructions about interacting with weeklynet link to commands for octez-node; do I need to work with octez-node, or is there something similar I should be doing with octez-client?
  • Tutorials are much easier to follow if each thing that the user needs to do is a numbered step, with the actual thing that the user is doing in the first sentence of that numbered step. Otherwise I don't know if statements like "The proto-alpha feature is necessary..." are telling me to do something or not.
  • I got as far as the originate smart rollup command. I have my private key aliased in octez but I'm not sure if it has any tez, because of the issues with the faucet. When I run the command I get this:
Node is bootstrapped.
Error:
  Rpc request failed:
     - meth: GET
     - uri: https://rpc.weeklynet-2024-01-10.teztnets.com/chains/main/blocks/head/context/constants
     - error: Failed to parse the answer (application/json):
                error:
                  Missing object field smart_rollup_enable
                content:

@lthms
Copy link
Collaborator Author

lthms commented Jan 12, 2024

Are there any prerequisites? Obviously it needs Rust; does it need any other things, like Clang and LLVM like the existing Smart Rollup tutorial?

Any prerequisite for building kernels apply. In addition, the Octez binaries compatible with the current Weeklynet are required. I tried to hint that in a “Note” block, with a link the weeklynet page, but it’s probably too light.

We probably need a tutorial specifically for interacting with Weeklynet on docs.tezos.com actually.

I needed to run rustup target add wasm32-unknown-unknown before building -- this is mentioned but make it a prereq or an explicit step; I missed it the first time.

Can do indeed. We should add a proper prerequisite section as you suggested.

How do I install the proto-alpha feature mentioned after the Cargo.toml file? Is it enough to have it in the features field in Cargo.toml?

Yes indeed.

Weeklynet faucet doesn't seem to work with temple browser extension; I get a message that says "Parameters invalid;" is that a known issue with Weeklynet? I can connect by scanning the qr code in the mobile app and the faucet tried to send test, but it says "An error occurred" and the console shows a 500 with no further info.

That’s not surprising: Weeklynet is the very last version of Tezos, not yet deployed on Mainnet. Temple cannot keep track. We come back to the benefit of having a tutorial for Weeklynet.

Instructions about interacting with weeklynet link to commands for octez-node; do I need to work with octez-node, or is there something similar I should be doing with octez-client?

You don’t need to start an Octez node directly. You just have to use the octez-client from the correct commit.

Tutorials are much easier to follow if each thing that the user needs to do is a numbered step, with the actual thing that the user is doing in the first sentence of that numbered step. Otherwise I don't know if statements like "The proto-alpha feature is necessary..." are telling me to do something or not.

Any input for improving the tutorial is welcome. Readers familiar with Rust are likely to understand this sentence in particular I think, but maybe it’s not good enough to assume familiarity with Rust.

I got as far as the originate smart rollup command. I have my private key aliased in octez but I'm not sure if it has any tez, because of the issues with the faucet. When I run the command I get this:

This error is most certainly coming from the fact you are not using the correct version of Octez-client.


Thanks a lot for your feedback. It’s very helpful.

Out of curiosity, would you have the bandwidth to contribute directly to the tutorial to improve its structure, fluidity, etc.?

@timothymcmackin
Copy link
Collaborator

Out of curiosity, would you have the bandwidth to contribute directly to the tutorial to improve its structure, fluidity, etc.?

Yes, I can find some time for that this week. I can also look into writing a page on using weeklynet.

timothymcmackin and others added 8 commits January 15, 2024 13:57
Co-authored-by: Raphaël Cauderlier <[email protected]>
Co-authored-by: Raphaël Cauderlier <[email protected]>
Co-authored-by: Raphaël Cauderlier <[email protected]>
Co-authored-by: Raphaël Cauderlier <[email protected]>
Co-authored-by: Raphaël Cauderlier <[email protected]>
Co-authored-by: Raphaël Cauderlier <[email protected]>
Co-authored-by: Raphaël Cauderlier <[email protected]>
Copy link
Collaborator

@NicNomadic NicNomadic left a comment

Choose a reason for hiding this comment

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

Very nice and entertaining tutorial. Just left a few comments and fixed some typos.

3. Following the creation of a block containing the certificate, a subset of
Tezos’ bakers tries to download the file from the DAL, and if they succeed
they attest it with a dedicated operation. They have a certain number of
blocks to do so, and if they don’t by the end of this period, the certificate
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
blocks to do so, and if they don’t by the end of this period, the certificate
block levels to do so, and if they don’t by the end of this period, the certificate

Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't "block levels" redundant?

is considered bogus and the related data is dropped.
4. In parallel, new Tezos blocks are produced. Each time, our files archive
Smart Rollup get some execution time (as all Smart Rollups do). Everytime this
happens, our files archive Smart Rollup will try to fetch the latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

consider abbreviating (here and elsewhere):

Suggested change
happens, our files archive Smart Rollup will try to fetch the latest
happens, our Smart Rollup will try to fetch the latest


#### Hello, World!

The DAL node exposes a RPC just for that: `POST /slots`, whose body is the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Beware, the RPC is called /slots here, but /slot in the example below. Which one is right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

argument given with `--data` is a valid JSON string.
:::

To post this certificate, takes the `commitment` and `commitment_proof` value
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
To post this certificate, takes the `commitment` and `commitment_proof` value
To post this certificate, take the `commitment` and `commitment_proof` value

should see a line of log giving you the hash used to register the slot.

Assuming you have set the `hash` variable accordingly, then you can check the
file indeed exists (and later use `hexdump -C` to inspect its contents.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
file indeed exists (and later use `hexdump -C` to inspect its contents.
file indeed exists, and later use `hexdump -C` to inspect its contents.

You cannot use `diff` to ensure the file you originally published and the one
you downloaded from the rollup node are equal. Indeed, they are not: since
the size of a slot is fixed, the DAL node pads the value it receives from
`POST /slot`. in order to ensure it has the correct size.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
`POST /slot`. in order to ensure it has the correct size.
`POST /slot` in order to ensure it has the correct slot size.

Copy link
Collaborator

Choose a reason for hiding this comment

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

allowing them to deposit tez to the Smart Rollup, and sign the files they
publish). We could also build a frontend to visualize the files published in
our archive. Or, we could deal with the fact that for now, it is not possible
for a consumer of the file to know it’s original size (we could fix that by
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for a consumer of the file to know it’s original size (we could fix that by
for a consumer of the file to know its original size (we could fix that by

@lthms lthms closed this Jan 19, 2024
@lthms lthms reopened this Jan 19, 2024
@lthms
Copy link
Collaborator Author

lthms commented Jan 19, 2024

Can I close this now that #264 is good to go?

@timothymcmackin
Copy link
Collaborator

Can I close this now that #264 is good to go?

Yes, go ahead and close if you are happy with #264 .

@lthms lthms closed this Jan 20, 2024
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.

5 participants