From 4f5a583dee3ba326e6a211c8ec37076162b25da1 Mon Sep 17 00:00:00 2001 From: Fiiranek Date: Wed, 11 Sep 2024 16:24:14 +0200 Subject: [PATCH 1/6] Use `main` branch for `app-starknet`; Re-enable ledger signer tests --- .github/workflows/checks.yml | 4 ++- .../tests/unit/signer/test_ledger_signer.py | 30 ++++++++++++++----- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 002c873e1..d71b8435e 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -188,7 +188,9 @@ jobs: - name: Clone LedgerHQ Starknet app repository - run: git clone https://github.com/LedgerHQ/app-starknet.git + run: | + git clone https://github.com/LedgerHQ/app-starknet.git + git checkout main - name: Build the app inside Docker container uses: addnab/docker-run-action@v3 diff --git a/starknet_py/tests/unit/signer/test_ledger_signer.py b/starknet_py/tests/unit/signer/test_ledger_signer.py index daef9dfa9..75392efa9 100644 --- a/starknet_py/tests/unit/signer/test_ledger_signer.py +++ b/starknet_py/tests/unit/signer/test_ledger_signer.py @@ -1,3 +1,5 @@ +from sys import platform + import pytest from starknet_py.common import create_sierra_compiled_contract @@ -18,8 +20,11 @@ from starknet_py.tests.e2e.fixtures.misc import load_contract -# TODO (#1476): Should be re-enabled once Ledger signer is updated with new APDU spec. -@pytest.mark.skip +# TODO (#1425): Currently Ledger tests are skipped on Windows due to different Speculos setup. +@pytest.mark.skipif( + platform == "win32", + reason="Testing Ledger is skipped on Windows due to different Speculos setup.", +) def test_init_with_invalid_derivation_path(): with pytest.raises(ValueError, match="Empty derivation path"): LedgerSigner(derivation_path_str="", chain_id=StarknetChainId.SEPOLIA) @@ -76,8 +81,11 @@ def test_init_with_invalid_derivation_path(): ), ], ) -# TODO (#1476): Should be re-enabled once Ledger signer is updated with new APDU spec. -@pytest.mark.skip +# TODO (#1425): Currently Ledger tests are skipped on Windows due to different Speculos setup. +@pytest.mark.skipif( + platform == "win32", + reason="Testing Ledger is skipped on Windows due to different Speculos setup.", +) def test_sign_transaction(transaction): # docs: start @@ -97,8 +105,11 @@ def test_sign_transaction(transaction): assert all(i != 0 for i in signature) -# TODO (#1476): Should be re-enabled once Ledger signer is updated with new APDU spec. -@pytest.mark.skip +# TODO (#1425): Currently Ledger tests are skipped on Windows due to different Speculos setup. +@pytest.mark.skipif( + platform == "win32", + reason="Testing Ledger is skipped on Windows due to different Speculos setup.", +) def test_create_account_with_ledger_signer(): # pylint: disable=unused-variable signer = LedgerSigner( @@ -132,8 +143,11 @@ async def _get_account_balance_strk(client: FullNodeClient, address: int): @pytest.mark.asyncio -# TODO (#1476): Should be re-enabled once Ledger signer is updated with new APDU spec. -@pytest.mark.skip +# TODO (#1425): Currently Ledger tests are skipped on Windows due to different Speculos setup. +@pytest.mark.skipif( + platform == "win32", + reason="Testing Ledger is skipped on Windows due to different Speculos setup.", +) async def test_deploy_account_and_transfer(client): signer = LedgerSigner( derivation_path_str="m/2645'/1195502025'/1470455285'/0'/0'/0", From a7044b23b98fac914cd90903116a0c3dee2ad8f4 Mon Sep 17 00:00:00 2001 From: Fiiranek Date: Wed, 11 Sep 2024 16:30:48 +0200 Subject: [PATCH 2/6] Use commit SHA instead of branch name --- .github/workflows/checks.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index d71b8435e..e7d9bbf0f 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -4,6 +4,7 @@ env: CAIRO_LANG_VERSION: "0.13.1" DEVNET_VERSION: "0.1.2" DEVNET_SHA: 7e7dbb5 + LEDGER_APP_SHA: dd58c5c on: push: @@ -190,7 +191,7 @@ jobs: - name: Clone LedgerHQ Starknet app repository run: | git clone https://github.com/LedgerHQ/app-starknet.git - git checkout main + git checkout ${{ env.LEDGER_APP_SHA }} - name: Build the app inside Docker container uses: addnab/docker-run-action@v3 From 2276eac07d6194116e3951edcb8858828a104a74 Mon Sep 17 00:00:00 2001 From: Fiiranek Date: Wed, 11 Sep 2024 16:45:06 +0200 Subject: [PATCH 3/6] Add `cd` command --- .github/workflows/checks.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index e7d9bbf0f..72dbfa616 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -191,6 +191,7 @@ jobs: - name: Clone LedgerHQ Starknet app repository run: | git clone https://github.com/LedgerHQ/app-starknet.git + cd app-starknet git checkout ${{ env.LEDGER_APP_SHA }} - name: Build the app inside Docker container From a93591d8ffefd81e3dc6e62067048a2f48a896ab Mon Sep 17 00:00:00 2001 From: Fiiranek Date: Wed, 11 Sep 2024 16:59:39 +0200 Subject: [PATCH 4/6] Perform `git checkout` in building app step --- .github/workflows/checks.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 72dbfa616..f981da4bb 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -191,8 +191,6 @@ jobs: - name: Clone LedgerHQ Starknet app repository run: | git clone https://github.com/LedgerHQ/app-starknet.git - cd app-starknet - git checkout ${{ env.LEDGER_APP_SHA }} - name: Build the app inside Docker container uses: addnab/docker-run-action@v3 @@ -200,7 +198,8 @@ jobs: image: ghcr.io/ledgerhq/ledger-app-builder/ledger-app-dev-tools options: --rm -v ${{ github.workspace }}:/apps run: | - cd /apps/app-starknet/starknet + cd /apps/app-starknet + git checkout ${{ env.LEDGER_APP_SHA }} cargo clean cargo ledger build nanox From 022dabd449558a3b8066ae62e5e1dd2439adfeca Mon Sep 17 00:00:00 2001 From: Fiiranek Date: Wed, 11 Sep 2024 17:17:44 +0200 Subject: [PATCH 5/6] Remove unnecessary change in clone command --- .github/workflows/checks.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index f981da4bb..2046aa74d 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -189,8 +189,7 @@ jobs: - name: Clone LedgerHQ Starknet app repository - run: | - git clone https://github.com/LedgerHQ/app-starknet.git + run: git clone https://github.com/LedgerHQ/app-starknet.git - name: Build the app inside Docker container uses: addnab/docker-run-action@v3 From f288be4f3b51ff4c32bb997e378eb90a92b49b53 Mon Sep 17 00:00:00 2001 From: Fiiranek Date: Thu, 12 Sep 2024 10:44:46 +0200 Subject: [PATCH 6/6] Update ledger signer docs --- docs/guide/signing.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/guide/signing.rst b/docs/guide/signing.rst index e482ed3d1..49b7b2aa3 100644 --- a/docs/guide/signing.rst +++ b/docs/guide/signing.rst @@ -15,6 +15,7 @@ signing algorithm, it is possible to create ``Account`` with custom Signing with Ledger ------------------- :ref:`LedgerSigner` allows you to sign transactions using a Ledger device. The device must be unlocked and Starknet app needs to be open. +Currently used version of Starknet app is ``1.1.1`` and only blind-signing is possible. Clear-signing will be available in the near future. .. codesnippet:: ../../starknet_py/tests/unit/signer/test_ledger_signer.py :language: python