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

use boringssl's upstream directly; removes need for our fork and committed generated files #945

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

spoonincode
Copy link
Member

When we added boringssl the only way to integrate it without requiring golang was to commit generated files in to the repository. This is quite the horror to me because it makes it difficult for anyone (including ourselves) to check that the generated files haven't been tampered with by whoever committed them. To try and mitigate that, the repo we store the generated files in (https://github.com/AntelopeIO/boringssl-build) contains a workflow to double check the committed files by generating them itself using golang etc. Still very undesirable though.

A handful of months ago boringssl modified their build structure so that golang was no longer required to use the cmake files. And, astonishingly, it Just Works even in our use case.

This is a big deal. It makes it trivial for anyone (including ourselves) to verify that we're using upstream verbatim. I think it's worth doing the upgrade.

I eyeballed the commit titles (and it some cases the commit contents) from our prior pinned commit of a1843d6. Didn't see anything alarming (anything that could affect our usage). Certainly could be worth another set of eyeballs skimming over the commit title history (https://boringssl.googlesource.com/boringssl/+log).

Didn't see any differences in our benchmark application (the merkle test is a bit persnickety for me to get consistent results though unless I increase the number of runs substantially, since these otherwise run very few runs)

Copy link
Member

@linh2931 linh2931 left a comment

Choose a reason for hiding this comment

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

Scanned the commit history list. It seems fine.

.gitmodules Show resolved Hide resolved
@spoonincode spoonincode merged commit d3d51ff into main Oct 17, 2024
76 checks passed
@spoonincode spoonincode deleted the boringssl_upstream_direct branch October 17, 2024 16:52
@ericpassmore
Copy link
Contributor

Note:start
category: Chores
component: Internal
summary: Pull boringssl submodule directly from upstream source.
Note:end

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.

4 participants