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

Make NodeManager optional #1142

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Make NodeManager optional #1142

wants to merge 2 commits into from

Conversation

benthecarman
Copy link
Collaborator

@benthecarman benthecarman commented Apr 16, 2024

Was able to fix #1071 while i was at it

This puts the node manager in a RwLock that is by default off, only until create_node_manager is called will we have a lightning node and on-chain wallet. Otherwise it will use fedimint when there is no nodemanager or throw an error if neither are configured.

@benthecarman benthecarman force-pushed the optional-node-manager branch from db1704a to f3aece1 Compare April 16, 2024 16:59
@benthecarman benthecarman force-pushed the optional-node-manager branch 2 times, most recently from 23692b2 to ae0eafa Compare April 17, 2024 04:55
@benthecarman
Copy link
Collaborator Author

This is getting close, want to write tests for everything but in my manual testing things seem to be working well

mutiny-core/src/lib.rs Outdated Show resolved Hide resolved
@benthecarman
Copy link
Collaborator Author

benthecarman commented Apr 17, 2024

note to self: need to make sure bringing back a node manager doesn't create a new node and instead brings back the old nodes

@benthecarman benthecarman force-pushed the optional-node-manager branch 3 times, most recently from 4d6002d to 4b9d5cb Compare April 22, 2024 22:46
@benthecarman benthecarman marked this pull request as ready for review April 22, 2024 22:47
@benthecarman benthecarman force-pushed the optional-node-manager branch from 4b9d5cb to 2f77928 Compare May 9, 2024 17:46
@benthecarman benthecarman force-pushed the optional-node-manager branch from 2f77928 to f04512e Compare June 4, 2024 20:39
@TonyGiorgio
Copy link
Contributor

needs rebase

@benthecarman benthecarman force-pushed the optional-node-manager branch from f04512e to 00c38ee Compare June 6, 2024 22:07
@TonyGiorgio
Copy link
Contributor

Errors when receiving on chain when only a federation is added

Screenshot 2024-06-06 at 8 01 02 PM

Copy link
Contributor

@TonyGiorgio TonyGiorgio left a comment

Choose a reason for hiding this comment

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

needs further testing, on chain receives don't work at all

@benthecarman benthecarman force-pushed the optional-node-manager branch from 00c38ee to 34165c5 Compare June 13, 2024 00:37
@benthecarman
Copy link
Collaborator Author

Went through full testing guide, everything is working as of now. Going to add a few more things to it that will now apply with this

@benthecarman
Copy link
Collaborator Author

discussed in meeting: don't default remove empty node managers

@benthecarman benthecarman force-pushed the optional-node-manager branch from 44bb3fd to b8551de Compare June 17, 2024 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deadlock in NodeManager::new_node
2 participants