Skip to content

Conversation

sipa
Copy link
Member

@sipa sipa commented Jan 25, 2012

Reported by onlineproof on IRC: Bitcoin does not verify whether private
keys and public keys correspond, when loading a wallet.

Reported by onlineproof on IRC: Bitcoin does not verify whether private
keys and public keys correspond, when loading a wallet.
@piratelinux
Copy link

Hi, this is onelineproof from IRC. I think you need to do a bit more than just that...If you're first setting pubkey to be vchpubkey, then obviously they will be equal. But eventually, I can test it to make sure...

If you wanna peek at my code that implements such a function in C, take a look at the function priv_to_pub in https://github.com/piratelinux/cwallet/blob/master/src/util.h

@sipa
Copy link
Member Author

sipa commented Jan 25, 2012

@piratelinux: CKey::GetPubKey extracts the public key from the OpenSSL Key structure, which is constructed at the time of the CKey::SetPrivKey() call. The information present there from the CKey::SetPubKey() is only used to know whether the key is compressed.

@TheBlueMatt
Copy link
Contributor

Why not check wkeys as well?

@sipa
Copy link
Member Author

sipa commented Jan 25, 2012

@TheBlueMatt do they exist?

@TheBlueMatt
Copy link
Contributor

Look at the block starting at line 868.

@gavinandresen
Copy link
Contributor

wkeys are dead code that should be removed, if I recall correctly.

gavinandresen added a commit that referenced this pull request Jan 25, 2012
Check consistency of private keys
@gavinandresen gavinandresen merged commit 4c932cc into bitcoin:master Jan 25, 2012
@TheBlueMatt
Copy link
Contributor

wkeys were used by very, very old clients instead of keys iirc, No reason to remove support for opening old wallets.

Gavin Andresen reply@reply.github.com wrote:

wkeys are dead code that should be removed, if I recall correctly.


Reply to this email directly or view it on GitHub:
#782 (comment)

@piratelinux
Copy link

I still don't think it will work, because I think what SetPrivKey does is take the public key part of the full private key, but if the secret part (usually 32 bytes) is corrupted, it wouldn't know. I did a rough test, but maybe if you could send me a release that I can compile and you're confident works, then I can do a more thorough test. The master branch of bitcoin is not compiling for me.
Thanks.
edit: I think it compiles fine now, so I can test with master branch

coblee referenced this pull request in litecoin-project/litecoin Jul 17, 2012
Check consistency of private keys
sipa added a commit to sipa/bitcoin that referenced this pull request Oct 14, 2020
c6b6b8f Merge bitcoin#830: Rip out non-endomorphism code + dependencies
c582aba Consistency improvements to the comments
63c6b71 Reorder comments/function around scalar_split_lambda
2edc514 WNAF of lambda_split output has max size 129
4232e5b Rip out non-endomorphism code
ebad841 Check correctness of lambda split without -DVERIFY
fe7fc1f Make lambda constant accessible
9d2f2b4 Add tests to exercise lambda split near bounds
9aca2f7 Add secp256k1_split_lambda_verify
acab934 Detailed comments for secp256k1_scalar_split_lambda
76ed922 Increase precision of g1 and g2
6173839 Switch to our own memcmp function
63150ab Merge bitcoin#827: Rename testrand functions to have test in name
c5257ae Merge bitcoin#821: travis: Explicitly set --with-valgrind
bb1f542 Merge bitcoin#818: Add static assertion that uint32_t is unsigned int or wider
a45c1fa Rename testrand functions to have test in name
5006895 Merge bitcoin#808: Exhaustive test improvements + exhaustive schnorrsig tests
4eecb4d travis: VALGRIND->RUN_VALGRIND to avoid confusion with WITH_VALGRIND
66a765c travis: Explicitly set --with-valgrind
d7838ba Merge bitcoin#813: Enable configuring Valgrind support
7ceb0b7 Merge bitcoin#819: Enable -Wundef warning
8b7dcdd Add exhaustive test for extrakeys and schnorrsig
08d7d89 Make pubkey parsing test whether points are in the correct subgroup
87af00b Abstract out challenge computation in schnorrsig
63e1b2a Disable output buffering in tests_exhaustive.c
39f67dd Support splitting exhaustive tests across cores
e99b26f Give exhaustive_tests count and seed cmdline inputs
49e6630 refactor: move RNG seeding to testrand
b110c10 Change exhaustive test groups so they have a point with X=1
cec7b18 Select exhaustive lambda in function of order
78f6cdf Make the curve B constant a secp256k1_fe
d7f39ae Delete gej_is_valid_var: unused outside tests
8bcd78c Make secp256k1_scalar_b32 detect overflow in scalar_low
c498366 Move exhaustive tests for recovery to module
be31791 Make group order purely compile-time in exhaustive tests
e73ff30 Enable -Wundef warning
c0041b5 Add static assertion that uint32_t is unsigned int or wider
4ad408f Merge bitcoin#782: Check if variable=yes instead of if var is set in travis.sh
412bf87 configure: Allow specifying --with[out]-valgrind explicitly
34debf7 Modify .travis.yml to explictly pass no in env vars instead of setting to nothing
a0e99fc Merge bitcoin#814: tests: Initialize random group elements fully
5738e86 tests: Initialize random group elements fully
c9939ba Merge bitcoin#812: travis: run bench_schnorrsig
a51f2af travis: run bench_schnorrsig
ef37761 Change travis.sh to check if variables are equal to yes instead of not-empty. Before this, setting `VALGRIND=wat` was considered as true, and to make it evaluate as false you had to unset the variable `VALGRIND=` but not it checks if `VALGRIND=yes` and if it's not `yes` then it's evaluated to false

git-subtree-dir: src/secp256k1
git-subtree-split: c6b6b8f
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants