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

brushingup crypto slides #30

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

Conversation

coax1d
Copy link

@coax1d coax1d commented Dec 12, 2023

Go over and comb through all crypto slides. Make any necessary edits

@coax1d coax1d self-assigned this Dec 12, 2023
@coax1d coax1d mentioned this pull request Dec 12, 2023
@coax1d coax1d changed the title Coax1d brushingup crypto slides brushingup crypto slides Dec 14, 2023
@coax1d coax1d marked this pull request as ready for review December 14, 2023 14:52
Copy link

@naterarmstrong naterarmstrong left a comment

Choose a reason for hiding this comment

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

None of the HTML changes even render for me, and I don't understand why you'd want to introduce HTML there, instead of just markdown.

See note on introducing excessive math into crypto slides.

Of course, the secret key is a point on an elliptic curve, not a phrase.

BIP39 applies 2,048 rounds of the SHA-512 hash function<br />to the mnemonic to derive a 64 byte key.
The secret key is a scalar value from the scalar field of the base field which the elliptic curve is defined over. Not a phrase.

Choose a reason for hiding this comment

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

This is probably too detailed for the level of math the cryptography module should talk about, and the number of words and terms will just make people confused.

Probably better to phrase it as either "a 64-byte value" (which is a bit imprecise, but more black-boxy) or "a mathematical construct specific to the cryptography you're using" (which is broadly accurate, and doesn't necessitate readers to try to figure out what a "scalar value", "scalar field", or "base field" are.

Copy link
Author

Choose a reason for hiding this comment

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

I am correcting the incorrect statement which was made that a secret key is a point on an elliptic curve which is just a false statement. If math is going to be talked about it should atleast be correct.

Copy link
Author

Choose a reason for hiding this comment

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

A secret key is not a curve point

Copy link
Author

@coax1d coax1d Dec 19, 2023

Choose a reason for hiding this comment

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

Instead we can just not reference any elliptic curve. But to be honest we are going to introduce a ZK section some small references to curves and fields isnt that bad. There isnt even a single actual math line in here I kind of disagree that it is too much math. I am not showing the scalar field times the base field element or something.

It is also important to note that there will be founders in crypto which probably wont understand half of it either so I think we are going to just have to wing it with what we choose to let some people scratch their head over.

Choose a reason for hiding this comment

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

I agree that we should be correct everywhere we do reference the math, and should for sure correct what's in here right now.

If you want to keep the definition as written, I'm fine with that if you add something in the notes for the lecturer to say that the definition is not important to know. My thought was that the definition written is a bit long and contains a few terms most won't know, and as people are learning most everything for the first time, it would be distracting.


---

## Portability

Different key derivation functions affect the ability to use the same mnemonic in multiple wallets as different wallets may use different functions to derive the secret from the mnemonic.

Notes:
i.e. May hash to a different base field because of a different elliptic curve

Choose a reason for hiding this comment

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

In here you can put the details previously added to the slide, saying that in ECC a secret key is actually a scalar value from the scalar field of the base field which the elliptic curve is defined over.

content/cryptography/addresses/slides.md Outdated Show resolved Hide resolved
- `fn public_key(sk) -> pk;` <br /> Return the `pk` (public key) from a `sk`.
- `fn sign(sk, msg) -> signature;` <br /> Takes `sk` and a message; returns a digital signature.
- `fn verify(pk, msg, signature) -> bool;` <br /> For the inputs `pk`, a message, and a signature; returns whether the signature is valid.
<ul>

Choose a reason for hiding this comment

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

All this stuff doesn't even render for me.

What's the reasoning for making this as inline HTML instead of just keeping it as markdown? I don't see anything in here that isn't supported in markdown (unordered lists, code).

Copy link
Author

Choose a reason for hiding this comment

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

It renders for me. But it already got pushed in the pba-content repo. Idk half of the slides in all modules is HTML or MD so it is a mix. There really hasnt been any enforcement. Also the MD cant be limiting sometimes it doesnt cover everything. If its really a big deal it can be changed. It wouldnt take long.

Choose a reason for hiding this comment

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

Ah, weird that it doesn't render for me locally but does otherwise

Copy link
Contributor

Choose a reason for hiding this comment

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

Glad to see the book getting some love 🌠 wishing the team for HK all the best 🙏 - hope to see the work for that wave end up translating here. I am not in the game for it (at least for now) but have some advice/notes if you wanna hear em 😁

Idk half of the slides in all modules is HTML or MD so it is a mix.

Ultimately not up to me anymore, though I tried to enforce MD. I did work with you @coax1d directly on this before in case you forgot 😝 🫶 - you even helped convert a subset of your HTML to MD where possible before too. I did intentionally archive the (private) pba-content repo that is not AFAIK up to date with the fair bit of polish I put in post UCB that is now in the initial publishing of this book that included moving HTML to MD where possible. I highly suggest pulling content from this repo before making changes to the what IMHO should remain archived older repo.

For some background, the chief motivator for using the reveal-md tooling for us from the beginning was:

Focus on content. Don't mess with style. You do not need to know anything but simple MD to make slides. Where MD is insufficiently expressive, there are minimal custom HTML slide style elements to get that content crafted and delivered, with easy templates/examples in https://github.com/Polkadot-Blockchain-Academy/pba-book/tree/main/content/contribute/copy-paste-slides.

Maintainability and uniformity. The book has a large number of contributors with zero up to expert HTML/CSS experience, and the next faculty to take on teaching may be anywhere in that range. It's much more sustainable to have as simple and clean content for everyone to reasonable in the long term, and much more portable (like in the pure reveal-md tooling -> to this book).

Maybe @Asamartino wants to do something different now, and that driving factor isn't as important now. 🤷 Happy to help outline and implement a strategy with maintainers here that lands in the contributor's guide, if the PBA team wants this.


For the rendering, sadly as it stands there are at least four different HTML servers depending on what commands you run (reveal has dev and build, bun with a minimal static server, and mdbook has a watching one). This could be cleaned up for a better overall UX, IMHO this would fork or replace the reveal-md tooling with something minimal here, and as I moved to bun for the book's static server here, I would opt for better configure mdbook (perhaps a custom plugin) as a tool to get slides -> reveal HTML -> full book.

Maybe related: webpro/reveal-md#433 but it's been a while since I played with different behavior.

In the end, the source of truth on what things will render as should be assessed via running markers or makers serve -> https://github.com/Polkadot-Blockchain-Academy/pba-book/blob/main/Makefile.toml#L108-L125 as this is what actually gets deployed to github pages.

Co-authored-by: Nate Armstrong <naterarmstrong@gmail.com>
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.

3 participants