Android: add Load & Sign unbroadcasted tx #1986

mpatc · 2020-09-01T15:59:01Z

same as before but with .gitignore and android/.idea/inspectionProfiles/Project_Default.xml added

not for vcs

cculianu · 2020-09-01T18:46:45Z

I don't have the capability to test this right now -- has @mhsmith or @fyookball or maybe even @EchterAgo tried this ? Or can you, @mpatc 100% vouch for it working?

mhsmith · 2020-09-01T19:05:55Z

I'll have a look at it tomorrow.

mpatc · 2020-09-01T21:42:26Z

Thanks mhsmith, I've tested on the emulator and a galaxy note 20 ultra and it works, but it's my first contribution, so I don't know what I might have missed. The changes are the addition of ColdSend.kt and ColdLoad.kt, and accompanying changes to the wallet menu xml

mhsmith · 2020-09-03T22:29:18Z

ColdSend.kt is mostly a duplicate of Send.kt, except for what happens after you enter the password. This isn't good, because any future change to the send process will require twice as many changes and twice as much testing.

Instead, I think saving a signed transaction should be combined with the existing send process, as it is in the desktop version. Here's one possible way, but if you've got any other ideas then let's discuss them:

Add a third "Save" button at the bottom left of the send password dialog, which runs the new code.
Since the send password dialog can now perform two different actions, its "OK" button should change to "Send".
In the main Send dialog, change the "OK" button to "Next", so a user who intends to save the tranaction won't worry that clicking it will cause an immediate send. Then for consistency, the Cancel button in the send password dialog would change to "Back". (The two-stage New Wallet process already uses these words.)

I've also got a few other minor comments which I'll add below.

mhsmith · 2020-09-03T22:31:50Z

By the way, there's no need to create a new pull request every time you make some changes: it should be enough just to push the changes to your branch on GitHub, and they'll appear in the existing PR.

android/app/.settings/org.eclipse.buildship.core.prefs

android/app/src/main/java/org/electroncash/electroncash3/ColdLoad.kt

android/app/src/main/java/org/electroncash/electroncash3/Util.kt

android/app/src/main/python/electroncash_gui/android/strings.py

mpatc · 2020-09-18T16:20:53Z

So at this point I've gotten rid of the ColdSend.kt and instead do the work it did in Send.kt by the use of a flag. Seems to work as intended. The only major thing is to get ColdLoad to toast any errors after it attempts to broadcast.

mpatc · 2020-09-19T12:43:50Z

As far as I can see, everything is done here, I'm going to work on implementing freeze/unfreeze addresses.

mhsmith · 2020-09-24T18:16:52Z

Thanks, I'll give it a last look over and probably merge it tomorrow.

It never seems to be empty on the emulator, but I was able to make the app crash on a physical device by trying to paste after a reboot.

It was passing a string to a method which expected a transaction, so the method always returned failure. Detecting "success" by looking for "txid" in the text of the error message isn't reliable, because that string could also appear in a real error. Changed to reuse the existing result checking code in Send.kt.

onShowDialog is called again after each rotation, so it can't unconditionally disable the button.

…be specified everywhere: Actually getBoolean already returns false by default, but it's better to be explicit.

* Make unbroadcasted flag an instance variable rather than a global. * Fix duplicate code in `if` statements.

mhsmith · 2020-09-26T22:49:27Z

Thanks for all your work on this. I did a few fixes and cleanups before merging: please look over my commits above before submitting your next PR.

By the way, I notice that the Electron Cash desktop app doesn't display transaction QRs as hex, but as an Electrum-specific base 43 format, so the app isn't able to scan them. How did you test this feature? Did you use some other wallet that displays hex QRs? Or did you just do the same as me, and copy a hex transaction into a QR generator?

mpatc · 2020-09-26T23:47:18Z

cool, I'll take a look later, yes, I just pasted the signed tx from virtual instance into chrome qr generator and then scanned it from my actual phone to load.

…

On Sat, Sep 26, 2020 at 6:49 PM Malcolm Smith ***@***.***> wrote: Thanks for all your work on this. I did a few fixes and cleanups before merging: please look over my commits above before submitting your next PR. By the way, I notice that the Electron Cash desktop app doesn't display transaction QRs as hex, but as base 43, so the app isn't able to scan them. How did you test this feature? Did you use some other wallet that displays hex QRs? Or did you just do the same as me, and copy a hex transaction into a QR generator? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1986 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADZVZPQUACQ6Z7NSNZAR243SHZVYFANCNFSM4QR4OY3Q> .

* added unbroadcasted send * added kt file for unbroadcast tx * fix coldsend dialog title * add load unbroadcasted tx * add load tx kotlin file * cleanup * Delete local.properties not for vcs * added missing files * removed generated files * remove files * cleanup * fix qr code in coldload * change buttontext ok to sign * cleanup whitespace * remove ColdSend.kt and add sign unbroadcasted to Send.kt * format comments and remove paste button enabler * removed accidently added package to main.kt * change name of paste button * remove toast used for testing * toast errors from broadcasting transactions * fix toast message on tx broadcast * comment fix * fix result toast * fixes * rename variables * change title to sign transaction * remove redundent declaration * remove extraneous generated files * copy Util.kt from original repo * replace original gitignore and project_default files * whitspace cleanup * check if connected before broadcasting Cold tx * refactor if statement * Check whether clipboard is empty before pasting: It never seems to be empty on the emulator, but I was able to make the app crash on a physical device by trying to paste after a reboot. * Fix ColdLoad toast issues: It was passing a string to a method which expected a transaction, so the method always returned failure. Detecting "success" by looking for "txid" in the text of the error message isn't reliable, because that string could also appear in a real error. Changed to reuse the existing result checking code in Send.kt. * Fix ColdLoad send button being disabled after rotating screen: onShowDialog is called again after each rotation, so it can't unconditionally disable the button. * Revert irrelevant whitespace changes in Send.kt * Make "unbroadcasted" argument false by default so it doesn't have to be specified everywhere: Actually getBoolean already returns false by default, but it's better to be explicit. * Send.kt cleanups: * Make unbroadcasted flag an instance variable rather than a global. * Fix duplicate code in `if` statements. Co-authored-by: Matthew Clancy <matthewpclancy@mpc.local> Co-authored-by: Malcolm Smith <smith@chaquo.com>

Matthew Clancy and others added 10 commits August 29, 2020 13:13

added unbroadcasted send

ebf096e

added kt file for unbroadcast tx

3704fbd

fix coldsend dialog title

8ad54c9

add load unbroadcasted tx

281c851

add load tx kotlin file

7c5cabc

cleanup

cd91072

Delete local.properties

441a0c0

not for vcs

added missing files

919dfb3

removed generated files

b808dfc

remove files

fa29fc9

cculianu added android enhancement labels Sep 1, 2020

This comment has been minimized.

Sign in to view