feat: Validator binary crate, Dockerfile, and Debian package#2053
feat: Validator binary crate, Dockerfile, and Debian package#2053
Conversation
|
Debian workflow works until release is required, as expected https://github.com/0xMiden/node/actions/runs/25411348891 |
Mirko-von-Leipzig
left a comment
There was a problem hiding this comment.
I didn't audit the old/moved code in depth, I assume its largely a copy-paste operation.
There was a problem hiding this comment.
This will come up a bunch. Should we create separate "base images" for building and runtime?
There was a problem hiding this comment.
As far as I understand that's not possible without using some extensions. We could add dockerfile-x though and move the builder and runner image definitions to a common Dockerfile referenced by the per-service Dockerfiles.
There was a problem hiding this comment.
I have gone with a merged Dockerfile with ARGs for bin and port. Might not be flexible enough for us in the end, but it might be (same will work fine for the ntx-builder image).
There was a problem hiding this comment.
Lets see how it goes. I don't like that it assumes the binaries are the same and can be coupled like this.
I think what I was assuming originally is that we would publish build and runtime base images. I don't understand why that isn't possible; isn't that how
FROM rust:1.93-slim-bookworm AS chef
exist?
There was a problem hiding this comment.
IIRC that requires the base image to exist in a registry
There was a problem hiding this comment.
I would really like if we could get rid of these. They're a pain to maintain.
I guess this could be done in tandem with removing Debian packages once we move to docker ourselves?
kkovaacs
left a comment
There was a problem hiding this comment.
Just two comments:
bin/genesis/README.mdstill has a reference tomiden-node validator bootstrapwhich is now gone.- Similarly,
scripts/run-node.shstill tries to bootstrap withmiden-node validator bootstrap, so it's effectively broken.
| homepage.workspace = true | ||
| keywords = ["miden", "validator"] | ||
| license.workspace = true | ||
| name = "miden-validator" |
There was a problem hiding this comment.
I think I saw a comment from @Mirko-von-Leipzig regarding naming somewhere: it does feel strange to have both miden-node-validator and miden-validator crates.
One potential option is to have both library and binary targets in the miden-node-validator crate - but I'm not sure if that's a good idea.
There was a problem hiding this comment.
I have renamed the lib crate to miden-validator-core. The -core suffix seems to be used in the ecosystem. Prefer -lib but doesn't seem to be as popular. cc @Mirko-von-Leipzig
There was a problem hiding this comment.
An alternate suggestion is to have both crate library and binary be miden-validator. The only downside is that the main entrypoint cannot live outside the crate, so we would have either have to move the entire create into bins, or move main out into the crate
Relates to #1961.