Move node crate to its own folder

[lemunozm]

Description

Fixes #1278

• Updated connectors rust edition to 2021 @NunoAlexandre

[lemunozm]

The compilation issue is because of this: mcginty/snow#146
I've tried to solve it without success. Not sure if any of you have found this before @NunoAlexandre @wischli

Note that the Cargo.lock is not updated in this PR, so all dependencies should work as they are.

[NunoAlexandre]

The compilation issue is because of this: mcginty/snow#146 I've tried to solve it without success. Not sure if any of you have found this before @NunoAlexandre @wischli

Note that the Cargo.lock is not updated in this PR, so all dependencies should work as they are.

@lemunozm I would try two things:

1. Run the nix command to make sure we don't have any messed up duplicated dependencies.
   I read the note about cargo.lock not changing but still I would double check this

2. Try cargo update

[lemunozm]

Thanks @NunoAlexandre, cargo update worked! But I'm still wondering why simply moving files adds a dependency error even though the Cargo.lock is not modified. Pretty weird!

[NunoAlexandre]

Thanks @NunoAlexandre, cargo update worked! But I'm still wondering why simply moving files adds a dependency error even though the Cargo.lock is not modified. Pretty weird!

Nice. yeah that is weird indeed. Sometimes cargo deals with dependencies differently when they are set in the root cargo.toml file, that could have something to do with it

[NunoAlexandre]

Thanks for doing this :) it was in the back of my mind for over a year.

[mustermeiszer]

Looks mostly good.

Two things:

• Pipeline is failing. Space in runner I guess and somehow we are missing an impl, I guess this is a feature flag issue, now that the node was moved to a non top level crate
• Why the cargo update?

[lemunozm]

I'm pretty sure the current clippy error is a clippy bug 😅

[lemunozm]

I'll recheck the features! Thanks for the suggestion

Regarding why the cargo update... this is the big question 😆

[NunoAlexandre]

I'll recheck the features! Thanks for the suggestion

You can try and run this subalfred at the root of the repo (credit to @wischli)

#!/usr/bin/env bash

# Find all child directories containing Cargo.toml files
dirs=$(find . -name Cargo.toml -print0 | xargs -0 -n1 dirname | sort -u)

# Execute the command "subalfred check" on each directory
for dir in $dirs; do
    echo $dir;
    subalfred check features $dir
done

[lemunozm]

subalfred says that everything is fine 😓

[NunoAlexandre]

subalfred says that everything is fine 😓

I can take a look later today / tomorrow morning, I have experienced s.o. m.a.n.y. cargo issues updating our chain to newer versions of polkadot in the last 1.5 years that I don't feel the pain anymore 😆

[lemunozm]

Hahaha, I appreciate it a lot @NunoAlexandre. Hope your experienced eye finds something

[wischli]

Couldn't find anything obvious as well comparing it with another parachain. So I agree it might be a feature flag issue. Please note that subalfred only checks std, runtime-benchmarks and try-runtime.

[lemunozm]

Thanks @wischli for taking the time to look into it. I didn't know about subalfred only check those three features.

[wischli]

Thanks @wischli for taking the time to look into it. I didn't know about subalfred only check those three features.

I wasn't fully sure if this is correct as I remembered it from looking through the Subalfred repo but the documentation states the same unfortunately: https://subalfred.hack.ink/user/cli/check.html#episode-2

[NunoAlexandre]

I just spent 2 hours hitting my head against a wall with this one 🙃

I actually faced the same issue when updating the chain to Polkadot 0.9.37 and I fixed it then by forking PureStake/frontier and adding the missing cargo features to pallet-ethereum (the pallet at fault at the time): moonbeam-foundation/frontier#164

I tried doing the same now, fetching the latest 0.9.38 revision of PureStake/frontier and adding all the missing cargo features (see https://github.com/NunoAlexandre/frontier/commits/moonbeam-polkadot-v0.9.38) and update a derived version of this branch (node-to-folder-fix) to have it point to this fork of mine but that didn't do it this time 🤔

I also tried to add patch rules to make sure any reference to frontier would be using my fork but that didn't cut it either.

Je ne sais pas 😢

[lemunozm]

Thanks a lot @NunoAlexandre for your research on this!! 🙌🏻 I'll give it another try with more time.

[lemunozm]

I still don't know what the bug is, but at least I know why it's falling now and not before.

Any command with the form cargo check in the top of the project folder was running the node crate before, it was translated to cargo check -p centrifuge-chain. Now if we do cargo check on the top folder, we are doing what previously we did as cargo check --workspace. So, the jobs are working in a different way, and we are finding a hidden issue we already had.

By now I think we can get this working by modifying the jobs to run the node instead of all workspace.

[lemunozm]

A little digging:

• cargo check ✅
• cargo check -r ✅
• cargo check -F runtime-benchmarks ❌
• cargo check -r -F runtime-benchmarks ❌
• cargo check -p centrifuge-chain ✅
• cargo check -r -p centrifuge-chain ✅
• cargo check -p centrifuge-chain -F runtime-benchmarks ✅
• cargo check -r -p centrifuge-chain -F runtime-benchmarks ✅

The issue comes from compiling the workspace (not the node) with runtime-benchmarks enabled. Release mode does not take effect.

[lemunozm]

And surprisingly, the issue is fixed if we add try-runtime along with runtime-benchmarks:

• cargo check ✅ (default features)
• cargo check -F runtime-benchmarks ❌ (no default features, only the specified)
• cargo check -F runtime-benchmarks,try-runtime ✅

[lemunozm]

I think all issues are solved. CI is now failing because we have reached the max size limit

[lemunozm]

No idea here why jobs are failing because of storage limits. Changes I did should not add extra storage, or at least I'm not able to see why

[mustermeiszer]

Maybe @gpmayorga can enlighten us CI noops here ^^

[lemunozm]

I do not now what have been changed in our CI config but it seems like this finally passes the CI jobs 😭

[NunoAlexandre]

I do not now what have been changed in our CI config but it seems like this finally passes the CI jobs 😭

I wonder if some external dependency fixed some missing feature which fixed it for us 🤔

[lemunozm]

The last CI errors were about space/memory. All "code" issues were already fixed before.

Maybe the doc CI job that was fixed generated a lot of artifacts in the docker, and made other CI jobs fail because of the available space. 🤔

[lemunozm]

I'm going to fix the conflict and maybe we can this finally merged 🎉

[lemunozm]

Closed in favor of: #1632