nochowderforyou / clams

Clam Project
MIT License
62 stars 58 forks source link

add option to allow dust from all addresses to be swept up when staking? #196

Closed dooglus closed 9 years ago

dooglus commented 9 years ago

I recently discovered that the CLAM consensus doesn't require all the inputs of a staking transaction to be from the same address. The only such rule is that the first input has the same address as the second output (the first output is always zero in a staking transaction).

So it's possible to gather value from multiple addresses while staking. For example: http://khashier.com/tx/78c1d2cd20bdd13f3497bed2091ccbd0f9811d08766308378166f76498a98578

This may not be desirable for people who want to use the balance at each address as a form of accounting, since the above staking transaction moves coins from one address to another. But for those who don't care about address balances this could be an easy (and free!) way of sweeping up the dust that collects in wallets. (Free, because staking transactions don't pay fees like other transactions do - who would they pay, anyway, other than their own creator).

Currently the client won't combine inputs from different addresses when staking, due to this code in wallet.cpp:

    // we're not splitting the output, so attempt to add more inputs
    BOOST_FOREACH(PAIRTYPE(const CWalletTx*, unsigned int) pcoin, setCoins)
    {
        // Only add coins of the same key/address as kernel
        if ((pcoin.first->vout[pcoin.second].scriptPubKey == scriptPubKeyKernel ||
             pcoin.first->vout[pcoin.second].scriptPubKey == txNew.vout[1].scriptPubKey) &&
            (pcoin.first->GetHash() != txNew.vin[0].prevout.hash ||
             pcoin.second != txNew.vin[0].prevout.n))
        {

Commenting out the first two lines of the if's condition is all that's needed to get the client merging dust from multiple addresses.

I'd propose not changing the default, but having a new config flag: -combineany perhaps, which enables it. (combine, because the flag that currently controls the merging on inputs when staking is called combinelimit)

Thoughts? It would be a trivial change to make, and would allow people (who stake at all regularly) to keep their wallets tidy for free.

I can see an argument against this that we don't want to encourage people to create large transactions without paying for them, but these large transactions are doing a public good - they are reducing the size of the set of unspent outputs - many of which are unspendable otherwise (requiring more in fees to spend than they are worth).

dooglus commented 9 years ago

197 implements this

dooglus commented 9 years ago

I'm going to go ahead and pull this to the master branch. Doesn't mean it can't be changed if anyone has any objections.

dooglus commented 9 years ago

Here it is in operation, sweeping up unspendable dust outputs for free:

http://khashier.com/tx/1141a4455d2c02788e090c2775c8b5f60c40b32395e37e2e61a6447585598d10

creativecuriosity commented 9 years ago

A good feature to have, especially for those whose primary concern is staking together dust.

However, privacy is an additional reason for this to remain off by default.

dooglus commented 9 years ago

Fair enough. We do have another solution for cleaning up 'dusty' wallets now (the 500 input limit on automatic coin selection that I implemented a few weeks ago), although that does involve spending more than the dust is worth to clean it up.