-
Notifications
You must be signed in to change notification settings - Fork 177
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
Block ancient dust spam #140
Open
domob1812
wants to merge
3
commits into
namecoin:namecoinq
Choose a base branch
from
domob1812:dust-spam
base: namecoinq
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
+1 |
Reward halving is at 210000. 220000 ? |
domob1812
force-pushed
the
dust-spam
branch
2 times, most recently
from
September 22, 2014 16:14
a7f41d2
to
5556718
Compare
Split the old code of CTxIndex::GetDepthInMainChain to allow also GetHeight() and GetContainingBlock() to be used.
Implement IsUnspendable() as additional check when verifying transactions. This method checks for provably unspendable outputs (will be useful later on for the UTXO set), and it also blocks dust spam outputs created between blocks 39k and 41k from being spent in the future. This enables them to be removed from the UTXO set.
Don't accept dust spam tx to the mempool even before the fork point. Blocks with them are still considered valid, though.
I'm not qualified to review the code, but I have no problem with the proposed changes on a concept level. |
BTW, I suggest to hold off with merging this one. I think we should first go with the other softforking changes and work on this later on. |
domob1812
pushed a commit
to domob1812/namecoin
that referenced
this pull request
Dec 16, 2014
bccaf86 Merge pull request namecoin#150 2a53a47 Merge pull request namecoin#151 5f5a31f Merge pull request namecoin#149 3907277 Merge pull request namecoin#142 a3e0611 Enable tests in x86 travis builds 45da235 x86 builder 8bb0e93 Merge pull request namecoin#155 971fe81 build: fix openssl detection for cross builds f22d73e Explicitly access %0..%2 as 64-bit so we use the right registers for x32 ABI e66d4d6 Avoid the stack in assembly and use explicit registers cf7b2b4 Fix ECDSA message hashes to 32 bytes 056ad31 Really compile with -O3 by default 74ad63a Merge pull request namecoin#146 9000458 Merge pull request namecoin#145 1f46b00 build: fix __builtin_expect detection for clang aaba2e0 Merge pull request namecoin#136 8a0775c Merge pull request namecoin#144 ee1eaa7 Merge pull request namecoin#141 c88e2b8 Compile with -O3 by default 6558a26 Make the benchmarks print out stats 000bdf6 Rename bench_verify to bench_recovery 7c6fed2 Add a few more additional tests. 992e03b travis: add clang to the test matrix b43b79a Merge pull request namecoin#143 e06a924 Include time.h header for time(). 8d11164 Add some additional tests. 3545627 Merge pull request namecoin#118 6a9901e Merge pull request namecoin#137 376b28b Merge pull request namecoin#128 1728806 Merge pull request namecoin#138 a5759c5 Check return value of malloc 39bd94d Variable time normalize ad86bdf Merge pull request namecoin#140 54b768c Another redundant secp256k1_fe_normalize 69dcaab Merge pull request #139 1c29f2e Remove redundant secp256k1_fe_normalize from secp256k1_gej_add_ge_var. 2b9388b Remove unused secp256k1_fe_inv_all f461b76 Allocate precomputation arrays on the heap b2c9681 Make {mul,sqr}_inner use the same argument order as {mul,sqr} 6793505 Convert YASM code into inline assembly f048615 Rewrite field assembly to match the C version 3ce74b1 Tweak precomputed table size for G git-subtree-dir: src/secp256k1 git-subtree-split: bccaf86caa9c44166e5a66600b742c516e03c3f0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch blocks "ancient" dust spam outputs (1 Swartz value, created between blocks 39k and 41k) from being spent in the future (starting at a predefined softfork block height - set to 300k for now until we decide about the actual value). This enables them to be removed from the UTXO set. See https://forum.namecoin.info/viewtopic.php?f=2&t=1877.
So far, I've only done rudimentary testing (not verified that the full chain still syncs fine, for instance). But the proposed solution here is open for discussion.
To test it, you can use a raw tx like this:
0100000002da11011c1bf3faf601ad8fc9eba9ef34feef0accf1df8590575390735345b8e80000000000ffffffff4b92dec3ce36a750f19b5ec9abfaab97b21b0512e5b8ffce399301adbdc624eb0000000000ffffffff0100e1f505000000001976a9144d92cbd00dc8e35d23329e3e858dc0d2c8f9014088ac00000000
With
sendrawtransaction
, it will be rejected before and after the patch (since it doesn't have any signatures on it). However, when the new rule strikes, the debug.log will read "previous txo is unspendable" instead of "signature verification failed" (since the new check comes before signature verification). Of course, you have to set FORK_HEIGHT_DUSTSPAM to some lower value first to see the effect.