Select validators for jumpstart DKG #1053
Conversation
ameba23
changed the title
| for validator_name in validators_names { | ||
| let mnemonic = development_mnemonic(&Some(validator_name)); | ||
| let (tss_signer, _static_secret) = | ||
| get_signer_and_x25519_secret_from_mnemonic(&mnemonic.to_string()).unwrap(); | ||
| let jump_start_confirm_request = | ||
| entropy::tx().registry().confirm_jump_start(BoundedVec(EVE_VERIFYING_KEY.to_vec())); | ||
|
|
||
| submit_transaction(api, rpc, &tss_signer, &jump_start_confirm_request, None).await.unwrap(); | ||
| // Ignore the error as one confirmation will fail |
There was a problem hiding this comment.
Here we don't know which validators are going to be picked, but we want all those who are picked to send confirmations. Since nothing bad happens if the wrong one confirms, we just have them all send confirmations here.
pallets/registry/src/lib.rs
Outdated
| // Check that the confirmation is coming from one of the selected validators | ||
| let (_block_number, selected_validators) = | ||
| JumpstartDkg::<T>::iter().last().ok_or(Error::<T>::JumpStartNotInProgress)?; | ||
| let selected_validators: Vec<_> = |
There was a problem hiding this comment.
Are you sure this is consistently giving you the entry with the latest block? From the docs it says that iterating on a map doesn't guarantee any order
There was a problem hiding this comment.
From the docs of the iter() function from IterableStorageMap which is a trait that StorageMap implements:
'Enumerate all elements in the map in lexicographical order of the encoded key'
https://paritytech.github.io/polkadot-sdk/master/frame_support/storage/trait.IterableStorageMap.html#tymethod.iter
So the question is whether lexicographically sorting scale encoded block numbers is the same as sorting them numerically. I think block numbers become a U256, but not sure about the encoding of a U256. I could try to write a test to figure it out.
There was a problem hiding this comment.
@HCastano so in the end - good catch!
I wrote a test and iterating over block numbers in a StorageMap gives them in what appears to be arbitrary order despite them being sorted after being encoded, so no idea what encoding is being used. So i've changed this to use a comparison on the decoded key. I've left the test in, not sure if we really need it.
pallets/registry/src/benchmarking.rs
Outdated
| <JumpstartDkg<T>>::set(block_number, vec![ | ||
| ValidatorInfo { | ||
| x25519_public_key: [0; 32], | ||
| ip_address: vec![20], | ||
| tss_account: accounts[1].encode(), | ||
| } |
There was a problem hiding this comment.
Since the size of this list can vary depending on the number of validators, we should probably take this into account during the benches
There was a problem hiding this comment.
Ok - i have changed these to add MAX_SIGNERS validators. I am not totally sure the way i have done it makes sense, but the benches seem to run successfully.
There was a problem hiding this comment.
It makes sense in the context of the benches, but I think the benches are actually parameterized wrong.
@JesseAbram doesn't look like we're using c in these benches, only the MAX_SIGNERS. Maybe something to follow up on?
pallets/registry/src/benchmarking.rs
Outdated
| <JumpstartDkg<T>>::set(block_number, vec![ | ||
| ValidatorInfo { | ||
| x25519_public_key: [0; 32], | ||
| ip_address: vec![20], | ||
| tss_account: accounts[1].encode(), | ||
| } |
There was a problem hiding this comment.
It makes sense in the context of the benches, but I think the benches are actually parameterized wrong.
@JesseAbram doesn't look like we're using c in these benches, only the MAX_SIGNERS. Maybe something to follow up on?
| @@ -117,12 +126,22 @@ benchmarks! { | |||
| let threshold_account: T::AccountId = whitelisted_caller(); | |||
| let expected_verifying_key = BoundedVec::default(); | |||
|
|
|||
| // add validators and a registering user | |||
| // add validators | |||
| for i in 0..MAX_SIGNERS { | |||
| let validators = add_non_syncing_validators::<T>(MAX_SIGNERS as u32, 0); | |||
There was a problem hiding this comment.
Not relevant to this PR, but while we are on the topic of whether stuff in this benchmark makes sense:
I am wondering why this line is inside the for loop and not before it.
Why do we need to call add_non_syncing_validators 15 times?
…/entropy-core into peg/select-initial-validators * 'peg/select-initial-validators' of github.com:entropyxyz/entropy-core: Update pallets/registry/src/benchmarking.rs
Closes #1052
This selects n validators for initial jumpstart DKG when handling the jumpstart extrinsic, using on-chain randomness.
I've also removed the signature request account field from some types relating to DKG, since DKG is now only used for network jumpstart.