Update Substrate to polkadot-stable2412-1

[vedhavyas]

This PR uplifts our current fork of polkadot-sdk to stable 2412-1

Some notable changes:

New Extrinsic Format

Introduced a new Extrinsic format with following types

Bare
General
Signed

Signed

This is our regular signed extrinsics that require a hard coded signature, carried over from previous extrinsic version.

Bare

Bare transactions are currently supporting the so called Unsigned extrinsics. This will be deprecated soon and is meant for inherents only once unsigned transactions are deprecated. This PR essentially converted our Unsigned calls to Bare calls. We would soon require to migrate our Unsigned Extrinsics to General Extrinsic format. More on this follows down.

General

A new type of transaction that does not require Hard Coded signatures. This Format uses the newly defined TransactionExtension which was previously called SignedExtra. This is the extrinsic format we should use for the Unsigned extrinsics going forward and move away completely from the Bare format. This seems much better in terms of how we validate and prepare, also previously called pre_dispatch, unsigned extrinsics.

TransactionExtension

Previously called SignedExtra, this extension will take Origin, Call and other details to streamline both Signed and General Extrinsics. Since both validate and prepare will have some pre processing before call dispatch, there are two types of weights defined. CallWeight and ExtensionWeight

Functions validate and prepare are essentially validate and pre_dispatch but one notable change is that we do not need to validate the call in prepare as these two functions are called one after the other and also has the capability to pass custom data from validate -> prepare -> post_dispatch

Signed Extrinsics

There are no notable changes from our end for extensions specific to signed extrinsics since we are using some out of the box substrate extensions and these come with proper benchmarking. For the extensions we have defined on our end, I have left TODOs to do benchmarks.

Unsigned Extrinsics, aka, General Extrinsics

These extrinsics also go through the same extensions defined in the runtime and need to be benchamarked when we introduce those validations and pre_dispatches.

When we migrate our unsigned extrinsics to General extrinsics, most of the code from validate_unsigned and pre_dispatch_unsigned remains the same but as mentioned above, since these are interconnected, we can pass data from validate to prepare. This comes handy for XDM where validate can verify XDM and then pass result to prepare to further process the validated result, which is XDM case and others, is quiet nice and makes the flow much simpler.

Each pallet that defines these General Extrinsics must provide the TransactionExtension with necessary plugs for validate and prepare.

CreateInherent

With the new Extrinsic format, the way we send offchain extrinsic submission of Unsigned has changed and needs to make use of CreateInherent trait to construct extrinsic from the RuntimeCall. Unfortuntely, name CreateInherent is confusing and there is a discussion upstream regarding the change of name.

Nevertheless, this use of trait is temporary until we migrate to General Extrinsics for Unsigned.

Transaction Pool

Unfortunately, upstream is making it very hard to define custom transaction pools like we have for Consensus since most of the required types are not exposed. Fortunately, our Consensus Wrapper does not have any extra logic and we decided to remove that and fallback to substrate's TransactionPool Impl. There is one notable issue we initiated discussion on regarding exposing necessary Apis from txpool to get Transactions by tag. When we decided to work on this, if we need to re-introduce our Custom transaction Pool, we should ensure those required private types are exposed. Having said that, its always a bad idea to write a wrapper for TxPool and we should find a better way through Hooks in to TXPools to do such additional checks from our end.

OnChargeTransaction

This trait introduced a new function called can_withdraw_fee that will ensure the required fee can be taken from the user for extrinsic execution. Not a big change but nevertheless worth mentioning for Context.

Council Pallet

This pallet introduce 3 more config types

	type DisapproveOrigin = TwoThirdsCouncil;
    type KillOrigin = TwoThirdsCouncil;.
    type Consideration = ()

I have use 2/3rd council for disapproving a proposal and same for Killing a Proposal. There is also a Consideration which essential signals runtime how much deposit to hold/free from the Council member who initiates the proposal. Currently set to zero but something this think on later.

frame_system

Frame system introduced ExtensionsWeightInfo which is similar to the Call weight info but for extensions as I mentioned above. For Frame, we default to provided impl for ()

Pallet EVM

This pallet introduced

	type AccountProvider = pallet_evm::FrameSystemAccountProvider<Self>;
    type GasLimitStorageGrowthRatio = ();

For GasLimitStorageGrowthRatio I have left is as is as I do not fully know which ratio to go with. I have left a TODO to check moonbeam and maybe start from there to either come up with our own ratio or take one used by Moonbeam itself.

Runtime EVM

Adjusted RuntimeEVM to use the new TransactionExtension for necessary pre_dispatches.

Note: I did notice a lot of duplicate code across multiple runtime mostly like due to copy paste when creating a new runtime. We should work on reducing the duplication and there has been some work done but a lot more to go.

Overall, new Extrinsic format is quiet nice

• separating out the Extension Weights and Call weights which was combined earlier
• Better streamline validations using same extensions unlike previous unsigned and signed validations in seperate paths.

Next steps

The big changes are we migrating our Unsigned Extrinsics to General using TransactionExtension validations. Unfortunately, this would be quiet a big lift from our end and help on these from the team is much appreciated 😉

Once we validate, test these changes, I will pick up task, with help from others ofcourse, to migrate over to new General extrinsic format.

I do not believe there are any breaking changes from my understanding and will only know full extent once we migrate atleast one Unsigned call, mostly on Consensus side, preferably a small one.

For this PR specifically, please look at the full diff as its quiet approachable given you have read the PR description.

## Tests
Mostly seems okay but I know a few failing tests. My mac gives up running all of them at once so want to rely on CI to see which ones are failing and why. But we can review the changes already

Code contributor checklist:

• I have read, understood and followed contributing guide

[vedhavyas]

I had put Benchmarks related code behind runtime-benchmarks feature since

• frame-benchmarking-cli still brings rocksdb even with --no-default-features. Issue here causing rocksdb_disabled_in_substrate test in subspace node and malicious-operator failing
• This also ensures benchmaking code does not get compiled only to be useless when run without runtime-benchmarks feature

[vedhavyas]

General extrinsics are disabled right now since we do not have any validations for those. This will ensure mistakenly or maliciously send Unsigned extrinsics through General Extrinsic format skipping all the necessary validations.

We will remove this once we migrated over to General extrinsics with proper extensions in place

[nazar-pc]

frame-benchmarking-cli still brings rocksdb even with --no-default-features. Issue paritytech/polkadot-sdk#3793 causing rocksdb_disabled_in_substrate test in subspace node and malicious-operator failing

It didn't in the past though, what has changed? I'd say open a PR upstream and backport into our fork, must be a simple regression.

[vedhavyas]

It didn't in the past though, what has changed? I'd say open a PR upstream and backport into our fork, must be a simple regression.

I did not spend time on this unfortunately and since I have moved the benchmarks under specific feature, I was happy with the way it came out.

I can take a look later but I believe this will not be a blocker for this PR

[teor2345]

I'd like to remove unnecessary dependencies before merging. This might need patches to our substrate fork. If we can't remove them all in this PR, let's make sure we open tickets so they get removed eventually.

For the remaining new/duplicate dependencies, I'd like to understand why they're needed.

I opened ticket #3397 to make dependency updates easier in future PRs.

Note: I did notice a lot of duplicate code across multiple runtime mostly like due to copy paste when creating a new runtime. We should work on reducing the duplication and there has been some work done but a lot more to go.

One of the challenges is code that uses the same text, but the types are different Rust types. So we'd need generics/traits or macros for that.

[nazar-pc]

I'd say we should definitely get rid of rocksdb and its relatives, for others it might require closer look and can be done in a separate PR.

Also note that due to rust-lang/cargo#10801 lock file may (and does) contain many more dependencies than needed.

[teor2345]

I'd say we should definitely get rid of rocksdb and its relatives, for others it might require closer look and can be done in a separate PR.

Also note that due to rust-lang/cargo#10801 lock file may (and does) contain many more dependencies than needed.

Apparently, the way to avoid that issue is to use cargo tree, which skips dependencies that aren't actually used.
rust-lang/cargo#10801 (comment)

[nazar-pc]

Yeah, there are apparently two dependency solvers with slightly different properties and it is unfortunate that the issue was open for so long and no one seems to be actively working on it right now. From my experience it also results in unnecessary crates being compiled, which prevented me from adding 64-bit RISC-V builds to official releases due to dependency that doesn't even seem to be used.

[vedhavyas]

This impl assumes RocksDB, are these the weights we actually want in production?
docs.rs/frame-system/latest/src/frame_system/extensions/weights.rs.html#139-209
(This issue happens in almost every runtime change in this PR.)

These weights are no initially exposed so I went with default ones

Actually, I think we want to ignore the entire preamble here.

Dont agree regarding the skipping of preamble. Preamble is part of the extrinsics and we we need to include it to ensure the offset points to extrinsic and be able to decode fully. The PR updates the object mapping to include preamble.

Let's check and see if we need two different calculations for the different extrinsic formats?

Why? How does different extrinsic format affect here. There is only one decode for Unchecked that include all possible extrinsic format. Maybe I mis understood you here ?

Here is how it's implemented in the pallet-evm template:
polkadot-evm/frontier@5a3f891/template/runtime/src/lib.rs#L349
We might want to use some reasonable default here, because () means 0, which gives unlimited storage usage during contract deployment. An "out of gas" error might be better if someone mistakenly deploys a contract with large storage usage.

I have seen how its implemented in Frontier does not mean we want to copy it blindly. The frontier template is meant for Solochain but ours is different and most closely resebles the Moonbeam albeit not exactly. Hence I left a TODO, if you notice, to adjust this value accordingly.

I do not want to be haste in this config as a wrongly set one is bad. We already have GasLimitPovSizeRatio to ensure POV size which is mostly what we care when it comes to fraud proofs.

I'd say we should definitely get rid of rocksdb and its relatives,

I would want to give sometime if someone else is already picked it and working to avoid double work and maybe pull those changes, if no one picked, I'll pick it up next

One of the challenges is code that uses the same text, but the types are different Rust types. So we'd need generics/traits or macros for that.

Not in the scope of this PR. Feel free to pick up if you have a preferred approach @teor2345

As for the rest of the new dependencies, and pointed out by Nazar, we would need to investigate, potentially provide patches to upstream to make them optional and pull them in our fork. So definetely out of scope for this PR

[teor2345]

This looks good to me now.

Actually, I think we want to ignore the entire preamble here.

Dont agree regarding the skipping of preamble. Preamble is part of the extrinsics and we we need to include it to ensure the offset points to extrinsic and be able to decode fully. The PR updates the object mapping to include preamble.

Yes, that’s what I meant by “ignore”, because it’s not part of the object 🙂

Let's check and see if we need two different calculations for the different extrinsic formats?

Why? How does different extrinsic format affect here. There is only one decode for Unchecked that include all possible extrinsic format. Maybe I mis understood you here ?

Ah, I assumed the data structure had changed because the underlying format had changed. Thank you for clarifying.

One of the challenges is code that uses the same text, but the types are different Rust types. So we'd need generics/traits or macros for that.

Not in the scope of this PR. Feel free to pick up if you have a preferred approach @teor2345

I never said it was 🙂

I don’t believe there is a common approach here, some kinds of copied code can be de-duplicated as-is. Others will need generics, and the more complicated cases will be easier with either declarative macros or something like derive-deftly.

Before anyone picks up this work, I’d suggest we make a list of the largest/easiest/most critical duplicate code, and prioritise it. Which we can do in Notion or on a ticket.

As for the rest of the new dependencies, and pointed out by Nazar, we would need to investigate, potentially provide patches to upstream to make them optional and pull them in our fork. So definetely out of scope for this PR

See ticket #3399 for a checklist, which we can check off as we resolve each one.