Conversation
|
@ylembachar this PR also has new assets version now which include new bootstrap address and |
blockchainluffy
force-pushed
the
release/keyper/v1.3.9
branch
from
d549c4e to
3a298d5
Compare
blockchainluffy
force-pushed
the
release/keyper/v1.3.9
branch
from
3a298d5 to
ea086a2
Compare
blockchainluffy
requested review from
konradkonrad and
ylembachar
and removed request for
konradkonrad
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.
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.
See above: I suggest to use ${NETWORK} instead of the array ${SUPPORTED_NETWORKS}.
konradkonrad
dismissed
their stale review
meant to say: lgtm after addressing the two comments
konradkonrad
left a comment
konradkonrad
left a comment
There was a problem hiding this comment.
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.
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.
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)
blockchainluffy
force-pushed
the
release/keyper/v1.3.9
branch
from
72e3f4d to
355ed8a
Compare
blockchainluffy
force-pushed
the
release/keyper/v1.3.9
branch
from
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.
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.
blockchainluffy
force-pushed
the
release/keyper/v1.3.9
branch
from
6baf70a to
70a1a02
Compare
blockchainluffy
force-pushed
the
release/keyper/v1.3.9
branch
from
55837bd to
6e5de32
Compare
blockchainluffy
force-pushed
the
release/keyper/v1.3.9
branch
from
7ee90c2 to
8105b62
Compare
3 participants
No description provided.