Hi colinistheman,

it's very good that people have concerns about the security of code, or the process used to assure it. I hope your concerns have been addressed by now.

Your post made me realize one thing though: you probably haven't seen gmaxwell's reddit post (http://www.reddit.com/r/Bitcoin/comments/2rrxq7/on_why_010s_release_notes_say_we_have_reason_to/). This explains the reason for the at the time somewhat cryptic "we have reason to believe it is better tested". I encourage you to read the details there, but in short: we found a very tricky (but most likely harmless) bug in OpenSSL itself while writing this library - because the tests did comparisons with OpenSSL and failed once. It's by no means a proof that libsecp256k1 is bug free (more review is always welcome), but it does show that its testing practices pay off.

We should probably change the language in the release notes, now that the OpenSSL bug it was referring to has been disclosed.


Most of the constants are taken directly from the secp256k1 standard, or computed using algorithms explained in code. But more comments to explain where they come from would not be a bad idea. We'll add some.

-checklevel only changes the level of consistency checks done at startup, it doesn't affect actual validation during network synchronization or otherwise.

If you're syncing from a known node which you fully trust, just copy its $DATADIR/blocks and $DATADIR/chainstate directories to the new system.

This is true for a softfork (a change where new blocks are acceptable to old software, like BIP16, BIP30 and BIP34 were).

For a hardfork (like increasing the block size limit), which is changing the rules that all full nodes already validate, you need to get those full node to agree with the upgrade, or they will not accept the change. Whether miners agree with that is not specifically relevant: the network will ignore the old miners anyway.

One way of looking at it is saying that it's the network deciding which rules they believe the system should enforce (by choosing to run software that implements those rules). Miners are just clients to the network, who create blocks satisfying the rules the network wants, and are paid for that. Miners can choose to enforce stricter rules than what is demanded by the network, however.

It's a DER requirement to prefix a 0x00 byte to an encoded integer if the integer otherwise starts with a byte >= 0x80. If not, it would be interpreted as a negative number.

BIP 62 already contains a proposal to make S values deterministic (by making it eventually a consensus rule inside version=3 transactions).

Instead of forcing an even value, it forces the value to be in the lower half of the range. Reason: makes signatures on average half a byte smaller. This is also what Bitcoin Core does since 0.9 (and afaik BitcoinJ as well).

EDIT: seems your code already does this, but the text says "even".

It's done to force people to verify the public key.

As any combination of signature and message results in a recovered public key. you may incorrectly assume it's a valid signature without verifying it is coming from whom you think it is.

v0.8.0

In due time. At this point, it hasn't by far received enough auditing to become responsible for securing millions of dollars in transfers.

Looks good to me.

Yes, that's correct.

If he has an extended private key, and gives you the corresponding extended public key, you will be able to derive all public keys (and thus addresses) that correspond to the private keys generated from his extended private key.

The reason for mapRelay (i.e., storing relayed data in wire format, rather than in parsed form), was I believe historically because Satoshi wanted to be able to extend the different data structures later in newer versions, and have old code relay them. I'm sure he didn't consider DoS attacks when designing this.

This may still be useful, and initially I was in favor of trying to keep this behaviour - there are certainly ideas for the future where this would be useful. However, each of those would require some form of authentication - a proof that the extension was in fact created by the creator of the transaction itself. The problem is that without having a definition for that extension, no old node can make this verification, and could easily be made to forward incorrect data with a transaction - potentially risking being considered a DoS attacker itself.

Given that, I think there is no safe way to use that feature anyway, and I'm in favor of just forwarding parsed data.

Do you have any idea what you're talking about?

The DER encoding for signatures is the most standard one that exists. We've been discussing enforcing the standard for a _year_ before implementing it - because, unfortunately, not everyone was following the same rules.

Do you know why we want to enforce this standard? To make sure Bitcoin can be implemented without relying on OpenSSL.

The standard was using DER-encoded signatures, we're not making anything up - we're just enforcing a correct encoding rule that should have been there all along. The code comment is just a nice summary of the rules. This was announced several times on the development list and on this forum, and in release notes. We did wait until only a minority of transactions with non-canonical encodings remained, but it's impossible to find every possible (future) misbehaving client code.

If you disagree that standard-conforming signatures should be enforced on the network, we have little to talk about.

In any case, kjj is right - there are 2 weird padding bytes there, and I don't understand why your code is adding them. In any case, it's clearly wrong - go find why and fix it.

@kokjo: mind sharing the code how to embed the OpenSSL-created signature into a sigScript? I expect the problem is caused by how you wrap/pad it, rather than with the signature itself.

No offence, but all this work is being done to actually make it comply to the standard.

The problem was that even though OpenSSL always created transactions that were compliant, it accepts signatures in non-standard encodings. This is bad, as every alternate implementation is required to implement the rules exactly as OpenSSL is. We've known this issue for a while, brought it up several times on the development mailing list and on this forum, investigated who was creating such transactions and gotten them to upgrade, until the frequency of such transactions was low enough. Then finally we added a rule to make bitcoind not relay or mine such transaction - without making them invalid even.

In this case, if the signature created made by bitcoind is non-standard, then we have a serious bug in the signature creation code. AFAIK, no bitcoin(d/qt) ever has ever created signatures that are currently considered non-standard however. If you're creating them yourself, it's possible to create valid transactions that are being rejected now. I'm sure you prefer your bitcoin node to reject the transaction immediately over only have the network reject it with you wondering why it doesn't confirm.

There is no risk for a hard fork at all right now, by the way. This is just local client policy about which transactions are relayed.

Or they added a tag of their own.

Deleting old blocks is perfectly possible - this was a design goal when the 0.8 database changes (called "ultraprune" for a reason, back then) were developed.

Old transactions are not affected, as the set of unspent transaction outputs is maintained separately from the block database. Their unspent outputs remain in the database, even if the block that contained the transaction itself isn't anymore.

The reason this is not yet implemented, is because we first need to make some protocol changes to help new nodes that start up find peers to download the blockchain from. If a majority of nodes goes to delete their old blocks, they'd have a very hard time finding a peer that still has those blocks otherwise.

If you start from an extended private key, and private derivation is used (the highest bit of the child id is 1), there are two ways to obtain the derived public key:

• Add the tweak to the parent public key.
• Convert the derived private key to a public key using normal EC multiplication with the generator.

Both are correct, but the latter is (negligably) more efficient.[/list]

Thanks, fixed!

I've made it 'Accepted' by now, as per BIP 01, 'Final' requires a reference implementation...