chore: add bootnodes,external-ip cli to bridge to support hive test #1009
Conversation
KolbyML
force-pushed
the
update-bridge-to-set-bootnotes
branch
from
007adc9 to
f154a0a
Compare
KolbyML
force-pushed
the
update-bridge-to-set-bootnotes
branch
from
f154a0a to
e72acc6
Compare
KolbyML
changed the title
portal-bridge/src/bridge.rs
Outdated
| @@ -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 | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Ok I just removed the sleep
KolbyML
changed the title
|
Well, I guess you're kinda blocked now by #1010 ... but if you don't mind, I'll leave that up for the rest of the day in case anyone else wants to 👀. And I'll get it merged early in the morning so you'll have the change (that removed the need for the sleep) available |
What was wrong?
this is required for the rewrite of the hive bridge test I am writing
Without the timeout bridge closes without the finishing uTP transfers.
Without being able to set bootnodes I can't test this as it wouldn't be valid
Without being able to set external ip the trin node can't be connected to from the other docker containers
below is the chat history of the problem we are trying to solve.
ethereum/portal-hive#110 (comment)
ethereum/portal-hive#110 (comment)
ethereum/portal-hive#110 (comment)
this PR is required to make the sister pr on portal hive ethereum/portal-hive#115
How was it fixed?
By adding 2 params required to write the test