Skip to content

Conversation

@rubensayshi
Copy link
Contributor

Work in Progress

I ported the code for pubkey recovery from bitcoin core using ctypes, the same way as the other code is written.

The VerifyMessage (and BitcoinMessage class) are still in the test_signverify_message.py because I have no idea where you would want these to go? please let me know where you want me to put them

I'm not entirely sure about the CPubKey.recover_compact, as a python dev I'd say it should be a @classmethod (like it is now) but to match the bitcoin core code (which is what you've been doing) should it be different?

tested against py2.7 and py3.4, to test it against libeay I have to run it on windows I guess, or is it 100% certain that everything 'just works'?

PS. is there any particular reason for not having travis-ci setup to run the tests on the various python versions?

@rubensayshi rubensayshi force-pushed the sign-verify-message branch 2 times, most recently from e1931bd to 798aad2 Compare February 23, 2015 14:17
@rubensayshi
Copy link
Contributor Author

okay, seem to have it working now except I keep running into some odd thing where if _ssl.BN_bin2bn stops when it runs into x00, eg;
b'\x00\xeb\xbb\x00\x17\x0c\x93(\xd4U\xfd\xc7\xfe\x1d\xf9&\xf9\xff\x1e\x8dB\xc8\x98~\xe1Y\x8e3g\xe4\x88\x0c\x90' becomes b'\xeb\xbb'
and
b'\x00\xc3\xd7!7\x01\x99\xcd%+\xef\xcd+n\xac\x00\x0b\x82\xdb\xb72+\xb4\xe6]\x80\xeb\xc0s\x99\x94"\xe3' becomes b'\xc3\xd7!7\x01\x99\xcd%+\xef\xcd+n\xac

Maybe @petertodd you can help me with this because tbh I this is the first time for be using this stuff and I have no clue why this happens.

PS. I made a DERSignature(ImmutableSerializable) class;

  1. idk where to put it
  2. maybe you don't like it and just prefer reusing the code in IsLowDERSignature to decode the signature?
  3. should I change it so that IsLowDERSignature uses it too?

@petertodd
Copy link
Owner

FWIW, I'm travelling right now and am pretty busy until the MIT Bitcoin Expo is over; I'll try to get to this when I get back after that.

Thanks!

@rubensayshi rubensayshi force-pushed the sign-verify-message branch from be91ff5 to 81c6084 Compare April 3, 2015 11:10
@rubensayshi
Copy link
Contributor Author

@petertodd I fixed the issue with the null bytes, if you could tell me where (which files) you'd prefer these things to go then this stuff should be ready:

  • class BitcoinMessage(ImmutableSerializable)
  • def SignMessage(key, message)
  • def VerifyMessage(address, message, sig)

@petertodd
Copy link
Owner

So bitcoin signatures aren't consensus critical, so rather than putting them in bitcoin.core.signature, let's put that functionality in bitcoin.signature instead. As for the code you added to bitcoin.core.key though... meh, leave it there for now; this can be cleaned up later.

@rubensayshi
Copy link
Contributor Author

@petertodd I've been slacking lately, sorry for that, I want / need to pick up this PR ...

I'll move bitcoin.core.signature to bitcoin.signature, the classes I listed before are currently in test_signverify_message.py, normally I'd move those into bitcoin.messages but ... that's already being used and that has a completely different context, any idea where these should go?

  • class BitcoinMessage(ImmutableSerializable)
  • def SignMessage(key, message)
  • def VerifyMessage(address, message, sig)

@rubensayshi
Copy link
Contributor Author

I just moved them into bitcoin.sign_verify_message, just to already seperate them, but it should get a more sensible name, I just can't come up with anything good ...

@petertodd
Copy link
Owner

How about just bitcoin.signmessage?

It'd be good to have signmessage.py and verifymessage.py scripts in the examples/ directory showing the functionality off too.

@rubensayshi rubensayshi force-pushed the sign-verify-message branch from 23aa0a3 to 0b8318c Compare June 30, 2015 08:50
@rubensayshi
Copy link
Contributor Author

okay, bitcoin.signmessage sounds good, moved!
also added example signmessage.py that does both, not really neccesary to have 2 scripts.

and I rebased and squashed all the commits into 1

@rubensayshi
Copy link
Contributor Author

@petertodd I think this is good to go?

@petertodd petertodd merged commit 0b8318c into petertodd:master Jul 9, 2015
petertodd added a commit that referenced this pull request Jul 9, 2015
0b8318c Add signing and verifying of a message compatible with bitcoin core, using pubkey recovery. (Ruben de Vries)
@petertodd
Copy link
Owner

Thanks!

Sorry, was out of the country, as usual. :/

@rubensayshi
Copy link
Contributor Author

awesome! thanks!

can we also get a v0.4.1 ;) ?

@petertodd
Copy link
Owner

Time for a v0.5.0 I think.

ghtdak pushed a commit to ghtdak/python-bitcoinlib that referenced this pull request Dec 1, 2015
0b8318c Add signing and verifying of a message compatible with bitcoin core, using pubkey recovery. (Ruben de Vries)

 [ yapified by gitreformat (github/ghtdak) on Mon Nov 30 21:12:19 2015 ]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants