Skip to content
This repository was archived by the owner on Jul 4, 2022. It is now read-only.

Conversation

@jordy25519
Copy link
Contributor

@jordy25519 jordy25519 commented Jan 20, 2021

Adds an RPC to trigger a "restart" of the grandpa voter
This allows the node to use new grandpa session/authority keys without a full node restart or waiting for authority set change event.

Steps to reproduce:

  1. Start a local node --chain=dev --validator --rpc-method=Unsafe
  2. Insert session keys for babe and gran
  3. trigger the worker restart: curl -H 'Content-Type: application/json' -d '{ "jsonrpc": "2.0", "method":"grandpa_restartVoter", "params":[], "id": 1 }' http://localhost:9933

Screen Shot 2021-01-21 at 11 53 12

cc: paritytech/substrate#7268

@jordy25519 jordy25519 force-pushed the spike/sp-grandpa-2768 branch from da82c69 to 3d83e5a Compare January 20, 2021 22:56
@jordy25519 jordy25519 requested a review from aliXsed January 21, 2021 01:25
@aliXsed
Copy link

aliXsed commented Jan 21, 2021

Do we know what is the value of voter_set_state when a grandpa authority change (the condition where we need to send the restart command) has happened?

@jordy25519
Copy link
Contributor Author

Do we know what is the value of voter_set_state when a grandpa authority change (the condition where we need to send the restart command) has happened?

My understanding is, every node always independently tracks the grandpa round state / votes etc. so we don't need any special handling.
The problem this PR fixes is that the grandpa voter worker is not aware it should be voting when it is newly given a session key that should be voting.

I think this is due to the value of the node's session key being cached in-memory somehow, the value is not properly refreshed until rebuild_voter happens which only happens on AuthoritySet change event or first start.
This PR allows the the refresh to happen on demand, the grandpa voter state is not altered in anyway by this PR, simply rebuilt with current available state.

@aliXsed
Copy link

aliXsed commented Jan 21, 2021

LGTM! though it's hard to track a potential side effect of rebuilding the grandpa state. But given that you have tested it, it should be safe. Also we know that this rpc is an unsafe one and wouldn't be exposed publicly.

@jordy25519 jordy25519 merged commit c8239c7 into develop Jan 21, 2021
@jordy25519 jordy25519 deleted the spike/sp-grandpa-2768 branch January 21, 2021 23:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants