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

Cannot reproduce 3.11.1 for Android #103

Closed
Giszmo opened this issue Nov 9, 2021 · 18 comments
Closed

Cannot reproduce 3.11.1 for Android #103

Giszmo opened this issue Nov 9, 2021 · 18 comments

Comments

@Giszmo
Copy link

Giszmo commented Nov 9, 2021

Sadly the issue with the minified main.js keeps cropping up and I wonder why and how to avoid it.

Objectionable diff is:

Files /tmp/fromPlay_it.airgap.vault_35477/assets/public/index.html and /tmp/fromBuild_it.airgap.vault_35477/assets/public/index.html differ
Only in /tmp/fromPlay_it.airgap.vault_35477/assets/public: main.6c0c07ffb31b2f9117c8.js
Only in /tmp/fromBuild_it.airgap.vault_35477/assets/public: main.6fac43b747228db945b3.js

In index.html only the main.js file names differ but the two main.*.js files differ in dozens of chunks (after preffifying) and with them being minified, it's hard to just trust them. Would not minifying be an option? Although that wouldn't fix reproducibility issues, it would help understand what's happening and why.

I noticed that dependencies are not well pinned. Not only aren't they pinned to a cryptographic hash of the dependency, they are not even pinned to a version number. From https://github.com/airgap-it/airgap-vault/blob/abbed9486d42fc10279018ec789566b71cf9cce2/package.json:

  "dependencies": {
    ...
    "@angular/common": "^11.2.9",
    "@angular/core": "^11.2.9",
    "@angular/forms": "^11.2.9",
@Giszmo
Copy link
Author

Giszmo commented Nov 9, 2021

Same issue for 3.11.2

@AndreasGassmann
Copy link
Member

Thanks for the report. I would love to track down the root cause of this as well.

We will discuss the removal of the minification step, but as you said, this will not resolve the original problem.

The versions in the package.json file are not locked, but the ones in the yarn.lock file are. During our build, we use the --frozen-lockfile flag, which should make sure that the exact dependencies are installed. https://classic.yarnpkg.com/en/docs/cli/install#yarn-install---frozen-lockfile-. This has to be done, because locking the versions in the package.json file means that only the direct dependencies are locked. All sub-dependencies would still always use the latest versions, so that's what the lock file is for.

@mohammadrafigh
Copy link
Contributor

@AndreasGassmann I tried 3.11.2 with recent changes on server and the npm error has gone, yarn did the trick. But unfortunately the steps (layers) of building Airgap docker image are getting huge, so applying simple changes on previous layer (like a simple cp command) takes a lot of time to be completed by server. So 60 mins of timeout was not enough to reproduce the apk. I increased the timeout to 90 mins but still we get timeout and a build failure from gradle at STEP 31 RUN /app/android/gradlew --project-dir /app/android build.

Here are the logs:

> Task :app:stripAppiumDebugSymbols
Unable to strip the following libraries, packaging them as they are: libsapling.so, libsqlc-ndk-native-driver.so, libtool-checker.so.

> Task :app:extractAppiumNativeSymbolTables
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86/libsapling.so because unable to locate the objcopy executable for the x86 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86/libsqlc-ndk-native-driver.so because unable to locate the objcopy executable for the x86 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86/libtool-checker.so because unable to locate the objcopy executable for the x86 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/arm64-v8a/libsapling.so because unable to locate the objcopy executable for the arm64-v8a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/arm64-v8a/libsqlc-ndk-native-driver.so because unable to locate the objcopy executable for the arm64-v8a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/arm64-v8a/libtool-checker.so because unable to locate the objcopy executable for the arm64-v8a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/armeabi-v7a/libsapling.so because unable to locate the objcopy executable for the armeabi-v7a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/armeabi-v7a/libsqlc-ndk-native-driver.so because unable to locate the objcopy executable for the armeabi-v7a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/armeabi-v7a/libtool-checker.so because unable to locate the objcopy executable for the armeabi-v7a ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86_64/libsapling.so because unable to locate the objcopy executable for the x86_64 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86_64/libsqlc-ndk-native-driver.so because unable to locate the objcopy executable for the x86_64 ABI.
Unable to extract native debug metadata from /app/android/app/build/intermediates/merged_native_libs/appium/out/lib/x86_64/libtool-checker.so because unable to locate the objcopy executable for the x86_64 ABI.

> Task :app:mergeAppiumNativeDebugMetadata NO-SOURCE
> Task :capacitor-app-launcher:bundleLibRuntimeToJarRelease
> Task :capacitor-clipboard:bundleLibRuntimeToJarRelease
> Task :capacitor-splash-screen:bundleLibRuntimeToJarRelease
> Task :capacitor-status-bar:bundleLibRuntimeToJarRelease
> Task :app:checkAppiumDuplicateClasses
> Task :app:compressAppiumAssets
> Task :app:dexBuilderAppium
> Task :app:desugarAppiumFileDependencies
Reproducibility check timed out!

Please let me know if we should increase the build timeout to 120 mins if it helps, Anyway I think some Dockerfile steps could be collapsed.

@AndreasGassmann
Copy link
Member

I didn't create the docker files and I'm not a docker expert, but we use variations of this setup for all our projects. I just checked, and in our internal gitlab runner, the android build takes less than 8 minutes: https://github.com/airgap-it/airgap-vault/blob/master/.gitlab-ci.yml#L42-L62

I can see in the logs that a few layers were cached, so I'm not sure how long the build would take if nothing was cached. But we have our timeout set to 1 hour, and we never hit that limit. If I remember correctly, when I ran the walletscrutiny script last time in my VM, it took around 20 minutes?

Could you increase the limit to 120m or even higher once, so we can see if it EVER finishes? Maybe it just hangs at a specific point and will never finish.

Do you have any suggestions what we could change in our build to make it more efficient?

@mohammadrafigh
Copy link
Contributor

I will take a look and update you.

@Giszmo
Copy link
Author

Giszmo commented Nov 14, 2021

For what it's worth, build now worked with the server. It had run out of RAM and we upgraded to 16GB.

mohammadrafigh added a commit to mohammadrafigh/airgap-vault that referenced this issue Nov 15, 2021
airgap-it#103 
This commit just removes unnecessary docker layers which results in smaller image and faster builds.
@mohammadrafigh
Copy link
Contributor

Sent a PR with minor changes in Dockerfile to remove unnecessary layers.
As @Giszmo the build is working on server but 3.11.2 is not reproducible due to some diffs in index.html, please check the logs with your API key in Walletscrutiny developers panel.

@AndreasGassmann
Copy link
Member

Thanks for looking into this and thanks @mohammadrafigh for the PR. I will look into it ASAP.

@AndreasGassmann
Copy link
Member

So I did some investigation yesterday and what I found is that it seems that the order of code is random in some cases. The diff was 3000 lines long, but there was only 1 huge chunk that was in a different place (but same code), and then a few individual lines that were different because the variable name of that chunk was different.

In my case, it was one specific package that was "in the wrong place". I will continue to investigate why that is.

I also disabled the optimisation step, but the file size of main.x.js doubled, so this isn't a feasible solution either.

@mohammadrafigh
Copy link
Contributor

Ok since 3.12.0 is reproducible then if more investigation is not needed, you can close this issue.

@AndreasGassmann
Copy link
Member

Thanks for all the help.

@mohammadrafigh
Copy link
Contributor

mohammadrafigh commented Nov 28, 2021

@AndreasGassmann Something random is happening, in your private test 3.12.0 was reproducible but the play store version seems to be not reproducible again please take a look at these logs:

===== Begin Results =====
appId:          it.airgap.vault
signer:         486381324d8669c80ca9b8c79d383dc972ec284227d65ebfe9e31cad5fd3f342
apkVersionName: 3.12.0
apkVersionCode: 36191
verdict:        
appHash:        2b5775c3d7d569d88b0c023d9f45dad1eca7902271dbba5b0c93bee0710851b2
commit:         cb039b59d211077c9568701afef7339ac34ef1f5

Diff:
Only in /tmp/fromPlay_it.airgap.vault_36191/assets/public: 3.8c9df035d827f0551568.js
Only in /tmp/fromBuild_it.airgap.vault_36191/assets/public: 3.ed396a67eea487509dc8.js
Files /tmp/fromPlay_it.airgap.vault_36191/assets/public/index.html and /tmp/fromBuild_it.airgap.vault_36191/assets/public/index.html differ
Only in /tmp/fromPlay_it.airgap.vault_36191/assets/public: main.58e5cb7e57243ca85507.js
Only in /tmp/fromBuild_it.airgap.vault_36191/assets/public: main.cf1142ad441b50c70620.js
Only in /tmp/fromBuild_it.airgap.vault_36191/assets/public: runtime.5cbd2fc5de80d469883b.js
Only in /tmp/fromPlay_it.airgap.vault_36191/assets/public: runtime.8d962b508f1357b3af89.js
Only in /tmp/fromPlay_it.airgap.vault_36191/META-INF: MANIFEST.MF
Only in /tmp/fromPlay_it.airgap.vault_36191/META-INF: PAPERS.RSA
Only in /tmp/fromPlay_it.airgap.vault_36191/META-INF: PAPERS.SF

Revision, tag (and its signature):
object cb039b59d211077c9568701afef7339ac34ef1f5
type commit
tag v3.12.0
tagger Andreas Gassmann  1637446858 +0100

v3.12.0
===== End Results =====

Do you use a specific API key or secret which is different between your production and development environment? maybe this is the case?

@AndreasGassmann
Copy link
Member

Did you compare the hash of the APK that I uploaded and the one from the play store? Unless google changed something during the release process, they must be the same. I actually downloaded the APK from the google play store developer panel while it was in review and uploaded that exact version to walletscrutiny to check if it was reproducible. I had to do that because I wasn't the one who signed the APK, so this was the easiest way for me to get the signed APK.

@mohammadrafigh
Copy link
Contributor

here is the hash of the APK in public state:
2b5775c3d7d569d88b0c023d9f45dad1eca7902271dbba5b0c93bee0710851b2
and it's the hash of the apk you uploaded using your api key:
bd5fed5058e007082ded70d520abbc3112022c33b77c43ed7ba09459180ddc20

The hashes are different which is weird. for more info please check developers panel, you can see both test results for public (without api key) and your private test.

@Giszmo
Copy link
Author

Giszmo commented Nov 28, 2021

It would be good to compare the two APKs with diffoscope. I have seen the hash change after upload to Google Play but it was because of different compression or something as the content was identical.

@AbakumovAlexandr
Copy link

@AndreasGassmann Is there any updates\progress on this?

The walletscrutiny status is still not good.

@AndreasGassmann
Copy link
Member

This is an old issue, let's continue this here: #127

@AbakumovAlexandr
Copy link

Great, let's track it there! Thank you!

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

No branches or pull requests

4 participants