mina icon indicating copy to clipboard operation
mina copied to clipboard

refactor node config to remove use of optcomp

Open glyh opened this issue 3 months ago • 3 comments

As title.

This is actually an effort to bump to OCaml 5, which requires bumping core libs which in turn requires bumping ppx_optcomp.

ppx_optcomp is completely redesigned to another library and could not be used by us anymore, hence this refactor.

It turns out we can just go one step further and make the comp time dispatch to runtime and we can use same build for different profiles. This, with some CI effort, could divide the time it takes for building stuff by the number of profiles we have. I believe that number is 3/4 now.

Bonus:

  • you can write any turing complete logic now to calculate node configs
  • you can have OCaml type system to help to harden the node config

glyh avatar Dec 06 '25 16:12 glyh

!ci-build-me

glyh avatar Dec 07 '25 03:12 glyh

I dont think this is any worse than what we have now, which is already terrible. I'm also excited about removing the mlh files, but I want to propose something else and see what you think:

IIUC, what you're doing here is using a dune build hook to write a module (ie a text file) which defines the value for the config we want based on an environment variable. This works, but does it also suffer from one of the most annoying parts of the process we have today: if you change the dev profile, you have to rebuild most of the project since the config is so low level.

The way we deal with signature kind is also different (and maybe incoherent). We have a virtual module with mainnet/testet(/others?) implementations, and the implementation gets chosen depending on the exectuable (e.g. see mina_testnet_signatures). I say "maybe incoherent" because I'm not sure which parts of our code use virtual modules vs the compile time string.

What if we changed the entire config module to a virtual module with the different implementations, and forced every executable to specify which one it wanted. This would allow you to get rid of the optcomp dep, and it would allow you to remove the mina_signature_kind virtual module and just depend on this one. And if I understand correctly that if we did this, it would allow building mina cli to share the whole build cache across builds

The downside would be that you need to change the ci / build pipelines because it forks all executables into multiple (like with the mina_testnet_signatures vs mina_mainnet_signatures

martyall avatar Dec 08 '25 18:12 martyall

it also suffer from one of the most annoying parts of the process we have today: if you change the dev profile, you have to rebuild most of the project since the config is so low level.

This is intended, I'm keeping all behaviors same so ensure this refactor is indeed a noop. To alleviate the issue you're talking about:

  • we can remove the comptime module, and resolve profiled config at runtime
  • we generate package with a file on disk telling the executable what is the preferred profile
  • user not using the new env var(say name it MINA_PROFILE) will fall back to this on disk file so there's no behavior change they'll notice
  • user using the new env var can just have another docker image/debian package we release, and mark MINA_PROFILE as they like.

The fallback logic would be roughly:

  • check if MINA_PROFILE exist, if so, derive profile from it
  • o.w. check if there's an on disk file somewhere in /etc/coda, derive profile from it
  • as a final resort, use dev profile

Since in this follow up we move stuff to runtime, the comptime module could be removed. Also the signature kind virtual modules could be removed and wired into profiled config.

I'm not sure which parts of our code use virtual modules vs the compile time string.

Just checked only src/config/dune is using profile, which this PR rewrites. The virtual module thing is not that relevant as it's set in code not at build time it seems, and we're always using mina_signature_kind.config

What if we changed the entire config module to a virtual module with the different implementations, and forced every executable to specify which one it wanted.

This will make our dune files more verbose. In additional, I think this is the wrong direction because we want "build once, run everywhere". What I proposed above, would also preserve build cache.

glyh avatar Dec 09 '25 08:12 glyh

!ci-nightly-me

glyh avatar Dec 12 '25 10:12 glyh

https://buildkite.com/o-1-labs-2/hardfork-package-generation-new/builds/198 https://buildkite.com/o-1-labs-2/mina-end-to-end-nightlies/builds/4030

glyh avatar Dec 12 '25 11:12 glyh

Rebased on origin/compatible to include https://github.com/MinaProtocol/mina/pull/18246

glyh avatar Dec 12 '25 12:12 glyh

!ci-nightly-me

glyh avatar Dec 12 '25 14:12 glyh

https://buildkite.com/o-1-labs-2/mina-end-to-end-nightlies/builds/4032

glyh avatar Dec 12 '25 14:12 glyh

Rebased to resolve merge conflict.

glyh avatar Dec 16 '25 04:12 glyh

!ci-build-me

glyh avatar Dec 16 '25 04:12 glyh

!ci-nightly-me

glyh avatar Dec 16 '25 04:12 glyh