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

Adding Signet option #149

Merged
merged 2 commits into from
May 24, 2024
Merged

Adding Signet option #149

merged 2 commits into from
May 24, 2024

Conversation

matthiasdebernardini
Copy link
Contributor

This is referencing the issue #148 adds Signet support by adding the keywords.

It seems to work but I am not sure how to guarantee quality, if there are tests then I would be happy to run them.

As far as I can tell, the only thing missing is being able to test boltz integration

, { Boss::Msg::Network_Testnet

Added Signet option
Adding signet config option to network
@openoms
Copy link

openoms commented Aug 20, 2023

Signet support would be welcome as it is preferred over testnet3.

@ksedgwic
Copy link
Collaborator

mempool.space thinks signet has 35 lightning nodes while testnet shows 470 and mainnet has 15,487

I'm pretty sure signet would require a patch to min_node_to_process see ad49445

I suspect this parameter should be different for each network

@ksedgwic
Copy link
Collaborator

I added a PR which sets min_nodes_to_process to a network specific value: #173

Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

OK make sense, if you help to mantaining it.

Also signet is a little bit strange because can be more signet network.

clboss keep a list of nodes (aka seeds) to start connecting with, so this is PR is missing to add it. In addition, we should make sure that this peer should be living enough.

An alternative approach is specify at least one seed as plugin option, this could help with signet, but not sure how the clboss metrics will react with a single node

@chrisguida chrisguida mentioned this pull request May 24, 2024
@chrisguida
Copy link
Contributor

Ended up reimplementing this myself before I knew this PR existed #207

xD

Let's just merge this, it works great for now

Copy link
Contributor

@chrisguida chrisguida left a comment

Choose a reason for hiding this comment

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

Works with mutinynet

@ksedgwic
Copy link
Collaborator

Looks good

@ksedgwic ksedgwic merged commit fa5c312 into ZmnSCPxj:master May 24, 2024
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

Sorry for being late to the discussion, but I think this PR is missing a list of seed nodes for signet. Or am I missing something?

Additionally, we may want to provide a seed list via a command line option, considering it is unclear how many signet network there are.

@chrisguida
Copy link
Contributor

chrisguida commented May 24, 2024

Ah I forgot about seed nodes. You can just connect to the faucet node on mutinynet, you can find the URI on https://faucet.mutinynet.com

@ksedgwic
Copy link
Collaborator

There is another PR for updating the mainnet seed nodes. The CLBOSS support isn't very good on any network. I generally make a few manual seed connections and bootstrap gossip that way ...

Maybe we should make a DNS based thing so at least the nodes aren't compiled in ...

@ksedgwic
Copy link
Collaborator

to clarify, I only make initial gossip connections manually, everything else works automatically

@ksedgwic ksedgwic mentioned this pull request Jun 17, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants