The thing that makes me nervous of this approach in general is that it uses c_void_p almost everywhere where a pointer is used in the underlying API (including places which expect pointers to pointers). If I uncomment the preceding two lines, I get the following errors when running unit tests, which I can't explain:

% python -m unittest discover
...................F..............................................................................F...
======================================================================
FAIL: test (bitcoin.tests.test_key.Test_CPubKey)
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".../python-bitcoinlib/bitcoin/tests/test_key.py", line 27, in test
    T('', False, False, False)
  File ".../python-bitcoinlib/bitcoin/tests/test_key.py", line 24, in T
    self.assertEqual(key.is_fullyvalid, is_fullyvalid)
AssertionError: True != False

======================================================================
FAIL: test_from_invalid_pubkeys (bitcoin.tests.test_wallet.Test_P2PKHBitcoinAddress)
Create P2PKHBitcoinAddress's from invalid pubkeys
----------------------------------------------------------------------
Traceback (most recent call last):
  File ".../python-bitcoinlib/bitcoin/tests/test_wallet.py", line 196, in test_from_invalid_pubkeys
    P2PKHBitcoinAddress.from_pubkey(x(''))
AssertionError: CBitcoinAddressError not raised

----------------------------------------------------------------------
Ran 102 tests in 1.670s

FAILED (failures=2)

Basically, it treats a zero-length public key as valid. I have no idea why. Maybe this is correct? 😕 See also this comment from test_key.py#L28.

Investigating further, it appears that o2i_ECPublicKey calls EC_POINT_oct2point, which in turn either calls ec_GFp_simple_oct2point (which should reject zero-length keys), or it calls something else that I haven't yet tracked down.

Ok, that's promising.

@posita Can you check if #78 makes the above issue go away? (IE uncommenting those two lines?)

@petertodd, unfortunately, it doesn't. Merging #78 into this fix and uncommenting the above two lines results in the same two test failures. 😞

The (sort of?) good news is that even without the ctypes specification, calls to o2i_ECPublicKey don't segfault. Whether they're doing what they're supposed to is another matter which I hope is adequately covered in the unit tests. However, the fact that providing a ctypes spec for o2i_ECPublicKey causes unit test failures still makes me nervous about using a similar technique with the other OpenSSL call-out functions. Unless the unit tests are wrong, but that doesn't appear to be the case. I don't think zero-length octal-encoded public keys should be considered valid.

I guess the alternative would be to write (or port) unit tests for each OpenSSL function to make sure the ctypes specifications didn't screw things up. That's probably well beyond the scope of what was originally intended here.

I found the bug. In CPubKey.__new__, we have the line self.is_fullyvalid = _cec_key.set_pubkey(self) != 0. Without these declarations, pointers are returned as integers, and can be compared to 0. With these declarations, pointers are almost always returned as integers, except when they are NULL (which used to be 0). Then Python translates them to None, and None != 0. Therefore is_fullyvalid gets set to True. This can be easily fixed by using self.is_fullyvalid = bool(_cec_key.set_pubkey(self)), which works whether 0 or None is returned. Tested on my computer, this fixes both test failures.

Hah! Brilliant! Working on a PR update now....

P.S. @jashug, do you want a job? 😉