BenWestgate / yeti.Bash

A port of Yeticold.com to Shell.
https://github.com/JWWeatherman/yeticold
14 stars 2 forks source link

Use shellcheck and shfmt #1

Open secinthenet opened 3 years ago

secinthenet commented 3 years ago

shellcheck will add checks that the script uses bash safely, and shfmt will make the formatting consistent

BenWestgate commented 3 years ago

Shell check has been used.

Removed input and output of the same variable in one pipe, now it starts as xprv_desc and pipes out canonical form to descriptor.

I've made some improvements to the formatting following this guide here: https://google.github.io/styleguide/shellguide.html

TODO remove pipe to while loop and replace sed substitutions with built-in "${string/#foo/bar}" make text prompts not split words across lines.

I'll keep the updated style in CustomWallet.sh as I add coinflips/dice this weekend and a compressed descriptor format for easily handwriting (and confirming it's correct) for a large n descriptor.

I'll delete the new code blocks afterwards to transfer the improved style back to the yeti-clone version.

secinthenet commented 3 years ago

Yes, using the google style guide is a good start, but it seems the scripts don't comply yet, for example use [ instead of [[, and the formatting is awkward and not very readable. Would be helpful to use much shorter lines, say a limit of 80 (or 100 or 120 but something consistent and not very wide). Also, put most comments on separate lines, etc.

BenWestgate commented 3 years ago

Would you like to reformat an awkward code block for me so I can see what style to copy for the rest?

One advantage this repository is supposed to bring to Yeti is easier code review without a GUI.

I'm not a professional developer this is my first project meant for others to read since University a decade ago. So I need some help.

secinthenet commented 3 years ago

You can just use shfmt and it will output something reasonable, can manually tweak later if needed (rare). Much better to use an automatic formatter and get consistent results than do it manually. BTW, it seems it's highly incompatible with the google style guide right now?

Example code block before

for ((i = 1 ; i <= $n ; i++)); do           # loop thru idented steps n times
    ./bitcoin-cli createwallet $i           #create a wallet
    ./bitcoin-cli -rpcwallet=$i dumpwallet $i   # dump the wallet to a walletdump file, this contains xprv and seed
    sed '6q;d' $i|tail -c112 >xprv$i        # find line 6 in file, trim line to xprv data, save as xprv
    grep "hdseed=1" $i|head -c52 >seed$i        #search for line with hdseed, trims line to WIF seed, save as seed
    sed -i s/$/$(<xprv$i)"\/*,"/ descriptor     #append "xprv/*," to descriptor
    done; wait
sed -i s/.$/"))"/ descriptor; wait          #replace last character , of string with )) to close parentheses from wsh(multi( and finish descriptor

After:

for ((i = 1; i <= $n; i++)); do # loop thru idented steps n times
  ./bitcoin-cli createwallet $i             #create a wallet
  ./bitcoin-cli -rpcwallet=$i dumpwallet $i # dump the wallet to a walletdump file, this contains xprv and seed
  sed '6q;d' $i | tail -c112 > xprv$i        # find line 6 in file, trim line to xprv data, save as xprv
  grep "hdseed=1" $i | head -c52 > seed$i    #search for line with hdseed, trims line to WIF seed, save as seed
  sed -i s/$/$(< xprv$i)"\/*,"/ descriptor   #append "xprv/*," to descriptor
done
wait
sed -i s/.$/"))"/ descriptor
wait #replace last character , of string with )) to close parentheses from wsh(multi( and finish descriptor
secinthenet commented 3 years ago

Some things are not taken care of by shfmt, I suggest you read and fully apply the google style guide to improve things. For example, some comments have a leading space char, some don't, this is hard to read

BenWestgate commented 3 years ago

I will standardize the comments to have leading space and not wrap on my 1920 pixel ubunutu desktop. Keep spaces between alphanumeric next and < or >.

I don't quite understand how to install shfmt so I will put that off until my new features are finished.

One of my Goals is to implement new features that improve Yeti:

Wallet Encryption so one backup alone doesn't contain any info about the main wallet storing significant funds. Compressed Descriptors so they can be handwritten without a printer Dice entropy support (just added)

I consider these more urgent since any great Ideas once written and tested here will be implemented by Yeti. And this repo is still more readable and commented than yeti's python.

ghost commented 3 years ago

I just ran shellcheck over your latest commit f00c11b55a6f85d5589907759c556b80a278fc17 and there are more syntax errors then lines of code.

Rspigler commented 3 years ago

See https://github.com/koalaman/shellcheck#user-content-installing or https://www.shellcheck.net/ for shellcheck and https://snapcraft.io/install/shfmt/ubuntu for shfmt