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

BIP39 to SSKR and reverse #63

Closed
nanostos opened this issue Sep 1, 2021 · 15 comments · May be fixed by #69
Closed

BIP39 to SSKR and reverse #63

nanostos opened this issue Sep 1, 2021 · 15 comments · May be fixed by #69

Comments

@nanostos
Copy link

nanostos commented Sep 1, 2021

Please allow for direct conversion of BIP39 to SSKR shares and vice versa, without having to manually convert to and from hex. Currently it sates seedtool: Input format bip39 cannot be used with output format sskr

@nanostos
Copy link
Author

nanostos commented Sep 1, 2021

After some experimentation, I've determined that the following commands can achieve the desired effect

seedtool --in bip39 "slender horse coil sketch public wink nest point royal believe inform lyrics critic harbor scheme enrich burden glance song chicken grief first panic actor" | seedtool --deterministic 1 --in hex --out sskr --group 2-of-3
and
seedtool --in sskr | seedtool --in hex --out bip39

It would still be nice if this could be achieved more directly.

@Sjlver
Copy link

Sjlver commented Mar 25, 2022

+1 to this feature request. I can see no reason why seedtool would not allow SSKR input with bip39 output (although I might be missing something)

@wolfmcnally
Copy link
Collaborator

+1 to this feature request. I can see no reason why seedtool would not allow SSKR input with bip39 output (although I might be missing something)

Seedtool was designed with the same idea in mind as many Unix command line tools: do one thing and do it well, and support piping the results from one operation into the next. It would actually complicate the design and maintenance of Seedtool quite a lot to support all the conversion/transformation scenarios one might want. Better to provide a tool that can integrate with itself and with shell scripts to become a component in transformations of arbitrary complexity.

@Sjlver
Copy link

Sjlver commented Mar 29, 2022

Hmm... I think I don't agree with this argument.

It's a bit like saying: Google Translate should not offer Russian to French translation. It's sufficient if there is Russian -> English and English -> French; we'll let people use their command-line skills to fill the gaps.

I think SSKR -> bip39 is a really common use case. Because SSKR is my preferred format for storing seeds, and bip39 is the one format that hardware wallets actually understand. I understand that seedtool likes its "hex" output format... but from a user's perspective, bip39 is what's needed far more often.

One more way to look at this: Seedtool is a single tool with many input/output formats, for a good reason. Of course we could go full unix-style and have separate tools, e.g., seedtool-generate | seedtool-convert --bip39. This would split seed generation and format conversion into separate tools. But it would be less user-friendly than the current tool, IMO.

Of course, seedtool should use a "hub-spoke" architecture internally, where all input formats are first converted to hex, and then hex is converted to the desired output format. I'm not saying that seedtool should have code for all the n**2 input/output format pairs. In that light, this feature request would potentially simplify seedtool rather than complicate it, since it removes the need for specific error messages like "bip39 output cannot be used with sskr input".

@Sjlver
Copy link

Sjlver commented Mar 29, 2022

Maybe iconv would be a better example than Google Translate, since iconv is a Unix command-line tool. I think it is really useful that iconv can go directly from each input encoding to each output encoding, without the user having to use an arbitrary intermediate representation.

Seedtool seems quite similar to iconv in many ways, being a tool that can convert between many representations of cryptographic seeds.

@alandefreitas
Copy link

It would actually complicate the design and maintenance of Seedtool quite a lot to support all the conversion/transformation scenarios one might want

It's sufficient if there is Russian -> English and English -> French

I don't how the specifics of the design. But these kinds of conversions are usually complex when there are no intermediary language/encoding for the languages/encodings. In the case of seedtool there seems to be an obvious lossless candidate, which is just the direct byte representation, or hex, depending on how the code would be reused.

do one thing and do it well

Seedtool is a single tool with many input/output formats, for a good reason. Of course we could go full unix-style and have separate tools (...)

From the perspective of seedtool, it seems like the tool does one thing well with a variety of input and output format options. It's different from a tool whose "one thing" it's doing is encoding from a particular encoding to another because the encoding is a representation option rather than the thing the tool does.

Given this design, when a combination of input and output doesn't work, the tool is still doing its "one thing" and it's the thing it promised to do. This seems more related to the "do it well" part because it's imposing on the user to find an external intermediary representation format.

@wolfmcnally
Copy link
Collaborator

I have added two example scripts to the seedtool repo that perform your use-case in a single step.

The first script is just:

#!/bin/bash

# bip39-to-sskr.sh

BIP39=$1
SSKR_OPTIONS=${@:2}
seedtool --in bip39 ${BIP39} | seedtool --in hex --out sskr ${SSKR_OPTIONS}

You can invoke it on a single line like this:

./bip39-to-sskr.sh "slender horse coil sketch public wink nest point royal believe inform lyrics critic harbor scheme enrich burden glance song chicken grief first panic actor" --group-threshold 1 --group 2-of-3 --group 3-of-5
tuna acid epic hard data skew wand acid acid able jugs noon kiln aunt roof leaf race figs horn exit knob wave next fuel pose taco miss work ruby webs huts film mild wolf zoom free puff wave whiz sets beta wave tent time kiwi vibe
tuna acid epic hard data skew wand acid acid acid hill oboe lazy stub hard fair diet days task jowl glow jade hope into hope undo game toys view flux vibe navy time judo duty main eyes hope junk what news lazy redo half yank gyro
tuna acid epic hard data skew wand acid acid also diet visa miss liar jowl warm mint bald data mint code zaps also aunt gush wand even purr belt plus judo join chef waxy kiln soap rich memo warm plus each epic task numb unit kept
tuna acid epic hard data skew wand acid brag able leaf wall toys trip undo exit ruby navy girl work legs bulb purr luau girl wasp poem quiz able void skew puff yawn gyro kite rich cook each peck belt lazy time void luck zaps maze
tuna acid epic hard data skew wand acid brag acid yoga jolt gems song mild hard inky when zest keep surf work hard news calm gear poem plus zone cook numb lava leaf work judo ruin zinc oval task next acid gyro oboe road lion runs
tuna acid epic hard data skew wand acid brag also high runs memo onyx note skew good jury oval peck pose menu toil lung news note iris hard slot kite arch navy heat note tent high item very barn limp zone quad taxi judo fuel eyes
tuna acid epic hard data skew wand acid brag apex days exit axis ramp tomb oval luau cost bulb down vast junk fish navy sets exit iris fizz eyes limp idea peck drum film unit help loud keno iron buzz knob epic mild frog fair cash
tuna acid epic hard data skew wand acid brag aqua lung grim visa help echo deli into ruby webs glow data item wand deli iced memo arch door buzz chef into toys ruin numb user wall idle echo half zest work tiny tied pose apex vibe

Or it can read the BIP-39 seed from STDIN:

./bip39-to-sskr.sh "" --group-threshold 1 --group 2-of-3 --group 3-of-5
slender horse coil sketch public wink nest point royal believe inform lyrics critic harbor scheme enrich burden glance song chicken grief first panic actor
^D
tuna acid epic hard data tied visa acid acid able hope glow iced meow flew ruby omit fizz need task gear claw door hope meow waxy dark keep barn math beta tent half wasp idea fern peck void limp peck fact half cook game down fish
tuna acid epic hard data tied visa acid acid acid keno kept oboe data rock skew plus vast free kick horn horn fund foxy tomb gray diet soap roof runs time pose yell able inch fern crux lung into twin pool back user purr fish guru
tuna acid epic hard data tied visa acid acid also belt fish yurt wave play gear ruby claw fern redo cola wave waxy iron blue luau film bulb kept taco pose drop bias dull jowl fern open echo heat game axis waxy jury list king judo
tuna acid epic hard data tied visa acid brag able drop mild paid what rich fair jolt urge user exit race saga lion user play cyan crux heat fizz wolf figs chef oval void half numb king gems puma exam away oval scar play vibe cook
tuna acid epic hard data tied visa acid brag acid cyan axis mild trip fizz unit dice calm memo ramp flew sets toil trip view waxy mild days lung news gush diet yank yank back pool aunt scar roof draw even zest many rust axis memo
tuna acid epic hard data tied visa acid brag also menu noon flux lamb quiz gift lazy slot twin very axis high hope ruby aqua surf acid very rich each knob fund purr guru oboe mint flux pool fizz gear sets apex deli kiwi tent bald
tuna acid epic hard data tied visa acid brag apex next also knob hang gift play slot bias miss jowl zone heat barn road game acid quiz next jury holy cash keys very flap zone monk fish curl gift good zaps hawk jade calm fern list
tuna acid epic hard data tied visa acid brag aqua work cyan cash onyx cook inch fern memo note sets wall need soap puff cusp free eyes crux grim code solo lamb judo veto main idle days cusp solo zest kiwi quad foxy claw play unit

The second script is just one line:

#!/bin/bash

# sskr-to-bip39.sh

seedtool --in sskr | seedtool --in hex --out bip39

It takes the shares from STDIN and writes the BIP-39 seed to STDOUT.

./sskr-to-bip39.sh
tuna acid epic hard data tied visa acid acid able hope glow iced meow flew ruby omit fizz need task gear claw door hope meow waxy dark keep barn math beta tent half wasp idea fern peck void limp peck fact half cook game down fish
tuna acid epic hard data tied visa acid acid acid keno kept oboe data rock skew plus vast free kick horn horn fund foxy tomb gray diet soap roof runs time pose yell able inch fern crux lung into twin pool back user purr fish guru
^D
slender horse coil sketch public wink nest point royal believe inform lyrics critic harbor scheme enrich burden glance song chicken grief first panic actor

@Sjlver
Copy link

Sjlver commented Apr 29, 2022

Hmm... Thanks :)

Why is a separate script preferable to having the tool itself do "right thing" (right from the point of view of @nanostos, @alandefreitas and yours truly)?

Personally, I'm interested in having this feature because it makes seedtool.info easier to build. That's an environment where I cannot run shell scripts. I would much rather have a single invocation of seedtool-cli, than carry intermediate results around in JavaScript. In seedtool.info's case, it's also much easier for users if all the output fields work with any input.

@Sjlver
Copy link

Sjlver commented Apr 29, 2022

Would you accept patches for this issue, or do you deeply think that going directly from SSKR to BIP39 is wrong?

@ChristopherA
Copy link
Contributor

I'm torn. @wolfmcnally as the original creator is trying to keep some constraints on the architecture of the code base carefully segregated, for a variety of reasons which also includes his more easily being able to support it.

On the other hand, with your involvement, seed-tool is clearly is a more community project now (with @wolfmcnally still as lead, but your contributions are very valuable). You are also not the only one to raise this issue, including myself. The new batch scripts solve it for me, but as you suggest, there are other complications when offering this through a non-cli environment with Javascript & web assembly.

I'm open to see a draft-PR that investigates how this can be done in a minimally hacky way, as a proposal for @wolfmcnally to review. But in the end, we need long-term support for this code and his opinion on long-term maintainability of the code base is also important. If this draft-PR introduces complications that Wolf finds risky, we may not be able to accept it. But equally, it may prove to him that it isn't too risky and worthwhile to merge.

Sorry for the wishy-washy here — this type of thing comes up often as community projects grow. My lessons from early on is that as long as the lead has concerns, they have to feel like it is an acceptable decision if they merge it. When we reach the point where Wolf chooses not to continue to be lead, the new lead should feel empowered the same way.

@wolfmcnally
Copy link
Collaborator

My concern is with having to maintain complex edge cases in the codebase. If a patch is truly a generalization, or better yet a simplification of what is there already, I'd consider it. But to be general if you want to go from SSKR to BIP39, then you need to consider going from anything to anything, or all you are scratching is your own personal itch.

The exact thing I'm doing in a shell script can be done from any language that invokes the code, including Javascript or anything else compiled to WebAssembly. No, you can't easily write pipes into a simple web page text field, but as I've recommended before, I don't think we should be striving to make seedtool available in all its generality on a web page: this would be intimidating for most users. Rather, a web page that offers seedtool functionality should focus on providing a subset of the most useful functionality and leave esoteric case-completeness for those able and willing to run it from the command line.

@Sjlver
Copy link

Sjlver commented Apr 30, 2022

Sounds good. Let me take an hour or so to look at the codebase. If I find a way to simplify things, I'll send you a PR; otherwise we'll leave this closed and I'll work around it for seedtool.info.

Sjlver added a commit to Sjlver/seedtool-cli that referenced this issue May 1, 2022
There's no technical reason why this shouldn't be allowed.

Closes BlockchainCommons#63
@Sjlver
Copy link

Sjlver commented May 1, 2022

Sent you #69

@wolfmcnally
Copy link
Collaborator

Sent you #69

Please see my review.

@wolfmcnally
Copy link
Collaborator

As your PR is actually far bigger in scope than the stated purpose of your original feature request, I am locking this thread as complete. If you wish to discuss your PR specifically, I recommend you do it on the review.

@BlockchainCommons BlockchainCommons locked as off-topic and limited conversation to collaborators May 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants