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.

Sign up for GitHub

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

chore: add bootnodes,external-ip cli to bridge to support hive test #1009

Merged
merged 2 commits into from
Oct 31, 2023
Merged

chore: add bootnodes,external-ip cli to bridge to support hive test #1009

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e72acc6
chore: add bootnodes,external-ip cli to bridge + sleep to support hiv…
KolbyML Oct 25, 2023
559c45c
Update bridge.rs
KolbyML Oct 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions portal-bridge/src/bridge.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,10 @@ impl Bridge {
BridgeMode::Latest => self.launch_latest().await,
_ => self.launch_backfill().await,
}
// let test finish uTP transfer before shutting down gossip nodes
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm curious, but wouldn't this be a problem for all bridge mode branches?

Anyway, I do think the need for this timeout goes away after #1010 since the bridge is updated to use the portal_*TraceGossip endpoint, which implicitly waits for all utp transfers to finish before responding. If you agree, then I'd advocate for removing this sleep since it would be unnecessary

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess it would be I was just thinking from the view point of getting my test working.

Will the bridge not always use portal_*TraceGossip?

I will remove this sleep if that is the case then

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, makes sense. As of #1010, the bridge will always use portal_*TraceGossip for both beacon & history content. Idk if it'll be like this forever, but I can't think of a good enough reason to suggest it might change, especially since consuming the gossip trace is fairly important for the bridge metrics.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok I just removed the sleep

if let BridgeMode::Test(_) = self.mode.clone() {
sleep(Duration::from_secs(2)).await;
}
info!("Bridge mode: {:?} complete.", self.mode);
}

Expand Down
14 changes: 14 additions & 0 deletions portal-bridge/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,20 @@ pub struct BridgeConfig {
#[arg(long, help = "Url for metrics reporting")]
pub metrics_url: Option<Url>,

#[arg(
default_value = "default",
long = "bootnodes",
help = "One or more comma-delimited base64-encoded ENR's or multiaddr strings of peers to initially add to the local routing table"
)]
pub bootnodes: String,

#[arg(
default_value = "none",
long = "external-ip",
help = "(Only use this if you are behind a NAT) The address which will be advertised to peers (in an ENR). Changing it does not change which address trin binds to, ex: 127.0.0.1"
)]
pub external_ip: String,

#[command(subcommand)]
pub client_type: ClientType,
}
Expand Down
16 changes: 15 additions & 1 deletion portal-bridge/src/client_handles.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ pub fn fluffy_handle(
.arg(format!("--metrics-address:{address}"))
.arg(format!("--metrics-port:{port}"));
}
if bridge_config.bootnodes != "default" {
for enr in bridge_config.bootnodes.split(',') {
command.args(["--bootstrap-node", enr]);
}
}
if !bridge_config.external_ip.is_empty() {
command.arg(format!("--nat:extip:{}", bridge_config.external_ip));
}
Ok(command.spawn()?)
}

Expand Down Expand Up @@ -67,7 +75,13 @@ pub fn trin_handle(
&format!("http://127.0.0.1:{rpc_port}"),
])
.args(["--discovery-port", &format!("{udp_port}")])
.args(["--bootnodes", "default"]);
.args(["--bootnodes", &bridge_config.bootnodes]);
if !bridge_config.external_ip.is_empty() {
command.args([
"--external-address",
&format!("{}:{}", bridge_config.external_ip, udp_port),
]);
}
if let Some(metrics_url) = bridge_config.metrics_url {
let url: String = metrics_url.into();
command.args(["--enable-metrics-with-url", &url]);
Expand Down