Skip to content

build: make vendored OpenSSL opt-in#5152

Draft
eval-exec wants to merge 2 commits intonervosnetwork:developfrom
eval-exec:ckb.remove-openssl-vendor
Draft

build: make vendored OpenSSL opt-in#5152
eval-exec wants to merge 2 commits intonervosnetwork:developfrom
eval-exec:ckb.remove-openssl-vendor

Conversation

@eval-exec
Copy link
Copy Markdown
Collaborator

@eval-exec eval-exec commented Mar 20, 2026

What problem does this PR solve?

Problem Summary:
Default non-WASM CKB builds always enable vendored OpenSSL, and the x86_64 Docker image relied on a focal/static OpenSSL path that is not ideal for reproducibility.

What is changed and how it works?

What's Changed:

  • gate openssl-vendored behind an explicit feature
  • keep prod_portable on the vendored path
  • move the x86_64 Docker build/runtime to Ubuntu 22.04 and remove the LD_LIBRARY_PATH workaround

Related changes

  • None

Check List

Tests

  • Manual test
    • cargo tree -p ckb-network -i openssl-src
    • cargo tree --features openssl-vendored -p ckb-network -i openssl-src
    • cargo check --locked -p ckb-network --features openssl-vendored
    • docker build -f docker/hub/Dockerfile -t ckb:x64-jammy-test .
    • docker run --rm --entrypoint bash ckb:x64-jammy-test -lc 'ldd /bin/ckb | grep -E "libssl|libcrypto"; /bin/ckb --version'

@eval-exec eval-exec requested a review from a team as a code owner March 20, 2026 07:51
@eval-exec eval-exec requested review from Copilot and quake and removed request for a team March 20, 2026 07:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR makes vendored OpenSSL opt-in for non-WASM builds to improve reproducibility, while preserving the “portable production” build path using vendored OpenSSL and updating the x86_64 Docker image to stop forcing static OpenSSL.

Changes:

  • Introduces an explicit openssl-vendored feature and wires it through ckb-networkckb-bin → workspace root.
  • Updates prod_portable to explicitly enable openssl-vendored.
  • Updates the x86_64 Docker build to stop forcing static OpenSSL and instead ship the required shared OpenSSL libraries in the runtime image.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
network/Cargo.toml Removes default p2p vendored OpenSSL and adds an opt-in feature forwarding to p2p/openssl-vendored.
ckb-bin/Cargo.toml Forwards openssl-vendored feature from ckb-bin to ckb-network.
Cargo.toml Exposes workspace-level openssl-vendored feature that forwards to ckb-bin.
Makefile Ensures prod_portable continues using vendored OpenSSL explicitly.
docker/hub/Dockerfile Stops forcing static OpenSSL in x86_64 Docker builds; copies shared libs and sets runtime loader path.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docker/hub/Dockerfile Outdated

USER ckb
ENV CKB_CHAIN=mainnet
ENV LD_LIBRARY_PATH=/usr/local/lib64
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

Setting LD_LIBRARY_PATH here overwrites any existing value from the base image or runtime environment. To avoid clobbering, consider appending/prepending instead (e.g., include the prior $LD_LIBRARY_PATH in the value).

Suggested change
ENV LD_LIBRARY_PATH=/usr/local/lib64
ENV LD_LIBRARY_PATH=/usr/local/lib64:${LD_LIBRARY_PATH}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 52cbcbe. The x86_64 Docker image now builds and runs on Ubuntu 22.04, so the final image resolves OpenSSL from Jammys standard library path and no longer sets LD_LIBRARY_PATH.

Comment thread docker/hub/Dockerfile Outdated
Comment on lines +23 to +28
RUN chown -R ckb:ckb /var/lib/ckb \
&& chmod 755 /var/lib/ckb

USER ckb
ENV CKB_CHAIN=mainnet
ENV LD_LIBRARY_PATH=/usr/local/lib64
Copy link

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

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

The x86_64 Docker image places libssl/libcrypto under /usr/local/lib64 (and then relies on LD_LIBRARY_PATH). If possible, consider copying these into a standard linker path (e.g., /usr/lib/x86_64-linux-gnu) or registering /usr/local/lib64 via ld.so.conf.d + ldconfig, to reduce reliance on LD_LIBRARY_PATH and keep the approach closer to the aarch64 Dockerfile.

Suggested change
RUN chown -R ckb:ckb /var/lib/ckb \
&& chmod 755 /var/lib/ckb
USER ckb
ENV CKB_CHAIN=mainnet
ENV LD_LIBRARY_PATH=/usr/local/lib64
RUN echo "/usr/local/lib64" > /etc/ld.so.conf.d/ckb-local-lib64.conf \
&& ldconfig
RUN chown -R ckb:ckb /var/lib/ckb \
&& chmod 755 /var/lib/ckb
USER ckb
ENV CKB_CHAIN=mainnet

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Addressed in 52cbcbe. Rather than registering a copied custom library path, the x86_64 Docker image now uses Ubuntu 22.04 for both build and runtime and links against the runtime images standard libssl.so.3 / libcrypto.so.3.

@eval-exec eval-exec marked this pull request as draft March 20, 2026 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants