Update to not use `--parachain-id` flag #160

jimmychu0807 · 2021-12-13T05:13:38Z

Many update mixed in this PR...

Parachains now create chainSpec and rawChainSpec, and then the paraId are updated within their rawChainSpec.
Added -o output flag to specify the asset output folder, logs will be stored at <output>/logs.
Added -v verbose flag that if specify, will display the actual commands being run
Updated yarn to yarn3 - it is faster.

AWARE

With this PR, I am able to start a 3-node relay chain and a 2-node parachain network, and the parachain blocks got finalized. But I cannot get:

3-node relay chain and two 1-node parachain networks to connect and have the parachains blocks got finalized. Need your help @shawntabrizi @apopiak
Cannot get the simpleParachain (using polkadot adder-collator) to connect to the relaychain. It is always syncing to the main adder-collator network in production.
I go thru the code in test, they are really moonbeam specific (refer to the constants.ts). I wonder if we still want to keep them in our repo.

If the code looks good, I will update the README, and also the cumulus tutorial in docs.substrate.io (this is what I originally intend to address: polkadot-developers/substrate-docs#634 😄 )

close #156
close #151
close #148
close #146

# Conflicts: # tsconfig.json # yarn.lock

jimmychu0807 · 2021-12-14T04:22:16Z

@joelamouche since you are the main participate in contributing to this code base, want to hear from you if changes made in this PR is also compatible with use cases on your side.

@shawntabrizi I see there are conflicts and I can resolve them. But before putting in more effort into this PR, I would like to get an initial indication from you that you are okay with the approach & direction of the change. I will then resolve any remaining conflicts.

Also need your help on getting the case of launching one relay-chain and two 1-node parachains networks back to work.

joelamouche · 2021-12-14T08:45:39Z

@jimmychu0807 Thanks I will look at it.
I copied your repo and ran yarn run para-test and I got some errors. Did you make sur the tests pass?

HCastano · 2021-12-16T17:45:27Z

@jimmychu0807 this PR should really be split into two or three PRs - at the very least the yarn3 update should be split out

shawntabrizi · 2021-12-16T23:26:20Z

100% agree with Hernando.

Added -o output flag to specify the asset output folder, logs will be stored at /logs.

Seems fine. Default behavior without the flag should stay the same.

Added -v verbose flag that if specify, will display the actual commands being run.

Also seems fine.

Updated yarn to yarn3 - it is faster.

I am not a fan of this change if it requires users to install something which is not the normal yarn.

I don't really see that the speed of yarn affects anyone's quality of life on this project, however installing new packages just for this application will def be annoying.

# Conflicts: # src/runner.ts # src/spawn.ts # src/types.d.ts

jimmychu0807 · 2021-12-21T05:41:13Z

I have merged with last change, and updated so yarn para-test run.

@joelamouche: But I realized that it stucks at this line and then the before part of describeParachain timed out (I changed that from 5 mins to 3 mins, btw). I look deeper into it. Eventually it calls providePolkadotApi and just return api connection from that port. Are you guys expecting there is a node running and accepting connection at the port?

Eventually, are para-test suppose to be runnable(and pass) in non-moonbeam specific hosts/environments?

@HCastano @shawntabrizi I have removed yarn3. Will be great if you could take a look at the scenario that having:

3-node relay chain and two 1-node parachain networks

They connect, but the parachains blocks DO NOT finalized. And I couldn't get that to work.

Thank you all!

shawntabrizi · 2021-12-30T00:22:07Z

src/runner.ts


 	for (const parachain of paras) {
-		const { resolvedId, chain } = parachain;
-		const bin = resolve(config_dir, parachain.bin);
+		const { isSimple, id, resolvedId, protocolId, chain = "local" } = parachain;


Suggested change

const { isSimple, id, resolvedId, protocolId, chain = "local" } = parachain;

const { isSimple, id, resolvedId, protocolId, chain = "local" } = parachain;

Where are you getting chain = "local" from?

This is breaking the application from running.

Are you testing polkadot-launch after the changes you made?

If within the parachain section of the config file the chain value is not specified, then what should its default value be?

Also, I tried with one relay chain of 3-node and one parachain of 2-node with config file listed here and works.

But as mentioned above, I cannot make it to work with 3-node relay chain and two 1-node parachain networks.

What test config do you use when testing?

Right now I was trying with just the default config, generating a binary from cumulus and polkadot master.

Cumulus polkadot-collator node does not have a local chain. By default, I don't think it needs a chain input.

jimmychu0807 · 2022-01-01T04:43:55Z

@shawntabrizi updated to remove setting chain type to local when missing.

But using polkadot as relay-chain bin and polkadot-collator as parachain bin, this is the log produced:

The point is the parachain does have the PoV block candidate generated but is never added to the parachain blockchain.

Is there something wrong? If yes, how should I go about debugging it?

mrq1911 · 2022-01-06T03:42:50Z

@jimmychu0807 i have tried your fork, but it transforms large numbers in chainspec like

        "balances": [
          [
            "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY",
            1,
            1000000000000000000000
          ],

to exponential notation

        "balances": [
          [
            "5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY",
            1,
            1e+21
          ],

which then node refuses to parse when trying to use it

also I have noticed that relative path moved one level up when trying to execute it as dependency

you can replicate it by running this https://github.com/galacticcouncil/Basilisk-node/blob/polkadot-v0.9.13/scripts/wait-for-inclusion/package.json#L9

nuke-web3 · 2022-01-07T15:12:47Z

Sorry this is out of any context, just found this https://github.com/paritytech/cumulus/pull/858/files

Might be useful to this PR? I hope so 🙏

jimmychu0807 · 2022-01-09T12:33:30Z

@lumir-mrkva thanks for trying my fork. Let me test out what you do next week.

jimmychu0807 · 2022-01-13T10:53:22Z

@lumir-mrkva

I fixed the way binary filepaths are resolved, so they are resolve with respect to the config file, not where you run the parachain-launch script.
I don't see your issue of converting a balance 1000000000000000000000 to 1e+21. Setting the balance in config file will directly send a transaction at era 0 to the chain of setting alice of that balance amount, not writing it to the chainSpec value. If the balance value is updated to a scientific notation, this is how the build-spec subcommand work and I would rather look into how the particular build-spec logic is written.

@shawntabrizi
When you have time this week or next, could you take a look at this PR if it is good to merge? Thanks.

CertainLach · 2022-02-07T09:17:51Z

If the balance value is updated to a scientific notation, this is how the build-spec subcommand work and I would rather look into how the particular build-spec logic is written.

It gets converted here: https://github.com/paritytech/polkadot-launch/pull/160/files#diff-2e46e281a9ce50fb008324ac073f40d11857c82caa1a495145416e31b3cb14a1R236-R247

Due to standard JSON not supporting large integers:

JSON.stringify(JSON.parse('{"balance":1000000000000000000000}'))
// {\"balance\":1e+21}

And then serde_json parser not accepting scientific notation as integers (because it is not lossless)
The solution for this may be using something like https://www.npmjs.com/package/json-bigint

jimmychu0807 added 8 commits December 3, 2021 11:12

my dev

fa245dd

Merge remote-tracking branch 'gh-parity/master' into jc/dev

8098d62

# Conflicts: # tsconfig.json # yarn.lock

Parachains generating chainSpecs

3ebfec5

This compiles

1d25012

updated

7c29608

yarn lint:write'ed

9e9e7bc

Updated test

29919c3

Reverting config as how they look originally

fc572a6

jimmychu0807 requested review from shawntabrizi and apopiak December 13, 2021 05:14

yarn lint:write

7835773

jimmychu0807 mentioned this pull request Dec 13, 2021

Use para id in specs instead of flag #157

Merged

jimmychu0807 added 4 commits December 20, 2021 20:19

Revert back to yarn v1 based on feedback

9afc8ab

Merge branch 'master' into jc/dev

74b10b9

# Conflicts: # src/runner.ts # src/spawn.ts # src/types.d.ts

compiled and updated polkadot-js api libs

57538dc

Updated

a51d1b8

jimmychu0807 added 2 commits December 21, 2021 13:44

yarn lint:write

a37eb83

Merge remote-tracking branch 'gh-parity/master' into jc/dev

d33b6ec

shawntabrizi reviewed Dec 30, 2021

View reviewed changes

jimmychu0807 added 2 commits January 1, 2022 12:31

Updated based on peer review

ea4a29d

linted

bf7ea94

Fix bin path resolving from config file and remove custom types

1191ac2

jimmychu0807 added 2 commits January 13, 2022 19:10

Updated readme and linted

0375876

Minor edit

c545e4c

shawntabrizi closed this Jun 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to not use `--parachain-id` flag #160

Update to not use `--parachain-id` flag #160

jimmychu0807 commented Dec 13, 2021 •

edited

Loading

jimmychu0807 commented Dec 14, 2021

joelamouche commented Dec 14, 2021

HCastano commented Dec 16, 2021

shawntabrizi commented Dec 16, 2021 •

edited

Loading

jimmychu0807 commented Dec 21, 2021 •

edited

Loading

shawntabrizi Dec 30, 2021

jimmychu0807 Dec 30, 2021 •

edited

Loading

jimmychu0807 Dec 30, 2021

shawntabrizi Dec 30, 2021

jimmychu0807 commented Jan 1, 2022

mrq1911 commented Jan 6, 2022 •

edited

Loading

nuke-web3 commented Jan 7, 2022

jimmychu0807 commented Jan 9, 2022

jimmychu0807 commented Jan 13, 2022

CertainLach commented Feb 7, 2022

	const { isSimple, id, resolvedId, protocolId, chain = "local" } = parachain;
	const { isSimple, id, resolvedId, protocolId, chain = "local" } = parachain;

Update to not use --parachain-id flag #160

Update to not use --parachain-id flag #160

Conversation

jimmychu0807 commented Dec 13, 2021 • edited Loading

jimmychu0807 commented Dec 14, 2021

joelamouche commented Dec 14, 2021

HCastano commented Dec 16, 2021

shawntabrizi commented Dec 16, 2021 • edited Loading

jimmychu0807 commented Dec 21, 2021 • edited Loading

shawntabrizi Dec 30, 2021

Choose a reason for hiding this comment

jimmychu0807 Dec 30, 2021 • edited Loading

Choose a reason for hiding this comment

jimmychu0807 Dec 30, 2021

Choose a reason for hiding this comment

shawntabrizi Dec 30, 2021

Choose a reason for hiding this comment

jimmychu0807 commented Jan 1, 2022

mrq1911 commented Jan 6, 2022 • edited Loading

nuke-web3 commented Jan 7, 2022

jimmychu0807 commented Jan 9, 2022

jimmychu0807 commented Jan 13, 2022

CertainLach commented Feb 7, 2022

Update to not use `--parachain-id` flag #160

Update to not use `--parachain-id` flag #160

jimmychu0807 commented Dec 13, 2021 •

edited

Loading

shawntabrizi commented Dec 16, 2021 •

edited

Loading

jimmychu0807 commented Dec 21, 2021 •

edited

Loading

jimmychu0807 Dec 30, 2021 •

edited

Loading

mrq1911 commented Jan 6, 2022 •

edited

Loading