-
Notifications
You must be signed in to change notification settings - Fork 64
feat: add shadow network simulator support #869
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: turuslan <[email protected]>
| ethereum_hashing = { git = "https://github.com/ReamLabs/ethereum_hashing.git" } | ||
|
|
||
| # https://github.com/shadow/shadow | ||
| libp2p = { git = "https://github.com/qdrvm/rust-libp2p.git", branch = "feature/shadow-0.55.0" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can figure out a better way, not patching the full libp2p with forked one. Also it seems we only need to patch quinn-udp but it's a transitive dep so I should check.
@KolbyML Is it possible to patch only when a specific rust flag is enabled? For example in this case, --feature shadow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not a fan of adding unmaintained dependencies to our Cargo.toml
Regardless if we were to do something like this we would instead
diff --git a/.cargo/config-shadow.toml b/.cargo/config-shadow.toml
new file mode 100644
index 0000000..cb8b6ca
--- /dev/null
+++ b/.cargo/config-shadow.toml
@@ -0,0 +1,7 @@
+[patch.crates-io]
+# https://github.com/shadow/shadow
+libp2p = { git = "https://github.com/qdrvm/rust-libp2p.git", branch = "feature/shadow-0.55.0" }
+libp2p-mplex = { git = "https://github.com/qdrvm/rust-libp2p.git", branch = "feature/shadow-0.55.0" }
+libp2p-quic = { git = "https://github.com/qdrvm/rust-libp2p.git", branch = "feature/shadow-0.55.0" }
+quinn = { git = "https://github.com/qdrvm/quinn.git", branch = "feature/shadow-0.5.11" }
+quinn-udp = { git = "https://github.com/qdrvm/quinn.git", branch = "feature/shadow-0.5.11" }and build Ream with cargo build --config .cargo/config-shadow.toml to include these patches.
But since we will be upgrading to libp2p version 0.56 shortly, I don't think we should merge this.
Even then once version 0.57 came out it would break again. I don't want to maintain this so it would probably be removed later.
So maybe it would be best to keep this on your local testing branch of ream?
| libp2p = { version = "0.55", default-features = false, features = ["identify", "yamux", "noise", "dns", "serde", "tcp", "tokio", "plaintext", "secp256k1", "macros", "ecdsa", "metrics", "quic", "upnp", "gossipsub", "ping", "shadow"] } | ||
| libp2p-identity = "0.2.12" | ||
| libp2p-mplex = "0.43.1" | ||
| libp2p-mplex = "0.43.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should downgrade our packages
What was wrong?
Couldn't run ream in shadow network simulator.
Shadow doesn't implement some
setsockoptoptions.How was it fixed?
Patch
quinn-udpto avoid unsupportedsetsockoptby using fallback.rs instead of unix.rs.quinn-udpis transitive dependencyream -> libp2p -> libp2p-quic -> quinn -> quinn-udpso
feature = "shadow"was added to every crate in that chain.To-Do