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

Already on GitHub? Sign in to your account

Compile in the LS 32bits as checkpoints #1577

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

TheBlueMatt commented Jul 10, 2012

Details:
Uses a src/checkpoint_def.cpp file to store a static array of checkpoints. That file is easily generate-able using -saveblockcheckpointfile and gitian builds require the file as an input.

Contributor

gavinandresen commented Jul 13, 2012

checkpoints_def.cpp needs some newlines-- see http://stackoverflow.com/questions/10519738/source-line-length-limit

And would it be better to NOT check in the full checkpoints_def.cpp to git? gitian builders could generate it themselves or download a copy (using github 'download a file' feature maybe).

Contributor

TheBlueMatt commented Jul 13, 2012

Oops, yea Ill add some \n, I had previously dropped them because of the increase in file size...

Gitian builders are required to have a checkpoints_def.cpp in their inputs, so they have to compile their own in anyway. Its only checked into git so that people who git clone && cd src && make can continue to do that. (and one can't simply build with an empty one as they will end up rejecting the 2 BIP30-invalid blocks which are in the chain before BIP30 was enforced). But if people believe its better to force people building bitcoin to download the file, thats fair too.

Contributor

TheBlueMatt commented Jul 17, 2012

New version:
"\n"s in checkpoints_def.cpp
Default checkpoints_def.cpp is now empty with a #warning
BIP30 checkpoints are now in checkpoints.cpp using full hashes.

@gavinandresen gavinandresen commented on the diff Jul 17, 2012

src/checkpoints_def.cpp
@@ -0,0 +1 @@
+const unsigned int LSBCheckpoints[] = {};
@gavinandresen

gavinandresen Jul 17, 2012

Contributor

Maybe a nice big comment at the top of this explaining why this do-nothing file exists? Something like:

/*
This is a placholder file that should be replaced by the output of bitcoind -saveblockcheckpointfileto
It will be filled iwth the least-significant 32 bits of block hashes in the best block chain. By checkpointing
the early, difficulty-1 blocks several potential denial-of-service attacks are prevented.
*/

Diapolo commented Jul 17, 2012

So, when doing own builds on Windows I have to use -saveblockcheckpointfileto to generate a new checkpoints_def.cpp? If this is the case wouldn't it be nicer if I don't need to specify that filename and even as default src\ is used :)?

Contributor

TheBlueMatt commented Jul 17, 2012

If you are running bitcoin-qt, the output would be src/checkpoints_def.cpp, or if you are running it on windows, it would be ../src/checkpoints_def.cpp, or if you are running bitcoin it would be checkpoints_def.cpp...I dont really feel like doing some #ifdefs there...

Contributor

gavinandresen commented Jul 18, 2012

Not loving this:

If I get rid of the compile #warnings, then I get a dirty tree, and I know sooner or later I'll accidentally commit checkpoints_def.cpp.

It will be considered a dirty git tree during gitian building, yes? That will screw up the auto-version-determination code...

Also, with a generated checkpoints_def.cpp, I get a gazillion:
checkpoints_def.cpp:185334: warning: this decimal constant is unsigned only in ISO C90

I think adding the 4 checkpoints and doing BIP30-check-all-except-checkpointed-blocks is the right thing to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment