Create workspace for multiple networks #54

jacobkaufmann · 2021-07-27T21:29:21Z

Create individual packages for the state and history networks.
Move the main() function from main.rs to lib.rs for the state network.
Remove main.rs for the state nework.
Add a basic main() function in the history network lib.rs.
Create a 'trin' package to host a single entry point for the program.

- Create individual packages for the state and history networks. - Move the main() function from main.rs to lib.rs for the state network. - Remove main.rs for the state nework. - Add a basic main() function in the history network lib.rs. - Create a 'trin' package to host a single entry point for the program.

carver · 2021-07-28T00:27:46Z

Hey, moving from side-channel to the PR:

I could not remember whether we wanted to maintain the ability to run the binaries individually or always spin them all up at once. The latter is what is implemented.

It's fine to launch them combined. We can add separate launches later, if we want.

Though it does become extra important for the version of chain-history on master to not crash on launch, so it doesn't halt development on state network. (that seems like a good invariant to target anyway)

I was also not sure how command line args are handled with this approach, once you CLI args for history

Yeah, similar to full clients, there will probably just be a ton of arguments that need to be named in a disambiguated way. The more we can just use good defaults, the better.

Feel fee to mention me when it's ready for final review.

SamWilsn · 2021-07-28T01:50:22Z

I was also not sure how command line args are handled with this approach, once you CLI args for history

If you use structopt (which I think we are now), you can define the argument structs in each library crate and merge them together in the binary crate, which might be a neat way to do it.

jacobkaufmann · 2021-07-28T15:04:24Z

trin/src/main.rs

+    tokio::select! {
+        history = trin_history::main() => {
+            history
+        },
+        state = trin_state::main() => {
+            state
+        },
+    }


Is there a cleaner way to do this? Do we want any logging here, or should we leave that to the individual network main() functions?

Yeah, fine to leave it bland for now. Seems okay to me.

At some point we can play with whether we want them in separate threads/runtimes, but for now a 👍🏻 from me.

jacobkaufmann · 2021-07-28T15:13:10Z

It looks like the win-build CI test has been failing ever since the merge of PR #49. Perhaps we want to create an issue to figure out why.

A number of additional items we could handle in this PR or as separate tasks:

Reorganize structopt to differentiate arguments for state vs history
- The history network has no CLI arguments as of now, but we could go ahead and create a structure in the binary crate to wrap the state network's arguments.
Move shareable jsonrpc.rs and portalnet code to a separate library crate to be shared among state and history.

KolbyML · 2021-07-28T22:25:06Z

It looks like the win-build CI test has been failing ever since the merge of PR #49. Perhaps we want to create an issue to figure out why.

@jacobkaufmann I think it is because I used a ton of credits when I was originally making the CI test for windows then after I realized that I started using the credits on my circleci account, so it should be fine whenever it resets.

carver · 2021-07-30T22:20:55Z

A number of additional items we could handle in this PR or as separate tasks:

Reorganize structopt to differentiate arguments for state vs history

The history network has no CLI arguments as of now, but we could go ahead and create a structure in the binary crate to wrap the state network's arguments.

Move shareable jsonrpc.rs and portalnet code to a separate library crate to be shared among state and history.

I'm all for adding issues to tackle these both after #54 is merged. I would only consider letting them delay merging this PR if it either was necessary to unblock work on chain history.

carver

Make sure the Cargo.toml change in #53 makes it in here, then should be good to go after handling some nitpicks.

carver · 2021-07-30T22:23:09Z

README.md

@@ -12,6 +12,16 @@ This should sound similar to a light client. It is, but with a peer-to-peer
 philosophy rather than the LES client/server model, which has introduced
 challenges in an altruistic environment.

+## Repository Structure
+
+Trin is a collection of networks.


Maybe:

Suggested change

Trin is a collection of networks.

The portal protocol is a collection of networks. Trin needs to connect to all of them.

carver · 2021-07-30T22:26:11Z

trin/Cargo.toml

+[package]
+name = "trin"
+version = "0.1.0"
+authors = ["Jacob Kaufmann <jacobkaufmann18@gmail.com>"]


Maybe add both of us for the joint one? At some point, this will probably turn into a joint member list, like trin@ethereum.org. But for now, we can just list individual maintainers of the individual crates.

carver · 2021-07-30T22:28:49Z

trin/src/main.rs

+    tokio::select! {
+        history = trin_history::main() => {
+            history
+        },
+        state = trin_state::main() => {
+            state
+        },
+    }


Yeah, fine to leave it bland for now. Seems okay to me.

At some point we can play with whether we want them in separate threads/runtimes, but for now a 👍🏻 from me.

jacobkaufmann added 2 commits July 27, 2021 15:19

Update for 'cargo fmt'

bb41c9d

Fix failing lint-build-test and update README.md

1ba6adc

jacobkaufmann commented Jul 28, 2021

View reviewed changes

jacobkaufmann marked this pull request as ready for review July 28, 2021 15:13

jacobkaufmann requested a review from carver July 28, 2021 17:11

carver approved these changes Jul 30, 2021

View reviewed changes

mrferris added 3 commits July 30, 2021 22:41

Fix merge conflicts in utils.rs and Cargo.toml

efcb614

Minor fixes to README and Cargo.toml

cb4a6ab

Fix Cargo.toml merge

ab3ef4a

mrferris merged commit 0b38528 into master Jul 31, 2021

lithp deleted the workspace branch October 22, 2021 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create workspace for multiple networks #54

Create workspace for multiple networks #54

jacobkaufmann commented Jul 27, 2021

carver commented Jul 28, 2021

SamWilsn commented Jul 28, 2021

jacobkaufmann Jul 28, 2021

carver Jul 30, 2021

jacobkaufmann commented Jul 28, 2021

KolbyML commented Jul 28, 2021

carver commented Jul 30, 2021

carver left a comment •

edited

Loading

carver Jul 30, 2021

carver Jul 30, 2021

carver Jul 30, 2021

	Trin is a collection of networks.
	The portal protocol is a collection of networks. Trin needs to connect to all of them.

Create workspace for multiple networks #54

Create workspace for multiple networks #54

Conversation

jacobkaufmann commented Jul 27, 2021

carver commented Jul 28, 2021

SamWilsn commented Jul 28, 2021

jacobkaufmann Jul 28, 2021

Choose a reason for hiding this comment

carver Jul 30, 2021

Choose a reason for hiding this comment

jacobkaufmann commented Jul 28, 2021

KolbyML commented Jul 28, 2021

carver commented Jul 30, 2021

carver left a comment • edited Loading

Choose a reason for hiding this comment

carver Jul 30, 2021

Choose a reason for hiding this comment

carver Jul 30, 2021

Choose a reason for hiding this comment

carver Jul 30, 2021

Choose a reason for hiding this comment

carver left a comment •

edited

Loading