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

move IP generators into an abstract base class #215

Conversation

TChukwuleta
Copy link
Contributor

A proposed fix for #189

Created an abstract base class for the IP generator and let every other class that needs the fake IP generator including the Tank as well as LNNode inherit it.

@josibake Kindly review. Thanks

Copy link

vercel bot commented Dec 7, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
warnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 7, 2023 6:43am

@josibake
Copy link
Collaborator

josibake commented Dec 7, 2023

Hey @TChukwuleta ! Welcome to warnet 😄

Now that we are closer to having the kubernetes backend finished, I think we are going to move away from setting the IP addresses manually all together, and instead just apply a patch to bitcoin core that ignores the IsRoutable() function. This simplifies things quite a bit and means we will probably get rid of all of this IP related code altogether.

The patched docker image is being worked on here: #206

I think we can keep this PR open for now until we have the patched docker images full ironed out, in case we need to back to generating the addresses.

@TChukwuleta
Copy link
Contributor Author

Hey @TChukwuleta ! Welcome to warnet 😄

Now that we are closer to having the kubernetes backend finished, I think we are going to move away from setting the IP addresses manually all together, and instead just apply a patch to bitcoin core that ignores the IsRoutable() function. This simplifies things quite a bit and means we will probably get rid of all of this IP related code altogether.

The patched docker image is being worked on here: #206

I think we can keep this PR open for now until we have the patched docker images full ironed out, in case we need to back to generating the addresses.

Thanks Josie. would there be anything to do to assist on getting out the patched docker image? else, I'd look for another issue to hop on

@willcl-ark
Copy link
Contributor

If we use the k8s images (post #241 ) in docker, we can get rid of all the ip address generation stuff from the repo.

Even if we want to add ASMap (#217) we can simply map the assigned ip address to a new AS manually, as I did in the linked PR...

@m3dwards @josibake @pinheadmz

@josibake
Copy link
Collaborator

josibake commented Feb 3, 2024

If we use the k8s images (post #241 ) in docker, we can get rid of all the ip address generation stuff from the repo.

Even if we want to add ASMap (#217) we can simply map the assigned ip address to a new AS manually, as I did in the linked PR...

@m3dwards @josibake @pinheadmz

Tend to agree as I don't think we'd even have the ability to set IP addresses on a managed k8s cluster. Also, with the isroutable patch for docker, I don't think we need to manually specify addresses, just need to pick the appropriate subnet. I'd say we close this for now and then can reopen if we do end up needing the IP addresses.

@m3dwards
Copy link
Collaborator

m3dwards commented Feb 5, 2024

Thanks for PR @TChukwuleta but I have to agree that it shouldn't be needed now.

@m3dwards m3dwards closed this Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants