Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EquivocationReportSystem trait bounds can recursively use T::KeyOwnerProof in some cases #2641

Closed
2 tasks done
StackOverflowExcept1on opened this issue Dec 6, 2023 · 0 comments · Fixed by #2644
Closed
2 tasks done
Labels
I2-bug The node fails to follow expected behavior. I10-unconfirmed Issue might be valid, but it's not yet known.

Comments

@StackOverflowExcept1on
Copy link

StackOverflowExcept1on commented Dec 6, 2023

Is there an existing issue?

  • I have searched the existing issues

Experiencing problems? Have you tried our Stack Exchange first?

  • This is not a support question.

Description of bug

This PR was merged recently: rust-lang/rust#115801 and it breaks some things. You can't recursively get Trait::AssociatedType. Now this won't compile.

We encountered this problem when building with the -C instrument-coverage flag, but maybe there is some other way to cause it.

The problem is that in rococo runtime or some custom runtime you can do the following:

type KeyOwnerProof =
<Historical as KeyOwnerProofSystem<(KeyTypeId, pallet_babe::AuthorityId)>>::Proof;
type EquivocationReportSystem =
pallet_babe::EquivocationReportSystem<Self, Offences, Historical, ReportLongevity>;

So, it goes to EquivocationReportSystem:

impl<T, R, P, L>
	OffenceReportSystem<Option<T::AccountId>, (EquivocationProof<T::Header>, T::KeyOwnerProof)>
	for EquivocationReportSystem<T, R, P, L>
where
	T: Config + pallet_authorship::Config + frame_system::offchain::SendTransactionTypes<Call<T>>,
	R: ReportOffence<
		T::AccountId,
		P::IdentificationTuple,
		EquivocationOffence<P::IdentificationTuple>,
	>,
	P: KeyOwnerProofSystem<(KeyTypeId, AuthorityId), Proof = T::KeyOwnerProof>, //recursion occurs in associated types
	P::IdentificationTuple: Clone,
	L: Get<u64>,
{ ... }

The compiler cannot resolve recursion in associated types:

type KeyOwnerProof = <Historical as KeyOwnerProofSystem<(KeyTypeId, pallet_babe::AuthorityId)>>::Proof; 
P: KeyOwnerProofSystem<(KeyTypeId, AuthorityId), Proof = T::KeyOwnerProof>

We can see Proof = T::KeyOwnerProof in P: ..., but the KeyOwnerProof from Config trying to resolve associated type ::Proof. It causes recursion and we do <<T::Proof>::Proof>::Proof...

Steps to reproduce

You can try build rococo runtime with RUSTFLAGS="-C instrument-coverage". Also see: rust-lang/rust#117211

$ RUSTFLAGS="-Cinstrument-coverage" cargo +nightly-2023-10-14 build -p rococo-runtime --release

   Compiling rococo-runtime v1.0.0 (/home/.../work/polkadot-sdk/polkadot/runtime/rococo)
    Building [=======================> ] 639/640: rococo-runtime
error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

For more information about this error, try `rustc --explain E0275`.
error: could not compile `rococo-runtime` (lib) due to previous error
@StackOverflowExcept1on StackOverflowExcept1on added I10-unconfirmed Issue might be valid, but it's not yet known. I2-bug The node fails to follow expected behavior. labels Dec 6, 2023
bkchr added a commit that referenced this issue Dec 6, 2023
bkchr added a commit that referenced this issue Dec 7, 2023
atodorov added a commit to gluwa/creditcoin3 that referenced this issue Mar 8, 2024
cargo llvm-cov was hitting this issue:

error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

which is documented upstream in
rust-lang/rust#117211
paritytech/polkadot-sdk#2641

and fixed in polkadot-sdk v1.6.0 and later, see
paritytech/polkadot-sdk#2644
atodorov added a commit to gluwa/creditcoin3 that referenced this issue Mar 8, 2024
cargo llvm-cov was hitting this issue:

error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

which is documented upstream in
rust-lang/rust#117211
paritytech/polkadot-sdk#2641

and fixed in polkadot-sdk v1.6.0 and later, see
paritytech/polkadot-sdk#2644
atodorov added a commit to gluwa/creditcoin3 that referenced this issue Mar 22, 2024
cargo llvm-cov was hitting this issue:

error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

which is documented upstream in
rust-lang/rust#117211
paritytech/polkadot-sdk#2641

and fixed in polkadot-sdk v1.6.0 and later, see
paritytech/polkadot-sdk#2644
atodorov added a commit to gluwa/creditcoin3 that referenced this issue Mar 22, 2024
cargo llvm-cov was hitting this issue:

error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

which is documented upstream in
rust-lang/rust#117211
paritytech/polkadot-sdk#2641

and fixed in polkadot-sdk v1.6.0 and later, see
paritytech/polkadot-sdk#2644
atodorov added a commit to gluwa/creditcoin3 that referenced this issue Mar 22, 2024
cargo llvm-cov was hitting this issue:

error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

which is documented upstream in
rust-lang/rust#117211
paritytech/polkadot-sdk#2641

and fixed in polkadot-sdk v1.6.0 and later, see
paritytech/polkadot-sdk#2644
atodorov added a commit to gluwa/creditcoin3 that referenced this issue Mar 22, 2024
cargo llvm-cov was hitting this issue:

error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

which is documented upstream in
rust-lang/rust#117211
paritytech/polkadot-sdk#2641

and fixed in polkadot-sdk v1.6.0 and later, see
paritytech/polkadot-sdk#2644
atodorov added a commit to gluwa/creditcoin3 that referenced this issue Mar 25, 2024
cargo llvm-cov was hitting this issue:

error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

which is documented upstream in
rust-lang/rust#117211
paritytech/polkadot-sdk#2641

and fixed in polkadot-sdk v1.6.0 and later, see
paritytech/polkadot-sdk#2644
atodorov added a commit to gluwa/creditcoin3 that referenced this issue Mar 25, 2024
cargo llvm-cov was hitting this issue:

error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

which is documented upstream in
rust-lang/rust#117211
paritytech/polkadot-sdk#2641

and fixed in polkadot-sdk v1.6.0 and later, see
paritytech/polkadot-sdk#2644
atodorov added a commit to gluwa/creditcoin3 that referenced this issue Mar 26, 2024
cargo llvm-cov was hitting this issue:

error[E0275]: overflow evaluating the requirement `<Runtime as pallet_grandpa::Config>::KeyOwnerProof == _`

which is documented upstream in
rust-lang/rust#117211
paritytech/polkadot-sdk#2641

and fixed in polkadot-sdk v1.6.0 and later, see
paritytech/polkadot-sdk#2644
github-merge-queue bot pushed a commit that referenced this issue May 7, 2024
Added manual jobs for code coverage (triggered via `codecov-start` job):
 - **codecov-start** - initialize Codecov report for commit/pr
- **test-linux-stable-codecov** - perform `nextest run` and upload
coverage data parts
- **codecov-finish** - finalize uploading of data parts and generate
Codecov report

Coverage requires code to be built with `-C instrument-coverage` which
causes build errors (e .g. ```error[E0275]: overflow evaluating the
requirement `<mock::Test as pallet::Config>::KeyOwnerProof == _\` ```,
seems like related to
[2641](#2641)) and
unstable tests behavior
([example](https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6004731)).
This is where we'll nee the developers assistance

closing [[polkadot-sdk] Add code coverage
#902](paritytech/ci_cd#902)
paritytech-ci pushed a commit that referenced this issue May 8, 2024
Added manual jobs for code coverage (triggered via `codecov-start` job):
 - **codecov-start** - initialize Codecov report for commit/pr
- **test-linux-stable-codecov** - perform `nextest run` and upload
coverage data parts
- **codecov-finish** - finalize uploading of data parts and generate
Codecov report

Coverage requires code to be built with `-C instrument-coverage` which
causes build errors (e .g. ```error[E0275]: overflow evaluating the
requirement `<mock::Test as pallet::Config>::KeyOwnerProof == _\` ```,
seems like related to
[2641](#2641)) and
unstable tests behavior
([example](https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6004731)).
This is where we'll nee the developers assistance

closing [[polkadot-sdk] Add code coverage
#902](paritytech/ci_cd#902)
hitchhooker pushed a commit to ibp-network/polkadot-sdk that referenced this issue Jun 5, 2024
Added manual jobs for code coverage (triggered via `codecov-start` job):
 - **codecov-start** - initialize Codecov report for commit/pr
- **test-linux-stable-codecov** - perform `nextest run` and upload
coverage data parts
- **codecov-finish** - finalize uploading of data parts and generate
Codecov report

Coverage requires code to be built with `-C instrument-coverage` which
causes build errors (e .g. ```error[E0275]: overflow evaluating the
requirement `<mock::Test as pallet::Config>::KeyOwnerProof == _\` ```,
seems like related to
[2641](paritytech#2641)) and
unstable tests behavior
([example](https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6004731)).
This is where we'll nee the developers assistance

closing [[polkadot-sdk] Add code coverage
paritytech#902](https://github.com/paritytech/ci_cd/issues/902)
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this issue Aug 2, 2024
Added manual jobs for code coverage (triggered via `codecov-start` job):
 - **codecov-start** - initialize Codecov report for commit/pr
- **test-linux-stable-codecov** - perform `nextest run` and upload
coverage data parts
- **codecov-finish** - finalize uploading of data parts and generate
Codecov report

Coverage requires code to be built with `-C instrument-coverage` which
causes build errors (e .g. ```error[E0275]: overflow evaluating the
requirement `<mock::Test as pallet::Config>::KeyOwnerProof == _\` ```,
seems like related to
[2641](paritytech#2641)) and
unstable tests behavior
([example](https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6004731)).
This is where we'll nee the developers assistance

closing [[polkadot-sdk] Add code coverage
paritytech#902](https://github.com/paritytech/ci_cd/issues/902)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I2-bug The node fails to follow expected behavior. I10-unconfirmed Issue might be valid, but it's not yet known.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant