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

Support collators in starlight #664

Merged
merged 12 commits into from
Sep 16, 2024
Merged

Conversation

tmpolaczyk
Copy link
Contributor

@tmpolaczyk tmpolaczyk commented Aug 26, 2024

Allow collators to connect to starlight using tanssi-node binary, and create blocks for container chains there.

Can be tested with pnpm moonwall run zombie_tanssi_relay.

List of hacks:

  • In zombienet, collators are spawned as nodes connected to container 2000. This makes it easier to spawn the nodes because zombienet automatically injects all the args. So in tanssi-node, the "orchestrator_client" is not an orchestrator client, but a parachain template client. This is nice because then it will fail if we try to use it for orchestrator stuff, but it would be better if there was no client at all. I removed the "start_network" function so I believe collators will never connect to parachain 2000, it would be nice to remove more stuff.

  • parathreads not supported because it's missing the buy_core function

  • all runtime apis called through the RelayChainInterface must be hardcoded, so we won't get a compile error if we put the wrong input or output type

@tmpolaczyk tmpolaczyk changed the title WIP Support collators in starlight Support collators in starlight Aug 29, 2024
@tmpolaczyk tmpolaczyk marked this pull request as ready for review August 29, 2024 10:08
Copy link
Contributor

github-actions bot commented Aug 29, 2024

Coverage Report

(master)

@@                         Coverage Diff                          @@
##           master   tomasz-wip-solochain-tanssi-node      +/-   ##
====================================================================
- Coverage   70.37%                             69.85%   -0.52%     
  Files         264                                264              
+ Lines       48037                              48404     +367     
====================================================================
+ Hits        33805                              33811       +6     
+ Misses      14232                              14593     +361     
Files Changed Coverage
/client/consensus/src/lib.rs 31.09% (+3.27%) 🔼
/client/consensus/src/mocks.rs 64.86% (+0.05%) 🔼
/client/service-container-chain/src/data_preservers.rs 80.48% (-1.97%) 🔽
/client/service-container-chain/src/service.rs 7.33% (-1.53%) 🔽
/client/service-container-chain/src/spawner.rs 44.40% (-0.26%) 🔽
/node/src/cli.rs 61.82% (-1.14%) 🔽
/node/src/command.rs 38.42% (-0.30%) 🔽
/node/src/service.rs 14.53% (-8.37%) 🔽

Coverage generated Mon Sep 16 10:27:59 UTC 2024

@tmpolaczyk tmpolaczyk added B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. not-breaking Does not need to be mentioned in breaking changes labels Sep 6, 2024
@girazoki
Copy link
Collaborator

girazoki commented Sep 6, 2024

I general this PR is as good as it can be for now. There is work to be done on the node-side, but that is planned for another PR.

@tmpolaczyk let's fix the small things, and let's see what the rest has to say

@tmpolaczyk tmpolaczyk force-pushed the tomasz-wip-solochain-tanssi-node branch from f2cb139 to 9b2d5cd Compare September 6, 2024 15:06
Copy link
Collaborator

@girazoki girazoki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's an approval for me, but I want more eyes on it @ParthDesai @nanocryk @Agusrodri

node/src/service.rs Outdated Show resolved Hide resolved
node/src/service.rs Outdated Show resolved Hide resolved
node/src/service.rs Outdated Show resolved Hide resolved
node/src/service.rs Outdated Show resolved Hide resolved
node/src/service.rs Outdated Show resolved Hide resolved
node/src/service.rs Outdated Show resolved Hide resolved
node/src/service.rs Outdated Show resolved Hide resolved
@tmpolaczyk tmpolaczyk merged commit e438161 into master Sep 16, 2024
37 of 38 checks passed
@tmpolaczyk tmpolaczyk deleted the tomasz-wip-solochain-tanssi-node branch September 16, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B5-clientnoteworthy Changes should be mentioned in any downstream projects' release notes D5-nicetohaveaudit⚠️ PR contains trivial changes to logic that should be properly reviewed. not-breaking Does not need to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants