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

feat: new behavior for network bootstrapping #1257

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

cylewitruk
Copy link
Member

@cylewitruk cylewitruk commented Jan 22, 2025

Description

Closes: #1133

Changes

  • Adds a new libp2p swarm behavior for handling network bootstrapping.
  • The swarm will now re-try connections to all seed peers if the connection count is 0, and re-try every minute until the swarm has peers.

Testing Information

Existing tests pass and I've tested this quite a bit in devenv. I'll add some actual tests for the behavior, but PR'ing this now for preview.

Checklist:

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@cylewitruk cylewitruk added sbtc signer binary The sBTC Bootstrap Signer. signer communication Communication across sBTC bootstrap signers. labels Jan 22, 2025
@cylewitruk cylewitruk added this to the sBTC: Deposits milestone Jan 22, 2025
@cylewitruk cylewitruk self-assigned this Jan 22, 2025
@cylewitruk cylewitruk linked an issue Jan 22, 2025 that may be closed by this pull request
1 task
@aldur aldur requested a review from Jiloc January 27, 2025 14:34
@cylewitruk
Copy link
Member Author

@matteojug I ended up doing quite a bit of refactoring, sorry :( So this probably needs a fresh look.

@djordon djordon removed this from the sBTC: Deposits milestone Feb 12, 2025
Copy link
Collaborator

@djordon djordon left a comment

Choose a reason for hiding this comment

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

Just had some nits, and one question.

.map(|either, _| either.into_inner())
.boxed();
}
let tcp_config = tcp::Config::default().nodelay(true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the significance of the nodelay here.

Copy link
Member Author

@cylewitruk cylewitruk Mar 6, 2025

Choose a reason for hiding this comment

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

It disables Nagle's Algorithm on the socket which is enabled by default on most OS's. I think this option might be set by default now, but I added it explicitly to be, well, explicit as to our intentions.

.map_err(|e| SignerSwarmError::LibP2P(Box::new(e)))?;
}
{
let mut swarm = self.swarm.lock().await;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We just locked, and unlock the swarm a moment ago for the local_peer_id. Maybe we should move that log down here.

enable_quic_transport: false,
enable_memory_transport: false,
initial_bootstrap_delay: Duration::from_secs(0),
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: this could be Duration::ZERO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sbtc signer binary The sBTC Bootstrap Signer. signer communication Communication across sBTC bootstrap signers. wait-until-next-release
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

[Feature]: Retry p2p seed peers
3 participants