Fixes issue #1957 #2252

Aditi-Singh16 · 2023-02-23T18:10:09Z

fixes #1957

* Some smaller improvements and fixes * mitigating 2078 * removed Microsoft link tracking * wrap up a failed update properly * better decode and better error_handling

* change passwordaspin auth implementation * env-var for RASPIBLITZ_SPECTER_RPC_LOGIN_BITCOIN_CONF_LOCATION * remove default * fix tests * fix the tests part2 * fix cypress tests * docstrings and other default removals * improve error_handling * do not raise exception

* added websocket reconnect javascript * timeout fix * FIXED server timout issue * working port config * added websockets * fixed cypress test * graceful exit, but client is slow * fast shutdown * Switch to other port, if this one is blocked * better error handling * undid the port retrying * trying CI with different port for websockets * added getting port from ENV * test CI with longer wait time for cypress * reset wait time to 1000 * test fix * doc * Bugfix: Flash cannot be called without a context. If a WebAPI (initiated from a different thread) fails, and flash is acalled as a backup the flash is without context. Now I simply added a attribute that flash cannot be called from any thread. It now skips it during the "Trying to rebroadcast" * restore feature that the WebAPI permission is updated * js formatting * doc and reshuffling methods * localhost fix for cypress * fixed bug that i introduced recently. * doc * refactoring * refactoring * fix bug and refactoring * removed use_reloader=false * made the CreateNotification more robust * better log * bug fix * pytest fix * bug fix * pytest works... but ugly * do not start the websockets server in the tests * Introducing SPECTER_WEBSOCKET_ACTIVE * fix for incomplete SPECTER_WEBSOCKET_ACTIVE * fix for incomplete SPECTER_WEBSOCKET_ACTIVE * working websockets server shutdown when main thread dies * doc * camelCase for js also changed WebAPI --> webapi * better naming * ssl working for python client and server, but not javascript * minimal working example for self-signed ssl * working minimal echo server in specter works with self signed cert * working user recognition * working websocket server. The client shows an ssl error * added ssl_context, but doesn't work yet * no error... not tested yet * working * working, but not in js yet * almost working!!! * client is not working * still cannot get a 1 server to many clients working * minimal client example * working minimal exaple to send to all connected clients * working (at first, but then disconnecting) * working. but cient needs some simplifications * working great!!! * fix closing connection handling * also for non ssl working * doc and requirements * better config * fix for multiple connections of 1 user * imports * doc * better logging * added endpoint to test * cleanup * added a possibility to quit websocket connections via a special command. * fix error message * doc * refactoring * remove old code * undo * Docs: Add mobile access question to the FAQ (#1829) * starting pytest pytest mocking doesn't work yet working pytest * better descriptions * more notification arguments * added pytest * move notifications.js such that it is always available * fix css issue * A test-page in the devhelper extension * defaults that shows a message without changing values * doc and rename: clarifying point about the flask flash * improved logging formatting * js_message_box reflects noy the notification_type * doc addressing #1766 (comment) * doc addressing #1766 (comment) * fix pytest * doc and use get instead of [] * redirect flash call * feature now complete * documentation * rename callback * Adding add_wallettab * moving to server_endpoints * proper removal * refactor robust_json_dumps import * forgot one reference of robust_json_dumps * remove unintended changes * import fixes and polishing * docs revised * working notifications * working almost completely moved * not working yet.... somehow sending to the server doesn't work. * working * added TODO * redirect flash * fix for flash * implemented callbacks Can test with js code a = await pythonCommand("app.specter.service_manager.execute_ext_callbacks( 'create_and_show_notification', 'title', target_uis='js_message_box', timeout=3000)") * invisible in sidebar and enabled by default * added logo * adding wallet_alias and docs * pictures for the docs * added import * forgotten change * pytest fix * forgotten part 2 * add HOST to config and fix pytest * examples in settings and wallet menu * app is not required in callback_after_serverpy_init_app * better error logging * fix: better error logging * fix: include * Removing Werkzeug Request logs for Cypress * selectWallet twice to fix weird issues * explicitly returing None if no user found * better structuring in server function * reset lots of flash changes * add timeout for websocket server connections * reset helpers.py * added connection_report - also changed the registering logic slightly, to now have an easy overview of the open connections - also added constants INTERNAL_NOTIFICATION_TITLES * better default services in user.py * tiny cleanup * avoid slow_request_detection_stop * move css #1766 (comment) * typo #1766 (comment) * callback_cleanup_on_exit #1766 (comment) * move also css colors to service * upgrade simple_websocket got rid of ssl error on close connection * fix tests * better default loading of notifications * fix bug see #1766 (comment) * better callback_cleanup_on_exit * deleted files * fix 1 pytest * removed example code #1766 (comment) * cleaned up js logic for the initial connection * moved sendRequest * doc * doc * increase ping/timeout interval, because the websocket connection closure works well now * increase migration count * renaming * changed order of notifications cypress test * disable the websockets during all pytest * reenable websockets for cypress * fix bug * pytest debugging ONLY. Have to rollback * Revert "pytest debugging ONLY. Have to rollback" This reverts commit ccee38f. * info output for pytest debugging * pytest experiment... Can be rolled back. * Revert "info output for pytest debugging" This reverts commit 07f3aaa. * fix see #1907 (comment) * added view_functions comment * trying to fix flaky cpyress * fix for hardcoded url * added auto-reconnection on request failiure * remove user_secret_decrypted_required * remove user_secret_decrypted_required * Removed verbose logging unless logging level < 10 * remove verbose_debug from notification.id * fix * fix pytest * fix non-working migration * set devstatus_alpha and opt-in * delete cypress test and remove migration from pytest * len(classlist) seems to differ in CI and locally * remove useless code * done #1766 (comment) - removed duplicate sendRequest * moved service_manager_cleanup_on_exit see #1766 (comment) * added comment see #1766 (comment) * attempt to fix #1766 (comment) * pytest works. Was a variation in requirements.txt --------- Co-authored-by: Manolis Mandrapilias <70536101+moneymanolis@users.noreply.github.com> Co-authored-by: Kim Neunert <k9ert@gmx.de> Co-authored-by: moneymanolis <moneymanolis@protonmail.com>

…2109) * added support for qr message signing for jade * remove taproot support for jade --------- Co-authored-by: Manolis Mandrapilias <70536101+moneymanolis@users.noreply.github.com> Co-authored-by: moneymanolis <moneymanolis@protonmail.com>

* ping black version * change to version 22.3.0 as in pre-commit * proper version * kick

Bumps [http-cache-semantics](https://github.com/kornelski/http-cache-semantics) from 4.1.0 to 4.1.1. - [Release notes](https://github.com/kornelski/http-cache-semantics/releases) - [Commits](kornelski/http-cache-semantics@v4.1.0...v4.1.1) --- updated-dependencies: - dependency-name: http-cache-semantics dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fixes psbt key error * use lock to avoid concurrent access

Bumps [@sideway/formula](https://github.com/sideway/formula) from 3.0.0 to 3.0.1. - [Release notes](https://github.com/sideway/formula/releases) - [Commits](hapijs/formula@v3.0.0...v3.0.1) --- updated-dependencies: - dependency-name: "@sideway/formula" dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* additional tests for edge-cases * higher timeout for rpc test to avoid flakiness * remove default timeout in fixture Co-authored-by: moneymanolis <moneymanolis@protonmail.com>

netlify · 2023-02-23T18:10:15Z

✅ Deploy Preview for specter-desktop-docs canceled.

Name	Link
🔨 Latest commit	`fc831ba`
🔍 Latest deploy log	https://app.netlify.com/sites/specter-desktop-docs/deploys/63fb1058bf95c20008d371cb

k9ert · 2023-02-24T13:32:24Z

Great! However, the test is failing. You have to adjust it as you changed the constructor.

k9ert · 2023-02-27T08:36:07Z

src/cryptoadvance/specter/services/extension_gen.py

        self.isolated_client = isolated_client
        self.devicename = devicename
        self.author = author
        self.author_email = email
        vc = VersionChecker()
        version = vc._get_current_version()
-        if version == "custom":
+        if self.custom_version != "":
+            if vc._check_if_version_is_available(custom_version) == True:


== True is not needed. In order to improve readability, you could rename the method and call it:

Suggested change

if vc._check_if_version_is_available(custom_version) == True:

if vc._version_is_available(custom_version):

Comparing methods which return booleans explicitely is a common issue. It's not wrong but i hope the readability improvement above makes sense to you.

k9ert · 2023-02-27T08:47:38Z

src/cryptoadvance/specter/services/extension_gen.py

        self.isolated_client = isolated_client
        self.devicename = devicename
        self.author = author
        self.author_email = email
        vc = VersionChecker()
        version = vc._get_current_version()
-        if version == "custom":
+        if self.custom_version != "":


Please have a look how branch and version is used. the feature i mostly spoke about was using specific branches (tags) as the basis for the templating here.
Yes, the self.version is used for specific templates in order to render out the version this extension is dependent on. But the branch variable specifies which templates are used: branches (or tags, doesn't matter) in the specterext-dummy-repo. You can rename "branch" to "tag" which would make more sense.

k9ert · 2023-02-27T08:49:31Z

src/cryptoadvance/specter/cli/cli_ext.py

+    if custom_version == None:
+        print(
+            """
+            The extension generation uses the latest version of specter. 


I think a fallback should rather be not the lastest version, but the version ext_gen is running on. That can be found (in <= v2) in from cryptoadvance.specter._version import version.

k9ert · 2023-02-27T08:50:48Z

src/cryptoadvance/specter/util/version.py

@@ -72,6 +72,37 @@ def _get_current_version(self):
            current = "custom"
        return current

+    def _check_if_version_is_available(self, custom_version):
+        """
+        checks if version requested is available or not.


Suggested change

checks if version requested is available or not.

checks if that version is available as a package on pypi.

k9ert · 2023-02-27T08:53:12Z

Looks You haven't addressed the test yet. Please run the tests and fix them. If you click on "details" and then on "Raw output", you see the reason why the test is failing:

caplog = <_pytest.logging.LogCaptureFixture object at 0x7f4b0a34fdf0>

    def test_ExtGen(caplog):
    
        with patch("cryptoadvance.specter.services.extension_gen.GithubUrlLoader") as mock:
>           extgen = ExtGen(
                ".",
                "testorg",
                "testext",
                False,
                False,
                "Some Author",
                "some@mail",
                # uncomment the below line to see a real generation
                # tmpl_fs_source="../specterext-dummy",
                dry_run=True,
            )
E           TypeError: ExtGen.__init__() missing 2 required positional arguments: 'author' and 'email'

tests/test_services_extension_gen.py:14: TypeError

k9ert

see the above comments

k9ert · 2023-02-27T11:28:06Z

BTW, most important from all: This PR is going to master but should go to uiux-revamp branch.

k9ert · 2023-02-27T17:03:53Z

Sorry, we have moved the uiux-revamp branch to the master branch which now results with a lot of conflicts. So this PR needs to be recreated. Please do something like this:

git checkout master
git pull upstream master
git checkout -b ext-update-new
git cherry-pick 5814c687dd555e92fc7315c1749eb4fa9acf510a 1c7436b51a14794ffc637dab41bbcaf71e86ff1c cc706a0b7e20681ed6cec9b8549ae106e1e2e3a7 fc831ba86ea93d22b2328eeaaa45cfd5be2c5f4c
git commit
git push

And then please recreate the PR.

Aditi-Singh16 · 2023-02-28T18:46:36Z

Okay sure!

k9ert and others added 10 commits January 27, 2023 13:42

Bugfix: mitigating taproot hotwallet issue 2078 (#2083)

55d157b

* Some smaller improvements and fixes * mitigating 2078 * removed Microsoft link tracking * wrap up a failed update properly * better decode and better error_handling

Bugfix: black_version_pin (#2121) (#2160)

701f7a8

* ping black version * change to version 22.3.0 as in pre-commit * proper version * kick

Chore: Thread safety for TxList and Cypress test fix (#2148) (#2161)

4c12246

* fixes psbt key error * use lock to avoid concurrent access

Chore: Additional tests for auto-detection edge cases (#2084)

d369abe

* additional tests for edge-cases * higher timeout for rpc test to avoid flakiness * remove default timeout in fixture Co-authored-by: moneymanolis <moneymanolis@protonmail.com>

init commit

5814c68

Aditi-Singh16 marked this pull request as draft February 23, 2023 18:10

Aditi-Singh16 added 2 commits February 24, 2023 13:24

added version switch

1c7436b

removed unwanted changes

cc706a0

added check for version avaliability

fc831ba

Aditi-Singh16 marked this pull request as ready for review February 26, 2023 11:11

k9ert reviewed Feb 27, 2023

View reviewed changes

k9ert requested changes Feb 27, 2023

View reviewed changes

k9ert force-pushed the master branch from 09c5c2f to ce8a614 Compare February 27, 2023 11:32

Aditi-Singh16 closed this Feb 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes issue #1957 #2252

Fixes issue #1957 #2252

Aditi-Singh16 commented Feb 23, 2023 •

edited by k9ert

Loading

netlify bot commented Feb 23, 2023 •

edited

Loading

k9ert commented Feb 24, 2023

k9ert Feb 27, 2023

k9ert Feb 27, 2023

k9ert Feb 27, 2023

k9ert Feb 27, 2023

k9ert commented Feb 27, 2023

k9ert left a comment

k9ert commented Feb 27, 2023

k9ert commented Feb 27, 2023

Aditi-Singh16 commented Feb 28, 2023

	if vc._check_if_version_is_available(custom_version) == True:
	if vc._version_is_available(custom_version):

	checks if version requested is available or not.
	checks if that version is available as a package on pypi.

Fixes issue #1957 #2252

Fixes issue #1957 #2252

Conversation

Aditi-Singh16 commented Feb 23, 2023 • edited by k9ert Loading

netlify bot commented Feb 23, 2023 • edited Loading

✅ Deploy Preview for specter-desktop-docs canceled.

k9ert commented Feb 24, 2023

k9ert Feb 27, 2023

Choose a reason for hiding this comment

k9ert Feb 27, 2023

Choose a reason for hiding this comment

k9ert Feb 27, 2023

Choose a reason for hiding this comment

k9ert Feb 27, 2023

Choose a reason for hiding this comment

k9ert commented Feb 27, 2023

k9ert left a comment

Choose a reason for hiding this comment

k9ert commented Feb 27, 2023

k9ert commented Feb 27, 2023

Aditi-Singh16 commented Feb 28, 2023

Aditi-Singh16 commented Feb 23, 2023 •

edited by k9ert

Loading

netlify bot commented Feb 23, 2023 •

edited

Loading