-
Notifications
You must be signed in to change notification settings - Fork 109
Add cdk-mintd module and package #1153
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
base: main
Are you sure you want to change the base?
Conversation
remove Cargo.lock from .gitignore so we can build mintd binary with buildRustPackage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I'm a NixOS user planning to run cdk-mint so this is useful to me.
Nix flakes are great and I think its better to have the module in this project repo instead of nixpkgs (less likely to get out of sync). The module looks great, I don't think it will be too much to maintain, you've done the bulk of the work already. Using nix actually reduces user configuration errors so will help in the long run.
I'm happy to help out with the nix stuff on this project (I added the nix flake to the core lightning project).
|
|
||
| rustPlatform.buildRustPackage rec { | ||
| pname = "cdk-mintd"; | ||
| version = "0.12.0"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Version should be read from Cargo.toml.
| ''; | ||
| module = self.nixosModules.default; | ||
| in | ||
| pkgs.nixosTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried running the test but got an error "Ln backend must be set".
I think if there is going to be a module for cdk-mint in this project then there has to be a test as well. Yes its quite a bit of work to get it set up at first but it helps so much with maintenance/reliability/documentation.
There's no lightning module in nixpkgs so maybe you can just use ldk embedded or pull in nix-bitcoin.
| }; | ||
|
|
||
| # Open firewall port if listening on all interfaces | ||
| networking.firewall.allowedTCPPorts = mkIf (cfg.settings.info.listen_host == "0.0.0.0") [ cfg.settings.info.listen_port ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually there is an option to automatically open firewall or not. Some people may be using nftables for example.
| , protobuf | ||
| }: | ||
|
|
||
| rustPlatform.buildRustPackage rec { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice crane is in the flake inputs (but not actually used). Did you consider using it to build the package? If developers on the project want to make use of more nix features then it might be better to use crane. It allows for more flexibility and artifact reuse when building the whole workspace.
|
hey @JosephGoulden thanks so much for the review, solid feedback. I'm working on this and still have a few things to figure out but i'm getting there. |
|
This PR is stale because it has been open for 60 days with no activity. |
Description
Add a NixOS module for the
cdk-mintdbinary. This enables pulling in a mint as a systemd service. In order to do this, we also implement acdk-mintdpackage which is added to the flake.Notes to the reviewers
Looking for conceptual feedback, and how hard you think maintaining this would be--it's a very large module.
In order to get a package working, we need a
Cargo.lockfile. I added one in the root of the project, but maybe this should be in the crate directory itself?Was trying to get a test working but might not be worth it.
Haven't tested all the options, but the module seems to build without Nix errors.
Suggested CHANGELOG Updates
CHANGED
ADDED
Nix module and package.
Cargo.lock file
REMOVED
Cargo.lock in .gitignore
FIXED
Checklist
just final-checkbefore committing