Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change dust control #1384

Merged
merged 1 commit into from May 24, 2019

Conversation

Projects
2 participants
@lontivero
Copy link
Contributor

commented Apr 28, 2019

This PR is for changing the way Wasabi deals with dust coins. Currently Wasabi keeps the dust coins but with a mark that can be queried to know when it is relevant or not. With this PR dust coins are completely ignored by Wasabi as if they have never existed.

Close #1383

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented Apr 29, 2019

NACK for now, because dust protection works for me. We have to figure out what's the issue.

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented May 3, 2019

As I said here: #1383 (comment)

Dust protection doesn't work after restart. So not NACK.

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented May 3, 2019

NACK for this PR. This is a hack to hide things in the UI. The whole reason we are in this situation is, because I was hacking in the first place. Now you want to hack around my hacking? That won't end well.

@lontivero

This comment has been minimized.

Copy link
Contributor Author

commented May 3, 2019

I don't understand. How is this a hack? I don't what to hack anything here, I've just removed all the flags everywhere, removed many lines of code and replace it for 2 lines.

There is something that I don't get here: you say that "Dust protection doesn't work after restart". Are you talking about my change or about the current situation (before this PR)?

@nopara73 nopara73 added this to In Progress in v1.1.5 via automation May 19, 2019

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented May 23, 2019

Hey Lucas, just a heads up that I didn't forget this PR. I'll either merge or rework this before v1.1.5 is released.

@lontivero

This comment has been minimized.

Copy link
Contributor Author

commented May 23, 2019

Good to know because the dustThreshold is 0.0001 and I have dust in my testnet wallet:
image

@nopara73

This comment has been minimized.

Copy link
Collaborator

commented May 24, 2019

  1. You removed the GUI checks.
  2. You removed the IsDust SmartCoin property and all its checks.
  3. You made sure the ProcessTransaction doesn't concern itself with outputs those are <= dust.

Indeed it's not a hack, but a very clean solution. Why didn't I implement it this way in the first place? I'm struggled. I was either stupid or I had a very good reason I don't remember.

@nopara73 nopara73 merged commit 329acf9 into zkSNACKs:master May 24, 2019

4 checks passed

CodeFactor No issues found.
Details
Wasabi.Linux #20190428.1 succeeded
Details
Wasabi.Osx #20190428.1 succeeded
Details
Wasabi.Windows #20190428.1 succeeded
Details

v1.1.5 automation moved this from In Progress to Done May 24, 2019

This was referenced May 24, 2019

@lontivero lontivero deleted the lontivero:Fixes/Dust-Control branch Jun 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.