ChainSpec extensions

[tomusdrw]

Resolves #3605

This PR introduces a way to customize ChainSpec (and chain spec files), with additional data that is used as configuration for other parts of Substrate node.

The parameters may (but don't have to) be "forkable", which means that the actual set of parameters depends on the current block number.

It introduces a bunch of changes to how ChainSpec json is being parsed to enforce that we validate the entire content and give meaningful error messages if something is missing. The extensions are also strictly typed, so there is no way to add misspelled or invalid parameter. For this to work I had to introduce a custom derive, which implements a mirror struct with all the fields being optional and additional custom derive which allows composition of strictly typed parameters be passed as configuration without adding too much generics in the code (see Extension::get::<T>()).

As an example, I've implemented the requested mechanism of rejecting blocks on specific height which do not match the hash provided in the spec.

I think we could now move parameters like TelemetryEndpoints, ProtocolId and Properties to be just extensions - can do that in this or follow up PR.

The whole scheme ended up being a bit more complicated than I hoped for so I'm also ok with rejecting this PR (or some parts of it) and going with more relaxed and loosly-typed solution (i.e. just having an arbitrary json::Map<String, json::Value> and letting the modules do the parsing.

[gui1117]

I'm not sure I see the whole need for this derive macros, if it is only for constraining inputs this could be done with a simple function declared as such:

constrain! {
    non_forkable {
        ext1: Extension1,
    },
    forkable {
        ext1: Extension1,
        ext2: Extension2,
    },
}

With constrain declare as a proc macro or macro_rules, and it just check that input has one and only one field for each non_forkable and same for forkable, plus possible forks with at most one field for each forkable.

Do I miss some feature here ?

[tomusdrw]

ChainSpecGroup implements quite a lot of stuff, namely for struct:

struct Example {
 a: u64,
 b: u64,
}

it implements

struct ExampleFork {
 a: Option<u64>,
 b: Option<u64>,
}

and a bunch of traits to tie this too together. This is needed since you might want to hard fork only one of the parameters and leave previous one at previous values (see a test here:

substrate/core/chain-spec/src/extension.rs

Line 304 in 3171fc5

"ext1": { "test": 5 }

)

The ChainSpecExtension is a bit different as it implements Extension trait, which has two methods:

• get which allows you to get a parameter of specific type (no mater how it's named in the struct)
• forks which allows to get Forks for specific type.
  The #[forks] attribute is needed to figure out which field holds the forks.

But back to your proposal:

1. I think there is still a need to have ChainSpecGroup macro, to generate fork structs for Extension1 and Extension2.
2. We still need to generate intermediate struct Extension { ext1: Extension1, ext2: Extension2 } to (de)serialize to JSON.
3. With current trait structure we still need to implement Extension trait for it.

So I find the derive macro a bit more readable than your proposal, unless we could get rid of some additional things (like some traits).

[gui1117]

oh Fork are used for nested structure as well. then ok it make sense.
Example could be a field of another structure here.

[Demi-Marie]

There should probably be a struct that encapsulates the parameters that keep being passed around during block verification.

[Demi-Marie]

This should probably be a struct, since it is being passed around so much.

[gavofyork]

some tests failing and there's a fat line in there?

[tomusdrw]

Updated.

[xlc]

Are there any reasons to use Properties over typed ChainSpec extensions?

[tomusdrw]

@xlc Properties are exposed over RPC (typed extensions currently are not). I imagine that Properties will still be used as hints for the UIs, while typed extensions will be used within the codebase.
Other than that, if you don't need strict typing and proper chain spec validation (i.e. doing some PoC), you might still be fine to just Properties.

[JoshOrndorff]

Is there a reason that the ChainSpecGroup derive macro does not support Enums? I would like to use it on an enum like this:

/// A chain spec extension for a chain that starts with no real consensus rules (manual seal) 
/// but later adds a PoW requirement, and then later a Poa requirement.
#[derive(Clone, Debug, Deserialize, Serialize, ChainSpecGroup)]
pub enum HotSwapConsensus {
	/// No real consensus rules enforced. This allows manual seal.
	None,
	/// Proof of Work seal required with block hash below given difficulty threshold.
	Pow{ difficulty: u64 },
	/// Proof of Authority seal required by one of the authorities specified.
	Poa{ authorities: Vec<Authority> },
}

The idea is that we can start a chain with manual seal, then hard fork to PoW, then hard fork more times to change the difficulty threshold, then finally fork to PoA.

But this fails to compile:

error: ChainSpecGroup is only available for structs with named fields.
  --> node/src/chain_spec.rs:18:48
   |
18 | #[derive(Clone, Debug, Deserialize, Serialize, ChainSpecGroup)]
   |                                                ^^^^^^^^^^^^^^
   |
   = note: this error originates in the derive macro `ChainSpecGroup`

[JoshOrndorff]

I'm also struggling because the ChainSpecGroup trait is not implemented for basic types like Vec<_> or even tuples. Am I misunderstanding the intended use of this? Or is it just not widely used and nobody noticed these limitations?

[bkchr]

AFAIK it was never used. Looking at the code I also don't really get how it should work.

/// A chain spec extension for a chain that starts with no real consensus rules (manual seal) 
/// but later adds a PoW requirement, and then later a Poa requirement.
#[derive(Clone, Debug, Deserialize, Serialize)]
pub enum HotSwapConsensus {
	/// No real consensus rules enforced. This allows manual seal.
	None,
	/// Proof of Work seal required with block hash below given difficulty threshold.
	Pow{ difficulty: u64 },
	/// Proof of Authority seal required by one of the authorities specified.
	Poa{ authorities: Vec<Authority> },
}

#[derive(Clone, Debug, Deserialize, Serialize, ChainSpecGroup)]
struct Consensus {
    consensus: HotSwapConsensus
}

Doing something like this should work. However, I would let the runtime signal the consensus change. Not sure why you want to decide this on the node side.

[JoshOrndorff]

I tried exactly what you suggested already, but it seems that when you derive ChainSpecGroup, every field must also implement ChainSpecGroup same as with Clone or Debug or many others.

Not sure why you want to decide this on the node side.

This is for educational purposes (specifically the academy). I want to demonstrate that the traditional/legacy way to upgrade a network is through hard forks and show how it is hard and messy to coordinate. I'm trying to motivate why forkless upgrades are good and what problem they solve and demonstrate that later. I agree using the runtime is ultimately better.

Anyway, thanks for taking a look.

[bkchr]

This is for educational purposes (specifically the academy). I want to demonstrate that the traditional/legacy way to upgrade a network is through hard forks and show how it is hard and messy to coordinate.

The node would still be required to be ready for the consensus upgrade. The only thing you would "win" by letting the runtime signal the upgrade is that you don't need to "coordinate a fork block number" before. Besides that you don't get that much.