Skip to content

Update the parachain template to stable2412 #26

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Apr 8, 2025
Merged

Update the parachain template to stable2412 #26

merged 13 commits into from
Apr 8, 2025

Conversation

paritytech-polkadotsdk-templatebot[bot]
Copy link
Contributor

@paritytech-polkadotsdk-templatebot paritytech-polkadotsdk-templatebot bot commented Jan 17, 2025 •
edited
Loading

The template has NOT been successfully built and needs to be inspected.

@github-actions GitHub Actions
Copy link

Hello, this is an automatic reminder that any code changes should be made to the source.

@paritytech-polkadotsdk-templatebot paritytech-polkadotsdk-templatebot bot changed the title [Don't merge] Update the parachain template to stable2412 Update the parachain template to stable2412 Feb 7, 2025
@iulianbarbu iulianbarbu mentioned this pull request Feb 7, 2025
Closed
@iulianbarbu iulianbarbu mentioned this pull request Mar 12, 2025
Merged
EgorPopelyaev pushed a commit to paritytech/polkadot-sdk that referenced this pull request Mar 26, 2025
@iulianbarbu @athei
@franciscoaguirre @github-actions
[stable2412] pallet-revive: remove related crates from umbrella (#7894)
6a5be3e 
# Description

When building `polkadot-sdk-parachain-template` workspace (`cargo build
--workspace --all-features`), based on `stable2412` dependencies
(paritytech/polkadot-sdk-parachain-template#26),
it fails with an error like below.
<details>
  <summary><b>error message</b></summary>
error[E0080]: evaluation of constant value failed
-->
/home/ubuntu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sc-network-0.47.0/src/protocol/message.rs:79:40
   |
79 |     #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)]
| ^^^^^^ the evaluated program panicked at 'Found variants that have
duplicate indexes. Both `Consensus` and `RemoteCallResponse` have the
index `6`. Use different indexes for each variant.',
/home/ubuntu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sc-network-0.47.0/src/protocol/message.rs:79:43
   |
= note: this error originates in the macro `$crate::panic::panic_2021`
which comes from the expansion of the macro `::core::panic` (in Nightly
builds, run with -Z macro-backtrace for more info)

error[E0080]: evaluation of constant value failed
-->
/home/ubuntu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sc-network-0.47.0/src/protocol/message.rs:79:48
   |
79 |     #[derive(Debug, PartialEq, Eq, Clone, Encode, Decode)]
| ^^^^^^ the evaluated program panicked at 'Found variants that have
duplicate indexes. Both `Consensus` and `RemoteCallResponse` have the
index `6`. Use different indexes for each variant.',
/home/ubuntu/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sc-network-0.47.0/src/protocol/message.rs:79:51
   |
= note: this error originates in the macro `$crate::panic::panic_2021`
which comes from the expansion of the macro `::core::panic` (in Nightly
builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0080`.
error: could not compile `sc-network` (lib) due to 2 previous errors
</details>

The root cause is `sc-network 0.47.0` (which builds fine when >=0.48.0),
a dependency of `pallet-revive-eth-rpc 0.2.0`, which is a dependency of
`polkadot-sdk 0.12.1`, used for various things in
`polkadot-sdk-parachain-template` repo. While investigating how to fix
the issue on `stable2412`, I discovered a few things:

1. `pallet-revive 0.3.1` can not build with `runtime-benchmarks` based
on latest `stable2412`, and it is also a dependency of
`pallet-revive-eth-rpc 0.2.0`, and pulled by `polkadot-sdk 0.12.1` for
its `runtime-benchmarks` feature.

2. Based on some chats started here:
#7844 (comment),
we identified that `pallet-revive/pallet-revive-eth-rpc` were set with
`publish = false` for a good reason and it shouldn't have changed here:
31b5923.
We also identified as a potential fix what this PR does, meaning
removing the `pallet-revive/pallet-revive-eth-rpc` dependency of
`polkadot-sdk`. I removed the other dependencies as well since I feel it
does not make sense to still keep them once `pallet-revive` is out.

3. Releasing correct crates for `pallet-revive*` and use them in
`polkadot-sdk` isn't yet necessary IMO, since some things still need to
be polished, and given their builds fail when building certain features,
they are not usable by users fully, so maybe not worth keeping partial
working functionality around just for the sake of having the crates in
`polkadot-sdk` - please challenge this if you think differently.
Ideally, we will make these pallets available for developers to use via
`polkadot-sdk` when all builds correctly with all features.

## Integration

Runtime/Node devs will not be able to reference `pallet-revive*` pallets
anymore via `polkadot-sdk`, so this is a breaking change, but if their
usage is close to zero by polkadot devs we can tune it down and consider
it a minor change for `polkadot-sdk`.

## Review Notes

1. If we think this requires a major bump for `polkadot-sdk`.. then we
can't look at the PR as a fix for `stable2412` and associated published
`polkadot-sdk` version. If we consider `pallet-revive` logic not being
necessarily used by polkadot devs so not very breaking from usage
perspective, then we can go with a minor bump and finally have a working
`polkadot-sdk` for `stable2412.

2. We also considered fixing `pallet-revive` & `pallet-revive-eth-rpc`
on `stable2412`, and publish correct versions on `crates.io`, and then
follow with a `polkadot-sdk` publish that depends on them, and in the
end `polkadot-sdk-parachain-template` workspace would build successfully
based on `stable2412` crates, but this is more work which could've been
done by now if it was necessary - e.g. in a scenario where actual
developers were using them (instead of setting the crates with `publish
= false`), so I think it makes sense to remove them for now from
`polkadot-sdk`.

3. ~IMO this PR should be backported to stable2503 too. Releasing
`polkadot-sdk` with `pallet-revive*` dependencies can be done with the
June release if things are more stable by then.~ We actually want to fix
`pallet-revive*` crates for stable2503, so must be kept in master and
2503 branch.

---------

Signed-off-by: Iulian Barbu <[email protected]>
Co-authored-by: Alexander Theißen <[email protected]>
Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: cmd[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@paritytech-polkadotsdk-templatebot paritytech-polkadotsdk-templatebot bot changed the title Update the parachain template to stable2412 [Don't merge] Update the parachain template to stable2412 Mar 31, 2025
@iulianbarbu iulianbarbu force-pushed the update-template/stable2412 branch from a3cb129 to 14bf9ca Compare April 1, 2025 08:28
@iulianbarbu iulianbarbu mentioned this pull request Apr 1, 2025
Merged
@iulianbarbu iulianbarbu changed the title [Don't merge] Update the parachain template to stable2412 Update the parachain template to stable2412 Apr 1, 2025
back to sync with stable2412-4
9242d73 
Signed-off-by: Iulian Barbu <[email protected]>
@iulianbarbu iulianbarbu force-pushed the update-template/stable2412 branch from 57c48dc to 9242d73 Compare April 4, 2025 19:30
iulianbarbu and others added 7 commits April 7, 2025 10:39
@iulianbarbu
adjust CI
cc4fbb6 
Signed-off-by: Iulian Barbu <[email protected]>
@iulianbarbu
debug failing test
5062a3f 
Signed-off-by: Iulian Barbu <[email protected]>
fix test workflow
3340547 
Signed-off-by: Iulian Barbu <[email protected]>
increase zn test timeout
03e246b 
Signed-off-by: Iulian Barbu <[email protected]>
redo old way of running zn test
5270438 
Signed-off-by: Iulian Barbu <[email protected]>
Revert "redo old way of running zn test"
ce4ca2c 
This reverts commit 5270438.
remove tmp dir recursively
8987ee7 
Signed-off-by: Iulian Barbu <[email protected]>
@iulianbarbu
Copy link
Collaborator

iulianbarbu commented Apr 8, 2025 •
edited
Loading

Ran the failing zn test on both macos and ubuntu. On macos it passes, while on ubuntu it fails. The difference is a log line on ubuntu:

Running validation of malicious PVF code has a higher risk of compromising this machine.
  - Optional: Cannot unshare user namespace and change root, which are Linux-specific kernel security features: not available: mount MS_PRIVATE: Permission denied (os error 13)

I was not expecting the test to fail on error given the log pattern used, which is Error. It seems though it fails. The error is not critical, and looks more like a warning/info message. I will wait for the new CI run, since this time I hope it will upload the logs as artifacts, and I can validate if this is what is shows up in the logs as well. If yes, we should be able to merge this.

Iulian Barbu and others added 4 commits April 8, 2025 12:49
avoid creating a new dir
35710c2 
Signed-off-by: Iulian Barbu <[email protected]>
@iulianbarbu
handle gracefully zn test failure
ee3cad6 
Signed-off-by: Iulian Barbu <[email protected]>
@iulianbarbu
fix dummy issue
c5579d8 
Signed-off-by: Iulian Barbu <[email protected]>
@iulianbarbu
fix ci
a3f8a55 
Signed-off-by: Iulian Barbu <[email protected]>
@iulianbarbu iulianbarbu merged commit ba81070 into master Apr 8, 2025
5 checks passed
@iulianbarbu iulianbarbu deleted the update-template/stable2412 branch April 8, 2025 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iulianbarbu iulianbarbu iulianbarbu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@iulianbarbu