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

Move the enclaves into the same build directory #3775

Merged
merged 1 commit into from
Dec 12, 2023

Conversation

nick-mobilecoin
Copy link
Collaborator

@nick-mobilecoin nick-mobilecoin commented Dec 6, 2023

Moving the enclaves into the same build directory reduces compilation
times on this developer's machine from 7m45s to 6m31s. Which is ~15%
improvement. It also reduces the build directory from 76GiB to 71GiB.

@nick-mobilecoin
Copy link
Collaborator Author

nick-mobilecoin commented Dec 6, 2023

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@nick-mobilecoin
Copy link
Collaborator Author

nick-mobilecoin commented Dec 6, 2023

This is a cheap win on build performance.

In the future it would be nice to:

  1. consolidate to one workspace and thus one Cargo.lock file for the enclaves. The lock files are needing to be updated for common dependencies and they are almost all identical, the consensus one has a few more dependency packages.
  2. Move the sim generated enclave signing certificate into the enclave build directory so that it is rebuilt properly between builds. Currently it's in the source directory as it's the easiest way to find it, if the "common" enclave workspace owns it then it should be easy for all targets to find it.

Copy link
Contributor

@eranrund eranrund left a comment

Choose a reason for hiding this comment

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

LGTM, but I am not sure how to really verify there aren't any nuances we are missing... for example different enclaves building the same crate with different features. I suspect this kind of stuff should be fine, and I suppose we'll find out rather quickly if not.

@nick-mobilecoin
Copy link
Collaborator Author

nick-mobilecoin commented Dec 6, 2023

LGTM, but I am not sure how to really verify there aren't any nuances we are missing... for example different enclaves building the same crate with different features. I suspect this kind of stuff should be fine, and I suppose we'll find out rather quickly if not.

Right now what I'm doing "locally" is

cargo build --release \
        -p mc-consensus-enclave-measurement \
        -p mc-fog-ingest-enclave-measurement \
        -p mc-fog-ledger-enclave-measurement \
        -p mc-fog-view-enclave-measurement

Then cleaning and building each one individually and seeing how they compare binary, hopefully there's no date info in there
(edit: I'm doing that right now and if they are teh same I'll merge)

@nick-mobilecoin
Copy link
Collaborator Author

None are the same

nick@8bedbc26e0f2:/tmp/mobilenode$ md5sum all_together/docker/release/*.so
6d4ea37081d7e700cf84772dd5d94442  all_together/docker/release/libconsensus-enclave.signed.so
5b1a6fc0fedfe478bb333fb342082769  all_together/docker/release/libconsensus-enclave.so
5e0190f61f2f7a3d2574c9133ad985ca  all_together/docker/release/libingest-enclave.signed.so
7f7ffda101ec135dff35da8b111c1cb1  all_together/docker/release/libingest-enclave.so
6b170aa621979d2e6c4376b58097ed03  all_together/docker/release/libledger-enclave.signed.so
0defa8876477580ae844668459c8b0ad  all_together/docker/release/libledger-enclave.so
9244f94501a56121e84c4952e4179d29  all_together/docker/release/libview-enclave.signed.so
4a5f6a43b6370d6ce70f39f4d89049b0  all_together/docker/release/libview-enclave.so
nick@8bedbc26e0f2:/tmp/mobilenode$ md5sum consensus-target/docker/release/*.so
f7975adc2f5ea3d1817e57918877b5ff  consensus-target/docker/release/libconsensus-enclave.signed.so
4ee7df40c756f93f31e8d5e3b29fbe45  consensus-target/docker/release/libconsensus-enclave.so
nick@8bedbc26e0f2:/tmp/mobilenode$ md5sum fog-ingest-enclave-target/docker/release/*.so
7a78dd97ea649f9b3505301e147e7fef  fog-ingest-enclave-target/docker/release/libingest-enclave.signed.so
d48ed621f4994db3e0a605d4f7bd545b  fog-ingest-enclave-target/docker/release/libingest-enclave.so
nick@8bedbc26e0f2:/tmp/mobilenode$ md5sum fog-ledger-enclave-target/docker/release/*.so
0054d22fdef3383d793ac4eb6e29032d  fog-ledger-enclave-target/docker/release/libledger-enclave.signed.so
137cafcfa69d2f8ed9dc1c95379a9316  fog-ledger-enclave-target/docker/release/libledger-enclave.so
nick@8bedbc26e0f2:/tmp/mobilenode$ md5sum fog-view-enclave-target/docker/release/*.so
e859f2131fee41ba529311b63df8215c  fog-view-enclave-target/docker/release/libview-enclave.signed.so
2eaf0b223e93c7c528f23628c37e1488  fog-view-enclave-target/docker/release/libview-enclave.so

I'll go to main and see what it gives

@nick-mobilecoin
Copy link
Collaborator Author

first run on main

nick@8bedbc26e0f2:/tmp/mobilenode$ md5sum target/docker/release/*.so
11fe641834b85e3b5128e5f27a924286  target/docker/release/libconsensus-enclave.signed.so
e7a3cba1c35aa5c4ff60f7e1d12bed1c  target/docker/release/libconsensus-enclave.so
af3aa85e2bb76d98f8f824212c63aa1c  target/docker/release/libingest-enclave.signed.so
8bfeea289db566b9c96c9bd1431b30cd  target/docker/release/libingest-enclave.so
d6ea379a7acc149ef83edb3e349bb580  target/docker/release/libledger-enclave.signed.so
e6cc428758fd9f61425f0269af7a2b82  target/docker/release/libledger-enclave.so
0e7d965bf1fd7f92d916ea51f4e25d2d  target/docker/release/libview-enclave.signed.so
13462b6f597ee20492b15cf8e70c8b13  target/docker/release/libview-enclave.so

Second run

nick@8bedbc26e0f2:/tmp/mobilenode$ md5sum target/docker/release/*.so
3980efbf87e9319eda5b2ae0749edc8f  target/docker/release/libconsensus-enclave.signed.so
a4d027d9e5b01b358d90bbdbdb54c56c  target/docker/release/libconsensus-enclave.so
8764c40e0479ecf3837c3d8cc10b57b3  target/docker/release/libingest-enclave.signed.so
e4f6c2425a8a09e69211284597e98771  target/docker/release/libingest-enclave.so
0f58c148b856896fd4fb683205e6b510  target/docker/release/libledger-enclave.signed.so
01956838c7ad08763e5ca4fe04b8e819  target/docker/release/libledger-enclave.so
357697a968f0cf7b902fe2b8f422e7ec  target/docker/release/libview-enclave.signed.so
01ebf074a575269e201960c8604ec9d9  target/docker/release/libview-enclave.so

I'll do some thinking, clearly there is a timestamp in those binaries

@nick-mobilecoin
Copy link
Collaborator Author

Looking into this more, most importantly is the enclave signing material, as it needs to be consistent in order for the css and signature to be repeatable. The below is from the current main without the enclave changes.
This is running cargo build --release -p <enclave> -p <enclave ... so trying to build them all at the same time.
I'm going to jump back to 5.0.x and repeat to see what values I get

nick@22db8ed0d02f:/tmp/mobilenode$ md5sum all-enclaves-main-hw-mode/release/*.dat
181eeda941c63129d449e5f756c46052  all-enclaves-main-hw-mode/release/consensus-enclave.dat
4c262b09f806f3e4119262e6c2073990  all-enclaves-main-hw-mode/release/ingest-enclave.dat
92fefe0f7f231b128454538f5821f995  all-enclaves-main-hw-mode/release/ledger-enclave.dat
2891ff1fa587047e5802529461a5b121  all-enclaves-main-hw-mode/release/view-enclave.dat
nick@22db8ed0d02f:/tmp/mobilenode$ md5sum all-enclaves-main-hw-mode-run2/release/*.dat
4d3d57defe921e693dcf73a7622b2d37  all-enclaves-main-hw-mode-run2/release/consensus-enclave.dat
4c262b09f806f3e4119262e6c2073990  all-enclaves-main-hw-mode-run2/release/ingest-enclave.dat
faa094f711fd8db85d0ca3fc48c7b942  all-enclaves-main-hw-mode-run2/release/ledger-enclave.dat
e6e689d47a7e811bede5e15e5819c719  all-enclaves-main-hw-mode-run2/release/view-enclave.dat
nick@22db8ed0d02f:/tmp/mobilenode$ md5sum all-enclaves-main-hw-mode-run3/release/*.dat
4d3d57defe921e693dcf73a7622b2d37  all-enclaves-main-hw-mode-run3/release/consensus-enclave.dat
f34c4871dd71f2a0d7407a0678445211  all-enclaves-main-hw-mode-run3/release/ingest-enclave.dat
d805827cc1870a2c6048bc5d6a05a15c  all-enclaves-main-hw-mode-run3/release/ledger-enclave.dat
5ba6fd5a45edba28327bedd4fd1517ba  all-enclaves-main-hw-mode-run3/release/view-enclave.dat

@nick-mobilecoin
Copy link
Collaborator Author

nick-mobilecoin commented Dec 11, 2023

Ok after finding and fixing, mobilecoinfoundation/sgx#455

I'm able to get the same binary outputs for the enclaves "consolidated" is the change from this PR

nick@3e1af13bf9d9:/tmp/mobilenode$ md5sum fixed_main/release/*.dat
a3b32eade5db43e109578fcbe7e542a1  first_run/release/consensus-enclave.dat
8161788f23b066d7c59fae9b3e35c049  first_run/release/ingest-enclave.dat
fbaeaa096ad1b4a70b94e1644514442b  first_run/release/ledger-enclave.dat
d18a19ba787bb5532c46a148926e5d32  first_run/release/view-enclave.dat
nick@3e1af13bf9d9:/tmp/mobilenode$ md5sum consolidated/release/*.dat
a3b32eade5db43e109578fcbe7e542a1  consolidated/release/consensus-enclave.dat
8161788f23b066d7c59fae9b3e35c049  consolidated/release/ingest-enclave.dat
fbaeaa096ad1b4a70b94e1644514442b  consolidated/release/ledger-enclave.dat
d18a19ba787bb5532c46a148926e5d32  consolidated/release/view-enclave.dat
nick@3e1af13bf9d9:/tmp/mobilenode$ md5sum fixed_main/release/*.so
52a75138d36429339e189043d8424c75  first_run/release/libconsensus-enclave.so
01ae2b84d0423d3035abdb6abaa081f7  first_run/release/libingest-enclave.so
7117a0a344764e7765c6cc20d1b00c62  first_run/release/libledger-enclave.so
d140a5e856c7e3b322a1255ebb64dad7  first_run/release/libview-enclave.so
nick@3e1af13bf9d9:/tmp/mobilenode$ md5sum consolidated/release/*.so
52a75138d36429339e189043d8424c75  consolidated/release/libconsensus-enclave.so
01ae2b84d0423d3035abdb6abaa081f7  consolidated/release/libingest-enclave.so
7117a0a344764e7765c6cc20d1b00c62  consolidated/release/libledger-enclave.so
d140a5e856c7e3b322a1255ebb64dad7  consolidated/release/libview-enclave.so

@nick-mobilecoin nick-mobilecoin changed the base branch from main to nick/fix-reproducible-builds December 11, 2023 22:25
Base automatically changed from nick/fix-reproducible-builds to main December 11, 2023 23:51
Moving the enclaves into the same build directory reduces compilation
times on this developer's machine from 7m45s to 6m31s. Which is ~15%
improvement. It also reduces the build directory from 76GiB to 71GiB.
@nick-mobilecoin nick-mobilecoin enabled auto-merge (squash) December 12, 2023 00:44
@nick-mobilecoin nick-mobilecoin merged commit 2415592 into main Dec 12, 2023
22 checks passed
@nick-mobilecoin nick-mobilecoin deleted the nick/combine-enclaves branch December 12, 2023 15:07
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