Skip to content
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

Feature/remove docker sentry nodes #410

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

konradkonrad
Copy link
Contributor

This changes the local docker test setup for snapshot shutter to run without sentry nodes / without validator isolation.

It changes the chain init parameter --role validator to use parameters for an exposed validator. The old functionality is preserved in --role isolated-validator.

This now is a better match for the keyper configuration in https://github.com/rolling-shutter/snapshot-keyper

Copy link
Contributor

@ulope ulope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

ulope and others added 3 commits November 14, 2023 17:40
Currently all validators connect to validator-0 instead of bootstrap (see `02-init-chain.sh:56`)
This changes the `validator` role to create a configuration for a
network exposed validator. The previous setting, where
`--role validator` set the parameters for an isolated validator behind a
`sentry` mode can now be configured by using
`--role isolated-validator`.

This is now more in line with the setup used in
https://github.com/shutter-network/snapshot-keyper which means less
configuration changes are needed during the setup.

It also allows for a simpler local docker compose test setup.
@ulope ulope force-pushed the feature/remove-docker-sentry-nodes branch from 9b59cba to 2e36e74 Compare November 14, 2023 16:54
Copy link
Collaborator

@ezdac ezdac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just putting some comments here for reference if needed.
I am currently fixing some of the bugs / syntax errors in a local branch

Str("stateFile", privValStateFile).
Msg("Generated private validator")
}
if slices.Contains([]string{VALIDATOR, ISOLATEDVALIDATOR}, config.Role) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SEED role also needs privkey files

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ehh, are you sure about that?

AFAICS the point of the seed role is to not be an actual part of the network but just to exchange peer information.

Copy link
Collaborator

@ezdac ezdac Dec 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No not sure, but IIRC the seed role node errored on me because it was looking for the file that was configured in the config.
Before this PR, the seed validator did get a privkey file.

So maybe it doesn't need it, but then this needs to be reflected in the tendermint config file as well.
Maybe the config entry for the privkey file has to be the empty string then.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And apart from that - wouldn't it make sense that also seed nodes are authenticated by address /signature?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They do need a node key (node_key_file in the config) for participating on the p2p network but what we're talking about here is the validator key.

I've just checked the boot nodes on the shapshot installation. They definitely don't have the priv_* key files.

${BB} sed -i "/^unconditional_peer_ids =/c\unconditional_peer_ids = \"${validator_id}\"" data/${sentry_cmd}/config/config.toml
${BB} sed -i "/^external_address =/c\external_address = \"${sentry_cmd}:${TM_P2P_PORT}\"" data/${sentry_cmd}/config/config.toml
# set seed node for chain bootstrap
${BB} sed -i "/^bootstrap_peers =/c\bootstrap_peers = \"${seed_node}\"" "${validator_config_path}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the 0.0.37-rc2 this is call seeds now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants