spesmilo / electrum

Electrum Bitcoin Wallet
https://electrum.org
MIT License
7.54k stars 3.11k forks source link

Python3 support #1075

Closed asfin closed 7 years ago

asfin commented 9 years ago

Will electrum migrate/support Python3? What stops this process? If I provide pull request on this issue, is there any chance that it will be accepted?

I've applied 2to3 script + few patches in code, so i can import and use libs, and qtgui can start and looks like it alive and responsible(i don't know is it fully functional or not).

Sorry if this is a dublicate, but i can't find any information about python3 support in electrum.

ecdsa commented 9 years ago

yes it would be nice to do the migration. last time I tried it was not so easy, but maybe you know this better than me

ecdsa commented 9 years ago

should be easier now, because the daemon uses os.fork

gustavklopp commented 8 years ago

+1

asfin commented 8 years ago

Should port use python2 and python3 simultaneously, or you will migrate to python3 fully in time? I've have many troubles with bytes support in python2, and it looks ugly. (Maybe Electrum-3.0 branch will fully migrate to py3? :smile: )

P. S. Sorry for this delay, but we've temporarily changed our project plans to use electrum.

asfin commented 8 years ago

I have two main ways for porting: First - just use six module and make code compatible with python3 as is. Second - rewrite all "hex" logic to bytes and clear api from using "hex" strings, replacing all to bytes or bytearray for python2. It makes api and code more clear, because you shoudn't cast int to string and do hex encode(in py3 you can omit all .encode("hex")), and allows clearly distinguish between text and data, so code will be more strict. But it's hard to maintain with upstream changes, and looks like too big change for accepting on PR, and maybe compatibility issues with android or other. I've already done a lot of work on second way, but I'm in doubt about your plans, and I want to clarify this.

Also I have some library compatibility troubles with aes and protobuf in python3. I've replaced aes with pyaes. Protobuf 3.0 branch support py3, but in not yet released as stable. And some troubles with socket and jsonrpclib but it's not so serious.

vince06fr commented 8 years ago

Python2 is end to life so I think the second option is better.
You can maybe get help by creating a python3 branch or fork

goetzc commented 8 years ago

@asfin do you have some public repository with your changes to support Python 3? I would like to help in the process.

asfin commented 8 years ago

@goetzc i have pair of branches in my fork, but not all changes pushed to repo. So I'll do some fixes in a week and update it. I'll notify you when it will be ready.

goetzc commented 8 years ago

Great!

venzen commented 8 years ago

ACK port to Python3 due to Python2 end-of-life as mentioned above.

ecdsa commented 8 years ago

@asfin I am currently working on a major refactoring. I will push it to the dev branch later today.

asfin commented 8 years ago

@ecdsa, you are here! Please reply for my previous post about port, should i do simple types porting, or i can change interface of functions e.g. change args to bytearrays instead of hex strings.

ecdsa commented 8 years ago

@asfin if you keep your changes minimal they will be easier to review

asfin commented 8 years ago

@goetzc I've pushed branch slow_py3_port to my repo, gui starts in py2 and py3, but it don't work at all. It was a long story about porting, so it is a third try, and it is big step back from previous port, which had passed almost all unittests, but it easier to re-port, than rebase :)

About structure: After very long hesitations i've decided to change API to bytes. I think it looks more strict and pythonic, when core functions use encoding-independent strings(also hex-like manipulations with binary strings looks more native). And it can slightly speedup core processing, because we don't need to encode strings to hex and back all the time, only in UI part. As drawbacks - need to change a lot of code in core lib and in UI parts, and fact, that many API calls change format(i think that "public" API should be universal, and shouldn't differ bytes from strings(but what about return types?), but helpers and utils/* functions must be binary only).

I've added some wrappers to util.py, like __bytes, to_bytes, bh2u, bfh, assert_bytes, assert_str - all documented in source. Now code contains a lot of debug prints and other garbage, during merge i've marked some incompats with "#TODO: py3" mark, which of course is only a small part of incompatibilities. I want to fix most of this things during next week or two, and my goal is that GUI can provide basic functions(create wallet, list txs, send-receive), after this i think it should be easier to collaborate, because all hot points will be fixed, other incompats will be harder to find and fix, and it will be distributed more evenly, so it reduce headache during rebase-merge cycle.

If i forgot to mention anything, or we should discuss bytes API, feel free to start conversation.

asfin commented 8 years ago

Things with py2 not as easy as i think. Some objects(like txi, txo, verified_tx3), uses tx_hashes as keys, and hashes are binary objects, which represented in py2 like bytearray. So bytearray is unhashable type, and we need always convert it to appropriate format in py2, which makes very obtrusive to use hashes in binary form. I think i should redo some of this changes, and only "always binary" objects will be pubkeys. About porting status: I can send and receive transactions via QtGUI in base BIP32 wallet, also i've tested multisig wallet creation, and it's ok. All test have passed(but i broke again some of it). Areas which i didn't port yet: all plugins, kivy ui, multisig wallets.

ecdsa commented 7 years ago

any news on this?

asfin commented 7 years ago

Hi, no good news. Because of intensive workload, i don't have enough time to follow updates of main branch, and my branch goes outdated too fast. Still haven't got vacation to finish it. Also i realized that way of porting which involves bytes/bytearray support not very good, because a lot of troubles with py2 support arises(like dict keys of bytearray type, and a lot of wrappers for bytes constructors). So i can port current codebase in way only to have "syntax" support for py3, and if you can merge this non-very-stable version into master(a key issue), it would be great, because i can't(by myself) follow updates.

JustinTArthur commented 7 years ago

@asfin do you think it's easy enough for some folks to take over this branch where you left off?

We could also do a few piecemeal 2-and-3 compatible pull requests for the low hanging fruit like print statement->function, __futures__ division, up until it's only the hard stuff left.

I recently undertook a similar effort for electrum-server, and it was similarly hard to keep rebasing upstream changes as they came in, and by the time I got things kinda working, someone had come up with a replacement project.

ecdsa commented 7 years ago

@JustinTArthur I think that now would be a good time to do that, the code structure in lib should be fairly stable now. One issue remains: python2 is still required for kivy on iOS. https://github.com/kivy/kivy-ios/issues/184

venzen commented 7 years ago

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1

iOS is marginal, give Apple's plans for the future. Focus on Linux and Android, the rest is not so important?

On 01/16/2017 04:23 PM, ThomasV wrote:

@JustinTArthur https://github.com/JustinTArthur I think that now would be a good time to do that, the code structure in lib should be fairly stable now. One issue remains: python2 is still required for kivy on iOS. kivy/kivy-ios#184 https://github.com/kivy/kivy-ios/issues/184

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/spesmilo/electrum/issues/1075#issuecomment-2728107 42,

or mute the thread https://github.com/notifications/unsubscribe-auth/AGmH2NcJGX8jh8afN-I eI5CZJMAJGUWOks5rSzcegaJpZM4Dsk_g.

-----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux)

iQEcBAEBAgAGBQJYfJz7AAoJEGwAhlQc8H1m+E8H/07f4XQYvNsCKw+yzERKY4Ux ppoBoMF10zw6LO2uYZPgszvCBOHZxW0liDS4pai4i4udlIy0AcfUx9UJL8irhwJN MCuLVtu3rIvmWNt+vUGNqgRKR7YWUpNELJFaUhFzr7uZMGFle8IrAPUCxXz8DyND VtxzAfjrSg1v5PeRRMS3IcdiIsg1RNSr2B/tmLqiIsKAwOBNRkiBeVFq6vNhTrAP 1cIjf3Yji+kR85SpyYZJ0NcpQGdyPpOQUR6vHwVsYc1YFU5P9D22DG/rfZvcngDf h8lsjtj6rXFBnuWgKzOWk6U5j2u6DrL7jnLAKUvGKb230S5wigHO1gIiPBK/p+M= =ZaZX -----END PGP SIGNATURE-----

asfin commented 7 years ago

@JustinTArthur Ok, on current weekend i'll update my repo with new version. After this we can coordinate and continue with rest of it, like tests, plugins, and corner-cases.

ecdsa commented 7 years ago

I agree iOS is marginal and should not be blocking us. Also, the android app would likely run faster with python3

asfin commented 7 years ago

@ecdsa Does it mean that we can drop py2 support?

ecdsa commented 7 years ago

@asfin I will not be maintaining 2 versions

asfin commented 7 years ago

@JustinTArthur updated my master, all tests ok, but UI can't start fully, and no plugins. Still need more time for updating UI. Currently code support both py2 and py3, but i'll drop py2 support, all ok?

Unresolved issues: import imp - deprecated, need to update properly jsonrpclib dependency - didn't work in py3, commented out. Need fix in daemon.py plugins - not ported yet lots of unstable stuff in network.py - i've experienced a lot of problems with network communications, and silent errors here early. Need to check.

ecdsa commented 7 years ago

@asfin if you are working on this, it would make sense to use the 'segwit' branch. lots of unmerged changes there. I hope to merge it this week.

JustinTArthur commented 7 years ago

@asfin ok, thanks. There's a jsonrpclib fork I used for electrum-server py3 attempt. I can look into network.py stuff. What's your capacity looking like for these next couple weeks?

ecdsa commented 7 years ago

note: segwit is now merged

asfin commented 7 years ago

Monkey-patched local jsonrpclib with 2to3, and uncommented stuff in daemon.py, so now i can start ui, and it's not crashing on every action. Need to adapt qrscanner to use zbarlight, because zbar cannot be used with py3(used zbarlight with opencv in my qt project, because got some troubles with QCamera) Parts of py2 compatibility removed. Rebased onto fresh changes with segwit. Still no plugins, and still need to replace jsonrpclib.

ecdsa commented 7 years ago

@asfin that's great news, thanks a bunch

ecdsa commented 7 years ago

@asfin where is your repo? I could have a look

asfin commented 7 years ago

Master branch https://github.com/asfin/electrum And because protobuf3 may be hard to get, you can use this precompiled file. Put it into lib/ and remove .txt suffix. paymentrequest_pb2_py3.py.txt I'm not sure that it's ready to clone and run, because not all deps may be satisfied. Also you may need to manually fix icons_rc.py, and add b'somestring' string prefix to all data lines.

asfin commented 7 years ago

@ecdsa core libs and qt gui working fine with standard wallet - send-receive, encryption, signing. Some plugins ported. but we need to replace dependency for jsonrpclib, because i don't know how it should work. Single trouble - icons_rc.py. Also need to check building for other platforms - Win, Android(I'll check MacOS later). I didn't touched kivy part, should i also update it? With some updates, I'm ready for PR.

ecdsa commented 7 years ago

that's great news. I am testing your branch now. this works for me: pyrcc4 -py3 icons.qrc -o gui/qt/icons_rc.py

ecdsa commented 7 years ago

there's a bunch of name 'cmp' is not defined errors from network.py, is_recent

ecdsa commented 7 years ago

all right, I am able to start the qt gui. congrats!

ecdsa commented 7 years ago

I am trying to update the kivy GUI. Did you start looking at that? note: if you installed kivy for python2, it is better to clone it again in a separate directory for python3.

asfin commented 7 years ago

Not looked yet on kivy, can update on next weekend.

ecdsa commented 7 years ago

ok, porting the kivy GUI to python3 is not difficult; I did it with minimal changes, and I can add a PR to your repo, or create a branch in the electrum repo.

However, creating an Android APK with python3 is difficult. I did not succeed yet. I was told by @akshayaurora that the kivy project is working on that, and that I should probably wait a couple of months.

So I think it is better to wait until kivy has real python3 support, before we merge this in the electrum master branch. In the meantime we can see how plugins need to be upgraded.

ecdsa commented 7 years ago

note: it seems that aes can be replaced with pyaes with python2 too; I'm going to do that first.

ecdsa commented 7 years ago

jsonrpclib can be replaced with this fork: https://github.com/tcalmant/jsonrpclib pip3 install jsonrpclib-pelix

ecdsa commented 7 years ago

I am now rebasing your branch and merging with my changes. the new branch will be pushed today.

asfin commented 7 years ago

I'm correctly understand that it mean merge to your master?

ecdsa commented 7 years ago

not to master, to a separate branch. there are still some issues.

ecdsa commented 7 years ago

it looks like zbar needs to be replaced with https://github.com/zplab/zbar-py I'm going to give it a try, that package might even work on osx/windows

ecdsa commented 7 years ago

@asfin or shall we rather use zbarlight?

ecdsa commented 7 years ago

The new branch is available here: https://github.com/spesmilo/electrum/tree/python3

asfin commented 7 years ago

if zbar-py can be drop-in replacement, it's better. zbarlight must be adopted for this usage, i wanted to do it fast, but it wasn't simple.

ecdsa commented 7 years ago

unfortunately there is no direct replacement for the zbar wrapper we used with python2 the closest I have found is the following wrapper: https://github.com/brettatoms/zbar-ctypes it needs to be extended, though, because it does not support the zbar::Processor class

ecdsa commented 7 years ago

problem solved: I rewrote qrscanner using ctypes+zbar instead of the python wrapper d99855f06072c33cef144edce68acded561e339a note: I rebased and force pushed the python3 branch. you might want to delete your local copy of that branch.