dimxy / komodo

Komodo
https://komodoplatform.com/
Other
7 stars 4 forks source link

Tokel: fix build of cryptoconditions lib #78

Closed dimxy closed 2 years ago

dimxy commented 3 years ago

The issue: in some cases, if some cryptoconditions sources like threshold.c was modified the whole cryptoconditions lib is not rebuilt until it is rebuilt manually. To fix this: add 'make cryptoconditions' cmd into src/Makefile.am add dependencies to included .c files (like threshold.c) into cryptoconditions/Makefile.am add call of cryptocondition autogen.sh and configure to zcutil/build....sh for linux macos windos

dimxy commented 2 years ago

More info: still having cryptoconditions rebuild problems: apart from cryptoconditions not rebuilt if some included files like threshold.c was modified, found another issue: when I switched a branch growing from the research to a branch growing from the dev branch I had link reference errors for cryptoconditions symbols:

/home/ubuntu/repo/komodo/src/cryptoconditions/src/eval.c:29: undefined reference to `sha256'
cryptoconditions/.libs/libcryptoconditions_core.a(cryptoconditions.o): In function `ed25519Verify':
/home/ubuntu/repo/komodo/src/cryptoconditions/src/ed25519.c:38: undefined reference to `ed25519_verify'
cryptoconditions/.libs/libcryptoconditions_core.a(utils.o): In function `hashFingerprintContents':
/home/ubuntu/repo/komodo/src/cryptoconditions/src/utils.c:215: undefined reference to `der_encode_to_buffer'
/home/ubuntu/repo/komodo/src/cryptoconditions/src/utils.c:221: undefined reference to `sha256'

After 'make clean' the rebuild was all right but sure this needs to be fixed. A Tokel team developer also encountered this issue.

A research has shown that the problem may be with the dependency files in .../cryptoconditions/src/.deps dir. I noticed that those files may be different. Here is an excerpt from two versions of a dependency file:

admin@ip-172-31-28-221:~/repo/komodo$ cat src/cryptoconditions/src/.deps/cryptoconditions.Plo |grep threshold.c
 src/threshold.c src/asn/ThresholdFingerprintContents.h src/prefix.c \
src/threshold.c:

same file (saved) with another path:

admin@ip-172-31-28-221:~/repo/komodo$ cat src/cryptoconditions/src/.deps0/cryptoconditions.Plo |grep threshold.c
 cryptoconditions/src/threshold.c \
cryptoconditions/src/threshold.c:

So in one file the path is cryptocondtions/src/threshold.c (like it is called from komodo/src) and another path is src/threshold.c (like i was built from the cryptoconditions directory itself). So when the path was 'cryptocondtions/src/threshold.c' any modifications in threshold.c did not lead to rebuilding of the cryptoconditions lib (only 'make clean' helped) However I could not find how the first path case was created: when I rebuilt several times manually it was always the case two.

jmjatlanta commented 2 years ago

I believe the problem to be here: https://github.com/KomodoPlatform/komodo/blob/d7edae28b8f49de5c4ae6f7ab24b29fc5ab14320/src/Makefile.am#L83

My interpretation: If nothing in src/cryptoconditions/src or src/cryptoconditions/include changes, don't bother calling make in the src/cryptoconditions directory. That will basically ignore the dependencies in src/cryptoconditions/Makefile.am.

As src/cryptoconditions/Makefile.am is probably smarter about dependencies of the library than src/Makefile.am, my suggestion would be to always have the autotools equivalent of make -C src/cryptoconditions called each time, and let that Makefile determine if it needs to be rebuilt or not. I will find out the best way to accomplish that, and test it out.

Another side point is there are probably similar problems with libsnark, libsecp256k1, and libunivalue, although I doubt they change much.

jmjatlanta commented 2 years ago

This should fix it: https://github.com/jmjatlanta/komodo/commit/d3f78192b97ac9fec5002f682d21b4193a78206d (touching threshold.c causes a rebuild)

Note that a libcryptoconditions.la shared library is built, but doesn't seem to be used. There may be a history as to why, so I'll leave it be.

dimxy commented 2 years ago

added this to my tokel repo

DIST_SUBDIRS = secp256k1 univalue
SUBDIRS = cryptoconditions

seems this resolved cryptoconditions rebuild issues