-
Notifications
You must be signed in to change notification settings - Fork 0
bumped keyper version to v1.3.9 #17
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
Conversation
|
@ylembachar this PR also has new assets version now which include new bootstrap address and |
d549c4e to
3a298d5
Compare
3a298d5 to
ea086a2
Compare
shutter/scripts/configure_keyper.sh
Outdated
| echo "Could not find DAppNode RPC/WS url for this package!" | ||
| echo "Please configure 'ETHEREUM_WS' to point to an applicable websocket RPC service." | ||
| exit 1; | ||
| export SHUTTER_NETWORK_NODE_ETHEREUMURL=ws://execution.${SUPPORTED_NETWORKS}.dncore.dappnode:8545 |
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.
SUPPORTED_NETWORKS is potentially an array. So this shouldn't apply here.
shutter/scripts/configure_keyper.sh
Outdated
|
|
||
| export SHUTTER_NETWORK_NODE_ETHEREUMURL=${ETHEREUM_WS:-$(get_execution_ws_url_from_global_env "$NETWORK" "$SUPPORTED_NETWORKS")} | ||
| //FIXME: This is a workaround for the issue with the staker-scripts@v0.1.1 not setting get_execution_ws_url_from_global_env correctly in the environment variables. | ||
| export SHUTTER_NETWORK_NODE_ETHEREUMURL=${ETHEREUM_WS:-ws://execution.${SUPPORTED_NETWORKS}.dncore.dappnode:8546} |
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.
See above: I suggest to use ${NETWORK} instead of the array ${SUPPORTED_NETWORKS}.
konradkonrad
left a comment
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.
lgtm!
meant to say: lgtm after addressing the two comments
konradkonrad
left a comment
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 SUPPORTED_NETWORKS is the right env var for filling the url (see comments). Other than that it looks good to me.
shutter/scripts/configure_keyper.sh
Outdated
| echo "Could not find DAppNode RPC/WS url for this package!" | ||
| echo "Please configure 'ETHEREUM_WS' to point to an applicable websocket RPC service." | ||
| exit 1; | ||
| export SHUTTER_NETWORK_NODE_ETHEREUMURL=ws://execution.${SUPPORTED_NETWORKS}.dncore.dappnode:8545 |
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.
Also the logic here needs to check for both ports and if none of them work, ask the user to configure an ETHEREUM_WS
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.
it is checking for both, 8546 port is checked initially if it does not find ETHEREUM_WS url, then if it does not work, we check for 8545 port (set in the test_ethereum_url function)
72e3f4d to
355ed8a
Compare
355ed8a to
26ad3cc
Compare
shutter/scripts/configure_keyper.sh
Outdated
| //FIXME: This is a workaround for the issue with the staker-scripts@v0.1.1 not setting get_execution_ws_url_from_global_env correctly in the environment variables. | ||
| //Git Issue: https://github.com/dappnode/staker-package-scripts/issues/11 | ||
| export SHUTTER_NETWORK_NODE_ETHEREUMURL=${ETHEREUM_WS:-ws://execution.${NETWORK}.dncore.dappnode:8546} | ||
| echo "[DEBUG | configure] SHUTTER_NETWORK_NODE_ETHEREUMURL is ${SHUTTER_NETWORK_NODE_ETHEREUMURL}" |
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.
For debugging purposes, this echo is now in an awkward spot -- even though it may say ...:8546, we can end up with ...:8545 as the chosen working value.
6baf70a to
70a1a02
Compare
55837bd to
6e5de32
Compare
konradkonrad
left a comment
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.
lgtm!
7ee90c2 to
8105b62
Compare
No description provided.