Reshare confirmation #965
Reshare confirmation #965
Conversation
| Ok(StatusCode::OK) | ||
| } | ||
|
|
||
| /// Confirms that a address has finished registering on chain. |
| signers_info.confirmations.push(validator_stash.clone()); | ||
| NextSigners::<T>::put(signers_info); | ||
| Self::deposit_event(Event::SignerConfirmed(validator_stash)); | ||
| Ok(Pays::No.into()) |
There was a problem hiding this comment.
This makes it a free transaction, right?
There was a problem hiding this comment.
free may not be the right term, refunded, as in they pay up front then if they don't hit any errors they get their fees back
| @@ -276,6 +276,7 @@ pub fn development_genesis_config( | |||
| }) | |||
| .collect::<Vec<_>>(), | |||
| proactive_refresh_data: (vec![], vec![]), | |||
| mock_signer_rotate: false, | |||
There was a problem hiding this comment.
Im guessing this will be removed when this is all finished and we always rotate signers
There was a problem hiding this comment.
oh no, this is for rotating on spin up, it is the only way I can get the tests in the TSS in the right state to actually test it (like the proactive refresh data above)
|
|
||
| if self.mock_signer_rotate { | ||
| NextSigners::<T>::put(NextSignerInfo { | ||
| next_signers: self.inital_signers.clone(), |
There was a problem hiding this comment.
is next_signers initially set to initial_signers just as a placeholder? Presumably it is going to get changed on the next new session.
There was a problem hiding this comment.
pretty much if next signers do not exist then the chain isn't in a rotating state and when the nodes call confirm done they will get rejected
| NextSigners::<T>::put(NextSignerInfo { | ||
| next_signers: current_signers, | ||
| confirmations: vec![], | ||
| }); |
There was a problem hiding this comment.
Not directly related to this PR, but is it possible to test this with our 3 node integration test / docker-compose setup? I guess we need 4 nodes before the check above which checks that the validator size is greater than the number of current signers will pass.
There was a problem hiding this comment.
yes we would need an extra node added
| @@ -77,6 +76,9 @@ async fn test_reshare() { | |||
| for response_result in response_results { | |||
| assert_eq!(response_result.unwrap().text().await.unwrap(), ""); | |||
| } | |||
| let key_share_after = unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), 3001).await; | |||
| assert_ne!(key_share_before, key_share_after); | |||
There was a problem hiding this comment.
I guess we could also check that bob and charlie's keyshares got updated. Not sure if thats needed.
There was a problem hiding this comment.
seems like overkill but happy to make an issue for it after....maybe a good first issue
There was a problem hiding this comment.
nvm it was a good idea, I just did it
good point will add it to #941 |
Related #941
This PR also stores the new key and deletes the old one until the new flow is finished #968