horizontalsystems/unstoppable-wallet-android

if (tmpRawTransaction != null) {
    fullTransaction = adapter.send(tmpRawTransaction)

The base64 message from the swap API is signed as it is. Nothing checks that the fee payer and token authority are the user's wallet, or that the destination, mint and amount match the quote the user approved. A compromised or faulty provider response could move funds elsewhere.

Decode the message with the instruction parser the WalletConnect summary already has. Require the payer and authority to be the active wallet and the transfers to match the quote. Refuse unknown programs and instructions.

val transactionData = EvmTransactionData(
    to = signable.to ?: throw IllegalStateException("No tx `to`"),
    value = BigInteger((signable.value ?: "0x0").stripHexPrefix(), 16),

Only the THORChain memo destination is checked. For the other chains, the destination, value, operations and source account are taken from the API as they are. A compromised or faulty provider can have the wallet sign a transfer elsewhere.

Before signing, decode the payload and check that the value is at most the quoted input, the target is a known router or deposit address, and the recipient matches. Reject unexpected operations.

viewModelScope.launch(Dispatchers.Default) {
    val boc = transactionSigner.sign(sendRequestEntity, tonWallet)
    tonKitWrapper?.tonKit?.send(boc)

Signing and sending errors are uncaught, so the app crashes. If the kit is null, the send is skipped but the dApp is still told the request was approved. The UI shows success before anything is sent.

Make confirm a suspend function and await it. Catch errors and reject the request toward the dApp. Approve only after the send succeeds.

val tokenInfo = tokenProvider.getTokenInfo(reference)
...
decimals = tokenInfo.decimals

The send path scales amounts with these decimals. A wrong or spoofed API answer leads to mis-scaled transfers and a misleading symbol.

Read decimals and the token program owner from the mint account on chain. Use the API data only for display.

3 -> if (data.size >= 9) {
    transfers.add(Transfer(false, u64LE(1), null, payer = accountAt(2), destination = accountAt(1)))

Classic and Token-2022 programs are treated the same, the mint is never resolved, and plain Transfer amounts are shown as raw base units. A dApp can present a worthless token as an ordinary transfer row.

Resolve the mint (TransferChecked carries it; for Transfer, look up the source account's mint). Show the symbol and decimals only for known mints, and show the mint address otherwise. Check program, mint and instruction shape together.

val expectedBuyAmountOrZero: BigDecimal
    get() = expectedBuyAmount ?: BigDecimal.ZERO

When the server leaves out the expected amount, the confirm page shows that the user will receive 0, and route ranking treats the route as worth 0. A real zero and a missing value cannot be told apart.

Keep the value nullable and reject routes or quotes that lack it.

fun generateMac(derivedKey: ByteArray, cipherText: ByteArray): ByteArray {
    System.arraycopy(derivedKey, 16, result, 0, 16)

AES-CTR plus a Keccak MAC are combined by hand, and the MAC key overlaps the AES key. No break was demonstrated, but this design is easy to get wrong.

Use AES-256-GCM or ChaCha20-Poly1305 for new backups, or at least split the derived key into separate encryption and MAC keys. Keep the old format read-only.

val message = it.message?.takeIf { m -> m.isNotBlank() }
    ?: it::class.java.simpleName.ifBlank { "Error" }

Users see class names or English technical text. The same happens in Nav3.kt (lines 285 and 329) and WCSessionViewModel.setError.

Map known failures to localised messages and send the detail to the log.

targetSdk = libs.versions.compileSdk.get().toInt()

The app targets API 37, and users can add self-hosted nodes. No manifest or source file mentions ACCESS_LOCAL_NETWORK (all manifests and Kotlin sources were searched), so connections to LAN nodes will fail on newer Android.

Declare the permission, request it before connecting to a private-range host, and explain the failure if the user declines.

val amount = assetAmount.toBigDecimalOrNull() ?: BigDecimal.ZERO

A malformed payment request produces a zero-value transaction that is still offered for signing.

Show an error when the amount is missing, non-numeric or not positive.

suspend fun handle(scannedText: String, closeAppOnResult: Boolean = false) {
    val dAppRequest = kit.readData(scannedText)

A re-delivered intent opens a second connect prompt for the same request.

Keep a bounded set of handled request ids and ignore repeats.

if (cache.containsKey(value)) return true
...
return cache[value]!!

The cache is an unsynchronised map, and parseAddress depends on an earlier isSupported call. Concurrent calls, or a call order that changes, can throw a NullPointerException.

Use a ConcurrentHashMap and a single lookup with a null check. Resolve the name again instead of using !!.

if (!cache.containsKey(item.hashCode())) {

Two items whose hashes collide return the same view item.

Key the cache on a stable id and use getOrPut.

get() = preferences.getBoolean("chartIndicatorsEnabled", false)

Keys mix camelCase, kebab-case and snake_case as bare strings, which makes collisions and typos easy.

Define namespaced key constants in one place. Migrate stored values if you rename keys.

import androidx.compose.material.MaterialTheme

Material 3 components read defaults that the Material 2 theme never sets.

Bridge the app colours into a Material 3 theme, or finish the migration.

scrollToTopAfterUpdate = true
viewModel.onSelectSortingField(selected)

A coroutine is launched inside the LazyColumn content builder, and a flag is cleared during composition. The same pattern appears in several market screens.

Scroll from the click handler, or from a LaunchedEffect keyed on the sort change.

OverallScore.Excellent -> Color(0xFF05C46B)
OverallScore.Good -> Color(0xFFFFA800)

The same colours are repeated in OverallScoreInfoPage.kt, and the copies can drift apart.

Move them to named theme tokens.

<item name="android:actionMenuTextAppearance">@style/Headline2</item>

The night theme uses the AppCompat attribute without android:, so menu text styling can differ between the two themes. Both files also hard-code #6E7899.

Use the same attribute in both files and a colour resource.

val nightMode = when (themeType) {
    ThemeType.Light -> AppCompatDelegate.MODE_NIGHT_NO

The same mapping exists in core/App.kt, and the two copies can diverge.

Extract one ThemeType.toNightMode() function and call it from both places.

override fun import(accounts: List<Account>) {
...
    _accountsFlow.tryEmit(accounts)

The parameter shadows the full account list. Subscribers see only the restored subset. For example, "all backed up" can be computed as true while older accounts that were never backed up still exist.

Emit this.accounts, the full cache, after the import.

val currentAdapters = this.adaptersMap.toMutableMap()
this.adaptersMap.clear()

While the map is being refilled, getAdapter() returns nothing. Transaction lists can render empty, and spam scoring runs without context. The main adapter manager already swaps its map atomically.

Build a new map and swap it in as a single step.

private fun handleUpdateNetwork() {
    stop()
    _kitStoppedFlow.tryEmit(Unit)

getKitWrapper is synchronised and ends in kitWrapper!!, but stop() can set the wrapper to null concurrently, which throws a NullPointerException. The THORChain collector is also not exception-guarded, so one throw cancels the shared scope for every later kit.

Make handleUpdateNetwork and stop synchronised, as the sibling managers are. Wrap the THORChain collector body in try/catch.

fun addAll(coinUids: List<String>) {
    dao.insertAll(coinUids.map { FavoriteCoin(it) })

add() updates the manual order but addAll(), used by backup restore, does not. Restored coins jump to the top in Manual sort.

Route all favourite writes through one method that updates both the database and the order list.

blockchainSettingsStorage.save(syncSource.url, blockchainType)
...
blockchainSettingsStorage.saveMoneroNode(node.url)

The managers that restart kits and emit change events are bypassed. Running EVM, Tron and Monero kits keep the old source until the app restarts.

Call the sync-source and node manager save methods instead of the storage layer.

connectivityManager.registerNetworkCallback(NetworkRequest.Builder().build(), callback)

The same callback, with its activeNetworks list, is registered again on every foreground. Networks that disappeared while it was unregistered are never removed, so the app can report "connected" while offline.

Clear the list, or create a new callback, in willEnterForeground.

while (isActive) {
    syncPending()
    delay(POLL_INTERVAL_MS)

A stuck record is requested forever at the same rate, including after failures.

Add per-record backoff, an age cap for stale records, and foreground gating.

scannedTransactionStorage.getScannedTransaction(transactionHash)?.let {
    return it.isSpam
}

Changed thresholds or limits take effect only through hand-written migrations that wipe the table.

Store a scorer version with each row and rescore rows from older versions.

if (record.trackingHandle != null) return

Ownership is checked before a network call, and the status is written unconditionally afterwards. A send screen's concurrent status change can be overwritten.

Move the condition into the UPDATE's WHERE clause.

if (activeAccount?.id != activeAccountId) {

If the cached Account object is replaced but its id is unchanged, the stale object stays active.

Refresh activeAccount from the cache whenever the cache is rebuilt.

if (record.spam && spamManager.hideSuspiciousTx) return@forEach

The condition is repeated in TokenTransactionsService and TransactionFilterService, and a new reader can forget it.

Expose one isHidden(record) predicate and call it everywhere.

val type: String,
val words: SecretList?,
val key: SecretString?,

The type determines which secret column must be set, but nothing enforces or verifies this.

Document the invariant and add a startup or post-migration check.

val spamScore: Int,
val blockchainType: BlockchainType,
val address: String?

Address lookups depend on spam rows having an address, but the write path can store null.

Guard the write and add a verification query.

@Query("SELECT * FROM ScannedTransaction WHERE spamScore >= 7 AND address = :address LIMIT 1")

If the threshold constant changes, the queries and the scorer disagree.

Pass the threshold as a query parameter.

override var baseBitcoinProvider: String?
    get() = preferences.getString(BASE_BITCOIN_PROVIDER, null)

Several accessors, including an unused authToken slot, have no callers and suggest features that do not exist.

Delete them and their keys.

val result = sendTransactionService.sendTransaction(swapDefenseState.mevProtectionEnabled)
saveSwapRecord(result)

The database write and the recent-address write run after the funds are sent, with no error boundary. If either throws, the user sees a failure and can send again, which broadcasts a second transaction. SendV2ConfirmViewModel already guards this step.

Wrap post-broadcast bookkeeping in try/catch that logs the error and still returns the result.

FileOutputStream(file).use { fos ->
    fos.bufferedWriter().use { bw ->
        bw.write(asJsonString)

The file is truncated before the data is serialised. A crash, a full disk or an exception during serialisation leaves an empty or partial address book. The UI has already shown the change as saved, and the error is only logged.

Serialise first, write to a temporary file, then rename it atomically over the original (or use AtomicFile). Report write failures to the caller.

private var contactsMap: MutableMap<String, Contact> = mutableMapOf()

save and delete mutate the map on the caller thread while the writer thread iterates it. The startup load can also replace it. Results include a ConcurrentModificationException inside the hidden write, or a contact saved during startup being lost.

Confine the map to the single dispatcher or guard it with a Mutex, and make save wait for the initial load.

val bitmap = context.contentResolver.openInputStream(uri)
    ?.use { BitmapFactory.decodeStream(it) } ?: return null
val intArray = IntArray(bitmap.width * bitmap.height)

A large photo needs hundreds of MB. The resulting OutOfMemoryError is not caught by catch (Exception), so the app crashes or freezes.

Read the image bounds first, downsample to about 2048 px, decode off the main thread, and handle out-of-memory errors.

val jsonString = br.readText()
//validate json format
BackupFileValidator().validate(jsonString)

A very large file can freeze the UI. The same pattern appears in the backup manager and contacts screens.

Cap the size, read and parse on a background dispatcher, and parse once.

class USwapProvider(

At 1,209 lines, the class builds transactions for nine chains in the core module, although chain plugin modules exist.

Split out the API and DTOs, and move per-chain transaction construction behind the chain plugin interface.

class TransactionViewItemFactory(

At 1,457 lines, the factory holds the XRP, Stellar, TON, Solana and Tron converters in core, away from their chain modules.

Move one converter per chain into each chain module, and keep the factory as a dispatcher.

fun handleDeepLink(uri: Uri) {

The view model depends on an Android platform type, which makes it harder to test.

Convert the Uri to a String or parsed values in the Activity.

fun onClickSend() {
    viewModelScope.launch(Dispatchers.IO) {
        send()

Only the button composable stops a double send. If a second call reaches the view model, it would broadcast a second transaction.

Track the in-flight Job and return early while it is active.

suspend fun send(): SendTransactionResult = withContext(serviceDispatcher) {

The two 60-line blocks are near-identical, so a fix such as finding 33 has to be made twice.

Extract one shared send-with-record helper.

val qrScannerLauncher = rememberLauncherForActivityResult(ActivityResultContracts.StartActivityForResult()) { result: ActivityResult ->

The copies have already drifted: only one of them calls onScanQR.

Extract one reusable QR-scan button composable.

override val spamCoinValueLimits: Map<String, BigDecimal> = mapOf(
    "XLM" to BigDecimal("0.1"),

Tuning the spam limits or the swap fee requires a new release.

Consider serving a versioned rule config from the backend.

if (!tokenAutoEnableManager.autoEnableTokensOnReceive) return

val enabledWallets = newAssets.map { asset ->

Seven chain account managers repeat the same logic. TON drops blacklisted jettons, but Stellar and Tron enable any received asset and take its code from the sender. Someone can send a fake "USDT", and it appears in the user's wallet.

Extract one shared auto-enable helper with a per-chain acceptance hook, and apply the scam filtering on every chain.

builtApk=$workDir/app/app/build/outputs/apk/release/app-release-unsigned.apk

The release process depends on this script. The build now uses product flavours and a newer JDK, while the script installs JDK 11, runs :app:assembleRelease and expects an output path that no longer exists. The verification step cannot succeed.

Update the task name, output path and JDK, and run the script in CI on each release.

run: ./gradlew clean assembleCiRelease --stacktrace --no-daemon

No workflow runs ./gradlew test or lint (searched .github/).

Add a test job and make the build and publish jobs depend on it.

import io.horizontalsystems.walletkit.modules.multiswap.sendtransaction.toEvmTransactionData

Move the helper into the shared module and drop the dependency.

value = getFormattedValue(data.points.lastOrNull()?.tvl ?: return@let, currency),

A later edit can silently return from the wrong lambda and drop an analytics block.

Use explicit labels, or extract each block into a function.

Log.e("AAA", "billingClient.isReady: ${billingClient.isReady}")

Remove them or use debug level with a descriptive tag. The same applies in AdapterFactory.kt and ResultEventBus.kt.

Log.w("XRate", "Could not fetch xrate for ${key.coinUid}:${key.timestamp}, ${e.javaClass.simpleName}:${e.message}")

Remember failed keys for a cool-down period and log one summary per sync.

// Mirrors the registration in App.onCreate.
ChainRegistry.register(TronChainPlugin())

The test never registers the EVM plugins that the app does, so EVM regressions are not caught.

Call the app's real registration function from the test.

- name: Upload to GitHub Release

The release document requires .sha256 and .md5 assets, but nothing produces them (searched .github/ and docker/).

Generate and upload checksums after signing.

cp "${WORK_DIR}/${BUILT_APK_FILE}" "${PWD}/unstoppable_wallet_google_play_${TAG}.apk"

Check that the target does not exist (cp -n), or abort if it does.

buildConfigFieldString("TWITTER_BEARER_TOKEN", "AAAA…")
buildConfigFieldString("CHAINALYSIS_API_KEY", "928b…")
val uswapApiKeyAndroid = "a32d…"

The repository contains a Twitter bearer token, explorer and RPC keys, Chainalysis, HashDit, 1inch, swap-partner keys and the test keystore password. They stay in git history. Anyone can use them to exhaust quotas or impersonate the app to partners.

Rotate the keys. Inject them from CI secrets or an untracked local.properties. Proxy the paid or private ones through a backend, and restrict the rest by package and quota at each provider.

val timeInBackground = System.currentTimeMillis() - lastBackgroundTime

A negative elapsed time never reaches the lock interval. Someone holding the unlocked phone can set the clock back and reopen the wallet without the PIN.

Lock when the elapsed time is negative, and base the check on SystemClock.elapsedRealtime() plus a boot check.

val kdfParams = crypto.kdfparams
val key = EncryptDecryptManager.getKey(passphrase, kdfParams) ?: throw RestoreException.EncryptionKeyException

A crafted backup with a huge n or r exhausts memory. getKey only catches Exception, so the app crashes during restore.

Reject parameters outside fixed limits before deriving the key.

activeSubscriptions.clear()
if (!billingClient.isReady) {

When billing is disconnected on resume, paying users lose premium features until a later successful query. The list is also changed on a background thread while the UI reads it. In addition, the result of launchBillingFlow (line 250) is ignored. If it fails, the purchase coroutine never resumes.

Build a new list and swap it in only after a successful query. Resume with a failure when the launch result is not OK.

val transaction = stellarKit.getTransaction(xdr)

The XDR is decoded in createState(), outside the guarded action construction, so a connected dApp can crash the app. The sign-and-submit action and the fee estimation path have the same problem.

Decode the XDR in the constructor so that the existing try/catch handles failures, and bound its length.

nodeManager.addMoneroNode(url, username, password, true)

The wallet trusts data from any node the user adds, including a hostile one.

Add custom nodes as untrusted by default. Let users opt in explicitly, with a warning.

val hasRequiredProtocol = scheme == "https" || scheme == "http"

Node traffic, including alias answers, can be read or changed on the network. Monero and Zcash already require https or onion.

Allow http only for .onion hosts.

val baseUrl = zanoNodeManager.currentNode.host.trimEnd('/')

A hostile or intercepted node can return any address for an alias, and that address goes straight into the send flow.

Resolve aliases only through trusted https nodes, validate the returned address, and show it in full for confirmation.

String(message.hexStringToByteArray())

The hex message is converted to a String, and WCRequestEvmViewModel.kt:116 signs that String's UTF-8 bytes. For binary payloads such as hashes, the signature covers different bytes from the ones requested, so it is invalid.

Sign the original decoded bytes. Use the String only for display, with a hex fallback.

} catch (ex: Exception) {
    null
}

If decrypting a stored secret fails, the seed account vanishes from the UI with no log, while isAccountsEmpty still counts it. Users may think their wallet is gone.

Log and surface rows that cannot be decoded, and keep them recoverable.

delete(walletsToReAdd)
save(walletsToReAdd)

If the process dies between the two writes, the enabled wallets for that chain are lost. The one-slot flow that drops older values can also swallow the intermediate list, so adapters are not rebuilt.

Use one transaction plus an explicit rebuild signal.

addresses = contactJson.addresses.mapNotNull { addressJson ->
    marketKit.blockchain(addressJson.blockchain_uid)?.let { ContactAddress(it, addressJson.address) }

The load drops addresses whose blockchain is unknown at that moment. A failed read leaves the list empty. The next save then overwrites the file with what remains.

Keep unresolved entries as raw data. Do not overwrite the file after a partial or failed read.

object Translator {

Note: Strings resolve only from bundled resources. No override mechanism was found. If this is intentional, record the decision.