feat: state chain upgraded to substrate 2022-05 #1650

dandanlen · 2022-05-10T10:08:57Z

Old substrate tag: monthly-2021-09+1
New substrate tag: monthly-2022-05

This latter tag has been recreated as chainflip-2022-05 in our substrate fork, and the recent grandpa fix cherry-picked. Our grandpa fork has also been updated.

The main changes to our code:

TypeInfo added to all storage types. TypeInfo can be derived alongside Encode and Decode and is required for all types in storage. It allows auto-generated type metadata for front-end tools. See the scale-info docs for more details.
MaxEncodedLen has been added where it was easy to add / derive. See here and here for details on MaxEncodedLen. The idea is that every storage item should have statically-determined maximium size. This implies that everywhere we store a Vec or BTreeSet etc. we should convert this to the Bounded* equivalent. The reason for this, as far as I can tell, is to have deterministic worst-case performance bounds for benchmarking. It also allows derivation of the StorageInfo trait, although not sure benefits this brings. Judging by this issue, it seems related to parachains. I considered adding this for our entire runtime to be out of scope for this PR, so for now I have annotated most pallets as without_storage_info - however we may want to slowly move towards enforcing MaxEncodedLen everywhere. I'll raise an issue to track this.
Related to the above, Aura and Grandpa now expect a max_authorities parameter. I've set it to 150.
AccountId no longer implements default.
The benchmarks now contain a SUBSTRATE_REFERENCE_HARDWARE constant - we should update this with out own values.
construct_runtime no longer need to list all the items that it exports (Call, Event, Storage..).
Calls are now exposed as structs instead of tuples. Note this doesn't impact the encoding, but it makes them easier to read.
Events can now be defined as named structs instead of tuples.
Errors can now include values, like normal errors.
Some minor changes to benchmarking syntax.
No more subxt in the Cfe.
Bonus content: I added the grandpa rpcs to our node.

Also one last note about secp256k1: there was a conflict because all parity crates, and web3 have updated their dependency to 0.21 while our signing code still uses 0.20. I didn't want to dive too deep into this, so for now have settled on importing two versions of the crate to keep the changes here minimal.

For a full list of substrate changes, see https://github.com/paritytech/substrate/releases.

Were any changes to the genesis config of any pallets? Negative.
Is types.json up to date? Types.json is no more.
Have any new dependencies been added? Lots of dependencies have been updated.
Has the external interface been changed? No
Do the changes require a runtime upgrade? No - I think? Would suggest rolling this out as part of a runtime+cfe+node upgrade anyway.

Old substrate version: monthly-2021-09+1 New substrate version: monthly-2022-05 Main change: TypeInfo added to all storage types. See https://github.com/paritytech/substrate/releases for a full list of changes.

kylezs

Will let some other eyes go over this ofc.

state-chain/chains/src/eth.rs

state-chain/pallets/cf-staking/src/lib.rs

state-chain/runtime/src/chainflip.rs

engine/src/state_chain/client.rs

AlastairHolmes · 2022-05-10T17:47:41Z

engine/src/state_chain/client.rs

-            state_chain_runtime::Call::from(extrinsic.clone()).encode(),
-        ));
-        let expected_hash = BlakeTwo256::hash_of(&unsigned_tx);
+        let call = call.into();


In the signed version above, you changed it so in the errors the call variable is formatted as it is passed into the function. Instead of turning the call into a state_chain_runtime::Call (using the into first) and then formatting that. I like this change, but you also need to do the same thing here.

We can't do it the same way here because we need the state_chain_runtime::Call to calculate the expected_hash. I agree it's better to be consistent though. I'll re-shuffle a bit.

state-chain/chains/src/eth.rs

Add AURA digest and change block execution order.

instead of runtime call.

dandanlen · 2022-05-11T10:50:16Z

Note - runtime integration tests are currently failing - would like to merge recent changes from PR paritytech/substrate#1538 into develop and then merge develop back into this before fixing them.

Janislav · 2022-05-11T11:44:20Z

state-chain/chainflip-weight-template.hbs

-	}
-	{{~/each}}
-}
+		{{#if (eq pallet "frame_system")}}


If you format the file in this way it will produce rust files with no valid format.

Did you try it?

I merged this from core substrate - apparently there used to be a bug in the handlebars implementation that required pre/post-fixing ~ everywhere to handle newlines properly. This should no longer be required. Maybe we need to bump our version of handlebars?

Do you mean the extra tab of indentation? I'll take a look.

So the problem is the generated weights files will end up like the format in the hbs file. Back then when I created this I had to ensure that the file has a proper format that matches our fmt rules.

Looks like format-on-save messed up the formatting. I've reverted it and fixed the formatting, should be good now.

This includes TypeInfo derivations for new types in auction resolver and backup triage.

AlastairHolmes · 2022-05-11T15:00:46Z

engine/src/state_chain/client.rs

+            frame_system::CheckEra::from(Era::Immortal),
+            frame_system::CheckNonce::from(nonce),
+            frame_system::CheckWeight::new(),
+            state_chain_runtime::ChargeTransactionPayment::from(0),


What does this zero mean?

It's the tip - normally this is used to increase transaction priority, but we ignore it in our runtime since there's no fee market, and we don't want anyone manipulating the execution order.

(I'll add a comment)

AlastairHolmes · 2022-05-11T15:03:20Z

engine/src/state_chain/client.rs

-            state_chain_runtime::Call::from(extrinsic.clone()).encode(),
-        ));
-        let expected_hash = BlakeTwo256::hash_of(&unsigned_tx);
+        let runtime_call = call.clone().into();


You could inline this like you did above.

Not sure how, I need it in two places, once to calculate the expected_hash, then again to construct the extrinsic.

Ah wait I think I made a mistake in the encoding, i need the hash of the extrinsic, not of the call.

See 762810b

morelazers · 2022-05-12T07:20:58Z

Because this is such a significant PR I'm trying out the successful builds with testnet-tools.

There's some strange behaviour in the engine when the chain requests a threshold signature, the parties immediately respond with a failure report.

morelazers · 2022-05-12T16:32:56Z

Error reported in #1650 (comment) is actually due to another problem (#1658).

I'll double check again in the morning, but I think this one is good to merge otherwise. Awesome stuff.

dandanlen added 14 commits May 10, 2022 10:03

feat: state chain upgraded to substrate 2022-05

b8df7b8

Old substrate version: monthly-2021-09+1 New substrate version: monthly-2022-05 Main change: TypeInfo added to all storage types. See https://github.com/paritytech/substrate/releases for a full list of changes.

feat: merge monthly-2022-05 changes into Cfe

83b28a2

feat: remove subxt

bc8e541

chore: update benchmarks template

017cf37

fix: benchmarking and try-runtime compiles

34e333b

chore: updatre state chain mocks and tests

ee9a482

chore: update more tests

dd0d713

fix call

0bca1af

fix: dependency conflict in signing

2917f33

fix: validator tests session keys

9f3070f

chore: clippy

80fc06f

feat: grandpa rpcs

f179874

chore: remove types.json 💥

bb4185f

feat: new construct_runtime syntax for mocks

2d3d545

dandanlen requested review from Janislav, msgmaxim, morelazers, kylezs and AlastairHolmes May 10, 2022 10:08

fix: remove unused dependencies.

ed4b02d

kylezs linked an issue May 10, 2022 that may be closed by this pull request

[SC-2669] Merge in Substrate SCALEInfo patch #1236

Closed

kylezs approved these changes May 10, 2022

View reviewed changes

state-chain/chains/src/eth.rs Show resolved Hide resolved

state-chain/pallets/cf-staking/src/lib.rs Show resolved Hide resolved

state-chain/runtime/src/chainflip.rs Outdated Show resolved Hide resolved

AlastairHolmes reviewed May 10, 2022

View reviewed changes

engine/src/state_chain/client.rs Outdated Show resolved Hide resolved

dandanlen added 4 commits May 10, 2022 16:42

fix: no expected authorship slot at genesis

650dd1c

fix: spelling

8d3d8d5

fix: panic if no author is provided.

361dc2d

feat: add a test for SigData MaxEncodedLen impl.

207b9db

AlastairHolmes reviewed May 10, 2022

View reviewed changes

msgmaxim reviewed May 11, 2022

View reviewed changes

state-chain/chains/src/eth.rs Outdated Show resolved Hide resolved

fix: integration tests

727083a

Add AURA digest and change block execution order.

dandanlen added 3 commits May 11, 2022 10:49

fix: remove pointless assertion context.

e8a0b2f

fix: unsigned extrinsic submission logs pallet call

7608433

instead of runtime call.

fix: test all MaxEncodedLen impls

9850c3b

Janislav reviewed May 11, 2022

View reviewed changes

dandanlen added 4 commits May 11, 2022 15:33

fix: revert weight template

c8f6e66

fix: correctly formatted hbs template

7de785c

fix: update benchmark.sh

fcabb06

Merge branch 'develop' into feat/integrate-substrate-monthly-2022-05

47b9c2f

This includes TypeInfo derivations for new types in auction resolver and backup triage.

AlastairHolmes reviewed May 11, 2022

View reviewed changes

dandanlen added 3 commits May 11, 2022 17:29

fix: expected_hash encoding

762810b

fix: integration tests

e74f005

fix: missed authorship slot tests.

0566f6e

dandanlen added 3 commits May 12, 2022 09:39

feat: additional cfe logging on failed signing attempt.

7b78c96

feat: extra loggin on keygen failures.

e8692a9

chore: add comment re. tx fee tip

675ad51

dandanlen mentioned this pull request May 12, 2022

[SC-3438] Investigate if it's ok to panic on missing digests #1657

Closed

chore: add TODO for issue #1657

7a5f938

AlastairHolmes approved these changes May 12, 2022

View reviewed changes

kylezs mentioned this pull request May 12, 2022

Feat/eth fee witnessing #1638

Merged

7 tasks

dandanlen merged commit 3ad37c1 into develop May 13, 2022

dandanlen deleted the feat/integrate-substrate-monthly-2022-05 branch May 13, 2022 13:37

This was referenced May 20, 2022

Chore/ethabi mainline #1266

Closed

[SC-1946] Move back to ethabi master #1237

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: state chain upgraded to substrate 2022-05 #1650

feat: state chain upgraded to substrate 2022-05 #1650

dandanlen commented May 10, 2022 •

edited

Loading

kylezs left a comment

AlastairHolmes May 10, 2022

dandanlen May 11, 2022

dandanlen commented May 11, 2022

Janislav May 11, 2022

dandanlen May 11, 2022

dandanlen May 11, 2022 •

edited

Loading

Janislav May 11, 2022

dandanlen May 11, 2022

AlastairHolmes May 11, 2022

dandanlen May 11, 2022

dandanlen May 11, 2022

AlastairHolmes May 11, 2022

dandanlen May 11, 2022

dandanlen May 11, 2022

morelazers commented May 12, 2022

morelazers commented May 12, 2022

feat: state chain upgraded to substrate 2022-05 #1650

feat: state chain upgraded to substrate 2022-05 #1650

Conversation

dandanlen commented May 10, 2022 • edited Loading

kylezs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dandanlen commented May 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dandanlen May 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morelazers commented May 12, 2022

morelazers commented May 12, 2022

dandanlen commented May 10, 2022 •

edited

Loading

dandanlen May 11, 2022 •

edited

Loading