Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use previous Ledger app version and re-enable Ledger signer tests #1487

Merged
merged 6 commits into from
Sep 12, 2024
Merged

Use previous Ledger app version and re-enable Ledger signer tests #1487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4f5a583
Use `main` branch for `app-starknet`; Re-enable ledger signer tests
franciszekjob Sep 11, 2024
a7044b2
Use commit SHA instead of branch name
franciszekjob Sep 11, 2024
2276eac
Add `cd` command
franciszekjob Sep 11, 2024
a93591d
Perform `git checkout` in building app step
franciszekjob Sep 11, 2024
022dabd
Remove unnecessary change in clone command
franciszekjob Sep 11, 2024
f288be4
Update ledger signer docs
franciszekjob Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/checks.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -196,7 +197,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

Expand Down
1 change: 1 addition & 0 deletions docs/guide/signing.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 22 additions & 8 deletions starknet_py/tests/unit/signer/test_ledger_signer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from sys import platform

import pytest

from starknet_py.common import create_sierra_compiled_contract
Expand All @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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(
Expand Down Expand Up @@ -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",
Expand Down
Loading