-
Notifications
You must be signed in to change notification settings - Fork 83
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
I'm receiving the following error when running the script on a new vm:
mainnet-val-setup.sh: line 14: wget: command not found
tar: go1.15.2.linux-amd64.tar.gz: Cannot open: No such file or directory
tar: Error is not recoverable: exiting now
...
HEAD is now at 1b7c80e Update changelog for v1.0.0 (#325)
contrib/devtools/Makefile:15: *** could not find go. Is it in PATH? . Stop.
Creating keys
mainnet-val-setup.sh: line 49: regen: command not found
After you have copied the mnemonic phrase in a safe place,
press the space bar to continue.
We should be escaping on error by adding set -e
to the start of this script. You need to add wget
to the list of packages installed with sudo apt install
in order to resolve.
scripts/mainnet-val-setup.sh
Outdated
|
||
echo "-- Clear old regen data and install Regen-ledger and setup the node --" | ||
|
||
rm -rf ~/.regen |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should remove ~/.regen
without confirming with the user that this action is ok. We should check whether this directory exists and prompt the user to confirm removing the directory is ok. We are assuming that the user is starting from scratch but we should take the necessary precaution.
scripts/mainnet-val-setup.sh
Outdated
YOUR_KEY_NAME=$1 | ||
YOUR_NAME=$2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will need to add supporting documentation that instructs users on setting these variables. We might also want to consider being more specific about these variables. YOUR_KEY_NAME
might be VALIDATOR_KEY_NAME
and YOUR_NAME
might be NODE_MONIKER
.
echo "press the space bar to continue." | ||
read -s -d ' ' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use enter rather than the spacebar to confirm and continue?
rm -rf $GOPATH/src/github.com/cosmos/cosmos-sdk | ||
git clone https://github.com/cosmos/cosmos-sdk $GOPATH/src/github.com/cosmos/cosmos-sdk | ||
cd $GOPATH/src/github.com/cosmos/cosmos-sdk | ||
git checkout v0.40.0 | ||
cd cosmovisor | ||
make cosmovisor | ||
cp cosmovisor $GOBIN/cosmovisor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should be installing stable versions of cosmovisor rather than checking out versions within the sdk repository and then building from source. I recently updated the documentation in the sdk to use the following:
go install github.com/cosmos/cosmos-sdk/cosmovisor/cmd/cosmovisor@v0.1.0
This will require go version 1.16 or 1.17 (go install
is not available in go 1.15).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think we should be installing a stable version of cosmovisor rather than checking out a version within the sdk repository. Using the command above would replace lines 102-108.
That works for me. Ill update!
…On Wed, Sep 8, 2021, 11:22 AM Ryan Christoffersen ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In scripts/mainnet-val-setup.sh
<#126 (comment)>:
> + wget https://dl.google.com/go/go1.15.2.linux-amd64.tar.gz
+ tar -xvf go1.15.2.linux-amd64.tar.gz
I still think we should be using a later version of go.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#126 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACBIMSSYHD5AJGSBAPMU7T3UA6SX7ANCNFSM5CXNFCRA>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
I tested with ubuntu 20.04 using the following command:
bash <(curl -s https://raw.githubusercontent.com/regen-network/mainnet/12dca1688b2499c3201fa8b3f5e6cc955f87a340/scripts/mainnet-val-setup.sh)
When running with a fresh VM, the clear
action leaves my screen blank so I'm unable to view the prompt. I have to hit page up. I then I am able to view the prompt for removing ~/.regen
(even though the directory does not exist) but confirming with 1
results in the following error:
CAUTION!
-- If Regen was previously installed, the following step will remove ~/.regen from your system. Are you sure you would like to continue?--
1) Yes
2) No
#? 1
install regen-ledger
fatal: could not create leading directories of '/src/github.com/regen-network/regen-ledger': Permission denied
I don't think it's necessary to clone the repo into the src
directory but git clone will need sudo
before it to make this work.
echo "CAUTION!" | ||
echo "-- If Regen was previously installed, the following step will remove ~/.regen from your system. Are you sure you would like to continue?--" | ||
|
||
select yn in "Yes" "No"; do | ||
case $yn in | ||
Yes ) rm -rf ~/.regen; break;; | ||
No ) exit;; | ||
esac | ||
done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should only ask if the user wants to remove the folder if the folder exists. Can we check if ~/.regen
exists and only run the confirmation if so?
Most users will be starting from scratch so this will be irrelevant to them.
rm -rf $GOPATH/src/github.com/cosmos/cosmos-sdk | ||
git clone https://github.com/cosmos/cosmos-sdk $GOPATH/src/github.com/cosmos/cosmos-sdk | ||
cd $GOPATH/src/github.com/cosmos/cosmos-sdk | ||
git checkout v0.40.0 | ||
cd cosmovisor | ||
make cosmovisor | ||
cp cosmovisor $GOBIN/cosmovisor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think we should be installing a stable version of cosmovisor rather than checking out a version within the sdk repository. Using the command above would replace lines 102-108.
echo "-- Your Key Name is $YOUR_KEY_NAME and your moniker is $YOUR_NAME. Is this correct?" | ||
|
||
select yn in "Yes" "No" "Cancel"; do | ||
case $yn in | ||
Yes ) break;; | ||
No ) echo "-- Please choose a name for your key --"; | ||
read YOUR_KEY_NAME; | ||
echo "-- Please choose a moniker --"; | ||
read YOUR_NAME; break;; | ||
Cancel ) exit;; | ||
esac | ||
done | ||
|
||
echo "-- Your Key Name is $YOUR_KEY_NAME and your moniker is $YOUR_NAME. --" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this flow be changed to a loop? would be nice to continuously ask for correctness until they hit finally hit yes, rather than just check one time
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK. Approving with improvements listed in #127.
No description provided.