Merge branch 'release/v20171024'

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

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

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')
 

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)

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
 
 
-<FlashQrCodeScreen@BoxLayout>:
+<FlashQrCodeScreen>:
     on_pre_enter:
         zbarcam_id.play = True
         self.bind_on_symbols()

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()

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.

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.