-
Notifications
You must be signed in to change notification settings - Fork 2
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.
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
Blind sign #36
Blind sign #36
Conversation
app/src/parser_impl.c
Outdated
@@ -1163,7 +1182,17 @@ parser_error_t _read(parser_context_t *c, parser_tx_t *v) | |||
CHECK_ERROR(_readTxAssetConfig(c, v)) | |||
break; | |||
case TX_APPLICATION: | |||
#if defined(TARGET_NANOS) || defined(TARGET_NANOS2) || defined(TARGET_NANOX) || defined(TARGET_STAX) || defined(TARGET_FLEX) | |||
if (!app_mode_blindsign() && !app_mode_expert()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I might be missing the goal here. So if Bs not enabled and app in normal mode, we request BS mode ?
So I guess the user cannot sign any tx with app in normal mode.
I was not really in the discussion of this but in my opinion, in this case the BS mode is not required because we will not show certain information, but more of a shortcut mode imitation, thus the user can enable it to skip screens.
I think here we are removing the possibility of the user to check all the screens if he wants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just saw the 4 possible options on the Slack channel, I guess this is right then.
@@ -40,7 +40,7 @@ DEFINES += PRODUCTION_BUILD=$(PRODUCTION_BUILD) | |||
|
|||
include $(CURDIR)/../deps/ledger-zxlib/makefiles/Makefile.app_testing | |||
|
|||
DEFINES += REVIEW_SCREEN_ENABLED SHORTCUT_MODE_ENABLED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if we should remove the REVIEW_SCREEN_ENABLE this will lead to the removal of the "Review Address" screen on the show address command.
I sure hope the screens I'm seeing are not what the signng workflow will be for algorand apps. The existing shortcut mode on S/X still trumps what I'm seeing here for Flex/Stax users. Hopefully this is something we can test before it goes live because what was delivered the first time for Flex users was unusable for Algorand defi users. The signing exp for something costing 4x as much shouldn't be 10x worse. 😞 |
Yes, @pbennett! It will be better if you could test the app instead of the signing screenshots you're seeing at the moment. We'll release a version for you to test first. Thanks! |
@sourajyoti-gupta I have every Ledger model and would be happy to help with Algorand-related testing. |
Hi @SilentRhetoric and @pbennett , version 2.1.1 is our latest supporting the Blind Signing as per requested. Please sideload the app from the Zondax Hub here - https://hub.zondax.ch/algorand and let me know your feedback. Thanks! |
The installed app says it can only be used on DEMO network. What does this mean? Is it actually validating the genesisid/hash in app? |
Remove shortcut mode and add blind sign instead.