BlockchainCommons / seedtool-cli

Cryptographic Seed Tool for the command line
Other
25 stars 16 forks source link

Replace SLIP-39 with SSKR #30

Closed wolfmcnally closed 3 years ago

wolfmcnally commented 3 years ago

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA512

Replace SLIP-39 with SSKR. 454ce63 Wolf McNally wolf@wolfmcnally.com Sep 15, 2020 at 10:13 PM Bump version to 0.8.0. -----BEGIN PGP SIGNATURE----- Comment: GPGTools - https://gpgtools.org

iQIzBAEBCgAdFiEElDZS7jhEF2DD3DU2S2wvz4lHgK4FAl9hoQsACgkQS2wvz4lH gK57TRAAsnyZacHQgCPREq6VR1hjhNg5wfB9oT8KhSIOqS3kace3gc9fZoGpr5xi 7DFdiEhTlHO3gDpQ/hRpAu5SozD7iZ4BP83dt5oloqIbmLL4YdBSvs7AsZnHiR7M 498lffffEMyxjLiyYdgHijJWSzJ6XkJSC7aMxDcrMIqmpzRV8Kv0tKA22cdkpPoX UooSjqm808/3BWe1SEY6NHwsHsFovWCxDq0NNlBuejEpx5veDircpc8sy9ZF2L2I iQuIJpj5Y5VLpDZrQ5PC264oQtgGY7wEa4+jpr1f4hN4Mu8nWVU3ohIPVVXwENls BpegGrbwMUGd2ElcwWJCUDS0r93dP+PV2UmUIsWnaOwuriTa14U0RzqphzNPk61B +2xoo6RcuA5VB4fkahEA+zGoK/kfWlK5HrEeMl/yPDzgDcBO/kOslpb388ozGsIb qBrBHOMSzjVP4S38bUKJTb2rTKPi0VNmRm027ClYWNyhUks0jis11Fl2uhuY+kpA W7puSLfYIhS9wb0QTbcHbGzALGbkbkg5DcB7IVNbuzjvUkdbdPaYfesrQ248Mfrq dql1WVhDD58MtO1PNiJplBu2YFANdFrKQXmBU+hXPytv6GCNhCgewN70ljtcbZq9 +qIEa9CDaaq79sP52POa7yqQjce0o1FgXhym0bXPRGNN73Z6iiI= =JuSP -----END PGP SIGNATURE-----

gorazdko commented 3 years ago

This will not build on any platform, including macOS, Linux and Windows

The fix you need is in #28

- AC_CHECK_LIB([bc-bytewords], [bytewords_encode], [], [
-  echo "### Error! libbc-bytewords must be installed first."
-  exit -1
-  ])

@wolfmcnally I encourage you to also review my PRs #28 and #29

ChristopherA commented 3 years ago

I built on MacOS Catalina 10.15.6 without any errors, but I agree with @gorazdko that his build changes for Linux and Windows should probably be merged first, and then confirm that they don't break this Mac build. In particular it is important that we build on Linux when we announce the version with bc-sskr.

ChristopherA commented 3 years ago

@gorazdko I was not able to reproduce your error on macOS. What version of the macOS are you testing?

I have brew install autoconf automake libtool for my testing installation. Could you try installing those and trying again? My build script is for my VM of Catalina (where I can revert back to stock after testing easily) is :

brew update
brew upgrade
brew install gh autoconf  automake libtool
cd ~/Documents/GitHub/BlockchainCommons/
git clone git@github.com:BlockchainCommons/bc-seedtool-cli.git
cd bc-seedtool-cli
gh pr list --repo BlockchainCommons/bc-seedtool-cli

Then I insert the PR number into the below:

git pull --ff-only
git checkout master
git clean -ffdx
gh pr checkout 30
./build.sh
sudo make install

and then after, to confirm that distclean works do:

make distclean
git clean -n -ffdx  # should be no remnants left
git checkout master
git branch -D wolfmcnally/master
git clean -n -ffdx  # should be no remnants left

@wolfmcnally, I thought we had those brew install instructions in some old README for this project, but I'm not seeing them in this version — we should add them as a note for Mac users.

ChristopherA commented 3 years ago

@wolfmcnally, I note that with my distclean checks that there are still some remnants after distclean. My test script does git clean -ffdx to make sure there are no artifacts before the build, then I do it later witn -n to see what should have been removed:

git clean -n -ffdx
make distclean # should be no remnants left

and after PR #30 I get:

Would remove deps/bc-slip39/
Would remove sysroot/

It could be that this is an artifact of checking out master first, and I should do git clean --ffdx after checking out the PR. Please advise what you think best practices are.

wolfmcnally commented 3 years ago

I have merged all of @gorazdko 's changes to both dependencies and Seedtool. I did fix a couple places where those changes broke the Mac build due to omission of the standard C++ libraries, and verified everything builds correctly on the Mac. @gorazdko , the one dependency that Seedtool still needs updates for is bc-sskr, before it can be built for Linux and Windows.

wolfmcnally commented 3 years ago

@wolfmcnally, I note that with my distclean checks that there are still some remnants after distclean. My test script does git clean -ffdx to make sure there are no artifacts before the build, then I do it later witn -n to see what should have been removed:

git clean -n -ffdx
make distclean # should be no remnants left

and after PR #30 I get:

Would remove deps/bc-slip39/
Would remove sysroot/

It could be that this is an artifact of checking out master first, and I should do git clean --ffdx after checking out the PR. Please advise what you think best practices are.

bc-slip39 should be deleted manually as all dependencies on it have been removed.

sysroot contains the build products of the dependencies. The distclean target could be modified to remove it.

ChristopherA commented 3 years ago

sysroot contains the build products of the dependencies. The distclean target could be modified to remove it.

I think this is a good idea with make distclean. The other one will solve itself after merge.

wolfmcnally commented 3 years ago

Turns out the former rm -rf libs line should have been changed to rm -rf sysroot when we changed the name of that dir. Also, distclean now deletes the contents of deps to force a re-checkout of the submodules, but not the deps directory itself as that's where git tracks the submodule checkout information.

gorazdko commented 3 years ago

I have merged all of @gorazdko 's changes to both dependencies and Seedtool. I did fix a couple places where those changes broke the Mac build due to omission of the standard C++ libraries, and verified everything builds correctly on the Mac. @gorazdko , the one dependency that Seedtool still needs updates for is bc-sskr, before it can be built for Linux and Windows.

This dependency https://github.com/BlockchainCommons/bc-sskr/pull/1 now builds on linux and windows

I will then test everything from scratch once you rebase your PR