[UI] Exclude coins from coinjoin #12893

SuperJMN · 2024-04-18T10:38:41Z

Enables users to actually choose the coins to exclude using the MusicBox triple-dot menu.

soosr · 2024-04-22T12:26:07Z

@Bodnaralexa
Could you create an icon for the Exlude Coins feature?

soosr · 2024-04-22T12:34:44Z

@SuperJMN
The checkbox on those items that are currently coinjoining should be disabled.

Can you open a new PR where you add this option? We might use it for the Manual Control too so please implement the condition to be customazible, to be able to pass different conditions depending on the different Coin list views.

soosr · 2024-04-22T12:37:56Z

Got an exception when tried to open the Exclude Coins feature. Couldn't repro after restring the app.

EDIT: If I log in two of my wallets, I got this exception every time I try to open the feature from the first wallet.

2024-04-22 14:35:07.366 [1] ERROR	Program.RunAsGuiAsync (162)	System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
   at System.Collections.Generic.List`1.get_Item(Int32 index)
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.PocketViewModel.CommonOrDefault[T](IList`1 list) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\PocketViewModel.cs:line 107
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.PocketViewModel.GetAnonScore(IEnumerable`1 pocketCoins) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\PocketViewModel.cs:line 96
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.PocketViewModel..ctor(IWalletModel wallet, Pocket pocket, Boolean ignorePrivacyMode) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\PocketViewModel.cs:line 30
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.CoinListViewModel.<>c__DisplayClass25_0.<RefreshFromPockets>b__0(Pocket pocket) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\CoinListViewModel.cs:line 181
   at System.Linq.Enumerable.SelectEnumerableIterator`2.ToList()
   at DynamicData.Kernel.EnumerableEx.AsList[T](IEnumerable`1 source) in /_/src/DynamicData/Kernel/EnumerableEx.cs:line 45
   at DynamicData.RangeChange`1..ctor(IEnumerable`1 items, Int32 index) in /_/src/DynamicData/List/RangeChange.cs:line 29
   at DynamicData.Change`1..ctor(ListChangeReason reason, IEnumerable`1 items, Int32 index) in /_/src/DynamicData/List/Change.cs:line 47
   at DynamicData.ChangeAwareList`1.AddRange(IEnumerable`1 collection) in /_/src/DynamicData/List/ChangeAwareList.cs:line 131
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.CoinListViewModel.<>c__DisplayClass25_0.<RefreshFromPockets>b__1(IExtendedList`1 x) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\CoinListViewModel.cs:line 188
   at DynamicData.List.Internal.ReaderWriter`1.Write(Action`1 updateAction) in /_/src/DynamicData/List/Internal/ReaderWriter.cs:line 75
   at DynamicData.SourceList`1.Edit(Action`1 updateAction) in /_/src/DynamicData/List/SourceList.cs:line 129
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.CoinListViewModel.RefreshFromPockets(ISourceList`1 source, IEnumerable`1 pockets) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\CoinListViewModel.cs:line 184
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.CoinListViewModel.<>c__DisplayClass4_1.<.ctor>b__11(IReadOnlyCollection`1 pockets) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\CoinListViewModel.cs:line 108
   at System.Reactive.Linq.ObservableImpl.Do`1.OnNext._.OnNext(TSource value)
--- End of stack trace from previous location ---
   at System.Reactive.PlatformServices.ExceptionServicesImpl.Rethrow(Exception exception)
   at System.Reactive.ExceptionHelpers.Throw(Exception exception)
   at System.Reactive.Stubs.<>c.<.cctor>b__2_1(Exception ex)
   at System.Reactive.AnonymousSafeObserver`1.OnError(Exception error)
   at System.Reactive.Sink`1.ForwardOnError(Exception error)
   at System.Reactive.Linq.ObservableImpl.Do`1.OnNext._.OnNext(TSource value)
   at System.Reactive.Sink`1.ForwardOnNext(TTarget value)
   at System.Reactive.Linq.ObservableImpl.SkipWhile`1.Predicate._.OnNext(TSource value)
   at System.Reactive.Sink`1.ForwardOnNext(TTarget value)
   at System.Reactive.Linq.ObservableImpl.Select`2.Selector._.OnNext(TSource value)
   at System.Reactive.Sink`1.ForwardOnNext(TTarget value)
   at System.Reactive.Linq.ObservableImpl.Select`2.Selector._.OnNext(TSource value)
   at System.Reactive.Sink`1.ForwardOnNext(TTarget value)
   at System.Reactive.IdentitySink`1.OnNext(T value)
   at System.Reactive.Sink`1.ForwardOnNext(TTarget value)
   at System.Reactive.Linq.ObservableImpl.Scan`2._.OnNext(TSource value)
   at System.Reactive.AutoDetachObserver`1.OnNextCore(T value)
   at System.Reactive.ObserverBase`1.OnNext(T value)
   at System.Reactive.Sink`1.ForwardOnNext(TTarget value)
   at System.Reactive.Linq.ObservableImpl.Where`1.Predicate._.OnNext(TSource value)
   at System.Reactive.Sink`1.ForwardOnNext(TTarget value)
   at System.Reactive.IdentitySink`1.OnNext(T value)
   at System.Reactive.AutoDetachObserver`1.OnNextCore(T value)
   at System.Reactive.ObserverBase`1.OnNext(T value)
   at DynamicData.Kernel.InternalEx.<>c__DisplayClass6_0`1.<Return>b__0(IObserver`1 o) in /_/src/DynamicData/Kernel/InternalEx.cs:line 125
   at System.Reactive.Linq.QueryLanguage.CreateWithActionDisposable`1.SubscribeCore(IObserver`1 observer)
   at System.Reactive.ObservableBase`1.Subscribe(IObserver`1 observer)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)
   at System.Reactive.TailRecursiveSink`1.Drain()
   at System.Reactive.TailRecursiveSink`1.Run(IEnumerable`1 sources)
   at System.Reactive.Linq.ObservableImpl.Concat`1.Run(_ sink)
   at System.Reactive.Producer`2.SubscribeRaw(IObserver`1 observer, Boolean enableSafeguard)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)
   at System.Reactive.Sink`2.Run(IObservable`1 source)
   at System.Reactive.Linq.ObservableImpl.Where`1.Predicate.Run(_ sink)
   at System.Reactive.Producer`2.SubscribeRaw(IObserver`1 observer, Boolean enableSafeguard)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)
   at DynamicData.ObservableCache`2.<>c__DisplayClass19_0.<Connect>b__0(IObserver`1 observer) in /_/src/DynamicData/Cache/ObservableCache.cs:line 125
   at System.Reactive.Linq.QueryLanguage.CreateWithDisposableObservable`1.SubscribeCore(IObserver`1 observer)
   at System.Reactive.ObservableBase`1.Subscribe(IObserver`1 observer)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)
   at System.Reactive.Sink`2.Run(IObservable`1 source)
   at System.Reactive.Linq.ObservableImpl.Scan`2.Run(_ sink)
   at System.Reactive.Producer`2.SubscribeRaw(IObserver`1 observer, Boolean enableSafeguard)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)
   at System.Reactive.Sink`2.Run(IObservable`1 source)
   at System.Reactive.Linq.ObservableImpl.Defer`1._.Run()
   at System.Reactive.Linq.ObservableImpl.Defer`1.Run(_ sink)
   at System.Reactive.Producer`2.SubscribeRaw(IObserver`1 observer, Boolean enableSafeguard)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)
   at System.Reactive.Sink`2.Run(IObservable`1 source)
   at System.Reactive.Linq.ObservableImpl.Select`2.Selector.Run(_ sink)
   at System.Reactive.Producer`2.SubscribeRaw(IObserver`1 observer, Boolean enableSafeguard)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)
   at System.Reactive.Sink`2.Run(IObservable`1 source)
   at System.Reactive.Linq.ObservableImpl.Select`2.Selector.Run(_ sink)
   at System.Reactive.Producer`2.SubscribeRaw(IObserver`1 observer, Boolean enableSafeguard)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)
   at System.Reactive.Sink`2.Run(IObservable`1 source)
   at System.Reactive.Linq.ObservableImpl.SkipWhile`1.Predicate.Run(_ sink)
   at System.Reactive.Producer`2.SubscribeRaw(IObserver`1 observer, Boolean enableSafeguard)
   at System.ObservableExtensions.SubscribeSafe[T](IObservable`1 source, IObserver`1 observer)
   at System.Reactive.Sink`2.Run(IObservable`1 source)
   at System.Reactive.Linq.ObservableImpl.Do`1.OnNext.Run(_ sink)
   at System.Reactive.Producer`2.SubscribeRaw(IObserver`1 observer, Boolean enableSafeguard)
   at System.Reactive.Producer`2.Subscribe(IObserver`1 observer)
   at System.ObservableExtensions.Subscribe[T](IObservable`1 source)
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.CoinListViewModel..ctor(IWalletModel wallet, IList`1 initialCoinSelection, Boolean ignorePrivacyMode) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\CoinListViewModel.cs:line 101
   at WalletWasabi.Fluent.ViewModels.Wallets.Settings.ExcludedCoinsViewModel..ctor(IWalletModel wallet) in WalletWasabi.Fluent\ViewModels\Wallets\Settings\ExcludedCoinsViewModel.cs:line 20
   at WalletWasabi.Fluent.ViewModels.Navigation.FluentNavigate.ExcludedCoins(IWalletModel wallet, NavigationTarget navigationTarget, NavigationMode navigationMode) in WalletWasabi.Fluent\WalletWasabi.Fluent.Generators\WalletWasabi.Fluent.Generators.Generators.MainGenerator\FluentNavigate.g.cs:line 520
   at WalletWasabi.Fluent.ViewModels.Wallets.CoinJoinStateViewModel.<>c__DisplayClass44_0.<.ctor>b__16() in WalletWasabi.Fluent\ViewModels\Wallets\CoinJoinStateViewModel.cs:line 170
   at ReactiveUI.ReactiveCommand.<>c__DisplayClass1_0`1.<Create>b__1(IObserver`1 observer) in /_/src/ReactiveUI/ReactiveCommand/ReactiveCommand.cs:line 147
   at System.Reactive.Linq.QueryLanguage.CreateWithDisposableObservable`1.SubscribeCore(IObserver`1 observer)
   at System.Reactive.ObservableBase`1.Subscribe(IObserver`1 observer)

soosr · 2024-04-22T12:41:44Z

Don't forget to add the select all button.

Bodnaralexa · 2024-04-22T12:56:48Z

Here's a version for exclude coins. :)

SuperJMN · 2024-04-24T09:18:47Z

Don't forget to add the select all button.

Implemented using a regular CheckBox. Tell me your opinion :)

SuperJMN · 2024-04-24T09:19:02Z

Here's a version for exclude coins. :)

@Bodnaralexa Wonderful icon! Added.

SuperJMN · 2024-04-24T09:22:16Z

Got an exception when tried to open the Exclude Coins feature. Couldn't repro after restring the app.

EDIT: If I log in two of my wallets, I got this exception every time I try to open the feature from the first wallet.

2024-04-22 14:35:07.366 [1] ERROR	Program.RunAsGuiAsync (162)	System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
   at System.Collections.Generic.List`1.get_Item(Int32 index)
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.PocketViewModel.CommonOrDefault[T](IList`1 list) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\PocketViewModel.cs:line 107
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.PocketViewModel.GetAnonScore(IEnumerable`1 pocketCoins) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\PocketViewModel.cs:line 96
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.PocketViewModel..ctor(IWalletModel wallet, Pocket pocket, Boolean ignorePrivacyMode) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\PocketViewModel.cs:line 30
   at WalletWasabi.Fluent.ViewModels.Wallets.Coins.CoinListViewModel.<>c__DisplayClass25_0.<RefreshFromPockets>b__0(Pocket pocket) in WalletWasabi.Fluent\ViewModels\Wallets\Coins\CoinListViewModel.cs:line 181
...

I haven't been able to repro with my wallets open. Maybe it's not related with this feature? Can you repro in the Coin List? I've seen the error happens in the GetAnonScore method, that is unchanged in this PR.

soosr · 2024-04-24T09:29:03Z

I haven't been able to repro with my wallets open. Maybe it's not related with this feature? Can you repro in the Coin List? I've seen the error happens in the GetAnonScore method, that is unchanged in this PR.

You were right. Opened an issue #12927

soosr · 2024-04-24T09:30:59Z

Don't forget to add the select all button.

Implemented using a regular CheckBox. Tell me your opinion :)

I can see it was easier to implement, but visually it doesn't fit there. Could you please implement the following looks:

WalletWasabi.Fluent/ViewModels/Wallets/Settings/ExcludedCoinsViewModel.cs

soosr · 2024-04-24T12:01:49Z

@SuperJMN Also please fix the conflict. Thanks!

soosr

tACK

soosr · 2024-04-29T09:05:41Z

@zkSNACKs/testing-team Please do a final test on the feature.

MarnixCroes

bug: have some coins of a pocket coinjoining -> select the pocket to be excluded from CJ (I did it in critical phase)-> CJ finishes -> excluded coins are undone, not excluded anymore
having the exclude feature available at Coinjoin Settings or at Wallet Coins is more appropriate imo. it's a feature that will, and should be used rarely. no need to have it at the musicbox
having a specific dialog for this is is not needed imo, having a freeze/block icon at wallet coins which can be clicked is cleaner.
excluded coins should be part of the sorting at wallet coins, like it is for coinjoin status. so when I sort I see all my excluded coins at the top
I have raised a topic in the UX channel about it would be much better if the exclusion would be applied immeditely the checkbox is checked. Waiting for decision now.

this means we have different behavior now. for example, at CJ strategy if I select another strategy, it is only applied after I click done, not if I go back or close the dialog. So this is different behavior, not best UX. Expected is for it to only apply after clicking done. or at least have the same behavior throughout the app

WalletWasabi.Fluent/Views/Wallets/Settings/ExcludedCoinsView.axaml

MarnixCroes · 2024-04-29T20:34:00Z

also, please merge master

Co-authored-by: Marnix Croes <93143998+MarnixCroes@users.noreply.github.com>

soosr · 2024-04-30T08:15:40Z

bug: have some coins of a pocket coinjoining -> select the pocket to be excluded from CJ (I did it in critical phase)-> CJ finishes -> excluded coins are undone, not excluded anymore

@SuperJMN Take a look please.

having the exclude feature available at Coinjoin Settings or at Wallet Coins is more appropriate imo. it's a feature that will, and should be used rarely. no need to have it at the musicbox

having a specific dialog for this is is not needed imo, having a freeze/block icon at wallet coins which can be clicked is cleaner.

The election in Wallet Coins is the bad UX, and it will be a read-only wallet list after #12888. Any coin selection should be put the closest to where it makes sense.
If you select coins with the intention to Send them, it should be around send.
If you select coins to exclude them from coinjoin, it should be around MusicBox.
So by just using common sense, the users can find it.

It is related to CJ but not really a setting (rather a feature) so wouldn't make sense to put it in the Settings. Someone who won't use, won't be bothered with it in its current place, but someone who will use it regularly would be annoyed to always go to settings.

excluded coins should be part of the sorting at wallet coins, like it is for coinjoin status. so when I sort I see all my excluded coins at the top

Make sense.
@SuperJMN Please update GetIndicatorPriority() method.

(Note: With the current UI we are able to have a separate column for each indicator, which would allow us to sort in each individual status. Maybe in the future, we can implement it.

I have raised a topic in the UX channel about it would be much better if the exclusion would be applied immeditely the checkbox is checked. Waiting for decision now.

this means we have different behavior now. for example, at CJ strategy if I select another strategy, it is only applied after I click done, not if I go back or close the dialog. So this is different behavior, not best UX. Expected is for it to only apply after clicking done. or at least have the same behavior throughout the app

You can try out 0dbaa49 to see it is a much worse UX.

SuperJMN · 2024-04-30T08:26:14Z

bug: have some coins of a pocket coinjoining -> select the pocket to be excluded from CJ (I did it in critical phase)-> CJ finishes -> excluded coins are undone, not excluded anymore

@SuperJMN Take a look please.

Will do.

soosr · 2024-05-06T08:31:00Z

@MarnixCroes Could you test again?

soosr

When I opened the dialog, I had some coins excluded, but the checkbox wasn't checked. The initial coins selection is broken on Coin Control too.

SuperJMN · 2024-05-07T10:25:12Z

When I opened the dialog, I had some coins excluded, but the checkbox wasn't checked. The initial coins selection is broken on Coin Control too.

Fixed!

WalletWasabi.Fluent/ViewModels/Wallets/Coins/CoinViewModel.cs

soosr

Receiving a coin into a empty wallet while the wallet coins dialog is opened, it doesn't react (stays empty).

soosr

tACK

Exclude coins

36f0129

pull-request-size bot added the size/L label Apr 18, 2024

SuperJMN changed the title ~~Exclude coins from coinjoin~~ [UI] Exclude coins from coinjoin Apr 18, 2024

SuperJMN added 4 commits April 18, 2024 12:44

Fix build

4cf4c12

Add "Forbidden" icon

019211b

Add logic

72dfb8d

Fix CF

35f7f27

SuperJMN marked this pull request as ready for review April 22, 2024 08:37

SuperJMN self-assigned this Apr 24, 2024

SuperJMN added 3 commits April 24, 2024 10:22

Implement selection policy

c9e7b17

New icon

10ad5a6

Implement select all/none

d7504a2

SuperJMN and others added 2 commits April 24, 2024 12:46

Change toggle selection button

1c1fffa

minor fixes

96cdacd

soosr requested changes Apr 24, 2024

View reviewed changes

SuperJMN and others added 4 commits April 25, 2024 10:28

Merge from master

c14ce31

Remove AppLifetime

8b71718

Reviewed code

27aaedd

Fix messed up selection column after scrolling

35b5c61

soosr marked this pull request as draft April 25, 2024 18:29

soosr marked this pull request as ready for review April 29, 2024 09:05

soosr previously approved these changes Apr 29, 2024

View reviewed changes

MarnixCroes reviewed Apr 29, 2024

View reviewed changes

WalletWasabi.Fluent/Views/Wallets/Settings/ExcludedCoinsView.axaml Outdated Show resolved Hide resolved

soosr dismissed their stale review via 0be2ee9 April 30, 2024 08:01

Coinjoin -> coinjoin

0be2ee9

Co-authored-by: Marnix Croes <93143998+MarnixCroes@users.noreply.github.com>

SuperJMN added 2 commits April 30, 2024 17:58

Fix selection desync

2e82fa8

Add priority for excluded coins

34ca433

soosr closed this May 2, 2024

soosr reopened this May 3, 2024

Merge branch 'master' into pr/12893

6269c0b

soosr requested changes May 7, 2024

View reviewed changes

Fix initial coin selection

7f806d8

SuperJMN requested a review from soosr May 7, 2024 10:25

soosr requested changes May 8, 2024

View reviewed changes

WalletWasabi.Fluent/ViewModels/Wallets/Coins/CoinViewModel.cs Outdated Show resolved Hide resolved

SuperJMN mentioned this pull request May 9, 2024

[UI] Add option to configure selectability of CJing coins #13006

Merged

Merge from master

71dca70

soosr requested changes May 10, 2024

View reviewed changes

Add Pockets is the source of truth

58b01d8

soosr approved these changes May 10, 2024

View reviewed changes

soosr merged commit 27e5374 into zkSNACKs:master May 10, 2024
6 of 8 checks passed

SuperJMN deleted the features/exclude-coins branch May 10, 2024 17:14

This was referenced May 13, 2024

exclude coins from coinjoin zkSNACKs/WasabiDoc#1717

Open

[Autocoinjoin] Freeze utxo from coinjoin #7487

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[UI] Exclude coins from coinjoin #12893

[UI] Exclude coins from coinjoin #12893

SuperJMN commented Apr 18, 2024 •

edited

soosr commented Apr 22, 2024

soosr commented Apr 22, 2024

soosr commented Apr 22, 2024 •

edited

soosr commented Apr 22, 2024

Bodnaralexa commented Apr 22, 2024

SuperJMN commented Apr 24, 2024

SuperJMN commented Apr 24, 2024 •

edited

SuperJMN commented Apr 24, 2024 •

edited

soosr commented Apr 24, 2024

soosr commented Apr 24, 2024

soosr commented Apr 24, 2024

soosr left a comment

soosr commented Apr 29, 2024

MarnixCroes left a comment

MarnixCroes commented Apr 29, 2024

soosr commented Apr 30, 2024

SuperJMN commented Apr 30, 2024

soosr commented May 6, 2024

soosr left a comment

SuperJMN commented May 7, 2024

soosr left a comment

soosr left a comment

[UI] Exclude coins from coinjoin #12893

[UI] Exclude coins from coinjoin #12893

Conversation

SuperJMN commented Apr 18, 2024 • edited

soosr commented Apr 22, 2024

soosr commented Apr 22, 2024

soosr commented Apr 22, 2024 • edited

soosr commented Apr 22, 2024

Bodnaralexa commented Apr 22, 2024

SuperJMN commented Apr 24, 2024

SuperJMN commented Apr 24, 2024 • edited

SuperJMN commented Apr 24, 2024 • edited

soosr commented Apr 24, 2024

soosr commented Apr 24, 2024

soosr commented Apr 24, 2024

soosr left a comment

Choose a reason for hiding this comment

soosr commented Apr 29, 2024

MarnixCroes left a comment

Choose a reason for hiding this comment

MarnixCroes commented Apr 29, 2024

soosr commented Apr 30, 2024

SuperJMN commented Apr 30, 2024

soosr commented May 6, 2024

soosr left a comment

Choose a reason for hiding this comment

SuperJMN commented May 7, 2024

soosr left a comment

Choose a reason for hiding this comment

soosr left a comment

Choose a reason for hiding this comment

SuperJMN commented Apr 18, 2024 •

edited

soosr commented Apr 22, 2024 •

edited

SuperJMN commented Apr 24, 2024 •

edited

SuperJMN commented Apr 24, 2024 •

edited