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

Update WalletGeneration.md #1813

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Update WalletGeneration.md #1813

wants to merge 7 commits into from

Conversation

watou
Copy link

@watou watou commented Sep 7, 2024

I noticed the ASCII flowchart and I thought I'd take a crack at converting it to mermaid.

I noticed the ASCII flowchart and I thought I'd take a crack at converting it to mermaid.
Copy link
Collaborator

@MarnixCroes MarnixCroes left a comment

Choose a reason for hiding this comment

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

hi,

Concept ACK, your current proposal doesn't render the chart though

master

image

pr

Screenshot from 2024-09-07 18-59-33

@MarnixCroes
Copy link
Collaborator

MarnixCroes commented Sep 7, 2024

it seems there is a vuepress pluging that would make it work
That plugin would need to be added to the config, I'm not sure if it's worth is tbh.

How about adding the mermaid chart as an image?
Screenshot from 2024-09-07 19-32-37

@watou
Copy link
Author

watou commented Sep 7, 2024

I didn't mean to give unwanted work to anyone! I'm just fond of viewing mermaid charts over ASCII versions -- less cognitive friction IMHO.

I won't take offence if you don't find it worthwhile. All the best!

@MarnixCroes
Copy link
Collaborator

MarnixCroes commented Sep 7, 2024

@watou
no worries!
Your proposal to use a mermaid chart is an improvement.
If you could add it as an image then I'm happy to add this change.

```mermaid
flowchart TD
    ent[Entropy]  --> jctA(( ))
    wl[Word list] --> jctA
    jctA --> mne[Mnemonics]
    mne --> jctB(( ))
    jctB --> seed[Seed]
    seed --> extkey[Extended Key]
    extkey --> privkey[Private Key]
    privkey --> jctC(( ))
    pass[Passphrase] ----> jctB & jctC
    net[Network] --> jctC
    jctC --> encsec[Encrypted Secret]
    encsec --> last["`Save encrypted
                   secret+chaincode+
                   Fingerprint+ExtPub`"]
```
replaced mermaid text with image
@MarnixCroes
Copy link
Collaborator

the image should be added to /docs/vuepress/public.

the mentioning of this is needed for bip38 is removed now.
I think it is still good to mention it, maybe add it like this:
image

pass[Passphrase] ----> jctB 
pass[Passphrase] -- This is needed to use BIP38 -->  jctC

The arrows consolidating in the small dots seems to be unnecessary.
Maybe remove the dots and just link them to the boxes? WDYT?

thanks

```mermaid
flowchart TD
    ent[Entropy] & wl[Word list] --> mne[Mnemonics]
    mne --> seed[Seed]
    seed --> extkey[Extended Key]
    extkey --> privkey[Private Key]
    privkey --> encsec[Encrypted Secret]
	pass[Passphrase] ----> seed 
	pass -- This is needed to use BIP38 -->  encsec
    net[Network] --> encsec
    encsec --> last["`Save encrypted secret + chaincode + Fingerprint + ExtPub`"]
```
Fixed image reference, to new image.
removed old image version in wrong place
@watou
Copy link
Author

watou commented Sep 7, 2024

Hopefully last commits are good!

Copy link
Collaborator

@yahiheb yahiheb left a comment

Choose a reason for hiding this comment

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

Why is the Passphrase at the top? I would rather see it aligned with the Mnemonics.

I am not sure This step is needed to use bip38 is at the right place.

@yahiheb yahiheb requested a review from lontivero September 8, 2024 00:07
```mermaid
flowchart TD
    ent[Entropy] & wl[Word list] --> mne[Mnemonics]
    mne --> seed[Seed]
    seed --> extkey[Extended Key]
    extkey --> privkey["`Private Key
(This is needed 
to use BIP38)`"]
    privkey --> encsec[Encrypted Secret]
	pass[Passphrase] --> seed 
	pass -->  encsec
    net[Network] --> encsec
    encsec --> last["`Save encrypted secret + chaincode + Fingerprint + ExtPub`"]
```
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