feat(dashpay): coinjoin entire wallet balance #1222
Conversation
… dashpay-core-20
HashEngineering
changed the title
| private var coinJoinSend = false | ||
| private val coroutineScope = CoroutineScope(Dispatchers.IO) | ||
|
|
||
| init { | ||
| coinJoinConfig | ||
| .observeMode() | ||
| .filterNotNull() | ||
| .onEach { mode -> | ||
| coinJoinSend = mode != CoinJoinMode.NONE | ||
| } | ||
| .launchIn(coroutineScope) | ||
| } |
There was a problem hiding this comment.
My first thought was to pass a "send with coinjoin" boolean value in many of these functions. But this would require that many ViewModels in many modules have access to the CoinJoinConfig.
My final idea was to load the CoinJoin mode here and no other modules or view models need to know anything about CoinJoin.
| @@ -53,11 +54,11 @@ class CoinJoinLevelFragment : Fragment(R.layout.fragment_coinjoin_level) { | |||
| lifecycleScope.launch { | |||
| if (viewModel.isMixing) { | |||
| if (confirmStopMixing()) { | |||
| viewModel.stopMixing() | |||
| viewModel.setMode(CoinJoinMode.NONE) | |||
There was a problem hiding this comment.
Setting the CoinJoinMode will trigger the CoinJoinService to start or stop mixing.
| @@ -100,7 +100,6 @@ data class BlockchainIdentityData(var creationState: CreationState = CreationSta | |||
| enum class CreationState { | |||
| NONE, // this should always be the first value | |||
| UPGRADING_WALLET, | |||
| MIXING_FUNDS, | |||
There was a problem hiding this comment.
Mixing is no longer part of the Username Creation Process, but the UI hasn't been updated to reflect that.
| fun BlockchainServiceImpl.initCoinJoin() { | ||
| val scope = CoroutineScope(Executors.newSingleThreadExecutor().asCoroutineDispatcher()) | ||
| scope.launch { | ||
| coinJoinService.configureMixing(true) | ||
| } | ||
| } |
There was a problem hiding this comment.
I would like to get rid of this extension. CoinJoinService is largely self-contained and should "configure itself" rather than be told to.
| override suspend fun waitForMixing() { | ||
| log.info("Waiting for mixing to complete by watching lock...") | ||
| mixingMutex.withLock {} | ||
| log.info("Mixing complete according to released lock") | ||
| } | ||
| override suspend fun waitForMixingWithException() { | ||
| waitForMixing() | ||
| exception?.let { | ||
| log.error("Mixing error: {}", it.message, it) | ||
| throw it | ||
| } | ||
| } | ||
| private fun setMixingComplete() { | ||
| mixingMutex.unlock() | ||
| } |
There was a problem hiding this comment.
Since mixing is not part of another process, there is no need for these waiting functions.
| .onEach { balance -> | ||
| log.info("coinjoin: total balance: $balance (before distinct)") | ||
| } | ||
| //.distinctUntilChanged() |
There was a problem hiding this comment.
These lines are for debugging.
I may remove this observer as it doesn't work as well as I would like. The blockchainStateProvider.observeState() may be sufficient. CoinJoin works on confirmed transactions, which means that having an observer that is triggered every block is ideal.
Sometimes there are a lot of ANR's when running this PR -- maybe its related to this observer. This observer doesn't seem to be triggered with each block, although it seems like it should.
| coroutineScope.launch(Dispatchers.IO) { updateMixingStatus(MixingStatus.FINISHED) } | ||
| coroutineScope.launch { | ||
| updateProgress() | ||
| updateMixingState(MixingStatus.FINISHED) | ||
| } |
There was a problem hiding this comment.
I removed Dispatchers.IO because it seemed to not use the threading pool created above. This resulted in missing Context exceptions.
| Executors.newFixedThreadPool(2).asCoroutineDispatcher(), | ||
| Executors.newFixedThreadPool(2, ContextPropagatingThreadFactory("coinjoin-pool")).asCoroutineDispatcher() |
There was a problem hiding this comment.
ContextPropagatingThreadFactory will call Context.propogate in the created threads and we don't need to worry about missing Context exceptions.
| // TODO: the coin selector will need to use CoinJoinCoinSelector if CoinJoin is ON | ||
| walletDataProvider.observeBalance(coinSelector = MaxOutputAmountCoinSelector()) | ||
| .distinctUntilChanged() | ||
| .onEach(_maxOutputAmount::postValue) |
There was a problem hiding this comment.
In the UI story for Mixing Information, this will need to be fixed where we select the correct CoinSelector based on the user selected CoinJoinMode.
MaxOutputAmountCoinSelector does not account for CoinJoinMode != NONE
| interface CoinJoinService { | ||
| var mode: CoinJoinMode | ||
| val mixingStatus: MixingStatus | ||
|
|
||
| fun needsToMix(amount: Coin): Boolean | ||
| suspend fun configureMixing( | ||
| amount: Coin, | ||
| requestKeyParameter: RequestKeyParameter, | ||
| requestDecryptedKey: RequestDecryptedKey, | ||
| restoreFromConfig: Boolean | ||
| ) | ||
|
|
||
| suspend fun prepareAndStartMixing() | ||
| suspend fun waitForMixing() | ||
| suspend fun waitForMixingWithException() | ||
| val mixingProgress: Flow<Double> |
There was a problem hiding this comment.
The interface has been simplified to the point where this is used only for viewing progress.
| Executors.newFixedThreadPool(2).asCoroutineDispatcher(), | ||
| Executors.newFixedThreadPool(2, ContextPropagatingThreadFactory("coinjoin-pool")).asCoroutineDispatcher() |
| updateBalance(balance) | ||
| } | ||
| } | ||
| .launchIn(uiCoroutineScope) // required for observeBalance |
There was a problem hiding this comment.
This might be an oversight in WalletBalanceObserver. We should be able to observe balance in any thread. I'll check it when have a chance.
Issue being fixed or feature implemented
Related PR's and Dependencies
dashpay/dashj#235
Screenshots / Videos
How Has This Been Tested?
Checklist: