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

feat: Add docker reproducible builds #6799

Open
wants to merge 3 commits into
base: unstable
Choose a base branch
from

Conversation

MoeMahhouk
Copy link

@MoeMahhouk MoeMahhouk commented Jan 13, 2025

Issue Addressed

Which issue # does this PR address?

This PR addresses reproducible builds. The current dockerfile builds the lighthouse binary but not reproducibly.
You can verify that by following these steps:

docker build --no-cache --output=. .
mv usr/local/bin/lighthouse lighthouse1
rm usr/local/bin/lighthouse 
docker build --no-cache --output=. .
mv usr/local/bin/lighthouse lighthouse2
sha256sum lighthouse1 lighthouse2

You will notice that each one of the binaries has a different checksum upon each build. This is critical for systems that depends on requiring reproducible builds, such as running lighthouse in confidential computing, like Intel TDX.

Proposed Changes

This PR adds a new build profile as well as a Dockerfile.reproducible that enables building the lighthouse binary reproducibly.
By following the steps I listed above, you will be able to verify that the resulted binary has the same hash upon several subsequent builds for the same version.

How to test it:

mkdir output1 output2
docker build --no-cache -f Dockerfile.reproducible --output=output1 .
docker build --no-cache -f Dockerfile.reproducible --output=output2 .
sha256sum output1/lighthouse output2/lighthouse
# hashes should be identical
rm -rf output1 output2

Additional Info

We at Flashbots are using Lighthouse as CL client in our BuilderNet TDX image and I am currently working on a containerized approach which would require the container image to be reproducibly buildable for verification purposes.
This would facilitate a way to make it work. I would also appreciate it, if you could extend CI pipeline with a new target that generates a reproducible image on each release if that's ok.

Thanks a lot and I'm looking forward for your feedback

@CLAassistant
Copy link

CLAassistant commented Jan 13, 2025

CLA assistant check
All committers have signed the CLA.

@michaelsproul
Copy link
Member

This is fantastic, thank you!

We have an open issue for repro builds here that we can close once this PR is merged:

To get the merge train rolling can you please:

  • Rebase on unstable and change the target branch to unstable
  • Sign the CLA using your github login (comment above this one).

Thanks!

@michaelsproul michaelsproul added enhancement New feature or request infra-ci labels Jan 13, 2025
@michaelsproul michaelsproul added the waiting-on-author The reviewer has suggested changes and awaits thier implementation. label Jan 13, 2025
@MoeMahhouk MoeMahhouk changed the base branch from stable to unstable January 14, 2025 10:10
@MoeMahhouk MoeMahhouk closed this Jan 14, 2025
@MoeMahhouk MoeMahhouk deleted the stable branch January 14, 2025 10:14
@MoeMahhouk MoeMahhouk restored the stable branch January 14, 2025 10:14
@MoeMahhouk MoeMahhouk reopened this Jan 14, 2025
@MoeMahhouk
Copy link
Author

Hey @michaelsproul, thanks for the quick response!
I have rebased my changes to unstable and changed the PR's base target to unstable too.
I also signed the CLA.
Lastly, I tried renaming my branch to avoid confusion but it caused the PR to close because from its perspective it got deleted. So I restored it back. Sorry for the confusion

@michaelsproul
Copy link
Member

No worries, thanks! I'll do some testing and we can merge this soon!

@MoeMahhouk
Copy link
Author

I have adjusted the dockerfile to support both architectures (x86_64 and aarch64) and verified the reproducible build for both targets.
Hint: for aarch build I would recommend building it on a machine with native arm CPU because an emulated one takes a lot of time that I had to stop it after 16 hours and spin up a native arm VM on azure. On native arm VM it built rather quickly ~ 15 mins

@chong-he chong-he added ready-for-review The code is ready for review and removed waiting-on-author The reviewer has suggested changes and awaits thier implementation. labels Jan 20, 2025
Copy link
Member

@chong-he chong-he left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this on x86_64 machines and it works, producing the same sha256sum hash on the Lighthouse binary. A make build-reproducible-x86_64, followed by extracting the binary file, and sha256sum on the binary file will result it:

a5838aebbe5ceaf2a9202c50c433f0c8d615551c32927de71d6df9a29eb76a5f lighthouse

A rebuilding gives the same hash.

A minor comment as below

Comment on lines 14 to 15

# Get the latest commit timestamp and set SOURCE_DATE_EPOCH
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, upon finish building the docker image, there will be a warning shown at the end:

 1 warning found (use docker --debug to expand):
 - UndefinedVar: Usage of undefined variable '$SOURCE_DATE_EPOCH' (line 21)

It's not really an issue but I find that if we define the SOURCE_DATE_EPOCH as an argument beforehand, then the warning no longer appears. I am not sure if this is a proper way to do, but I have tested this and the build is still reproducible with the added code.

Suggested change
# Get the latest commit timestamp and set SOURCE_DATE_EPOCH
# Define SOURCE_DATE_EPOCH as an ARG first
ARG SOURCE_DATE_EPOCH
# Get the latest commit timestamp and set SOURCE_DATE_EPOCH

If we add this in the code, the sha256sum of the resulting binary would be:
7ececbec414b0cc51e42a0f81e8441b9d5ae2f915c2e924424f67a9e932b5e95 lighthouse

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for highlighting this warning.
It appears that Docker is not keeping the state nor really getting the value from the command git log -1 --pretty=%ct
So it was setting SOURCE_DATE_EPOCH to empty string silently which is wrong.
Upon uncovering such issue, I am preparing a fix where we pass it as build arg similar to the build target and currently validating that it works fine and the variable is captured correctly

Copy link
Member

@chong-he chong-he Jan 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested again and confirm that the warning is gone. The hashes of the binary files are the same upon rebuilding
d85f5ff5fecc515b55982181e8a642788c6dde2a20727a60b1f21b0112e8257b lighthouse

@MoeMahhouk MoeMahhouk requested a review from chong-he January 20, 2025 16:37
@michaelsproul michaelsproul added the v7.0.0-beta.0 New release c. Q1 2025 label Jan 22, 2025
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. We can include this in our v7.0.0 release.

Will flashbots handle reproducible builds/releases?

@michaelsproul
Copy link
Member

I got a segfault building the aarch64 image on x86_64. I guess that's expected? The build is intended to run on aarch64 I'm guessing?

@michaelsproul michaelsproul added v7.1.0 Post-Electra release waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed v7.0.0-beta.0 New release c. Q1 2025 ready-for-review The code is ready for review labels Feb 6, 2025
@MoeMahhouk
Copy link
Author

I got a segfault building the aarch64 image on x86_64. I guess that's expected? The build is intended to run on aarch64 I'm guessing?

I tried building it directly on a native arm machine because on x86_64 it took more than 16hours and didn't finish. So I had to terminate the build manually and re-run it again on an Azure native Arm VM.
I didn't get segfault there and generated the same build hash of the binary

@MoeMahhouk
Copy link
Author

LGTM. We can include this in our v7.0.0 release.

Will flashbots handle reproducible builds/releases?

Optimally, there would be a github action in your release.yaml workflow that on each tag release, it generates a reproducibly built container and pushes it to a public registry like ghcr or dockerhub.
If you want, I can look into your script and create a PR for that. But it should be rather straight forward process if I am not wrong

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request infra-ci v7.1.0 Post-Electra release waiting-on-author The reviewer has suggested changes and awaits thier implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants