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

ZombieNet: ensure that asynchronous backing logic is backwards compatible with v1 #6001

Closed
rphmeier opened this issue Sep 13, 2022 · 17 comments · Fixed by #6314
Closed

ZombieNet: ensure that asynchronous backing logic is backwards compatible with v1 #6001

rphmeier opened this issue Sep 13, 2022 · 17 comments · Fixed by #6314
Assignees
Labels
I5-tests Tests need fixing, improving or augmenting. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.

Comments

@rphmeier
Copy link
Contributor

The test is simple: start a 2-node testnet with 1 parachain, where one node runs master and the other runs the asynchronous backing feature branch. Set the group size to 2 in genesis and ensure that the parachain progresses at the expected rate.

@rphmeier rphmeier added I5-tests Tests need fixing, improving or augmenting. T5-parachains_protocol This PR/Issue is related to Parachains features and protocol changes. labels Sep 13, 2022
@eskimor
Copy link
Member

eskimor commented Sep 20, 2022

@sandreim Can someone on Engineering take that?

@sandreim sandreim added T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance. and removed T5-parachains_protocol This PR/Issue is related to Parachains features and protocol changes. labels Sep 20, 2022
@sandreim
Copy link
Contributor

Sure, is the branch in healthy state right now ?

@eskimor
Copy link
Member

eskimor commented Sep 20, 2022

No, but I am working on it.

@bredamatt
Copy link
Contributor

bredamatt commented Sep 21, 2022

@eskimor
Copy link
Member

eskimor commented Sep 21, 2022

No actually it is: rh-async-backing-feature
#5022

The branch referenced by you will be merged into rh-async-backing-feature once ready.

@eskimor
Copy link
Member

eskimor commented Sep 26, 2022

Correction, there seems to be an issue with prospective parachains.

@eskimor
Copy link
Member

eskimor commented Sep 28, 2022

Ok, branch should be ready - I believe the failing Zombienet tests are unrelated.

@bredamatt
Copy link
Contributor

bredamatt commented Oct 4, 2022

Not sure why this happens, but I did a checkout of the branch and attempted building - then I saw this issue:

error: `sp_trie::recorder::Recorder<H>::as_trie_recorder::{opaque#0}<'_>` does not live long enough
   --> /Users/mattia/.cargo/git/checkouts/substrate-7e08433d4c370a21/1f720c1/primitives/state-machine/src/trie_backend_essence.rs:181:44
    |
181 |         let recorder = recorder.as_mut().map(|r| r as _);
    |                                                  ^

error: `sp_trie::recorder::Recorder<H>::as_trie_recorder::{opaque#0}<'_>` does not live long enough
   --> /Users/mattia/.cargo/git/checkouts/substrate-7e08433d4c370a21/1f720c1/primitives/state-machine/src/trie_backend_essence.rs:219:44
    |
219 |         let recorder = recorder.as_mut().map(|r| r as _);
    |                                                  ^

   Compiling sp-weights v4.0.0 (https://github.com/paritytech/substrate?branch=master#1f720c11)
error: could not compile `sp-state-machine` due to 2 previous errors

I had a similar issue on a branch related to a different PR, so not sure if its solely my repo. What solved it was a merge of the latest commits on master to the branch. @eskimor do you have this issue when you try cargo check or cargo build at all?

@bkchr
Copy link
Member

bkchr commented Oct 4, 2022

@bredamatt this is fine. The latest rustc nightly has some regression. We fixed this for now in Substrate

@bredamatt
Copy link
Contributor

@bkchr indeed a toolchain thing, thanks for pointing me in that direction.

@bredamatt
Copy link
Contributor

Seems to be working just fine with this zombienet configuration:

[settings]
timeout = 1000

[relaychain]
chain = "rococo-local"

[relaychain.genesis.runtime.runtime_genesis_config.configuration.config]
  max_validators_per_core = 2

  [[relaychain.nodes]]
  name = "alice"
  command = "polkadot-async" // This is the binary from the async backing branch
  extra_args = [ "--alice" ]

  [[relaychain.nodes]]
  name = "bob"
  command = "polkadot" // this is the master branch binary
  extra_args = [ "--bob" ]

[[parachains]]
id = 100
cumulus_based = true

  [parachains.collator]
  name = "collator01"
  command = "parachain-template-node"
  args = ["-lparachain=debug"]

What is the best way to share the findings for this type of issue? Should we simply share the logs here, or use some other form of visibility, or potentially write a zombienet feature test and add it to the branch?

@sandreim
Copy link
Contributor

sandreim commented Oct 4, 2022

I think it makes sense to keep the CI test in the async branch for now, just to make sure we dont break things as we go fwd.

@rphmeier
Copy link
Contributor Author

rphmeier commented Oct 5, 2022

Yeah, @bredamatt please open a PR against the feature branch and file a follow-up issue to remove the test once synchronous backing is obsolete.

@bredamatt
Copy link
Contributor

@rphmeier @eskimor @sandreim waiting for the above before finishing up the CI test on the async branch.

@bredamatt
Copy link
Contributor

@eskimor seems there are some conflicts on the rh-async-backing-feature branch wrt. to the CI tests. @pepoviola, could you mention which files need to be checked out from the master branch into the rh-async-backing-feature branch? If they are not too scary, I could take them on myself so I can sync them into the branch I am adding the integration tests into.

@rphmeier
Copy link
Contributor Author

rphmeier commented Oct 6, 2022

We should just merge rh-async-backing-feature with master. I believe @slumber has plans to do this in the near future

@bredamatt bredamatt linked a pull request Nov 19, 2022 that will close this issue
@sandreim
Copy link
Contributor

done in #6314

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I5-tests Tests need fixing, improving or augmenting. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.
Projects
No open projects
Status: Done
5 participants