Skip to content
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.

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

Handle unique validation when parachain id is same #102

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

wirednkod
Copy link
Contributor

Checks if the parachain ID exists and if so error is sent;

Note: Should we increase the parachain ID in case it already exists instead of panicking?

@github-actions
Copy link

Coverage after merging nik-ensure-parachain-id-uniqueness into main

87.38%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
crates/configuration/src
   global_settings.rs96.48%100%85.71%97.99%47
   hrmp_channel.rs93.81%100%84.21%96.15%45
   network.rs99.11%100%95.79%99.50%135
   parachain.rs98.61%100%95.28%99%1034–1036, 168
   relaychain.rs98.65%100%93.75%99.24%90
   utils.rs100%100%100%100%
crates/configuration/src/shared
   errors.rs50%100%50%50%
   helpers.rs100%100%100%100%
   node.rs98.30%100%92.50%98.93%112, 118, 237
   resources.rs97.77%100%95.45%98.29%109, 97
   types.rs93.26%100%86.05%95.56%
crates/orchestrator/src
   lib.rs100%100%100%100%
crates/provider/src
   errors.rs25%100%33.33%20%6–8
   lib.rs0%100%0%0%60–72, 74–85, 88–90
   native.rs46.71%100%48.10%46.54%100–103, 105, 108–109, 111–114, 129, 137, 163–180, 183–191, 193–202, 204–209, 212–258, 260–264, 266, 268–276, 278–279, 281, 283, 290–295, 297–299, 30, 300–302, 304–308, 311–321, 324, 334–335, 337–341, 344–349, 363, 367, 372–378, 385, 402, 410–411, 427, 429–434, 437–468, 471, 473–479, 481–491, 498, 511, 530, 552, 555–560, 564–567, 570–573, 575–584, 586–590, 592, 596–601, 603–610, 612–625, 627–628, 630–632, 635–641, 643–644, 795, 90, 95–99
crates/provider/src/shared
   types.rs9.36%100%10%8.79%116, 123–128, 133, 141, 148, 178, 20, 30, 37, 52–58, 66, 72, 78
crates/support/src
   net.rs0%100%0%0%10, 4–9
crates/support/src/fs
   errors.rs0%100%0%0%3
   local_file.rs36.36%100%33.33%37.50%23–25, 27–29, 33–35, 7
   mock.rs44.83%100%45.83%44.63%109, 124, 136–145, 147–156, 158–172, 52–57, 61–66, 70–75, 79–84
crates/test-runner/src
   lib.rs100%100%100%100%

@pepoviola
Copy link
Collaborator

We should allow use the same id for multiple parachains. There is already a test in cumulus using this feature (https://github.com/paritytech/polkadot-sdk/blob/master/cumulus/zombienet/tests/0005-migrate_solo_to_para.toml). I think we can use the same logic that we have in the current version where we use another identifier for the parachain we just append -n if is already used. Thoughts?

@l0r1s
Copy link
Contributor

l0r1s commented Sep 21, 2023

We should allow use the same id for multiple parachains. There is already a test in cumulus using this feature (https://github.com/paritytech/polkadot-sdk/blob/master/cumulus/zombienet/tests/0005-migrate_solo_to_para.toml). I think we can use the same logic that we have in the current version where we use another identifier for the parachain we just append -n if is already used. Thoughts?

I'm good for adding the field but we should find a better name 🙄

@pepoviola
Copy link
Collaborator

We should allow use the same id for multiple parachains. There is already a test in cumulus using this feature (https://github.com/paritytech/polkadot-sdk/blob/master/cumulus/zombienet/tests/0005-migrate_solo_to_para.toml). I think we can use the same logic that we have in the current version where we use another identifier for the parachain we just append -n if is already used. Thoughts?

I'm good for adding the field but we should find a better name 🙄

Yes, maybe something like uniq_id 🫣

@github-actions
Copy link

Coverage after merging nik-ensure-parachain-id-uniqueness into main

96.80%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
crates/configuration/src
   global_settings.rs97.36%100%89.29%98.49%
   hrmp_channel.rs95.88%100%89.47%97.44%
   network.rs99.34%100%97.78%99.49%111, 116, 124, 80
   parachain.rs98.14%100%95.50%98.44%1094–1096, 51–53, 67–70
   relaychain.rs98.99%100%95.38%99.44%
   utils.rs100%100%100%100%
crates/configuration/src/shared
   errors.rs50%100%50%50%
   helpers.rs100%100%100%100%
   node.rs98.32%100%92.68%98.93%121, 127
   resources.rs96.09%100%94.52%96.43%102, 114, 126–128, 147–150
   types.rs90.29%100%84.16%92.11%279–281, 350–352, 375–376, 378–380, 60–62
crates/orchestrator/src
   lib.rs100%100%100%100%
crates/provider/src
   lib.rs25%100%0%50%
   native.rs91.82%100%84.42%92.78%1013, 1022, 1043, 1050, 1081, 1108, 1132, 1137, 1165, 1191, 1201, 1208, 1225, 127, 1275, 1332–1334, 1379, 1455–1456, 1480, 150, 1521, 238, 262, 264, 271, 273–274, 287, 309, 343–345, 347–349, 390, 40, 410, 459, 48, 508, 537–538, 543–545, 663, 755, 762, 767, 772, 777, 784, 797, 85, 868, 892, 897, 901, 930, 946, 976, 980
crates/provider/src/shared
   types.rs91.54%100%86.67%92.40%132–138, 15–18
crates/support/src
   fs.rs75%100%66.67%80%
   net.rs0%100%0%0%10, 4–9
crates/support/src/fs
   in_memory.rs95.55%100%96.48%95.37%149, 271, 321, 372, 424, 432, 440, 448, 470, 478, 593, 615, 643, 660, 678, 685, 704, 731, 748, 79–82, 84, 86–89, 91, 96
   local.rs98.25%100%96.30%98.75%51, 8
crates/test-runner/src
   lib.rs100%100%100%100%

@wirednkod wirednkod added the block label Oct 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants