Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Chain behavior with less than minimum validator candidates #2494

Closed
kianenigma opened this issue May 7, 2019 · 9 comments
Closed

Chain behavior with less than minimum validator candidates #2494

kianenigma opened this issue May 7, 2019 · 9 comments
Labels
A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. I3-bug The node fails to follow expected behavior.

Comments

@kianenigma
Copy link
Contributor

kianenigma commented May 7, 2019

When this happens candidates.len() < <MinimumValidators<T>>,

here should be an approach to make sure:

  • The internal state of Staking + Session module is sensible. Example: exposures need to be probably removed as we don't really want to slash anyone anymore at this state. Or, we currently don't clean the session module's list of validator if this case happens.
  • The global behavior of the chain is properly changed (probably via some function calls or events). Quote:

} else {
// There were not enough candidates for even our minimal level of functionality.
// This is bad.
// We should probably disable all functionality except for block production
// and let the chain keep producing blocks until we can decide on a sufficiently
// substantial set.
// TODO: #2494
Self::slot_stake()
}

(not certain which milestone is this worth to be placed in)

@kianenigma kianenigma added the I3-bug The node fails to follow expected behavior. label May 7, 2019
@xlc
Copy link
Contributor

xlc commented May 10, 2019

Not sure if this is related.
I had an issue that with 4 minimum validators, 2 current validators (via misconfigured chain spec), I can't add new validators.
On election, it will have 2 + 1 = 3 new validators, which is less than minimum validators, and not updating validators because it is less than minimum and as a result there still only 2 validators.

@kianenigma
Copy link
Contributor Author

kianenigma commented May 10, 2019

On election, it will have 2 + 1 = 3 new validators, which is less than minimum validators, and not updating validators because it is less than minimum and as a result there still only 2 validators.

Based on this looks like it is related but it is the expected behavior. 3 candidates (assuming all have enough balance). 4 minimum. Nothing will happen. Correct?

Though I am pretty sure even upon the resolve of this issue your scenario will not change. The chain will probably not do anything if your candidates are less than the minimum.

@dvc94ch
Copy link
Contributor

dvc94ch commented Jun 27, 2019

Should we allow invalid chain specs? Why don't we do this instead?

diff --git a/srml/staking/src/lib.rs b/srml/staking/src/lib.rs
index c1b42a8b..4a4b0ce0 100644
--- a/srml/staking/src/lib.rs
+++ b/srml/staking/src/lib.rs
@@ -275,8 +275,8 @@ use runtime_io::with_storage;
 use rstd::{prelude::*, result, collections::btree_map::BTreeMap};
 use parity_codec::{HasCompact, Encode, Decode};
 use srml_support::{
-       StorageValue, StorageMap, EnumerableStorageMap, decl_module, decl_event,
-       decl_storage, ensure, traits::{
+       StorageValue, StorageMap, EnumerableStorageMap, assert_eq_uvec, decl_module,
+       decl_event, decl_storage, ensure, traits::{
                Currency, OnFreeBalanceZero, OnDilution, LockIdentifier, LockableCurrency,
                WithdrawReasons, OnUnbalanced, Imbalance, Get
        }
@@ -585,9 +585,11 @@ decl_storage! {
                                        };
                                }
 
-                               if let (_, Some(validators)) = <Module<T>>::select_validators() {
-                                       <session::Validators<T>>::put(&validators);
-                               }
+                               // Make sure the genesis state of the session module is consistent
+                               // with the genesis state of the staking module.
+                               let validators = <Module<T>>::select_validators().1
+                                       .expect("Genesis state of the staking module is valid.");
+                               assert_eq_uvec!(validators, <session::Validators<T>>::get());
                        });
                });
        }

@burdges
Copy link

burdges commented Feb 24, 2020

We could maybe degrade to some "staking and governance only" chain state. It'd make moving staking and governance off the relay chain "interesting". :(

@gui1117
Copy link
Contributor

gui1117 commented May 26, 2021

as discussed offline it seems a good solution would be to not create a new era until there is a successful election of more than the minimum number of validator.

@burdges
Copy link

burdges commented May 26, 2021

It makes election censorship more risky, but that's likely better than our alternatives.

@stale
Copy link

stale bot commented Jul 7, 2021

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jul 7, 2021
@gui1117
Copy link
Contributor

gui1117 commented Jul 8, 2021

The emergency phase is implemented, in the emergency phase the configured trusted origin (proportion of collective or root in kusama) can push the solution.
So I think we can close, or maybe we can reopen if we want to implement more functionality like:

We could maybe degrade to some "staking and governance only" chain state

@gui1117 gui1117 closed this as completed Jul 8, 2021
@kianenigma
Copy link
Contributor Author

Yeah, this can be closed now.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. I3-bug The node fails to follow expected behavior.
Projects
Status: Done
Development

No branches or pull requests

5 participants