diff --git a/TODO.md b/TODO.md index fde941b..c5a1811 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,9 @@ * `env['LDFLAGS'] += ' -L' + join(libsecp256k1_dir, '.libs')` # note the `.libs` * But in fact passing the library path ("LIB_DIR" and "LDFLAGS") may not be required because libsecp256k1 recipe has `self.install_libs(arch, *libs)` + * libsecp256k1 + * remove java symbols from libsecp256k1 (configure `--enable-jni=no`) to reduce library size + * `should_build()` with `.libs/libsecp256k1.so` check * MISC * kill running threads on application leave so it doesn't hangs when you quite while the thread tries to connect diff --git a/src/CHANGELOG.md b/src/CHANGELOG.md index 730ada0..ce7c5c3 100644 --- a/src/CHANGELOG.md +++ b/src/CHANGELOG.md @@ -1,5 +1,12 @@ # Change Log +## [v20171024] + + - Fix crash on startup, refs #94 + - Fix crash on null account deletion, refs #90 + - Fix crash on account deletion twice, refs #88 + - Fix crash on dialog dismiss, refs #89 + ## [v20171022] - Add flash QR Code support, refs #85 diff --git a/src/main.py b/src/main.py index 8fe3523..ce3e581 100755 --- a/src/main.py +++ b/src/main.py @@ -90,6 +90,10 @@ def setup(self): current_account=lambda _, value: self.on_current_account(value)) def on_current_account(self, account): + # e.g. deleting the last account, would set + # Controller.current_account to None + if account is None: + return address = "0x" + account.address.encode("hex") self.address_property = address @@ -263,7 +267,7 @@ def on_password(self, instance, password): class Receive(BoxLayout): - current_account = ObjectProperty() + current_account = ObjectProperty(allownone=True) address_property = StringProperty() def __init__(self, **kwargs): @@ -298,6 +302,8 @@ def update_address_property(self): self.address_property = address def on_current_account(self, instance, account): + if account is None: + return self.update_address_property() def on_address_property(self, instance, value): @@ -314,7 +320,7 @@ def on_alias_updated(self, instance, alias): class History(BoxLayout): - current_account = ObjectProperty() + current_account = ObjectProperty(allownone=True) def __init__(self, **kwargs): super(History, self).__init__(**kwargs) @@ -370,6 +376,8 @@ def update_history_list(self, list_items): @run_in_thread def _load_history(self): + if self.current_account is None: + return account = self.current_account address = '0x' + account.address.encode("hex") try: @@ -432,7 +440,7 @@ def load_account_list(self): class Overview(BoxLayout): - current_account = ObjectProperty() + current_account = ObjectProperty(allownone=True) current_account_string = StringProperty() def __init__(self, **kwargs): @@ -453,6 +461,8 @@ def update_current_account_string(self): """ Updates current_account_string from current_account. """ + if self.current_account is None: + return account = self.current_account address = "0x" + account.address.encode("hex") self.current_account_string = address @@ -516,7 +526,8 @@ def setup(self): # TODO: create a generic account form class ManageExisting(BoxLayout): - current_account = ObjectProperty() + # e.g. when the last account was deleted + current_account = ObjectProperty(allownone=True) address_property = StringProperty() current_password = StringProperty() new_password1 = StringProperty() @@ -580,13 +591,28 @@ def on_delete_account_yes(self, dialog): account = self.current_account self.pywalib.delete_account(account) dialog.dismiss() + # TODO + self.controller.current_account = None self.show_redirect_dialog() self.controller.load_landing_page() + def prompt_no_account_error(self): + """ + Prompts an error since no account are selected for deletion, refs: + https://github.com/AndreMiras/PyWallet/issues/90 + """ + title = "No account selected." + body = "No account selected for deletion." + dialog = Controller.create_dialog(title, body) + dialog.open() + def prompt_delete_account_dialog(self): """ - Not yet implemented. + Prompt a confirmation dialog before deleting the account. """ + if self.current_account is None: + self.prompt_no_account_error() + return title = "Are you sure?" body = "" body += "This action cannot be undone.\n" @@ -620,6 +646,10 @@ def update_password(self): Controller.snackbar_message("Updated!") def on_current_account(self, instance, account): + # e.g. deleting the last account, would set + # Controller.current_account to None + if account is None: + return address = "0x" + account.address.encode("hex") self.address_property = address @@ -980,7 +1010,8 @@ def on_symbols(self, instance, symbols): class Controller(FloatLayout): - current_account = ObjectProperty() + # e.g. when the keystore is void + current_account = ObjectProperty(allownone=True) current_account_balance = NumericProperty(0) # keeps track of all dialogs alive dialogs = [] @@ -1191,7 +1222,12 @@ def on_dialog_dismiss(dialog): """ Removes it from the dialogs track list. """ - Controller.dialogs.remove(dialog) + try: + Controller.dialogs.remove(dialog) + except ValueError: + # fails silently if the dialog was dismissed twice, refs: + # https://github.com/AndreMiras/PyWallet/issues/89 + pass @staticmethod def dismiss_all_dialogs(): @@ -1284,6 +1320,8 @@ def fetch_balance(self): """ Fetches the new balance and current_account_balance property. """ + if self.current_account is None: + return account = self.current_account try: self.current_account_balance = self.pywalib.get_balance( @@ -1376,6 +1414,9 @@ def load_flash_qr_code(self): """ Loads the flash QR Code screen. """ + # loads ZBarCam only when needed, refs: + # https://github.com/AndreMiras/PyWallet/issues/94 + from zbarcam import ZBarCam # noqa # loads the flash QR Code screen self.screen_manager_current('flashqrcode', direction='left') diff --git a/src/pywalib.py b/src/pywalib.py index a0b3f8c..c7412a6 100755 --- a/src/pywalib.py +++ b/src/pywalib.py @@ -275,7 +275,10 @@ def delete_account(self, account): if not os.path.exists(deleted_keystore_dir): os.makedirs(deleted_keystore_dir) # "removes" it from the file system - shutil.move(account.path, deleted_keystore_dir) + account_filename = os.path.basename(account.path) + deleted_account_path = os.path.join( + deleted_keystore_dir, account_filename) + shutil.move(account.path, deleted_account_path) # deletes it from the `AccountsService` account manager instance account_service = self.get_account_list() account_service.accounts.remove(account) diff --git a/src/pywallet.kv b/src/pywallet.kv index 4f6fcd0..d590088 100644 --- a/src/pywallet.kv +++ b/src/pywallet.kv @@ -1,6 +1,5 @@ #:kivy 1.10.0 -#:import ZBarCam zbarcam #:import ROUND_DIGITS pywalib.ROUND_DIGITS #:import webbrowser webbrowser #:import Window kivy.core.window.Window @@ -418,7 +417,7 @@ id: manage_keystores_id -: +: on_pre_enter: zbarcam_id.play = True self.bind_on_symbols() diff --git a/src/tests/test_import.py b/src/tests/test_import.py index 61dc88c..5c080bc 100644 --- a/src/tests/test_import.py +++ b/src/tests/test_import.py @@ -46,6 +46,14 @@ def test_pyethapp(self): self.assertIsNotNone(account) self.assertIsNotNone(address) + def test_zbarcam(self): + import zbarcam + # zbarcam imports PIL and monkey patches it so it has + # the same interfaces as Pillow + self.assertTrue(hasattr(zbarcam, 'PIL')) + self.assertTrue(hasattr(zbarcam.PIL.Image, 'frombytes')) + self.assertTrue(hasattr(zbarcam.PIL.Image.Image, 'tobytes')) + if __name__ == '__main__': unittest.main() diff --git a/src/tests/test_pywalib.py b/src/tests/test_pywalib.py index 6dba313..670de38 100644 --- a/src/tests/test_pywalib.py +++ b/src/tests/test_pywalib.py @@ -110,7 +110,7 @@ def test_deleted_account_dir(self): def test_delete_account(self): """ Creates a new account and delete it. - Then verify we can load the account from the backup location. + Then verify we can load the account from the backup/trash location. """ pywalib = self.pywalib account = self.helper_new_account() @@ -122,12 +122,36 @@ def test_delete_account(self): # even recreating the PyWalib object pywalib = PyWalib(self.keystore_dir) self.assertEqual(len(pywalib.get_account_list()), 0) - # tries to reload it from the backup location + # tries to reload it from the backup/trash location deleted_keystore_dir = PyWalib.deleted_account_dir(self.keystore_dir) pywalib = PyWalib(deleted_keystore_dir) self.assertEqual(len(pywalib.get_account_list()), 1) self.assertEqual(pywalib.get_account_list()[0].address, address) + def test_delete_account_already_exists(self): + """ + If the destination (backup/trash) directory where the account is moved + already exists, it should be handled gracefully. + This could happens if the account gets deleted, then reimported and + deleted again, refs: + https://github.com/AndreMiras/PyWallet/issues/88 + """ + pywalib = self.pywalib + account = self.helper_new_account() + # creates a file in the backup/trash folder that would conflict + # with the deleted account + deleted_keystore_dir = PyWalib.deleted_account_dir(self.keystore_dir) + os.makedirs(deleted_keystore_dir) + account_filename = os.path.basename(account.path) + deleted_account_path = os.path.join( + deleted_keystore_dir, account_filename) + # create that file + open(deleted_account_path, 'a').close() + # then deletes the account and verifies it worked + self.assertEqual(len(pywalib.get_account_list()), 1) + pywalib.delete_account(account) + self.assertEqual(len(pywalib.get_account_list()), 0) + def test_handle_etherscan_error(self): """ Checks handle_etherscan_error() error handling. diff --git a/src/tests/ui/test_ui_base.py b/src/tests/ui/test_ui_base.py index 85ee282..f3fe79f 100644 --- a/src/tests/ui/test_ui_base.py +++ b/src/tests/ui/test_ui_base.py @@ -320,6 +320,116 @@ def helper_test_delete_account(self, app): account_list_id = switch_account.ids.account_list_id self.assertEqual(len(account_list_id.children), 0) + def helper_test_delete_account_none_selected(self, app): + """ + Tries to delete account when none are selected, refs: + https://github.com/AndreMiras/PyWallet/issues/90 + """ + controller = app.controller + pywalib = controller.pywalib + manage_existing = controller.manage_existing + # makes sure an account is selected + pywalib.new_account(password="password", security_ratio=1) + controller.current_account = pywalib.get_account_list()[0] + # ManageExisting and Controller current_account should be in sync + self.assertEqual( + manage_existing.current_account, controller.current_account) + # chaning in the Controller, should trigger the change on the other + self.assertTrue(manage_existing.current_account is not None) + controller.current_account = None + self.assertIsNone(manage_existing.current_account) + # let's try to delete this "None account" + delete_button_id = manage_existing.ids.delete_button_id + delete_button_id.dispatch('on_release') + # an error dialog should pop + dialogs = controller.dialogs + self.assertEqual(len(dialogs), 1) + dialog = dialogs[0] + self.assertEqual(dialog.title, 'No account selected.') + controller.dismiss_all_dialogs() + + def helper_confirm_account_deletion(self, app): + """ + Helper method for confirming account deletion popups. + """ + controller = app.controller + manage_existing = controller.manage_existing + # a confirmation popup should show + dialogs = controller.dialogs + self.assertEqual(len(dialogs), 1) + dialog = dialogs[0] + self.assertEqual(dialog.title, 'Are you sure?') + # confirm it + # TODO: click on the dialog action button itself + manage_existing.on_delete_account_yes(dialog) + # the dialog should be replaced by another one + dialogs = controller.dialogs + self.assertEqual(len(dialogs), 1) + dialog = dialogs[0] + self.assertEqual(dialog.title, 'Account deleted, redirecting...') + controller.dismiss_all_dialogs() + self.assertEqual(len(dialogs), 0) + + def helper_test_delete_account_twice(self, app): + """ + Trying to delete the same account twice, shoult not crash the app: + https://github.com/AndreMiras/PyWallet/issues/51 + """ + controller = app.controller + pywalib = controller.pywalib + manage_existing = controller.manage_existing + # makes sure an account is selected + pywalib.new_account(password="password", security_ratio=1) + controller.current_account = pywalib.get_account_list()[0] + self.assertTrue(manage_existing.current_account is not None) + account_count_before = len(pywalib.get_account_list()) + # let's try to delete this account once + delete_button_id = manage_existing.ids.delete_button_id + delete_button_id.dispatch('on_release') + self.helper_confirm_account_deletion(app) + # the account should be deleted + self.assertEqual( + len(pywalib.get_account_list()), account_count_before - 1) + # makes sure the account was also cleared from the selection view + switch_account = self.helper_load_switch_account(app) + account_list_id = switch_account.ids.account_list_id + self.assertEqual( + len(account_list_id.children), len(pywalib.get_account_list())) + # TODO: the selected account should now be None + # self.assertIsNone(manage_existing.current_account) + # self.assertIsNone(controller.current_account) + # let's try to delete this account a second time + delete_button_id = manage_existing.ids.delete_button_id + delete_button_id.dispatch('on_release') + # TODO: the second time an error dialog should pop + # dialogs = controller.dialogs + # self.assertEqual(len(dialogs), 1) + # dialog = dialogs[0] + # self.assertEqual(dialog.title, 'No account selected.') + controller.dismiss_all_dialogs() + + def helper_test_dismiss_dialog_twice(self, app): + """ + If by some choice the dismiss event of a dialog created with + Controller.create_dialog_helper() is fired twice, it should be + handled gracefully, refs: + https://github.com/AndreMiras/PyWallet/issues/89 + """ + Controller = main.Controller + title = "title" + body = "body" + # makes sure the controller has no dialog + self.assertEqual(Controller.dialogs, []) + # creates one and verifies it was created + dialog = Controller.create_dialog_helper(title, body) + self.assertEqual(len(Controller.dialogs), 1) + # dimisses it once and verifies it was handled + dialog.dispatch('on_dismiss') + self.assertEqual(Controller.dialogs, []) + # then a second time and it should not crash + dialog.dispatch('on_dismiss') + self.assertEqual(Controller.dialogs, []) + # main test function def run_test(self, app, *args): Clock.schedule_interval(self.pause, 0.000001) @@ -329,6 +439,9 @@ def run_test(self, app, *args): self.helper_test_on_send_click(app) self.helper_test_address_alias(app) self.helper_test_delete_account(app) + self.helper_test_delete_account_none_selected(app) + self.helper_test_delete_account_twice(app) + self.helper_test_dismiss_dialog_twice(app) # Comment out if you are editing the test, it'll leave the # Window opened. diff --git a/src/version.py b/src/version.py index d2c9886..74c184b 100644 --- a/src/version.py +++ b/src/version.py @@ -1 +1 @@ -__version__ = '2017.1022' +__version__ = '2017.1024'