search

vbuterin / pybitcointools

SImple, common-sense Bitcoin-themed Python ECC library
1.28k stars 857 forks source link

Use low S-value for signatures #63

Closed erkmos closed 9 years ago

erkmos commented 9 years ago

The Satoshi client negates the S-value of a signature if the value is above half the order of the curve. The development version of the satoshi client enforces low S-values for signatures. But the stable version (0.9.3) only generates signatures this way, it does not enforce the check yet. From what I understand this change was made to help with malleability and has the added benefit of reducing the average serialised size of signatures.

See bitcoin/bitcoin#3016 and bitcoin/bitcoin#3637 and https://github.com/bitcoin/bips/blob/master/bip-0062.mediawiki for more information

jdb6167 commented 9 years ago

If I'm understanding that BIP correctly, that S value change is a characteristic of v3 transactions. This pull request changed the signing code, but the transactions generated are still v1 transactions. I don't believe signatures following the new S-value rule will validate if S > N/2 and the tx version is still 1. I could be mistaken, however.

erkmos commented 9 years ago

I see it's still a draft proposal too, so I might've jumped the gun a bit on adding this. But you are right that I should've updated the transaction versions aswell, but according to the BIP v3 transactions are still considered non-standard.. So I guess I will close this for now. :)

On 10 December 2014 at 21:48, jdb6167 notifications@github.com wrote:

If I'm understanding that BIP correctly, that S value change is a characteristic of v3 transactions. This pull request changed the signing code, but the transactions generated are still v1 transactions. I don't believe signatures following the new S-value rule will validate if S > N/2 and the tx version is still 1. I could be mistaken, however.

— Reply to this email directly or view it on GitHub https://github.com/vbuterin/pybitcointools/pull/63#issuecomment-66521169 .

jdb6167 commented 9 years ago

It appears I was incorrect. Sorry, my bad. I noticed that Trezor was already doing this in their ECDSA function, and raised an issue there, but I was corrected. When I reread that BIP more carefully, I found this line in the motivation: "Inherent ECDSA signature malleability ECDSA signatures themselves are already malleable: taking the negative of the number S inside (modulo the curve order) does not invalidate it." So, they're enforcing negation of S values because the S values are currently malleable.

erkmos commented 9 years ago

I guess this can be done without moving to version 3 transactions explicitly, as a precaution. Checking the 0.9.x satoshi client repo, it already does this https://github.com/bitcoin/bitcoin/blob/0.9/src/key.cpp#L214 without bumping the transaction version https://github.com/bitcoin/bitcoin/blob/0.9/src/core.h#L188

On 10 December 2014 at 23:39, jdb6167 notifications@github.com wrote:

It appears I was incorrect. Sorry, my bad. I noticed that Trezor was already doing this in their ECDSA function, and raised an issue there, but I was corrected. When I reread that BIP more carefully, I found this line in the motivation: "Inherent ECDSA signature malleability ECDSA signatures themselves are already malleable: taking the negative of the number S inside (modulo the curve order) does not invalidate it." So, they're enforcing negation of S values because the S values are currently malleable.

— Reply to this email directly or view it on GitHub https://github.com/vbuterin/pybitcointools/pull/63#issuecomment-66538035 .