nimbus: init at 23.2.0

[jakubgs]

Description of changes

Nimbus is a client for the Ethereum consensus layer that is lightweight, secure and easy to use.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 22.11 Release Notes (or backporting 22.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[jakubgs]

I have tested this using the following config.nix:

{
  services.geth = {
    mainnet = {
      enable = true;
      authrpc = {
        enable = true;
        port = 8551;
        vhosts = ["localhost"];
        jwtsecret = "/etc/geth/jwtsecret";
      };
    };
  };

  services.nimbus-beacon-node = {
    enable = true;
    web3Urls = ["http://localhost:8551"];
    jwtSecret = "/etc/geth/jwtsecret";
    rest.enable = true;
  };

  environment.etc."geth/jwtsecret" = {
    mode = "0444";
    text = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
  };

  services.getty.autologinUser = "root";

  systemd.services.geth-mainnet.serviceConfig.StandardOutput = "journal+console";
  systemd.services.nimbus-beacon-node.serviceConfig.StandardOutput = "journal+console";
}

And ran it using:

nixos-generate -f vm-nogui -c $PWD/config.nix -I nixpkgs=$HOME/work/nixpkgs

Which worked without issues.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-in-distress/3604/51

[aanderse]

I left a few comments after a look over the module. Please let me know if you have any questions.

[jakubgs]

Thanks for the feedback @aanderse, I didn't know types.path checks for null and that types.enum exists, that's much cleaner than what I was doing.

I've addressed most point aside from the user creation, which I think is necessary since the user has to manage their validator files permissions and for that a static user is necessary, so we can't use the DynamicUser option like some services.

[jakubgs]

Sorry if you're not interested by I'm desperate for reviews, and you've worked on the Geth package and service.

I'd appreciate some feedback from @pennae @ncfavier @iceman-p @bachp @ryantm.

[SuperSandro2000]

The enable option should probably also reference the doc

[SuperSandro2000]

Suggested change

	"--rest=${boolToString cfg.rest.enable}"
	] ++ optionals cfg.rest.enable [
	"--rest-address=${cfg.rest.address}"
	"--rest-port=${toString cfg.rest.port}"
	] ++ [
	"--metrics=${boolToString cfg.metrics.enable}"
	"--rest=${boolToString cfg.rest.enable}"
	"--metrics=${boolToString cfg.metrics.enable}"
	] ++ optionals cfg.rest.enable [
	"--rest-address=${cfg.rest.address}"
	"--rest-port=${toString cfg.rest.port}"

[jakubgs]

Why would I mix the order of flags with different prefixes? Looks ugly.

[SuperSandro2000]

We should group the flags together that are unconditional. Having some between the conditional ones is confusing.

[jakubgs]

I disagree. I'd say mixing different prefixes is what's confusing. All --rest* flags and all --metrics* flags should be grouped in their respective blocks.

[SuperSandro2000]

It is more confusing if conditional and none conditional flags are mixed.

[jakubgs]

No, in my opinion it's more confusing if you have flags with different prefixes mixed rather than grouped.

But if you intend to block this PR on this issue then I will change it.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1910