pendulum icon indicating copy to clipboard operation
pendulum copied to clipboard

Use Foucoco runtime with instant seal

Open gianfra-t opened this issue 1 year ago • 1 comments

Closes tasks/#235.

Node update notes:

This does involve changes to the node of course, but it is not required for collators to update since it involves modifications for testing the chain locally.

Modifications

Implementation of a new flag instant-seal that allows to run a local node with the current foucoco runtime on instant seal mode consensus, like foucoco standalone.

By not specifying the new flag the node should behave the same as before the changes.

Also modifies the genesis config to add the necessary token allowances used currently in the standalone chain.

Most relevant code changes

  • New node implementation implementation with instant seal. Based on foucoco-standalone.
  • Flag to specify instant seal mode.
  • New node identity as well as changes for handling the new entry in the enum (example). The only difference with foucoco node is the use of the foucoco_standalone_config genesis config by default.
  • New genesis config, which matches the definition used in foucoco-standalone.
  • Conditional handling for starting the node depending on the new flag.
  • Add a new function that will start the common services for the original and the instant seal implementation. This is done to avoid as much as possible critical code duplication.

Running in standalone mode

To run: cargo run -p pendulum-node -- --chain {x}-spec-raw.json --instant-seal. Most other flags are irrelevant when instant-seal is used.

For instance, to run with the standalone genesis config from chain_spec.rs: cargo run -p pendulum-node -- --chain res/foucoco-standalone-spec-raw.json --instant-seal

or simply : cargo run -p pendulum-node -- --chain foucoco-standalone --instant-seal

to use whatever current config in chain_spec.rs is defined for foucoco standalone.

Tests

The new standalone chain was tested by executing the deployment script for nabla from wasm-deploy .

gianfra-t avatar May 10 '24 19:05 gianfra-t

Running in standalone mode

To run:

It would be nice to have this how-to at the bottom of the README.md. :ok_hand:

b-yap avatar May 16 '24 08:05 b-yap

There are a couple of changes in this PR that are pure formatting changes in unrelated code. How come, is the code on the main branch currently not correctly formatted or is did you reformat the code with different parameters? Makes it harder to review this PR.

TorstenStueber avatar May 20 '24 18:05 TorstenStueber

It seems that actually the current code on main is not correctly formatted. For that reason I added this issue.

Is it possible to revert the formatting changes in unrelated code to keep this PR focussed and then correct the formatting when working on the linked issue?

TorstenStueber avatar May 20 '24 18:05 TorstenStueber

It would be nice to have this how-to at the bottom of the README.md.

@b-yap Totally! I was waiting to see if in review something came up and we decided to change the commands or implementation, as soon as we define it I will add this to the README.md.

@TorstenStueber Regarding formatting, I realized after the fact that it was a bad idea for reviewing the changes, so I rolled back the format and will maybe do at the end before merging. Regarding the changes in node/service.rs, I took inspiration mostly from how we start the foucoco-standalone service.

The idea was to maintain the same configurations of services we have right now for the production chain, as well as the same standalone uses without duplicating all the code. I tried to unify as much as I could the common initialization between both here, but it became cumbersome due mostly to parachain_confighere which does not implement clone, and so it is hard to move it to a common function implementation (especially since spawn_tasks takes ownership). And yes this all will probably change substantially when we upgrade.

gianfra-t avatar May 21 '24 13:05 gianfra-t

Okay, I appreciate your effort to try to unify the code. Then let's use it as it is now – as long as it works that way 👍.

TorstenStueber avatar May 22 '24 00:05 TorstenStueber

The README looks good, thanks @gianfra-t 👍 seems like something went wrong in your merge commit, that's why the CI is complaining.

ebma avatar May 23 '24 15:05 ebma

Ohh yes I was hoping I caught all conflicts. My vs code was buggy and not showing them.

gianfra-t avatar May 23 '24 15:05 gianfra-t

@ebma I think the new build error has to do with the runner changes, any idea what can cause it ?

gianfra-t avatar May 23 '24 16:05 gianfra-t

Yes, @b-yap mentioned a solution here. It should work if you make sure that the ahash dependency used is at least v0.8.x I think.

ebma avatar May 23 '24 16:05 ebma

Thanks! cargo update did the trick. 0.8.3 was the problematic one.

gianfra-t avatar May 23 '24 17:05 gianfra-t