Update Zcash SDK to v2.7.0-rc.2(Ironwood) and implement fee recalculation for unified balances

Author: oleonov

app/src/main/java/cash/p/terminal/core/adapters/zcash/ZcashAdapter.kt

@@ -172,6 +172,11 @@ class ZcashAdapter(
     private val balanceCheckMutex = Mutex()
     private var importUfvkError : Throwable? = null
 
+    private val feeLock = Any()
+    private var feeJob: Job? = null
+    private var lastFeeSnapshot: AccountBalance? = null
+    private var feeGeneration = 0L
+
     companion object {
         private const val DECIMAL_COUNT = 8
         private val DATABASE_CORRUPTION_MESSAGES = listOf(
@@ -181,6 +186,13 @@ class ZcashAdapter(
         )
 
         val MINERS_FEE = ZcashSdk.MINERS_FEE.convertZatoshiToZec(DECIMAL_COUNT)
+
+        // The fee probe steps down by MINERS_FEE (10,000 zat) per attempt. With funds spread
+        // over several pools ZIP-317 charges per bundle and per input, so a near-max send
+        // easily exceeds the former 4 steps (40,000 zat). 20 attempts cover fees up to
+        // 200,000 zat; the probe only runs to the end in the rare case where the whole
+        // balance cannot be sent at all.
+        private const val FEE_PROBE_ATTEMPTS = 20
     }
 
     init {
@@ -421,8 +433,8 @@ class ZcashAdapter(
         statusJob?.cancel()
         statusJob = scope.launch {
             synchronizer.status.collect {
-                if (it == Synchronizer.Status.SYNCED && fee.value == MINERS_FEE) {
-                    calculateFee()
+                if (it == Synchronizer.Status.SYNCED) {
+                    scheduleFeeRecalculation()
                 }
             }
         }
@@ -534,7 +546,18 @@ class ZcashAdapter(
                     valuePending = Zatoshi(0)
                 )
 
-                AddressSpecType.Unified -> synchronizer.walletBalances.value?.get(zcashAccount?.accountUuid)?.orchard
+                // After NU6.3 activation the turnstile forbids adding value to Orchard, so change
+                // and payments to Orchard recipients are built in an Ironwood bundle. A unified
+                // address therefore holds funds in both pools and its balance is their sum.
+                AddressSpecType.Unified -> synchronizer.walletBalances.value?.get(zcashAccount?.accountUuid)
+                    ?.let { accountBalance ->
+                        WalletBalance(
+                            available = accountBalance.orchard.available + accountBalance.ironwood.available,
+                            changePending = accountBalance.orchard.changePending +
+                                accountBalance.ironwood.changePending,
+                            valuePending = accountBalance.orchard.valuePending + accountBalance.ironwood.valuePending
+                        )
+                    }
                     ?: WalletBalance(Zatoshi(0), Zatoshi(0), Zatoshi(0))
             }
         }
@@ -609,22 +632,65 @@ class ZcashAdapter(
     private val _fee: MutableStateFlow<BigDecimal> = MutableStateFlow(MINERS_FEE)
     override val fee: StateFlow<BigDecimal> = _fee.asStateFlow()
 
+    /**
+     * Restarts the fee calculation whenever the account balance changes.
+     *
+     * The marker is the whole [AccountBalance] rather than `available` alone: under ZIP-317 the
+     * fee depends on which pools are involved, and after NU6.3 activation funds can move from
+     * Orchard to Ironwood without changing the total. The snapshot is read under the lock
+     * because this runs both from `onBalance` (main dispatcher) and from the status collector
+     * (IO): otherwise an older call could overwrite the marker and cancel the calculation
+     * started for the fresher balance.
+     *
+     * While the published fee is still the default one the snapshot is not enough to conclude
+     * the fee is current — ZIP-317 also depends on the proposal target height, which changes at
+     * NU6.3 activation without touching any balance field — so the calculation repeats on every
+     * trigger until a real fee is known.
+     */
+    private fun scheduleFeeRecalculation() {
+        synchronized(feeLock) {
+            val snapshot = synchronizer.walletBalances.value?.get(zcashAccount?.accountUuid)
+            if (snapshot == lastFeeSnapshot && _fee.value != MINERS_FEE) return
+            lastFeeSnapshot = snapshot
+            val generation = ++feeGeneration
+            val available = walletBalance.available
+            feeJob?.cancel()
+            feeJob = scope.launch {
+                val calculated = calculateFee(available)
+                synchronized(feeLock) {
+                    // The probe may have passed its last cancellation point and returned after a
+                    // fresher calculation already published its fee. Ownership is checked by
+                    // calculation number rather than by snapshot value: on an A -> B -> A balance
+                    // cycle the stale probe would match the snapshot again and publish an
+                    // outdated fee.
+                    if (feeGeneration != generation) return@launch
+                    if (calculated == null) {
+                        // The probe found no workable fee — clear the marker so the next balance
+                        // tick retries it instead of treating the fee as already calculated.
+                        lastFeeSnapshot = null
+                    } else {
+                        _fee.value = calculated
+                    }
+                }
+            }
+        }
+    }
+
+    /** Returns the discovered fee, or `null` when the probe failed outright. */
     private suspend fun calculateFee(
         balance: Zatoshi = walletBalance.available,
-        tryCounter: Int = 4
-    ): Unit = withContext(dispatcherProvider.io) {
+        tryCounter: Int = FEE_PROBE_ATTEMPTS
+    ): BigDecimal? = withContext(dispatcherProvider.io) {
         try {
             if (balance == Zatoshi(0)) {
-                _fee.value = MINERS_FEE
-                return@withContext
+                return@withContext MINERS_FEE
             }
-            val calculatedFee = synchronizer.proposeTransfer(
+            synchronizer.proposeTransfer(
                 account = getFirstAccount(),
                 recipient = AppConfigProvider.donateAddresses[BlockchainType.Zcash]
                     .orEmpty(),
                 amount = balance
-            ).totalFeeRequired()
-            _fee.value = calculatedFee.convertZatoshiToZec(DECIMAL_COUNT)
+            ).totalFeeRequired().convertZatoshiToZec(DECIMAL_COUNT)
         } catch (ex: Exception) {
             if (ex is TransactionEncoderException.ProposalFromParametersException && tryCounter > 0) {
                 // Not enough money to send with commission
@@ -634,7 +700,10 @@ class ZcashAdapter(
                 } catch (e: CancellationException) {
                     throw e
                 } catch (_: Throwable) {
+                    null
                 }
+            } else {
+                null
             }
         }
     }
@@ -977,6 +1046,12 @@ class ZcashAdapter(
         balance?.get(zcashAccount?.accountUuid)?.sapling?.let {
             balanceUpdatedSubject.onNext(Unit)
         }
+        // The pool composition changes at runtime: after NU6.3 activation change arrives in
+        // Ironwood and the available balance no longer matches a fee calculated for a single
+        // pool. Recalculate on every balance change, not only on the first sync.
+        if (syncState is AdapterState.Synced) {
+            scheduleFeeRecalculation()
+        }
         startOneTimeAddressBalanceCheck()
     }
 

app/src/test/java/cash/p/terminal/core/adapters/zcash/ZcashAdapterCorruptionRecoveryTest.kt

@@ -1,155 +1,59 @@
 package cash.p.terminal.core.adapters.zcash
 
-import android.content.Context
 import android.database.sqlite.SQLiteDatabaseCorruptException
-import cash.p.terminal.core.ILocalStorage
 import cash.p.terminal.core.TestDispatcherProvider
-import cash.p.terminal.core.managers.BackgroundKeepAliveManager
 import cash.p.terminal.core.managers.RestoreSettings
-import cash.p.terminal.domain.usecase.ClearZCashWalletDataUseCase
-import cash.p.terminal.wallet.Account
-import cash.p.terminal.wallet.AccountOrigin
-import cash.p.terminal.wallet.AccountType
 import cash.p.terminal.wallet.AdapterState
 import cash.p.terminal.wallet.Wallet
 import cash.z.ecc.android.sdk.SdkSynchronizer
 import cash.z.ecc.android.sdk.Synchronizer
 import cash.z.ecc.android.sdk.WalletInitMode
 import cash.z.ecc.android.sdk.block.processor.CompactBlockProcessor
 import cash.z.ecc.android.sdk.exception.CompactBlockProcessorException
-import cash.z.ecc.android.sdk.model.AccountBalance
-import cash.z.ecc.android.sdk.model.AccountUuid
 import cash.z.ecc.android.sdk.model.BlockHeight
 import cash.z.ecc.android.sdk.model.PercentDecimal
 import cash.z.ecc.android.sdk.model.TransactionOverview
 import cash.z.ecc.android.sdk.model.ZcashNetwork
 import io.horizontalsystems.core.BackgroundManager
 import io.horizontalsystems.core.BackgroundManagerState
-import io.horizontalsystems.core.CoreApp
 import io.horizontalsystems.core.entities.BlockchainType
 import io.mockk.coEvery
 import io.mockk.coVerify
 import io.mockk.every
 import io.mockk.mockk
-import io.mockk.mockkObject
 import io.mockk.slot
-import io.mockk.unmockkAll
 import io.mockk.verify
 import junit.framework.TestCase.assertEquals
 import junit.framework.TestCase.assertFalse
 import junit.framework.TestCase.assertTrue
 import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.CompletableDeferred
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
-import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.flow.MutableStateFlow
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.suspendCancellableCoroutine
-import kotlinx.coroutines.test.StandardTestDispatcher
 import kotlinx.coroutines.test.advanceTimeBy
 import kotlinx.coroutines.test.advanceUntilIdle
-import kotlinx.coroutines.test.resetMain
 import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
-import kotlinx.coroutines.test.setMain
-import org.junit.After
-import org.junit.Before
 import org.junit.Test
-import org.koin.core.context.startKoin
-import org.koin.core.context.stopKoin
-import org.koin.dsl.module
 
 /**
  * Tests for ZcashAdapter database corruption detection and recovery.
  *
- * All adapter coroutines (recovery, status/start/restart jobs, the subscriber scope) run on
- * [dispatcher], a single [StandardTestDispatcher] shared with `runTest`, so every wait below is
- * driven deterministically via the virtual-time scheduler instead of real timeouts.
+ * The shared harness lives in [ZcashAdapterTestFixture]: all adapter coroutines (recovery,
+ * status/start/restart jobs, the subscriber scope) run on its single `StandardTestDispatcher`
+ * shared with `runTest`, so every wait below is driven deterministically via the virtual-time
+ * scheduler instead of real timeouts.
  */
 @OptIn(ExperimentalCoroutinesApi::class)
-class ZcashAdapterCorruptionRecoveryTest {
-
-    private val dispatcher = StandardTestDispatcher()
-
-    // Separate from the `runTest` scope on purpose: the adapter's subscriber collectors are
-    // parented to synchronizer.coroutineScope (ZcashAdapter.subscribe()) and never complete on
-    // their own. If they were children of the `runTest` scope, `runTest` would hang waiting for
-    // them. They live in `appScope` instead, cancelled explicitly in tearDown().
-    private val appScope = CoroutineScope(SupervisorJob() + dispatcher)
-
-    private val context = mockk<Context>(relaxed = true)
-    private val wallet = mockk<Wallet>(relaxed = true)
-    private val localStorage = mockk<ILocalStorage>(relaxed = true)
-    private val backgroundManager = mockk<BackgroundManager>(relaxed = true)
-    private val singleUseAddressManager = mockk<ZcashSingleUseAddressManager>(relaxed = true)
-    private val clearZCashWalletDataUseCase = mockk<ClearZCashWalletDataUseCase>(relaxed = true)
-    private val backgroundKeepAliveManager = mockk<BackgroundKeepAliveManager>(relaxed = true)
-    private val restoreSettings = RestoreSettings().apply { birthdayHeight = 2000000L }
-
-    private lateinit var mockSynchronizer: SdkSynchronizer
-
-    private val statusFlow = MutableStateFlow(Synchronizer.Status.SYNCING)
-    private val progressFlow = MutableStateFlow(PercentDecimal.ZERO_PERCENT)
-    private val walletBalancesFlow = MutableStateFlow<Map<AccountUuid, AccountBalance>?>(null)
-    private val processorInfoFlow = MutableStateFlow(
-        CompactBlockProcessor.ProcessorInfo(null, null, null)
-    )
-    private val allTransactionsFlow = MutableStateFlow<List<TransactionOverview>>(emptyList())
+class ZcashAdapterCorruptionRecoveryTest : ZcashAdapterTestFixture() {
 
     private var capturedProcessorErrorHandler: ((Throwable?) -> Boolean)? = null
     private var capturedCriticalErrorHandler: ((Throwable?) -> Boolean)? = null
 
-    private lateinit var adapter: ZcashAdapter
-
-    @Before
-    fun setUp() {
-        Dispatchers.setMain(dispatcher)
-        CoreApp.instance = mockk(relaxed = true)
-
-        startKoin {
-            modules(module {
-                single { clearZCashWalletDataUseCase }
-                single { backgroundKeepAliveManager }
-            })
-        }
-
-        val testSeed = ByteArray(64) { it.toByte() }
-        val accountType = mockk<AccountType.Mnemonic>(relaxed = true) {
-            every { seed } returns testSeed
-        }
-        val account = mockk<Account>(relaxed = true) {
-            every { id } returns "test-account-id"
-            every { name } returns "Test"
-            every { type } returns accountType
-            every { origin } returns AccountOrigin.Created
-        }
-        every { wallet.account } returns account
-        every { localStorage.zcashAccountIds } returns setOf("test-account-id")
-        every { localStorage.torEnabled } returns false
-        every { backgroundManager.stateFlow } returns MutableStateFlow(BackgroundManagerState.Unknown)
-        every { clearZCashWalletDataUseCase.getValidAliasFromAccountId(any(), any()) } returns "zcash_test"
-
-        mockkObject(BlockHeight.Companion)
-        coEvery { BlockHeight.ofLatestCheckpoint(any(), any()) } returns BlockHeight.new(2500000L)
-
-        setupMockSynchronizer()
-        mockSynchronizerCompanion()
-    }
-
-    private fun setupMockSynchronizer() {
-        mockSynchronizer = mockk<SdkSynchronizer>(relaxed = true) {
-            every { status } returns statusFlow
-            every { progress } returns progressFlow
-            every { walletBalances } returns walletBalancesFlow
-            every { processorInfo } returns processorInfoFlow
-            every { allTransactions } returns allTransactionsFlow
-            every { coroutineScope } returns appScope
-            every { latestHeight } returns null
-        }
-
+    override fun stubSynchronizer() {
         val processorSlot = slot<(Throwable?) -> Boolean>()
         every { mockSynchronizer.onProcessorErrorHandler = capture(processorSlot) } answers {
             capturedProcessorErrorHandler = processorSlot.captured
@@ -160,17 +64,8 @@ class ZcashAdapterCorruptionRecoveryTest {
         }
     }
 
-    private fun mockSynchronizerCompanion() {
-        mockkObject(Synchronizer)
+    override fun stubSynchronizerCompanion() {
         coEvery { Synchronizer.erase(any(), any(), any()) } returns true
-        every {
-            Synchronizer.newBlocking(
-                context = any(), zcashNetwork = any(), alias = any(),
-                lightWalletEndpoint = any(), birthday = any(), walletInitMode = any(),
-                setup = any(), isTorEnabled = any(), isExchangeRateEnabled = any()
-            )
-        } returns mockSynchronizer
-
         coEvery {
             Synchronizer.new(
                 context = any(), zcashNetwork = any(), alias = any(),
@@ -180,19 +75,6 @@ class ZcashAdapterCorruptionRecoveryTest {
         } returns mockSynchronizer
     }
 
-    private fun createAdapter(): ZcashAdapter {
-        return ZcashAdapter(
-            context = context,
-            wallet = wallet,
-            restoreSettings = restoreSettings,
-            addressSpecTyped = null,
-            localStorage = localStorage,
-            backgroundManager = backgroundManager,
-            singleUseAddressManager = singleUseAddressManager,
-            dispatcherProvider = TestDispatcherProvider(dispatcher, appScope),
-        )
-    }
-
     private fun createAdapter(baseDelayMs: Long, maxDelayMs: Long): ZcashAdapter {
         return ZcashAdapter(
             context, wallet, restoreSettings, null, localStorage, backgroundManager,
@@ -201,17 +83,6 @@ class ZcashAdapterCorruptionRecoveryTest {
         )
     }
 
-    @After
-    fun tearDown() {
-        if (::adapter.isInitialized) {
-            adapter.stop()
-        }
-        appScope.cancel()
-        stopKoin()
-        Dispatchers.resetMain()
-        unmockkAll()
-    }
-
     // --- onProcessorErrorHandler ---
 
     @Test