namecoin / namecoin-legacy

Legacy client. New version here: https://github.com/namecoin/namecoin-core Note the release branch! - Official website:
https://namecoin.org
MIT License
448 stars 177 forks source link

Remove the mandatory "per-dust-output" fee rule. #201

Closed domob1812 closed 9 years ago

domob1812 commented 10 years ago

Instead, enforce a rule similar to Bitcoin: Completely disallow (no matter what fee) outputs less than 0.0001 NMC.

This helps to harmonise fee rules between Namecoin and what is common in Bitcoin, which simplifies development of things like the Armory port. (No special rules required for Namecoin.) Dust spam as it appeared historically (and was prevented by the now-removed rule) should be avoided by the dust output rule just as well.

josephbisch commented 10 years ago

I think this is a good idea. While I had the rule working in Armory for outputs less than 0.01NMC, I think making the fees more like those of Bitcoin makes implementing them easier.

JeremyRand commented 10 years ago

Do I understand correctly that this will refuse to relay such transactions, but will still accept them if they appear in a block?

josephbisch commented 10 years ago

Domob added the code disallowing dust outputs to CTransaction::AcceptToMemoryPool, so it only has to do with accepting a transaction into the memory pool so that it can be relayed to peers in the future. The code having to do with block validation is CBlock::CheckBlock and this commit does not touch that, so transactions in blocks should still be validated according to the rules in CTransaction::CheckTransaction. Since I'm not particularly familiar with the Namecoin source code, some or all of that explanation may be incorrect. Maybe Domob can confirm that I understand correctly.

Domob, it seems the code still adds the fee so that older clients will still relay transactions. I guess that means, for now, Armory needs to still add fees according to the old rule, so that older clients will still relay?

domob1812 commented 9 years ago

Yes, exactly: Transactions in blocks are always accepted, no matter how much "dust" they contain or how little fees are attached. (That is the same as with Bitcoin's dust, and also the same as with the old spam prevention rule I've removed in this patch.) Only relaying is affected. The new rules should be such that transactions created by both the old and also the future rebased client (unless patched specifically) are always accepted. (The current namecoind doesn't create outputs smaller than 0.01 NMC by itself, and the "dust limit" of the rebased client is somewhere between 0.0001 NMC and 0.001 NMC.)

@josephbisch: Yes, I designed the code to still attach the old fee when sending transactions. Eventually this rule will go, but for now it makes sense (IMHO). Whether or not you want to replicate it in Armory is your decision. I hope that the network will relatively soon be upgraded to clients that follow the new rules, so if you have some patience, you can just implement the new semantics. But since you already have the code (I think), I suggest to leave it in there for now and only remove it later.

phelixbtc commented 9 years ago

Might be a bit on the low side in regard of the aggregator issues. Maybe limit outputs to 0.001?

domob1812 commented 9 years ago

This would make it block more than the corresponding Bitcoin rules consider "dust". I think we should be liberal here to not cause comaptibility issues. 0.0001 NMC is already much higher than the outputs used in the dust spamming attacks (by a factor of 10,000). I don't think it causes problems.

phelixbtc commented 9 years ago

The change will make the aggregator attack cheaper by a factor of about 3 (fee per kb + other fees). Luckily the network should be much more resilient to this kind of attack with the recent changes. I don't quite follow how compatibility issues could arise from this difference to Bitcoin but if you prefer it this way it's ok with me.

phelixbtc commented 9 years ago

Compiles on Windows and downloads the blockchain.

domob1812 commented 9 years ago

I would like to merge this soon. Please review or speak up in the next days if you have objections.