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

Ensure that all symbols in the simplicity library begin with simplicity_ #248

Merged
merged 5 commits into from
Jul 31, 2024

Conversation

roconnor-blockstream
Copy link
Collaborator

C has no name spaces, and we want to ensure there are no symbol collisions with other libraries.

You can check the list of exported symbols by somelike

nm -C result/lib/libElementsSimplicity.a | grep " [A-TV-Z] "

The grep is largely to exclude non-exported symbols from the list.

This the first step into prefixing all symbol names in the simplicity static
library.

C has no name spaces, and we want to ensure there are no symbol collisions with
other libraries.
@roconnor-blockstream
Copy link
Collaborator Author

In rust-simplicty you will want to ammend

https://github.com/BlockstreamResearch/rust-simplicity/blob/c74f94dd9565a750abe47e3ea51045eea5793d0b/simplicity-sys/depend/wrapper.h#L6

to

  bool result = simplicity_##jet(dst, *src, env); \

Copy link
Collaborator

@uncomputable uncomputable left a comment

Choose a reason for hiding this comment

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

ACK d2cdd72 It's sad that we have to do this, but it seems common practice for C libraries :/

@apoelstra
Copy link
Contributor

The 5184676 haskell derivation doesn't compile. Is your goal that every individual commit of this PR will build?

This prefixes most symbol names in the simplicity static library.

C has no name spaces, and we want to ensure there are no symbol collisions with
other libraries.

clang-rename is very finicky, so the following bash script was used:

shopt -s globstar
for SYMBOL in closeBitstream readNBits decodeUptoMaxInt readBitstring computeWordCMR computeCommitmentMerkleRoot computeAnnotatedMerkleRoot verifyCanonicalOrder fillWitnessData verifyNoDuplicateIdentityRoots decodeMallocDag analyseBounds evalTCOExpression read_buffer8 write_buffer8 read_sha256_context write_sha256_context copyBits mallocBoundVars decodeJet rsort hasDuplicates sha256_compression_is_optimized sha256_bitstring computeTypeAnalyses mallocTypeInference sha256_confidential sha256_confAsset sha256_confNonce sha256_confAmt generateIssuanceEntropy calculateAsset calculateToken make_tapleaf make_tapbranch build_txEnv
do
    clang-rename -i -qualified-name=${SYMBOL} -new-name=simplicity_${SYMBOL} $(grep "${SYMBOL}(" *.c *.h include/**/*.h primitive/**/*.c primitive/**/*.h -l) -- -I include
done
C has no name spaces, and we want to ensure there are no symbol collisions with
other libraries.
clang-rename cannot rename macro generated functions, so this must be done by hand.

This prefixes the rest of the symbol names in the simplicity static library.

C has no name spaces, and we want to ensure there are no symbol collisions with
other libraries.
All of the other exported symbols begin with the "simplicity_" prefix.
We are renameing the "elements_simplicty_" API functions to be
"simplicity_elements_" instead so that everything consistently has the same prefix.
@roconnor-blockstream
Copy link
Collaborator Author

Might be working now.

Copy link
Contributor

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK bd89d2a

@roconnor-blockstream roconnor-blockstream merged commit bd89d2a into staging Jul 31, 2024
@roconnor-blockstream roconnor-blockstream deleted the prefix-symbols branch July 31, 2024 21:42
apoelstra added a commit to BlockstreamResearch/rust-simplicity that referenced this pull request Aug 8, 2024
This update renames a bunch of symbols from x to simplicity_x. This
commit has a small number of non-mechanical changes to address that.

The one "judgement call" was to update wrapper.h in accordance with
BlockstreamResearch/simplicity#248 (comment)
to cover all the jet wrapper renames.
apoelstra added a commit to BlockstreamResearch/rust-simplicity that referenced this pull request Aug 8, 2024
This update renames a bunch of symbols from x to simplicity_x. This
commit has a small number of non-mechanical changes to address that.

It also updates the benchmarks and therefore the CMRs and AMRs of many
jets.

The one "judgement call" was to update wrapper.h in accordance with
BlockstreamResearch/simplicity#248 (comment)
to cover all the jet wrapper renames.
apoelstra added a commit to BlockstreamResearch/rust-simplicity that referenced this pull request Aug 8, 2024
…5025067

c9e70ed update libsimplicity to 121b2951d8cca3d73dccd6fcbe721f2195025067 (Andrew Poelstra)

Pull request description:

  This update renames a bunch of symbols from x to simplicity_x. This commit has a small number of non-mechanical changes to address that.

  The one "judgement call" was to update wrapper.h in accordance with BlockstreamResearch/simplicity#248 (comment) to cover all the jet wrapper renames.

Top commit has no ACKs.

Tree-SHA512: 5aed096de52aac26651f627f4754e02ebcc3394e846e524f03024c75a75e3af7739d69b243427bfac1c86448dbcfa479347b05104daf6892471c507daed88596
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.

3 participants