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

Add user interface to set dust limit and filtered addresses #2383

Closed
wants to merge 2 commits into from

Conversation

jonls
Copy link
Contributor

@jonls jonls commented Mar 18, 2013

This patch adds a new "Advanced" pane to the options box where the dust limit for accepted transactions can be set, and where a filter on output addresses can be specified. CTxMemPool::accept() will drop any transaction with outputs below the dust limit, or with an output address corresponding to an address specified in the filter.

screenshot

Accept only transactions for which all output values are above a minimum amount (dust limit). The mimimum defaults to zero (everything above zero is accepted).
Added QT interface options pane called "Advanced" to set this value.
Transactions destined for these addresses will not be accepted by this node.
Added a field in the "Advanced" options pane to set the list of filtered addresses.
@paraipan
Copy link
Contributor

Needed feature, Bitcoin developers please review.

@grue0
Copy link

grue0 commented Mar 19, 2013

DO WANT

even if we need to make it a .conf option, this will be much better than manually patching the client with each build.

@PartTimeLegend
Copy link

I love you! No homo.

@petertodd
Copy link
Contributor

NAK

Don't encourage censorship based on where Bitcoins are going rather than the technical details about how they are being transferred.

My https://github.com/petertodd/bitcoin/tree/block-uneconomic-utxo-creation patch is an example of a way to approach the issue based on politically neutral technical metrics rather than singling out any particular entity. A "dust limit" is fine. Building ways to block specific addresses into the client itself isn't.

Additionally any "dust limit" should be written in a way that the behavior is predictable; end users are harmed when they don't have any idea what the dust limit might be set to.

@ghost
Copy link

ghost commented Mar 19, 2013

Address filtering is a DEFINITE no. Do not forget that addresses are disposable.

@luke-jr
Copy link
Member

luke-jr commented Mar 19, 2013

Looks good visually and functionality-wise. I'll have a look at the code when I get home.

@benwiebe
Copy link

Definitely needed as part of the client!

@grue0
Copy link

grue0 commented Mar 20, 2013

@petertodd @gladoscc I don't see what's wrong with giving people a choice to ignore transactions. Anyone can implement their own address filtering by editing the source code, so what do we gain by adding a technical barrier?

@petertodd
Copy link
Contributor

Including such a mechanism in the official Bitcoin reference client implies we support censorship; an obvious extension of this patch is supporting automated blacklists.

@gmaxwell
Copy link
Contributor

@Gruez "Anyone sufficiently motivated could build a chemical weapon, what do we gain by not equipping every adult with one?"

Having to modify the software gives people enough friction to think over their change. Not just "I should do this because some website said to click here for awesomeness". ... though my level of horror at this pull request is somewhat tempered by the fact that it's easily avoided by someone using Bitcoin properly (not reusing addresses).

Still, NAK.

It's also worth noting that inconsistent forwarding rules created by the dust setting makes it much harder to write reliable wallet software... since the software doesn't actually know when its peers are going to forward the transaction or not (and your peers don't tell you when they don't). Ultimately wallets will need to deal with that, but they don't currently.

I'd prefer to see a pull request that depriortizes all address reuse, as that will allow reusers with standing relationships to opt into lower priority handling and it encourages blacklisting resistant behavior in our ecosystem.

@laanwj
Copy link
Member

laanwj commented Mar 29, 2013

GUI looks good. But NAK on the filtering. This is a censorship measure, and an ineffective one at that.

@paraipan
Copy link
Contributor

I beg to differ @laanwj, I don't understand your point in not letting me control my own resources. Did someone put you there to decide over what bitcoiners have to have in their client software?

@paraipan
Copy link
Contributor

Let's see what the community has to say about this https://bitcointalk.org/index.php?topic=160785

@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/427dcfb215a5892c8108bc76b40fe9bab9eacb24 for binaries and test log.
This is an automated test script which runs test cases on each commit every time is updated.
It, however, dies sometimes and fails to test properly, if you are waiting on a test, please check timestamps and if the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/
and contact BlueMatt on freenode if something looks broken.

@gmaxwell
Copy link
Contributor

We won't be adding any blacklisting functionality at this time. There are some other active pull requests on dust relay limiting which should generally address the rest of the concerns here. Thanks to all involved to actually putting code on paper and making a concrete proposal.

@gmaxwell gmaxwell closed this Apr 29, 2013
@paraipan
Copy link
Contributor

paraipan commented May 6, 2013

@paraipan
Copy link
Contributor

paraipan commented Jun 5, 2013

This patch is a potential solution to MAX_BLOCK_SIZE limit give it permits Bitcoin users to ignore spam transactions under set limits.

https://bitcointalk.org/index.php?topic=221111

@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants