Stax/flex support

[steel97]

• split code to use nbgl for flex/stax, bagl for nanos+/nanox
• removed nanos target

Checklist

• App update process has been followed
• Target branch is develop
• Application version has been bumped

[steel97]

Great! Thanks for your quick response. I believe @kushti forwarded you an additional review from another developer.

I made few changes, tokens UI is now dynamically allocated

[arobsn]

Found some issues while doing functional tests.

Important: Please note that this review isn't complete, just wanted to report what I have ASAP so you can have something to work with, I'm still working on functional tests and new issues can eventually come up

Derive address operation

• It should be a one screen operation, just like in "Ext PubKey Export" command.
• Currently it displays "Confirm send address", it would be good to keep it consistent with other "export" commands, so please rename it to "Address Export".

Sign operation

• It's showing a zeroed AppID page even if no AppId is send.

[arobsn]

Found more issues on the Tx Signing flow:

1. The user is not able to navigate back on the signing screens, just forward.
2. Transaction signing crashes when the user clicks "confirm", with no error code, just a blank screen. Here you can see an minimal reproduction of the issue: https://github.com/arobsn/ledger-issue-minimal-reproduction
   Edit: Just confirmed that the minimal-reproduction code works perfectly on the nanosp build

[steel97]

* It should be a one screen operation, just like in "Ext PubKey Export" command.

Done

* Currently it displays "Confirm send address", it would be good to keep it consistent with other "export" commands, so please rename it to "Address Export".

Done

Sign operation

* It's showing a zeroed AppID page even if no `AppId` is send.

Should be fixed with last commits

[steel97]

2. Transaction signing crashes when the user clicks "confirm", with no error code, just a blank screen. Here you can see an minimal reproduction of the issue: https://github.com/arobsn/ledger-issue-minimal-reproduction
   **Edit**: Just confirmed that the `minimal-reproduction` code works perfectly on the `nanosp` build

Confirm is shown on Attest input stage, do you mean Approve signing stage?

For some reason it works for me in both debug/release builds.
Could you please try latest version?

[steel97]

1. The user is not able to navigate back on the signing screens, just forward.

I made few improvements. Now back button is active in certain cases (should match nano x/s+ behavior).
Basically it's possible to navigate forward/backward while reviewing tokens.

[steel97]

@arobsn actually I noticed that 0x00000000 app id is also shown in nano s+ build when signing tx:

Edit: fixed for nano s+/x as well
Edit 2: looks like test case requires app id even if it's zero:

ledger-app-ergo/tests/transaction-tests.js

Line 22 in d709571

flows[i].push({ header: 'Application', body: '0x00000000' });

commented out this check

[arobsn]

Address derivation

• On both "Address export" and "Address confirm", change and index nodes of the derivation path are getting omitted. If I request 44'/429'/0'/0/0, only 44'/429'/0 is displayed, for example.

Transaction signing

I can confirm that signing is not crashing as previously reported, it was a mistake on my side, I though that just clicking on "hold to sign" was enough, but I need to click-and-hold until signing process is finished, not sure if it's a VNC issue tho.

• Change must be displayed in the outputs confirm flow if CHANGE_INDEX_PATH - SIGN_INDEX_PATH > 19, this is important to avoid bad agents to set change to something like change_index = Int.MAX, see details in "Change confirmations" in Reduce the amount of confirmations on sign flow #11

[arobsn]

@arobsn actually I noticed that 0x00000000 app id is also shown in nano s+ build when signing tx:

Thank you

[steel97]

* On both "Address export" and "Address confirm", `change` and `index` nodes of the derivation path are getting omitted. If I request `44'/429'/0'/0/0`, only `44'/429'/0` is displayed, for example.

Fixed

* Change must be displayed in the outputs confirm flow if `CHANGE_INDEX_PATH - SIGN_INDEX_PATH > 19`, this is important to avoid bad agents to set change to something like `change_index = Int.MAX`, see details in "Change confirmations" in [Reduce the amount of confirmations on `sign` flow #11](https://github.com/ergoplatform/ledger-app-ergo/issues/11)

Stax/flex shares logic from s+/x, so this should work same. Screenshot from tx sign change 22 test:

[arobsn]

Everything seems to be working as expected now. Thanks