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.

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

Add an expert mode splash screen before the parameter field of the transaction operation #192

Merged

Conversation

spalmer25
Copy link
Collaborator

fixes #175

@spalmer25 spalmer25 added bug Something isn't working app::wallet issues relating to the wallet app device::NANOS Issues affecting NANOS device device::NANOSP Issues affecting NANOSP device device::NANOX Issues affecting NANOX device labels Jan 11, 2024
@spalmer25 spalmer25 added this to the Wallet: v3.0.0 - release milestone Jan 11, 2024
@spalmer25 spalmer25 self-assigned this Jan 11, 2024
@spalmer25 spalmer25 force-pushed the palmer@functori@expert-mode-splash-screen-before-parameter branch 2 times, most recently from 379b2fa to 64c9c5b Compare January 11, 2024 14:04
@spalmer25 spalmer25 marked this pull request as ready for review January 11, 2024 14:15
@spalmer25 spalmer25 force-pushed the palmer@functori@expert-mode-splash-screen-before-parameter branch 5 times, most recently from d129564 to 8619e03 Compare January 12, 2024 10:23
Copy link
Collaborator

@ajinkyaraj-23 ajinkyaraj-23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall LGTM.
Step by step commits are great to understand incremental changes done.

app/src/parser/operation_state.h Outdated Show resolved Hide resolved
TZ_OPERATION_FIELD("Destination", TZ_OPERATION_FIELD_DESTINATION),
TZ_OPERATION_OPTION_FIELD("_Parameters",
TZ_OPERATION_TUPLE_FIELD("_Parameters",
TZ_OPERATION_FIELD("Entrypoint", TZ_OPERATION_FIELD_SMART_ENTRYPOINT, .complex=true),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Entrypoint can be made complex=false 😄

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, let's do it in another PR: #194

@spalmer25 spalmer25 force-pushed the palmer@functori@expert-mode-splash-screen-before-parameter branch 2 times, most recently from 21567e5 to 64bd107 Compare January 12, 2024 15:53
* information
*/
static tz_parser_result
tz_step_field(tz_parser_state *state)

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 180 lines.
@spalmer25 spalmer25 force-pushed the palmer@functori@expert-mode-splash-screen-before-parameter branch from 64bd107 to 80762b9 Compare January 12, 2024 16:38
@ajinkyaraj-23 ajinkyaraj-23 merged commit 487015d into main Jan 12, 2024
111 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app::wallet issues relating to the wallet app bug Something isn't working device::NANOS Issues affecting NANOS device device::NANOSP Issues affecting NANOSP device device::NANOX Issues affecting NANOX device
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an expert mode splash screen before the parameter field of the transaction operation
3 participants