CometMock icon indicating copy to clipboard operation
CometMock copied to clipboard

Allow new nodes to be added during runtime

Open p-offtermatt opened this issue 2 years ago • 10 comments
trafficstars

CometMock should provide a way to connect to new apps during runtime, e.g. to add validators.

Proposed Solution

We add a new CometMock-specific RPC endpoint in https://github.com/informalsystems/CometMock/blob/ph/v0.38.x-upgrade/cometmock/rpc_server/routes.go#L56: register-node(address, node_home) which takes as arguments the listen-address of the new node and its home directory.

The node info needs to be added to abci_client.GlobalClient:

  • A new AbciCounterpartyClient needs to be added to the Clients list
  • It needs to get a signing status

Also, the application needs to be synced to the current height. I think it is probably best to stop all block production, automatic or triggered by the user, until the application has been synced, since it gives the most control over when exactly the node joins. This likely can utilize the blockMutex https://github.com/informalsystems/CometMock/blob/ph/v0.38.x-upgrade/cometmock/abci_client/client.go#L30

To sync, we need to load the information from the storage and essentially replay the blocks seen so far. See the storage here: https://github.com/informalsystems/CometMock/blob/ph/v0.38.x-upgrade/cometmock/storage/storage.go#L61 This might require storing additional things in the storage, but I don't think so - it has the commits and blocks, which should suffice as far as I can tell.

The workflow to add a new node would then be the following:

  • Start up the node
  • Register the node with CometMock

Workaround

There is a workaround for the need to do this: Start all nodes during chain startup, even if they are not yet validators. During runtime, all and only validator nodes will sign, so it suffices to make some nodes into validators during runtime purely on the app level.

Currently, I am trying to understand whether there are any big drawbacks to this workaround. In the wild, people might want to connect new validators during runtime, so there might be a good reason to support this, but it is not clear how people want to test in practice.

p-offtermatt avatar Oct 09 '23 15:10 p-offtermatt

What is the need for the node_home? Can we be more specific around information needed from the node_home. This is more for processes not running on the same host machine.

Anmol1696 avatar Oct 09 '23 15:10 Anmol1696

What is the need for the node_home? Can we be more specific around information needed from the node_home. This is more for processes not running on the same host machine.

The node home is used to build the validator, see here https://github.com/informalsystems/CometMock/blob/ph/v0.38.x-upgrade/cometmock/main.go#L30 So the only files needed are here:

privValidatorKeyFile := nodeHome + "/config/priv_validator_key.json"
privValidatorStateFile := nodeHome + "/data/priv_validator_state.json"

I think this makes a lot of sense to refactor to e.g. take just these two files as inputs. I imagine this works best by specifying explicitly just the paths to these two files, so you could clone them over from the other machine. Another option would be to read the file content from the command line, but it would make the command quite long in practice.

p-offtermatt avatar Oct 09 '23 15:10 p-offtermatt

Ahh. And does cometmock write anything back to the validator node home? If it is just passed in memory, and not used again, maybe it might make sense to provide it directly.

Anmol1696 avatar Oct 10 '23 05:10 Anmol1696

It does not write anything there, no, so I think in principle it should be fine to pass it directly. I'd only be concerned that it becomes annoying to start CometMock, my experience with passing jsons on the command line is that it's annoying since you sometimes have to worry about escaping quotes in the right places. could also think about allowing providing them either as files or on the cli, but that may be unnecessary complexity

p-offtermatt avatar Oct 10 '23 06:10 p-offtermatt

Ahh makes sense. I was thinking more like passing the json files itself with a path. We could also have something like:

cometMock \
  --home path-to-home   # default to nothing, use only if specified, set as $HOME \
  --priv-key            # path to priv_validator_key.json; default to $HOME/config/priv_validator_key.json \
  --priv-val-state      # path to priv_validator_key.json; default to $HOME/data/priv_validator_state.json

With this both will work:

cometMock --home <val-home>

## As well as, create the file first with cat or echo and the pass it to cometMock
cometMock --priv-val-key <path to val key> --priv-val-state <path to val state>

With something like this we might be able to solve both our usecases, as well as this seems to be the way for configuring the current cosmos apps as well, which i actually like.

Happy to contribute this as well.

Anmol1696 avatar Oct 10 '23 08:10 Anmol1696

Yes, having the node home or the two files directly seems like a great option, much better than my thought of passing the file content in directly - it makes it so normal users don't need to worry about the content of the node home, and advanced users can easily use it for other use cases as well.

Would be very happy about a PR for this if you feel up to it. The v0.38 upgrade is almost done and I will open a PR in an hour or two. If you want, you could branch off of https://github.com/informalsystems/CometMock/tree/ph/v0.38.x-upgrade to start working on this,

p-offtermatt avatar Oct 10 '23 08:10 p-offtermatt

With something like this we might be able to solve both our usecases, as well as this seems to be the way for configuring the current cosmos apps as well, which i actually like.

@Anmol1696 this is fine we just want to add one new validator, but we may want to add multiple at different times.

mmulji-ic avatar Nov 22 '23 11:11 mmulji-ic

With something like this we might be able to solve both our usecases, as well as this seems to be the way for configuring the current cosmos apps as well, which i actually like.

@Anmol1696 this is fine we just want to add one new validator, but we may want to add multiple at different times.

Could you clarify what you think is the problem with what Anmol proposed? By calling it multiple times and calling advance_time, it seems possible to add multiple validators at different times, assuming the command can be executed while the main cometmock binary is running (e.g. by calling into an rpc endpoint cometmock offers)

p-offtermatt avatar Nov 22 '23 13:11 p-offtermatt

The following api ...

cometMock --priv-val-key <path to val key> --priv-val-state <path to val state>

looked like a one-shot execution, but if cometmock calls using the above, connect to a running instance that would work.

mmulji-ic avatar Nov 22 '23 13:11 mmulji-ic

👍 you are right! I think this assumes the command is a replacement for an RPC call. There is a separate issue to provide nice One-Shot commands that hit the various endpoints cometmock offers that is helpful for context https://github.com/informalsystems/CometMock/issues/53

p-offtermatt avatar Nov 22 '23 14:11 p-offtermatt