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

Missing feature for parachain runtime? #2230

Closed
ntn-x2 opened this issue Feb 24, 2023 · 9 comments
Closed

Missing feature for parachain runtime? #2230

ntn-x2 opened this issue Feb 24, 2023 · 9 comments

Comments

@ntn-x2
Copy link

ntn-x2 commented Feb 24, 2023

After upgrading from 0.9.37 to 9.9.38, our parachain runtimes compiled only with the benchmarking flag have started failing.

cargo check -p peregrine-runtime --features runtime-benchmarks

⚡ Found 3 strongly connected components which includes at least one cycle each
cycle(001) ∈ α: DisputeCoordinator ~~{"DisputeDistributionMessage"}~~> DisputeDistribution ~~{"DisputeCoordinatorMessage"}~~>  *
cycle(002) ∈ β: CandidateBacking ~~{"CollatorProtocolMessage"}~~> CollatorProtocol ~~{"CandidateBackingMessage"}~~>  *
cycle(003) ∈ γ: NetworkBridgeRx ~~{"GossipSupportMessage"}~~> GossipSupport ~~{"NetworkBridgeRxMessage"}~~>  *
    Checking polkadot-runtime v0.9.38 (https://github.com/paritytech/polkadot?branch=release-v0.9.38#72309a2b)
error[E0046]: not all trait items implemented, missing: `ReachableDest`
   --> /home/antonio/.cargo/git/checkouts/polkadot-4038f27d5e4ea2e8/72309a2/runtime/polkadot/src/xcm_config.rs:366:1
    |
366 | impl pallet_xcm::Config for Runtime {
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `ReachableDest` in implementation
    |
    = help: implement the missing item: `type ReachableDest = Type;`

For more information about this error, try `rustc --explain E0046`.
error: could not compile `polkadot-runtime` due to previous error

The error is generated in the polkadot-runtime crate, which I am not sure how deep inside the dependency tree is, and it seems an issue with the runtime-benchmarks flag not being properly set by one of the crates that our runtime depends on.

On a related note, I was not even able to compile the statemine runtime on its own with just the runtime-benchmarks feature either, but with a different error regarding the new try_success_origin syntax, which gives me the hint that not all feature combinations are working for the cumulus repo.

EDIT: this is the PR to update from 0.9.37 to 0.9.38. If our parachain runtimes are compiled with cargo build -p peregrine-runtime --features runtime-benchmarks, then we have the compilation issue.

EDIT 2: compiling the whole kilt-parachain node with just the runtime-benchmarks feature works, so something in the client side is causing the feature from being correctly enabled by the dependencies.

ntn-x2 added a commit to KILTprotocol/kilt-node that referenced this issue Mar 1, 2023
fixes KILTprotocol/ticket#2464
Fixes #481 -> see issue
description for a complete list of PRs.

## Relevant changes

- paritytech/polkadot#4097 (XCM v3)
- paritytech/polkadot#6490 (root origin can
issue grants on behalf of the treasury)
- paritytech/substrate#13214 (we used in the the
staking pallet but we never called the `set` function, so we don't need
to do anything)
- paritytech/substrate#13216 (removes all calls
and intrinsics from the authorship pallet)

Release analysis:
https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-38/2122

## Open issues

If any of the runtimes is complied with just the `runtime-benchmarks`
features, there is a compilation issue due to some nested dependencies.
Linked issue: paritytech/cumulus#2230.

---------

Co-authored-by: Tino Rusch <tino@kilt.io>
@ntn-x2
Copy link
Author

ntn-x2 commented Mar 8, 2023

If I pull the latest version of the polkadot-v0.9.38 branch (commit 3275e27) and run cargo check --features runtime-benchmarks inside the parachain template runtime, I still get the following error:

error[E0046]: not all trait items implemented, missing: `try_successful_origin`
    --> /home/antonio/.cargo/git/checkouts/substrate-7e08433d4c370a21/18bb7c7/frame/collective/src/lib.rs:1109:1
     |
1109 | / impl<
1110 | |         O: Into<Result<RawOrigin<AccountId, I>, O>> + From<RawOrigin<AccountId, I>>,
1111 | |         I,
1112 | |         AccountId: Decode,
1113 | |     > EnsureOrigin<O> for EnsureMember<AccountId, I>
     | |____________________________________________________^ missing `try_successful_origin` in implementation
     |
     = help: implement the missing item: `fn try_successful_origin() -> Result<OuterOrigin, ()> { todo!() }`

error[E0046]: not all trait items implemented, missing: `try_successful_origin`
    --> /home/antonio/.cargo/git/checkouts/substrate-7e08433d4c370a21/18bb7c7/frame/collective/src/lib.rs:1133:1
     |
1133 | / impl<
1134 | |         O: Into<Result<RawOrigin<AccountId, I>, O>> + From<RawOrigin<AccountId, I>>,
1135 | |         AccountId,
1136 | |         I,
1137 | |         const N: u32,
1138 | |     > EnsureOrigin<O> for EnsureMembers<AccountId, I, N>
     | |________________________________________________________^ missing `try_successful_origin` in implementation
     |
     = help: implement the missing item: `fn try_successful_origin() -> Result<OuterOrigin, ()> { todo!() }`

error[E0046]: not all trait items implemented, missing: `try_successful_origin`
    --> /home/antonio/.cargo/git/checkouts/substrate-7e08433d4c370a21/18bb7c7/frame/collective/src/lib.rs:1157:1
     |
1157 | / impl<
1158 | |         O: Into<Result<RawOrigin<AccountId, I>, O>> + From<RawOrigin<AccountId, I>>,
1159 | |         AccountId,
1160 | |         I,
1161 | |         const N: u32,
1162 | |         const D: u32,
1163 | |     > EnsureOrigin<O> for EnsureProportionMoreThan<AccountId, I, N, D>
     | |______________________________________________________________________^ missing `try_successful_origin` in implementation
     |
     = help: implement the missing item: `fn try_successful_origin() -> Result<OuterOrigin, ()> { todo!() }`

error[E0046]: not all trait items implemented, missing: `try_successful_origin`
    --> /home/antonio/.cargo/git/checkouts/substrate-7e08433d4c370a21/18bb7c7/frame/collective/src/lib.rs:1182:1
     |
1182 | / impl<
1183 | |         O: Into<Result<RawOrigin<AccountId, I>, O>> + From<RawOrigin<AccountId, I>>,
1184 | |         AccountId,
1185 | |         I,
1186 | |         const N: u32,
1187 | |         const D: u32,
1188 | |     > EnsureOrigin<O> for EnsureProportionAtLeast<AccountId, I, N, D>
     | |_____________________________________________________________________^ missing `try_successful_origin` in implementation
     |
     = help: implement the missing item: `fn try_successful_origin() -> Result<OuterOrigin, ()> { todo!() }`

For more information about this error, try `rustc --explain E0046`.
error: could not compile `pallet-collective` due to 4 previous errors
warning: build failed, waiting for other jobs to finish...

@ggwpez
Copy link
Member

ggwpez commented Mar 8, 2023

Yea I looked into it locally but could not pinpoint where a feature is missing or erroneously being propagated…
But it does reproduce.

@bkontur
Copy link
Contributor

bkontur commented Mar 8, 2023

hmm, I have the similiar/same problem here: https://gitlab.parity.io/parity/mirrors/cumulus/-/jobs/2490428
I tried [this}(https://github.com//pull/2167/commits/24d02744811048fcd56e0aa453f9cf4c611f609c) but didnt help

@ntn-x2
Copy link
Author

ntn-x2 commented Mar 8, 2023

It is currently breaking our runtimes compilation whenever the feature is enabled. The node compilation is fine, probably because the default std somehow fixes the issue.

@bkchr
Copy link
Member

bkchr commented Mar 9, 2023

6d1ac27 cherry picked this pr to fix it. For me it works, can you test it @ntn-x2?

@ntn-x2
Copy link
Author

ntn-x2 commented Mar 10, 2023

@bkchr cool, the fix works when compiling the template runtime with the benchmark feature only. Could this be backported to at least the 0.9.38 branch? Or will it be only included in the 0.9.39?

@bkchr
Copy link
Member

bkchr commented Mar 10, 2023

I already backported it to the 0.9.38 branch. (this is the commit I linked above)

@bkchr
Copy link
Member

bkchr commented Mar 10, 2023

Can we close this @ntn-x2?

@ntn-x2
Copy link
Author

ntn-x2 commented Mar 10, 2023

Yes, issue seems fixed also for our runtime compilation. Thanks for the quick fix!

@ntn-x2 ntn-x2 closed this as completed Mar 10, 2023
ntn-x2 added a commit to KILTprotocol/kilt-node that referenced this issue Mar 15, 2023
Fixes compilation for the runtimes when the `runtime-benchmarks` feature
was enabled. For more context, see the [Cumulus GH
issue](paritytech/cumulus#2230).

Before, `cargo check -p peregrine-runtime --features runtime-benchmarks`
was broken. Not anymore.
Ad96el pushed a commit to KILTprotocol/kilt-node that referenced this issue Mar 20, 2023
fixes KILTprotocol/ticket#2464
Fixes #481 -> see issue
description for a complete list of PRs.

- paritytech/polkadot#4097 (XCM v3)
- paritytech/polkadot#6490 (root origin can
issue grants on behalf of the treasury)
- paritytech/substrate#13214 (we used in the the
staking pallet but we never called the `set` function, so we don't need
to do anything)
- paritytech/substrate#13216 (removes all calls
and intrinsics from the authorship pallet)

Release analysis:
https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-38/2122

If any of the runtimes is complied with just the `runtime-benchmarks`
features, there is a compilation issue due to some nested dependencies.
Linked issue: paritytech/cumulus#2230.

---------

Co-authored-by: Tino Rusch <tino@kilt.io>
Ad96el pushed a commit to KILTprotocol/kilt-node that referenced this issue Mar 20, 2023
Fixes compilation for the runtimes when the `runtime-benchmarks` feature
was enabled. For more context, see the [Cumulus GH
issue](paritytech/cumulus#2230).

Before, `cargo check -p peregrine-runtime --features runtime-benchmarks`
was broken. Not anymore.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants