trustwallet / wallet-core

Cross-platform, cross-blockchain wallet library.
https://developer.trustwallet.com/wallet-core
Apache License 2.0
2.86k stars 1.6k forks source link

JNI crash in wallet.core.jni.HDWallet.mnemonic() #961

Closed dmdeklerk closed 4 years ago

dmdeklerk commented 4 years ago

Describe the bug Google playstore test center triggers JNI crash on some devices.

JNI DETECTED ERROR IN APPLICATION: fid == null
  in call to GetLongField
  from java.lang.String wallet.core.jni.HDWallet.mnemonic()

Calling code is Kotlin

fun createWallet(@NonNull input: HDWallet.CreateWalletInput, @NonNull result: Result) {
  val wallet = wallet.core.jni.HDWallet(input.strength, input.passphrase)
  val value = wallet.mnemonic()
  return GeneralImpl.sendResult(GeneralImpl.stringValue(value), result)
}

We use the published lib "implementation 'com.trustwallet:wallet-core:2.0.12'".

To Reproduce I was unable to reproduce this locally yet. Error is reported by automated tester system at google only.

Expected behavior Dont crash in JNI

Additional context logcat section cutout

05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427] JNI DETECTED ERROR IN APPLICATION: fid == null
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]     in call to GetLongField
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]     from java.lang.String wallet.core.jni.HDWallet.mnemonic()
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427] "main" prio=5 tid=1 Runnable
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   | group="main" sCount=0 dsCount=0 obj=0x7534da88 self=0x75dc095a00
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   | sysTid=10039 nice=-4 cgrp=default sched=0/0 handle=0x75dff1da98
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   | state=R schedstat=( 796894167 312348491 2679 ) utm=61 stm=17 core=3 HZ=100
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   | stack=0x7fecfb3000-0x7fecfb5000 stackSize=8MB
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   | held mutexes= "mutator lock"(shared held)
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   native: #00 pc 000000000047ae90  /system/lib64/libart.so (_ZN3art15DumpNativeStackERNSt3__113basic_ostreamIcNS0_11char_traitsIcEEEEiP12BacktraceMapPKcPNS_9ArtMethodEPv+220)
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   native: #01 pc 000000000047ae8c  /system/lib64/libart.so (_ZN3art15DumpNativeStackERNSt3__113basic_ostreamIcNS0_11char_traitsIcEEEEiP12BacktraceMapPKcPNS_9ArtMethodEPv+216)
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   native: #02 pc 000000000044f3bc  /system/lib64/libart.so (_ZNK3art6Thread9DumpStackERNSt3__113basic_ostreamIcNS1_11char_traitsIcEEEEbP12BacktraceMap+472)
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   native: #03 pc 00000000002eeacc  /system/lib64/libart.so (_ZN3art9JavaVMExt8JniAbortEPKcS2_+1128)
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   native: #06 pc 00000000004649cc  /data/app/com.heatwallet.heat_wallet-1/split_config.arm64_v8a.apk (???)
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   native: #07 pc 0000000000014160  /data/app/com.heatwallet.heat_wallet-1/oat/arm64/base.odex (Java_wallet_core_jni_HDWallet_mnemonic__+124)
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   at wallet.core.jni.HDWallet.mnemonic(Native method)
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   at com.heatwallet.heat_wallet.e$a.a(:-1)
05-18 05:35:09.310: A/art(10039): art/runtime/runtime.cc:427]   at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:889)

Note that not all tested devices trigger these crashes.

Screenshot 2020-05-18 at 16 46 33
dmdeklerk commented 4 years ago

Our own generated protobuf classes are named HDWallet as well. Which is in @NonNull input: HDWallet.CreateWalletInput

dmdeklerk commented 4 years ago

Found a similar crash report.

JNI DETECTED ERROR IN APPLICATION: fid == null
   in call to GetLongField
   from wallet.core.jni.PrivateKey wallet.core.jni.HDWallet.getKey(java.lang.String)

Calling code

fun getKeyPair(@NonNull input: HDWallet.GetKeyPairInput, @NonNull result: Result) {
   val wallet = wallet.core.jni.HDWallet(input.mnemonic, input.passphrase)
   val privateKey = wallet.getKey(input.derivationPath)
05-18 05:37:51.830: A/zygote(9330): runtime.cc:531] JNI DETECTED ERROR IN APPLICATION: fid == null
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]     in call to GetLongField
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]     from wallet.core.jni.PrivateKey wallet.core.jni.HDWallet.getKey(java.lang.String)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531] "main" prio=5 tid=1 Runnable
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   | group="main" sCount=0 dsCount=0 flags=0 obj=0x7266a418 self=0xa8b2c000
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   | sysTid=9330 nice=-4 cgrp=default sched=0/0 handle=0xad2be4a4
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   | state=R schedstat=( 3553467611 1237922239 5671 ) utm=239 stm=115 core=1 HZ=100
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   | stack=0xbe471000-0xbe473000 stackSize=8MB
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   | held mutexes= "mutator lock"(shared held)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   native: #00 pc 002e8097  /system/lib/libart.so (art::DumpNativeStack(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, int, BacktraceMap*, char const*, art::ArtMethod*, void*)+130)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   native: #01 pc 00378ec1  /system/lib/libart.so (art::Thread::DumpStack(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, bool, BacktraceMap*, bool) const+204)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   native: #02 pc 003755f7  /system/lib/libart.so (art::Thread::Dump(std::__1::basic_ostream<char, std::__1::char_traits<char>>&, bool, BacktraceMap*, bool) const+34)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   native: #03 pc 0024d50d  /system/lib/libart.so (art::JavaVMExt::JniAbort(char const*, char const*)+720)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   native: #05 pc 00297d57  /system/lib/libart.so (art::JNI::GetLongField(_JNIEnv*, _jobject*, _jfieldID*)+586)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   native: #06 pc 003b0dd9  /data/app/com.heatwallet.heat_wallet-8ndqRJzdolqwEguLNDS6iA==/split_config.armeabi_v7a.apk (???)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   native: #07 pc 000016c5  /data/app/com.heatwallet.heat_wallet-8ndqRJzdolqwEguLNDS6iA==/oat/arm/base.odex (Java_wallet_core_jni_HDWallet_getKey__Ljava_lang_String_2+92)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at wallet.core.jni.HDWallet.getKey(Native method)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at com.heatwallet.heat_wallet.e$a.a(:-1)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at com.heatwallet.heat_wallet.MainActivity$a.a(:-1)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at d.a.b.a.j$a.a(:-1)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at io.flutter.embedding.engine.e.b.a(:-1)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at io.flutter.embedding.engine.FlutterJNI.handlePlatformMessage(:-1)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at android.os.MessageQueue.nativePollOnce(Native method)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at android.os.MessageQueue.next(MessageQueue.java:325)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at android.app.ActivityThread.main(ActivityThread.java:6523)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at java.lang.reflect.Method.invoke(Native method)
05-18 05:37:51.831: A/zygote(9330): runtime.cc:531]   at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:438)
hewigovens commented 4 years ago

could you share more contexts (how is the test executed, sample project to reproduce)? the code snippet looks quite normal

optout21 commented 4 years ago

Do these happen in the beginning of the tests? My first guess would be that this is related to how test environment is initialized.

dmdeklerk commented 4 years ago

The test is conducted through the google playstore submission system. They have an array of virtual and physical devices on which they run a crawler that just clicks away and enters random data in any app you submit.

If you share me your email I can send you the logcat output of the test and other information.

We are in the process of setting up test infrastructure to catch this better.

dmdeklerk commented 4 years ago

The finger print of the crash resembles this issue here.. https://stackoverflow.com/questions/13845430/code-flow-in-java-using-jni

From the JNI specification:

A pending exception raised through the JNI (by calling ThrowNew, for example) does not immediately disrupt the native method execution. This is different from how exceptions behave in the Java programming language. When an exception is thrown in the Java programming language, the virtual machine automatically transfers the control flow to the nearest enclosing try/catch statement that matches the exception type. The virtual machine then clears the pending exception and executes the exception handler. In contrast, JNI programmers must explicitly implement the control flow after an exception has occurred.

I imagine this could possibly be triggered by a race condition where wallet-core embedders invoke various in-paralel functions over for instance (our case) a Flutter asynchronous method channel. Where both paralel calls call into wallet-core, the one call will not be finished while the other raises an exception at the c++ level.

If this is the case it would mean wallet-core functions can only ever be invoked semi synchronously and developers on frameworks like Flutter should account for that.

Question: How does Trust Wallet itself invoke the code in the wallet-core libraries? Does it do that in paralel or is there a mechanism in place to only ever invoke one foreign function at a time and always deal with its exceptions?

Stack Overflow
Code flow in Java using JNI
I have the code on JNI level. And it can throws exceptions. The code is: #include "JavaGlueClass.h" #include <stdio.h> #include <windows.h> #include <string.h> jint
dmdeklerk commented 4 years ago

Do these happen in the beginning of the tests? My first guess would be that this is related to how test environment is initialized.

Nope. Tests run for a while (10 ish seconds) or longer or shorter

optout21 commented 4 years ago

I understood this is happening in a test farm with physical devices. I am saying that the root cause may lie somewhere else, not in the spot where the crash happens -- that may be sporadic, as it is sporadic on which devices it occurs (my guess it is not tied to specific devices/models, but more random). It may be that something is not fully correct with initialization -- and this is only a guess -- either in wallet core lib, or heat wallet app, or test environment.

You can contact me over Telegram, id catenocrypt (I would not leave an email address here).

Thanks for contributing to testing of walletcore, and reporting errors.

dmdeklerk commented 4 years ago

Understood, I will research further and contact you once I figure out more. Thanks!

hewigovens commented 4 years ago

or join https://t.me/trust_developers

Telegram
Trust Developers - Public
Public discussion for Trust Developers. Documentation available: developer.trustwallet.com Add new blockchain: developer.trustwallet.com/wallet-core/newblockchain
optout21 commented 4 years ago

I've added below code snippet to walletcore android sample app. I got some weird logs, no explicit crash, but it did not go all the way.

https://github.com/trustwallet/wallet-core/blob/master/samples/android/app/src/main/java/com/trust/walletcore/example/MainActivity.kt

        var sum = 0
        for (t in 0..20) {
            thread(start = true) {
                showLog("${t} ${sum} Mnemonic: ${wallet.mnemonic()}")
                for (i in 0..1000) {
                    val m = wallet.mnemonic()
                    val addressEth = wallet.getAddressForCoin(coinEth)
                    sum++
                }
                showLog("${t} ${sum} Mnemonic: ${wallet.mnemonic()}")
            }
        }
2020-05-19 11:19:16.140 23831-23904/com.trust.walletcore.example I/System.out: 16 19 Mnemonic: ...
2020-05-19 11:19:16.280 23831-23831/com.trust.walletcore.example W/letcore.exampl: Accessing hidden field Ljava/nio/Buffer;->address:J (light greylist, reflection)
2020-05-19 11:19:16.281 23831-23831/com.trust.walletcore.example W/b.a.a.r2: platform method missing - proto runtime falling back to safer methods: java.lang.NoSuchMethodException: copyMemory [class java.lang.Object, long, class java.lang.Object, long, long]

So there is an issue with JNI wrapper called in parallel.

GitHub
trustwallet/wallet-core
Cross-platform, cross-blockchain wallet library. Contribute to trustwallet/wallet-core development by creating an account on GitHub.
dmdeklerk commented 4 years ago

I was able to solve this. The problem was with R8 messing up the JNI code. For any flutter developers hitting this issue while using wallet-core. Either disable R8 completely.

flutter build appbundle --release --no-shrink

Or better fine tune R8 to keep wallet-core JNI code intact.

jayateertha043 commented 3 years ago

Hey how did you integrate the SDK with flutter ? Method channel or any plugins?

jayateertha043 commented 3 years ago

@dmdeklerk

dmdeklerk commented 3 years ago

Method channel @jayateertha043 , combined with Protobuf

VictorZhang2014 commented 3 years ago

Hi @dmdeklerk To build apk without shrinkResources false and minifyEnabled false can solve the crash. That's good! But we still want to obfuscate our code, and resources shrinker is not so important, then in this case, how do we enable minifyEnabled property?

ne3Vubeki commented 2 years ago

Hello, in my plugin when using TrustWalletCore, an error occurs:

java_vm_ext.cc:577]     JNI DETECTED ERROR IN APPLICATION: mid == null
java_vm_ext.cc:577]     in call to CallIntMethod
java_vm_ext.cc:577]     from int wallet.core.jni.BitcoinScript.hashTypeForCoin(wallet.core.jni.CoinType)

Methods for fixes indicated in the issue: https://github.com/trustwallet/wallet-core/issues/961 don't help. How can this error be corrected?

ne3Vubeki commented 2 years ago
import wallet.core.jni.BitcoinScript
import wallet.core.jni.proto.Bitcoin
import wallet.core.jni.proto.Tron
import wallet.core.jni.proto.Ethereum

import android.os.Build
import androidx.annotation.RequiresApi
import kotlin.math.round

/** WalletTrustPlugin */
class WalletTrustPlugin: FlutterPlugin, MethodCallHandler {

init {
System.loadLibrary("TrustWalletCore")
}

private lateinit var channel : MethodChannel
private val bitcoinCashType = CoinType.BITCOINCASH
private val bitcoinScript = BitcoinScript.hashTypeForCoin(CoinType.BITCOINCASH)
afterlifehy commented 2 years ago

@ne3Vubeki @dmdeklerk I solved this problem,By not confusing trustwallet, the program can run

-keep public class com.trustwallet.core.{*;} -keep public class wallet.core.*{;} -keep public class org.web3j.{*;}