From 5d2686060f5d7647b1774fc7034713c58a38433d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Fri, 8 Mar 2024 10:29:41 +0100 Subject: [PATCH 01/15] Idle: update baking idle screens specifically Not every NVRAM update - reset: done after confirmation - setup: done after confirmation - authorize baking: done after confirmation - sign: done after screen-saver exit Only deauthorize needs an explicit update --- src/apdu_setup.c | 3 +++ src/globals.h | 2 -- src/ui.h | 4 ++++ src/ui_nbgl.c | 3 --- 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/apdu_setup.c b/src/apdu_setup.c index 89482409..c5d41970 100644 --- a/src/apdu_setup.c +++ b/src/apdu_setup.c @@ -129,6 +129,9 @@ size_t handle_apdu_deauthorize(__attribute__((unused)) uint8_t instruction, THROW(EXC_PARSE_ERROR); } UPDATE_NVRAM(ram, { memset(&ram->baking_key, 0, sizeof(ram->baking_key)); }); +#ifdef HAVE_BAGL + update_baking_idle_screens(); +#endif // HAVE_BAGL return finalize_successful_send(0); } diff --git a/src/globals.h b/src/globals.h index 83e9c80b..d1979c61 100644 --- a/src/globals.h +++ b/src/globals.h @@ -154,7 +154,6 @@ extern globals_t global; extern nvram_data const N_data_real; #define N_data (*(volatile nvram_data *) PIC(&N_data_real)) -void update_baking_idle_screens(void); high_watermark_t volatile *select_hwm_by_chain(chain_id_t const chain_id, nvram_data volatile *const ram); @@ -169,5 +168,4 @@ high_watermark_t volatile *select_hwm_by_chain(chain_id_t const chain_id, sizeof(global.apdu.baking_auth.new_data)); \ body; \ nvm_write((void *) &N_data, &global.apdu.baking_auth.new_data, sizeof(N_data)); \ - update_baking_idle_screens(); \ }) diff --git a/src/ui.h b/src/ui.h index ff4060d3..7b5262c2 100644 --- a/src/ui.h +++ b/src/ui.h @@ -34,6 +34,10 @@ void ui_refresh(void); void exit_app(void); // Might want to send it arguments to use as callback +#ifdef HAVE_BAGL +void update_baking_idle_screens(void); +#endif // HAVE_BAGL + void ux_confirm_screen(ui_callback_t ok_c, ui_callback_t cxl_c); void ux_idle_screen(ui_callback_t ok_c, ui_callback_t cxl_c); diff --git a/src/ui_nbgl.c b/src/ui_nbgl.c index cfaf8fd1..37f267ad 100644 --- a/src/ui_nbgl.c +++ b/src/ui_nbgl.c @@ -54,9 +54,6 @@ static const char* const bakeInfoTypes[] = { "High Watermark", }; -void update_baking_idle_screens(void) { -} - static bool navigation_cb_baking(uint8_t page, nbgl_pageContent_t* content) { UNUSED(page); From 796a4df6d4190aee903eed9155c368ff46472d89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 10:20:21 +0100 Subject: [PATCH 02/15] UI: remove unused code --- src/ui.h | 7 +------ src/ui_bagl.c | 32 +++++++++++++++----------------- src/ui_common.c | 23 ----------------------- src/ui_empty.c | 5 +---- src/ui_nbgl.c | 9 +-------- 5 files changed, 18 insertions(+), 58 deletions(-) diff --git a/src/ui.h b/src/ui.h index 7b5262c2..184dbf48 100644 --- a/src/ui.h +++ b/src/ui.h @@ -29,18 +29,13 @@ #include "keys.h" void ui_initial_screen(void); -void ui_init(void); -void ui_refresh(void); void exit_app(void); // Might want to send it arguments to use as callback #ifdef HAVE_BAGL void update_baking_idle_screens(void); -#endif // HAVE_BAGL - void ux_confirm_screen(ui_callback_t ok_c, ui_callback_t cxl_c); - -void ux_idle_screen(ui_callback_t ok_c, ui_callback_t cxl_c); +#endif // HAVE_BAGL /* Initializes the formatter stack. Should be called once before calling `push_ui_callback()`. */ void init_screen_stack(); diff --git a/src/ui_bagl.c b/src/ui_bagl.c index f2a9fe5c..3b2cc1e1 100644 --- a/src/ui_bagl.c +++ b/src/ui_bagl.c @@ -51,7 +51,8 @@ void calculate_baking_idle_screens_data(void) { void update_baking_idle_screens(void) { init_screen_stack(); calculate_baking_idle_screens_data(); - ui_refresh(); + /// refresh + ux_stack_display(0); } // User MUST call `init_screen_stack()` before the first call to this function. @@ -244,18 +245,6 @@ void display_next_state(bool is_left_ux_step) { } } -void ui_initial_screen(void) { - // reserve a display stack slot if none yet - if (G_ux.stack_count == 0) { - ux_stack_push(); - } - - init_screen_stack(); - calculate_baking_idle_screens_data(); - - ux_idle_screen(NULL, NULL); -} - void ux_prepare_display(ui_callback_t ok_c, ui_callback_t cxl_c) { global.dynamic_display.screen_stack_size = global.dynamic_display.formatter_index; global.dynamic_display.formatter_index = 0; @@ -269,14 +258,23 @@ void ux_prepare_display(ui_callback_t ok_c, ui_callback_t cxl_c) { } } +void ui_initial_screen(void) { + // reserve a display stack slot if none yet + if (G_ux.stack_count == 0) { + ux_stack_push(); + } + + init_screen_stack(); + calculate_baking_idle_screens_data(); + + ux_prepare_display(NULL, NULL); + ux_flow_init(0, ux_idle_flow, NULL); +} + void ux_confirm_screen(ui_callback_t ok_c, ui_callback_t cxl_c) { ux_prepare_display(ok_c, cxl_c); ux_flow_init(0, ux_confirm_flow, NULL); THROW(ASYNC_EXCEPTION); } -void ux_idle_screen(ui_callback_t ok_c, ui_callback_t cxl_c) { - ux_prepare_display(ok_c, cxl_c); - ux_flow_init(0, ux_idle_flow, NULL); -} #endif // HAVE_BAGL diff --git a/src/ui_common.c b/src/ui_common.c index 0c275088..626ae742 100644 --- a/src/ui_common.c +++ b/src/ui_common.c @@ -25,21 +25,6 @@ #include "globals.h" #include "os.h" -#ifdef HAVE_BAGL -void io_seproxyhal_display(const bagl_element_t *element); - -void io_seproxyhal_display(const bagl_element_t *element) { - return io_seproxyhal_display_default((bagl_element_t *) element); -} - -void ui_refresh(void) { - ux_stack_display(0); -} -#endif // HAVE_BAGL - -// CALLED BY THE SDK -unsigned char io_event(unsigned char channel); - unsigned char io_event(__attribute__((unused)) unsigned char channel) { // nothing done with the event, throw an error on the transport layer if // needed @@ -95,14 +80,6 @@ unsigned char io_event(__attribute__((unused)) unsigned char channel) { // command has been processed, DO NOT reset the current APDU transport return 1; } -void ui_init(void) { -#ifdef HAVE_BAGL - UX_INIT(); -#endif // HAVE_BAGL -#ifdef HAVE_NBGL - nbgl_objInit(); -#endif // HAVE_NBGL -} void require_pin(void) { os_global_pin_invalidate(); diff --git a/src/ui_empty.c b/src/ui_empty.c index f39ac75e..ad9c4174 100644 --- a/src/ui_empty.c +++ b/src/ui_empty.c @@ -53,12 +53,9 @@ void ux_layout_empty_init(unsigned int stack_slot) { ux_stack_display(stack_slot); } -void ux_layout_empty_screen_init(__attribute__((unused)) unsigned int x) { -} - void return_to_idle() { global.dynamic_display.is_blank_screen = false; - ux_idle_screen(NULL, NULL); + ui_initial_screen(); } UX_STEP_CB(empty_screen_step, empty, return_to_idle(), {}); diff --git a/src/ui_nbgl.c b/src/ui_nbgl.c index 37f267ad..77a99360 100644 --- a/src/ui_nbgl.c +++ b/src/ui_nbgl.c @@ -40,10 +40,6 @@ static const char* const infoTypes[] = {"Version", "Developer", "Copyright"}; static const char* const infoContents[] = {VERSION, "Ledger", "(c) 2023 Ledger"}; -void ui_initial_screen(void) { - ux_idle_screen(NULL, NULL); -} - #define MAX_LENGTH 200 static char* bakeInfoContents[3]; static char buffer[3][MAX_LENGTH]; @@ -90,10 +86,7 @@ void ui_menu_about_baking(void) { NULL); } -void ux_idle_screen(ui_callback_t ok_c, ui_callback_t cxl_c) { - (void) ok_c; - (void) cxl_c; - +void ui_initial_screen(void) { nbgl_useCaseHome("Tezos Baking", &C_tezos, NULL, false, ui_menu_about_baking, exit_app); } From c7923e109a768fcfc8dc9543891e2cbb0ddf2ac2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Fri, 8 Mar 2024 10:06:56 +0100 Subject: [PATCH 03/15] UI: reorder code --- src/ui.h | 10 +-- src/ui_bagl.c | 185 ++++++++++++++++++++++++-------------------------- 2 files changed, 94 insertions(+), 101 deletions(-) diff --git a/src/ui.h b/src/ui.h index 184dbf48..6ba32dbf 100644 --- a/src/ui.h +++ b/src/ui.h @@ -28,6 +28,11 @@ #include "keys.h" +/* Initializes the formatter stack. Should be called once before calling `push_ui_callback()`. */ +void init_screen_stack(void); +/* Initializes the formatter stack. Should be called once before calling `push_ui_callback()`. */ +void push_ui_callback(char *title, string_generation_callback cb, void *data); + void ui_initial_screen(void); void exit_app(void); // Might want to send it arguments to use as callback @@ -36,8 +41,3 @@ void exit_app(void); // Might want to send it arguments to use as callback void update_baking_idle_screens(void); void ux_confirm_screen(ui_callback_t ok_c, ui_callback_t cxl_c); #endif // HAVE_BAGL - -/* Initializes the formatter stack. Should be called once before calling `push_ui_callback()`. */ -void init_screen_stack(); -/* User MUST call `init_screen_stack()` before calling this function for the first time. */ -void push_ui_callback(char *title, string_generation_callback cb, void *data); diff --git a/src/ui_bagl.c b/src/ui_bagl.c index 3b2cc1e1..e611119e 100644 --- a/src/ui_bagl.c +++ b/src/ui_bagl.c @@ -39,20 +39,27 @@ #define G global.ui -void display_next_state(bool is_left_ux_step); +#define G_display global.dynamic_display -void calculate_baking_idle_screens_data(void) { - push_ui_callback("Tezos Baking", copy_string, VERSION); - push_ui_callback("Chain", copy_chain, &N_data.main_chain_id); - push_ui_callback("Public Key Hash", copy_key, &N_data.baking_key); - push_ui_callback("High Watermark", copy_hwm, &N_data.hwm.main); +void init_screen_stack() { + explicit_bzero(&global.dynamic_display.screen_stack, + sizeof(global.dynamic_display.screen_stack)); + global.dynamic_display.formatter_index = 0; + global.dynamic_display.screen_stack_size = 0; + global.dynamic_display.current_state = STATIC_SCREEN; } -void update_baking_idle_screens(void) { - init_screen_stack(); - calculate_baking_idle_screens_data(); - /// refresh - ux_stack_display(0); +void ux_prepare_display(ui_callback_t ok_c, ui_callback_t cxl_c) { + global.dynamic_display.screen_stack_size = global.dynamic_display.formatter_index; + global.dynamic_display.formatter_index = 0; + global.dynamic_display.current_state = STATIC_SCREEN; + + if (ok_c) { + global.dynamic_display.ok_callback = ok_c; + } + if (cxl_c) { + global.dynamic_display.cxl_callback = cxl_c; + } } // User MUST call `init_screen_stack()` before the first call to this function. @@ -70,81 +77,6 @@ void push_ui_callback(char *title, string_generation_callback cb, void *data) { global.dynamic_display.screen_stack_size++; } -void init_screen_stack() { - explicit_bzero(&global.dynamic_display.screen_stack, - sizeof(global.dynamic_display.screen_stack)); - global.dynamic_display.formatter_index = 0; - global.dynamic_display.screen_stack_size = 0; - global.dynamic_display.current_state = STATIC_SCREEN; -} - -UX_STEP_INIT(ux_init_upper_border, NULL, NULL, { display_next_state(true); }); -UX_STEP_NOCB(ux_variable_display, - bnnn_paging, - { - .title = global.dynamic_display.screen_title, - .text = global.dynamic_display.screen_value, - }); -UX_STEP_INIT(ux_init_lower_border, NULL, NULL, { display_next_state(false); }); - -UX_STEP_CB(ux_app_is_ready_step, - nn, - ux_empty_screen(), - { - "Application", - "is ready", - }); - -UX_STEP_CB(ux_idle_quit_step, - pb, - exit_app(), - { - &C_icon_dashboard_x, - "Quit", - }); - -UX_FLOW(ux_idle_flow, - &ux_app_is_ready_step, - - &ux_init_upper_border, - &ux_variable_display, - &ux_init_lower_border, - - &ux_idle_quit_step, - FLOW_LOOP); - -static void prompt_response(bool const accepted) { - ui_initial_screen(); - if (accepted) { - global.dynamic_display.ok_callback(); - } else { - global.dynamic_display.cxl_callback(); - } -} - -UX_STEP_CB(ux_prompt_flow_accept_step, pb, prompt_response(true), {&C_icon_validate_14, "Accept"}); - -UX_STEP_CB(ux_prompt_flow_reject_step, pb, prompt_response(false), {&C_icon_crossmark, "Reject"}); - -UX_STEP_NOCB(ux_eye_step, - nn, - { - "Review", - "Request", - }); - -UX_FLOW(ux_confirm_flow, - &ux_eye_step, - - &ux_init_upper_border, - &ux_variable_display, - &ux_init_lower_border, - - &ux_prompt_flow_reject_step, - &ux_prompt_flow_accept_step); - -#define G_display global.dynamic_display - void clear_data() { explicit_bzero(&G_display.screen_title, sizeof(G_display.screen_title)); explicit_bzero(&G_display.screen_value, sizeof(G_display.screen_value)); @@ -245,17 +177,44 @@ void display_next_state(bool is_left_ux_step) { } } -void ux_prepare_display(ui_callback_t ok_c, ui_callback_t cxl_c) { - global.dynamic_display.screen_stack_size = global.dynamic_display.formatter_index; - global.dynamic_display.formatter_index = 0; - global.dynamic_display.current_state = STATIC_SCREEN; +UX_STEP_INIT(ux_init_upper_border, NULL, NULL, { display_next_state(true); }); +UX_STEP_NOCB(ux_variable_display, + bnnn_paging, + { + .title = global.dynamic_display.screen_title, + .text = global.dynamic_display.screen_value, + }); +UX_STEP_INIT(ux_init_lower_border, NULL, NULL, { display_next_state(false); }); - if (ok_c) { - global.dynamic_display.ok_callback = ok_c; - } - if (cxl_c) { - global.dynamic_display.cxl_callback = cxl_c; - } +UX_STEP_CB(ux_app_is_ready_step, + nn, + ux_empty_screen(), + { + "Application", + "is ready", + }); +UX_STEP_CB(ux_idle_quit_step, + pb, + exit_app(), + { + &C_icon_dashboard_x, + "Quit", + }); +UX_FLOW(ux_idle_flow, + &ux_app_is_ready_step, + + &ux_init_upper_border, + &ux_variable_display, + &ux_init_lower_border, + + &ux_idle_quit_step, + FLOW_LOOP); + +void calculate_baking_idle_screens_data(void) { + push_ui_callback("Tezos Baking", copy_string, VERSION); + push_ui_callback("Chain", copy_chain, &N_data.main_chain_id); + push_ui_callback("Public Key Hash", copy_key, &N_data.baking_key); + push_ui_callback("High Watermark", copy_hwm, &N_data.hwm.main); } void ui_initial_screen(void) { @@ -271,6 +230,40 @@ void ui_initial_screen(void) { ux_flow_init(0, ux_idle_flow, NULL); } +void update_baking_idle_screens(void) { + init_screen_stack(); + calculate_baking_idle_screens_data(); + /// refresh + ux_stack_display(0); +} + +static void prompt_response(bool const accepted) { + ui_initial_screen(); + if (accepted) { + global.dynamic_display.ok_callback(); + } else { + global.dynamic_display.cxl_callback(); + } +} + +UX_STEP_CB(ux_prompt_flow_reject_step, pb, prompt_response(false), {&C_icon_crossmark, "Reject"}); +UX_STEP_CB(ux_prompt_flow_accept_step, pb, prompt_response(true), {&C_icon_validate_14, "Accept"}); +UX_STEP_NOCB(ux_eye_step, + nn, + { + "Review", + "Request", + }); +UX_FLOW(ux_confirm_flow, + &ux_eye_step, + + &ux_init_upper_border, + &ux_variable_display, + &ux_init_lower_border, + + &ux_prompt_flow_reject_step, + &ux_prompt_flow_accept_step); + void ux_confirm_screen(ui_callback_t ok_c, ui_callback_t cxl_c) { ux_prepare_display(ok_c, cxl_c); ux_flow_init(0, ux_confirm_flow, NULL); From 52d0b74e1b1aa0c7f296f93c82f1b3574af19bb1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Fri, 8 Mar 2024 10:09:41 +0100 Subject: [PATCH 04/15] ui_bagl: use G_display --- src/ui_bagl.c | 36 +++++++++++++++++------------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/src/ui_bagl.c b/src/ui_bagl.c index e611119e..681da7f9 100644 --- a/src/ui_bagl.c +++ b/src/ui_bagl.c @@ -42,39 +42,37 @@ #define G_display global.dynamic_display void init_screen_stack() { - explicit_bzero(&global.dynamic_display.screen_stack, - sizeof(global.dynamic_display.screen_stack)); - global.dynamic_display.formatter_index = 0; - global.dynamic_display.screen_stack_size = 0; - global.dynamic_display.current_state = STATIC_SCREEN; + explicit_bzero(&G_display.screen_stack, sizeof(G_display.screen_stack)); + G_display.formatter_index = 0; + G_display.screen_stack_size = 0; + G_display.current_state = STATIC_SCREEN; } void ux_prepare_display(ui_callback_t ok_c, ui_callback_t cxl_c) { - global.dynamic_display.screen_stack_size = global.dynamic_display.formatter_index; - global.dynamic_display.formatter_index = 0; - global.dynamic_display.current_state = STATIC_SCREEN; + G_display.screen_stack_size = G_display.formatter_index; + G_display.formatter_index = 0; + G_display.current_state = STATIC_SCREEN; if (ok_c) { - global.dynamic_display.ok_callback = ok_c; + G_display.ok_callback = ok_c; } if (cxl_c) { - global.dynamic_display.cxl_callback = cxl_c; + G_display.cxl_callback = cxl_c; } } // User MUST call `init_screen_stack()` before the first call to this function. void push_ui_callback(char *title, string_generation_callback cb, void *data) { - if (global.dynamic_display.formatter_index + 1 >= MAX_SCREEN_STACK_SIZE) { + if (G_display.formatter_index + 1 >= MAX_SCREEN_STACK_SIZE) { THROW(0x6124); } - struct screen_data *fmt = - &global.dynamic_display.screen_stack[global.dynamic_display.formatter_index]; + struct screen_data *fmt = &G_display.screen_stack[G_display.formatter_index]; fmt->title = title; fmt->callback_fn = cb; fmt->data = data; - global.dynamic_display.formatter_index++; - global.dynamic_display.screen_stack_size++; + G_display.formatter_index++; + G_display.screen_stack_size++; } void clear_data() { @@ -181,8 +179,8 @@ UX_STEP_INIT(ux_init_upper_border, NULL, NULL, { display_next_state(true); }); UX_STEP_NOCB(ux_variable_display, bnnn_paging, { - .title = global.dynamic_display.screen_title, - .text = global.dynamic_display.screen_value, + .title = G_display.screen_title, + .text = G_display.screen_value, }); UX_STEP_INIT(ux_init_lower_border, NULL, NULL, { display_next_state(false); }); @@ -240,9 +238,9 @@ void update_baking_idle_screens(void) { static void prompt_response(bool const accepted) { ui_initial_screen(); if (accepted) { - global.dynamic_display.ok_callback(); + G_display.ok_callback(); } else { - global.dynamic_display.cxl_callback(); + G_display.cxl_callback(); } } From 8d7d539c536d7d4ca931bc9c23feb24ccdb72276 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 15:13:59 +0100 Subject: [PATCH 05/15] Doc: documents ui files --- src/globals.h | 1 + src/ui.h | 41 ++++++++++++++++++++++-- src/ui_bagl.c | 84 +++++++++++++++++++++++++++++++++++++++++++------ src/ui_common.c | 15 ++++++++- src/ui_empty.c | 25 +++++++++++++++ src/ui_empty.h | 8 +++++ src/ui_nbgl.c | 11 +++++++ 7 files changed, 171 insertions(+), 14 deletions(-) diff --git a/src/globals.h b/src/globals.h index d1979c61..07c1ce5d 100644 --- a/src/globals.h +++ b/src/globals.h @@ -27,6 +27,7 @@ #include "bolos_target.h" #include "operations.h" +#include "ui.h" // Zeros out all globals that can keep track of APDU instruction state. // Notably this does *not* include UI state. diff --git a/src/ui.h b/src/ui.h index 6ba32dbf..1dac5f8c 100644 --- a/src/ui.h +++ b/src/ui.h @@ -28,16 +28,51 @@ #include "keys.h" -/* Initializes the formatter stack. Should be called once before calling `push_ui_callback()`. */ +/** + * @brief Initializes the formatter stack + * + * Must be called once before calling `push_ui_callback()` for the first time + * + */ void init_screen_stack(void); -/* Initializes the formatter stack. Should be called once before calling `push_ui_callback()`. */ + +/** + * @brief Pushes a new screen to the flow + * + * Must call `init_screen_stack()` before calling this function for the first time + * + * @param title: title of the screen + * @param cb: callback to generate the string version of the data + * @param data: data to display on the screen + */ void push_ui_callback(char *title, string_generation_callback cb, void *data); +/** + * @brief Starts the idle flow + * + */ void ui_initial_screen(void); -void exit_app(void); // Might want to send it arguments to use as callback +/** + * @brief Exits the app + * + */ +void exit_app(void); #ifdef HAVE_BAGL + +/** + * @brief Updates the baking screens + * + */ void update_baking_idle_screens(void); + +/** + * @brief Prepare confirmation screens + * + * @param ok_c: accept callback + * @param cxl_c: cancel callback + */ void ux_confirm_screen(ui_callback_t ok_c, ui_callback_t cxl_c); + #endif // HAVE_BAGL diff --git a/src/ui_bagl.c b/src/ui_bagl.c index 681da7f9..80a1fd27 100644 --- a/src/ui_bagl.c +++ b/src/ui_bagl.c @@ -41,13 +41,19 @@ #define G_display global.dynamic_display -void init_screen_stack() { +void init_screen_stack(void) { explicit_bzero(&G_display.screen_stack, sizeof(G_display.screen_stack)); G_display.formatter_index = 0; G_display.screen_stack_size = 0; G_display.current_state = STATIC_SCREEN; } +/** + * @brief Prepare the display + * + * @param ok_c: accept callback + * @param cxl_c: cancel callback + */ void ux_prepare_display(ui_callback_t ok_c, ui_callback_t cxl_c) { G_display.screen_stack_size = G_display.formatter_index; G_display.formatter_index = 0; @@ -61,7 +67,6 @@ void ux_prepare_display(ui_callback_t ok_c, ui_callback_t cxl_c) { } } -// User MUST call `init_screen_stack()` before the first call to this function. void push_ui_callback(char *title, string_generation_callback cb, void *data) { if (G_display.formatter_index + 1 >= MAX_SCREEN_STACK_SIZE) { THROW(0x6124); @@ -75,15 +80,24 @@ void push_ui_callback(char *title, string_generation_callback cb, void *data) { G_display.screen_stack_size++; } -void clear_data() { +/** + * @brief Clear screen related values + * + */ +void clear_data(void) { explicit_bzero(&G_display.screen_title, sizeof(G_display.screen_title)); explicit_bzero(&G_display.screen_value, sizeof(G_display.screen_value)); } -// Fills the screen with the data in the `screen_stack` pointed by the index -// `G_display.formatter_index`. Fills the `screen_title` by copying the `.title` field and fills the -// `screen_value` by computing `callback_fn` with the `.data` field as a parameter -void set_screen_data() { +/** + * @brief Fills the screen with the data in the `screen_stack` pointed + * by the index `G_display.formatter_index`. Fills the + * `screen_title` by copying the `.title` field and fills the + * `screen_value` by computing `callback_fn` with the `.data` + * field as a parameter + * + */ +void set_screen_data(void) { struct screen_data *fmt = &G_display.screen_stack[G_display.formatter_index]; if (fmt->title == NULL) { // Avoid seg faulting for bad reasons... @@ -95,9 +109,10 @@ void set_screen_data() { fmt->callback_fn(G_display.screen_value, sizeof(G_display.screen_value), fmt->data); } -/* - * Enables coherent behavior on bnnn_paging when there are multiple - * screens. +/** + * @brief Enables coherent behavior on bnnn_paging when there are + * multiple screens. + * */ void update_layout() { G_ux.flow_stack[G_ux.stack_count - 1].prev_index = @@ -106,6 +121,19 @@ void update_layout() { ux_flow_relayout(); } +/** + * @brief Selects the next screen to display + * + * It allows to navigate through the screens in the order in + * which they were pushed. + * + * The left-hand side shows the first screens pushed, the + * right-hand side the last. + * + * Goes back to standard navigation by leaving the boundaries. + * + * @param is_left_ux_step: if come from the left screen + */ void display_next_state(bool is_left_ux_step) { if (is_left_ux_step) { // We're called from the LEFT ux step if (G_display.current_state == STATIC_SCREEN) { @@ -175,6 +203,13 @@ void display_next_state(bool is_left_ux_step) { } } +/** + * @brief Generic way to display screens + * + * The border helps to stay in the variable_display page + * See `display_next_state(is_left_ux_step)` + * + */ UX_STEP_INIT(ux_init_upper_border, NULL, NULL, { display_next_state(true); }); UX_STEP_NOCB(ux_variable_display, bnnn_paging, @@ -184,6 +219,17 @@ UX_STEP_NOCB(ux_variable_display, }); UX_STEP_INIT(ux_init_lower_border, NULL, NULL, { display_next_state(false); }); +/** + * @brief Idle flow + * + * - Home screen + * - Version screen + * - Chain-id screen + * - Public key hash screen + * - High Watermark screen + * - Exit screen + * + */ UX_STEP_CB(ux_app_is_ready_step, nn, ux_empty_screen(), @@ -208,6 +254,10 @@ UX_FLOW(ux_idle_flow, &ux_idle_quit_step, FLOW_LOOP); +/** + * @brief Pushes the baking screens + * + */ void calculate_baking_idle_screens_data(void) { push_ui_callback("Tezos Baking", copy_string, VERSION); push_ui_callback("Chain", copy_chain, &N_data.main_chain_id); @@ -235,6 +285,11 @@ void update_baking_idle_screens(void) { ux_stack_display(0); } +/** + * @brief Callback called on accept or cancel + * + * @param accepted: true if accepted, false if cancelled + */ static void prompt_response(bool const accepted) { ui_initial_screen(); if (accepted) { @@ -244,6 +299,15 @@ static void prompt_response(bool const accepted) { } } +/** + * @brief Confirmation flow + * + * - Initial screen + * - Values + * - Reject screen + * - Accept screen + * + */ UX_STEP_CB(ux_prompt_flow_reject_step, pb, prompt_response(false), {&C_icon_crossmark, "Reject"}); UX_STEP_CB(ux_prompt_flow_accept_step, pb, prompt_response(true), {&C_icon_validate_14, "Accept"}); UX_STEP_NOCB(ux_eye_step, diff --git a/src/ui_common.c b/src/ui_common.c index 626ae742..935cd327 100644 --- a/src/ui_common.c +++ b/src/ui_common.c @@ -25,7 +25,16 @@ #include "globals.h" #include "os.h" -unsigned char io_event(__attribute__((unused)) unsigned char channel) { +/** + * @brief Redefinition of lib_standard_app/io.io_event + * + * In order to disable ticker event handling to prevent + * screen-saver from starting + * + * @param channel: requested channel + * @return 1 + */ +uint8_t io_event(__attribute__((unused)) unsigned char channel) { // nothing done with the event, throw an error on the transport layer if // needed @@ -81,6 +90,10 @@ unsigned char io_event(__attribute__((unused)) unsigned char channel) { return 1; } +/** + * @brief Invalidates the pin to enforce its requirement. + * + */ void require_pin(void) { os_global_pin_invalidate(); } diff --git a/src/ui_empty.c b/src/ui_empty.c index ad9c4174..1589a9f9 100644 --- a/src/ui_empty.c +++ b/src/ui_empty.c @@ -25,6 +25,10 @@ #include "globals.h" #include "ui.h" +/** + * @brief Black screen element + * + */ const bagl_element_t empty_screen_elements[] = {{{BAGL_RECTANGLE, BAGL_NONE, 0, @@ -41,9 +45,20 @@ const bagl_element_t empty_screen_elements[] = {{{BAGL_RECTANGLE, .text = NULL}, {}}; +/** + * @brief This structure represents the parameters needed for the empty layout + * + * No parameters required + * + */ typedef struct ux_layout_empty_params_s { } ux_layout_empty_params_t; +/** + * @brief Initializes the empty layout + * + * @param stack_slot: index stack_slot + */ void ux_layout_empty_init(unsigned int stack_slot) { ux_stack_init(stack_slot); G_ux.stack[stack_slot].element_arrays[0].element_array = empty_screen_elements; @@ -53,11 +68,21 @@ void ux_layout_empty_init(unsigned int stack_slot) { ux_stack_display(stack_slot); } +/** + * @brief Exits the empty screen + * + */ void return_to_idle() { global.dynamic_display.is_blank_screen = false; ui_initial_screen(); } +/** + * @brief Empty screens flow + * + * On any click: return_to_idle + * + */ UX_STEP_CB(empty_screen_step, empty, return_to_idle(), {}); UX_STEP_INIT(empty_screen_border, NULL, NULL, { return_to_idle(); }); UX_FLOW(ux_empty_flow, &empty_screen_step, &empty_screen_border, FLOW_LOOP); diff --git a/src/ui_empty.h b/src/ui_empty.h index 345ac98e..cf9bb382 100644 --- a/src/ui_empty.h +++ b/src/ui_empty.h @@ -19,4 +19,12 @@ */ #pragma once +/** + * @brief Empties the screen + * + * Waits a click to return to home screen + * + * Applies only for Nano devices + * + */ void ux_empty_screen(void); diff --git a/src/ui_nbgl.c b/src/ui_nbgl.c index 77a99360..ca5edf2e 100644 --- a/src/ui_nbgl.c +++ b/src/ui_nbgl.c @@ -50,6 +50,13 @@ static const char* const bakeInfoTypes[] = { "High Watermark", }; +/** + * @brief Callback to fill the settings page content + * + * @param page: page of the settings + * @param content: content to fill + * @return bool: if the page is not out of bounds + */ static bool navigation_cb_baking(uint8_t page, nbgl_pageContent_t* content) { UNUSED(page); @@ -76,6 +83,10 @@ static bool navigation_cb_baking(uint8_t page, nbgl_pageContent_t* content) { return true; } +/** + * @brief Draws settings pages + * + */ void ui_menu_about_baking(void) { nbgl_useCaseSettings("Tezos baking", 0, From cc4a7ae38bc91c57d680f00043eb40dbad417787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Fri, 8 Mar 2024 14:38:39 +0100 Subject: [PATCH 06/15] Doc: documents exception.h file --- src/exception.h | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/exception.h b/src/exception.h index 0c106be9..3e3700c3 100644 --- a/src/exception.h +++ b/src/exception.h @@ -26,9 +26,12 @@ // Throw this to indicate prompting #define ASYNC_EXCEPTION 0x2000 -// Standard APDU error codes: -// https://www.eftlab.co.uk/index.php/site-map/knowledge-base/118-apdu-response-list - +/** + * @brief Standard APDU error codes + * + * https://www.eftlab.co.uk/index.php/site-map/knowledge-base/118-apdu-response-list + * + */ #define EXC_WRONG_PARAM 0x6B00 #define EXC_WRONG_LENGTH 0x6C00 #define EXC_INVALID_INS 0x6D00 @@ -41,7 +44,13 @@ #define EXC_CLASS 0x6E00 #define EXC_MEMORY_ERROR 0x9200 -// Crashes can be harder to debug than exceptions and latency isn't a big concern +/** + * @brief Checks if a pointer is NULL + * + * Crashes can be harder to debug than exceptions and latency isn't a big concern + * + * @param ptr: pointer + */ static inline void check_null(void volatile const *const ptr) { if (ptr == NULL) { THROW(EXC_MEMORY_ERROR); From 850db78e0d598459382f62f681bce549da17633c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 10:47:02 +0100 Subject: [PATCH 07/15] Doc: documents memory.h file --- src/memory.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/memory.h b/src/memory.h index 9c564c5d..281d75f0 100644 --- a/src/memory.h +++ b/src/memory.h @@ -25,6 +25,14 @@ #include "exception.h" +/** + * @brief Compares two buffer + * + * Statically guarantees that buffer sizes match + * + * @param a: first buffer + * @param b: second buffer + */ #define COMPARE(a, b) \ ({ \ _Static_assert(sizeof(*a) == sizeof(*b), "Size mismatch in COMPARE"); \ @@ -32,4 +40,10 @@ check_null(b); \ memcmp(a, b, sizeof(*a)); \ }) + +/** + * @brief Returns the number of element of an array + * + * @param a: array + */ #define NUM_ELEMENTS(a) (sizeof(a) / sizeof(*a)) From be8ca0bacd9c633993bbc0d653710b52449f7ff9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 10:47:47 +0100 Subject: [PATCH 08/15] Doc: documents type.h file --- src/types.h | 224 ++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 154 insertions(+), 70 deletions(-) diff --git a/src/types.h b/src/types.h index 425c1f53..b8f8bf46 100644 --- a/src/types.h +++ b/src/types.h @@ -35,24 +35,25 @@ #undef false #define false ((bool) 0) -// NOTE: There are *two* ways that "key type" or "curve code" are represented in -// this code base: -// 1. `derivation_type` represents how a key will be derived from the seed. It -// is almost the same as `signature_type` but allows for multiple derivation -// strategies for ed25519. This type is often parsed from the APDU -// instruction. See `parse_derivation_type` for the mapping. -// 2. `signature_type` represents how a key will be used for signing. -// The mapping from `derivation_type` to `signature_type` is injective. -// See `derivation_type_to_signature_type`. -// This type is parsed from Tezos data headers. See the relevant parsing -// code for the mapping. +/** + * NOTE: There are *two* ways that "key type" or "curve code" are represented in + * this code base: + * 1. `derivation_type` represents how a key will be derived from the seed. It + * is almost the same as `signature_type` but allows for multiple derivation + * strategies for ed25519. This type is often parsed from the APDU + * instruction. See `parse_derivation_type` for the mapping. + * 2. `signature_type` represents how a key will be used for signing. + * The mapping from `derivation_type` to `signature_type` is injective. + * See `derivation_type_to_signature_type`. + * This type is parsed from Tezos data headers. See the relevant parsing + * code for the mapping. + */ typedef enum { DERIVATION_TYPE_SECP256K1 = 1, DERIVATION_TYPE_SECP256R1 = 2, DERIVATION_TYPE_ED25519 = 3, DERIVATION_TYPE_BIP32_ED25519 = 4 } derivation_type_t; - typedef enum { SIGNATURE_TYPE_UNSET = 0, SIGNATURE_TYPE_SECP256K1 = 1, @@ -60,13 +61,23 @@ typedef enum { SIGNATURE_TYPE_ED25519 = 3 } signature_type_t; +/** + * @brief Type of baking message + * + */ typedef enum { - BAKING_TYPE_BLOCK = 2, - BAKING_TYPE_ATTESTATION = 3, - BAKING_TYPE_PREATTESTATION = 4 + BAKING_TYPE_BLOCK, + BAKING_TYPE_ATTESTATION, + BAKING_TYPE_PREATTESTATION } baking_type_t; -// Return number of bytes to transmit (tx) +/** + * @brief Generic APDU instruction handler + * + * @param instruction: apdu instruction + * @param flags: io flags + * @return size_t: offset of the apdu response + */ typedef size_t (*apdu_handler)(uint8_t instruction, volatile uint32_t *flags); typedef uint32_t level_t; @@ -76,35 +87,63 @@ typedef uint32_t round_t; #define MAX_INT_DIGITS 20 +/** + * @brief This structure represents chain id + * + */ typedef struct { - uint32_t v; + uint32_t v; ///< value of the chain id } chain_id_t; // Mainnet Chain ID: NetXdQprcVkpaWU static chain_id_t const mainnet_chain_id = {.v = 0x7A06A770}; -// UI -typedef bool (*ui_callback_t)(void); // return true to go back to idle screen - -// Uses K&R style declaration to avoid being stuck on const void *, to avoid having to cast the -// function pointers. +/** + * @brief UI callback + * + * @return bool: true to go back to idle screen + */ +typedef bool (*ui_callback_t)(void); + +/** + * @brief String generator callback + * + * Uses K&R style declaration to avoid being stuck on const + * void *, to avoid having to cast the function pointers + * + * @param buffer: output buffer + * @param buffer_size: output size + * @param data: data + */ typedef void (*string_generation_callback)( /* char *buffer, size_t buffer_size, const void *data */); -// Keys +/** + * @brief Pair of public and private key + * + */ typedef struct { - cx_ecfp_public_key_t public_key; - cx_ecfp_private_key_t private_key; + cx_ecfp_public_key_t public_key; ///< public key + cx_ecfp_private_key_t private_key; ///< private key } key_pair_t; -// Baking Auth #define MAX_BIP32_LEN 10 +/** + * @brief This structure represents a bip32 path + * + */ typedef struct { - uint8_t length; - uint32_t components[MAX_BIP32_LEN]; + uint8_t length; ///< length of the path + uint32_t components[MAX_BIP32_LEN]; ///< array of components } bip32_path_t; +/** + * @brief Copies a bip32 path + * + * @param out: bip32 path output + * @param in: bip32 path copied + */ static inline void copy_bip32_path(bip32_path_t *const out, bip32_path_t volatile const *const in) { check_null(out); check_null(in); @@ -112,6 +151,13 @@ static inline void copy_bip32_path(bip32_path_t *const out, bip32_path_t volatil out->length = in->length; } +/** + * @brief Checks that two bip32 paths are equals + * + * @param a: first bip32 path + * @param b: second bip32 path + * @return bool: result + */ static inline bool bip32_paths_eq(bip32_path_t volatile const *const a, bip32_path_t volatile const *const b) { return a == b || (a != NULL && b != NULL && a->length == b->length && @@ -120,11 +166,23 @@ static inline bool bip32_paths_eq(bip32_path_t volatile const *const a, a->length * sizeof(*a->components)) == 0); } +/** + * @brief This structure represents a bip32 path and its curve + * + * Defines a key when associates with a mnemonic seed + * + */ typedef struct { - bip32_path_t bip32_path; - derivation_type_t derivation_type; + bip32_path_t bip32_path; ///< bip32 path of the key + derivation_type_t derivation_type; ///< curve of the key } bip32_path_with_curve_t; +/** + * @brief Copies a bip32 path and its curve + * + * @param out: output + * @param in: bip32 path and curve copied + */ static inline void copy_bip32_path_with_curve(bip32_path_with_curve_t *const out, bip32_path_with_curve_t volatile const *const in) { check_null(out); @@ -133,27 +191,44 @@ static inline void copy_bip32_path_with_curve(bip32_path_with_curve_t *const out out->derivation_type = in->derivation_type; } +/** + * @brief Checks that two bip32 paths with their paths are equals + * + * @param a: first bip32 path and curve + * @param b: first bip32 path and curve + * @return bool: result + */ static inline bool bip32_path_with_curve_eq(bip32_path_with_curve_t volatile const *const a, bip32_path_with_curve_t volatile const *const b) { return a == b || (a != NULL && b != NULL && bip32_paths_eq(&a->bip32_path, &b->bip32_path) && a->derivation_type == b->derivation_type); } +/** + * @brief This structure represents a High Watermark (HWM) + * + */ typedef struct { - level_t highest_level; - round_t highest_round; - bool had_attestation; - bool had_preattestation; - bool migrated_to_tenderbake; + level_t highest_level; ///< highest level seen + round_t highest_round; ///< highest round seen + bool had_attestation; ///< if an attestation has been seen at current level/round + bool had_preattestation; ///< if a pre-attestation has been seen at current level/round + bool migrated_to_tenderbake; ///< if chain has migrated to tenderbake } high_watermark_t; +/** + * @brief This structure represents data store in NVRAM + * + */ typedef struct { - chain_id_t main_chain_id; + chain_id_t main_chain_id; ///< main chain id + + /// high watermarks information struct { - high_watermark_t main; - high_watermark_t test; + high_watermark_t main; ///< HWM of main + high_watermark_t test; ///< HWM of test } hwm; - bip32_path_with_curve_t baking_key; + bip32_path_with_curve_t baking_key; ///< authorized key } nvram_data; #define SIGN_HASH_SIZE 32 // TODO: Rename or use a different constant. @@ -166,52 +241,68 @@ typedef struct { #define PROMPT_WIDTH 16 #define VALUE_WIDTH PROTOCOL_HASH_BASE58_STRING_SIZE -// Operations -#define PROTOCOL_HASH_SIZE 32 - // TODO: Rename to KEY_HASH_SIZE #define HASH_SIZE 20 +/** + * @brief This structure represents the content of a parsed baking data + * + */ typedef struct { - chain_id_t chain_id; - baking_type_t type; - level_t level; - round_t round; - bool is_tenderbake; + chain_id_t chain_id; ///< chain id + baking_type_t type; ///< kind of the baking message + level_t level; ///< level of the baking message + round_t round; ///< round of the baking message + bool is_tenderbake; ///< if belongs to the tenderbake consensus protocol } parsed_baking_data_t; +/** + * @brief This structure represents information about parsed contract + * + */ typedef struct parsed_contract { - uint8_t originated; // a lightweight bool + uint8_t originated; ///< a lightweight bool signature_type_t - signature_type; // 0 in originated case - // An implicit contract with signature_type of 0 means not present - uint8_t hash[HASH_SIZE]; + signature_type; ///< 0 in originated case + ///< An implicit contract with signature_type of 0 means not present + uint8_t hash[HASH_SIZE]; ///< hash of the contract } parsed_contract_t; +/** + * @brief Tag of operation + * + */ enum operation_tag { OPERATION_TAG_NONE = -1, // Sentinal value, as 0 is possibly used for something OPERATION_TAG_REVEAL = 107, OPERATION_TAG_DELEGATION = 110, }; +/** + * @brief This structure represents information about parsed operation + * + */ struct parsed_operation { - enum operation_tag tag; - struct parsed_contract source; - struct parsed_contract destination; - - uint32_t flags; // Interpretation depends on operation type + enum operation_tag tag; ///< operation tag + struct parsed_contract source; ///< source of the operation + struct parsed_contract destination; ///< destination of the operation }; +/** + * @brief This structure represents information about parsed a bundle of operations + * + * Except for reveals, only one operation can be parsed per bundle + * + */ struct parsed_operation_group { - cx_ecfp_public_key_t public_key; // compressed - uint64_t total_fee; - uint64_t total_storage_limit; - bool has_reveal; - struct parsed_contract signing; - struct parsed_operation operation; + cx_ecfp_public_key_t public_key; ///< signer public key + uint64_t total_fee; ///< sum of all fees + uint64_t total_storage_limit; ///< sum of all storage limits + bool has_reveal; ///< if the bundle contains at least a reveal + struct parsed_contract signing; ///< contract form of signer + struct parsed_operation operation; ///< operation parsed }; -// Maximum number of APDU instructions #define INS_MAX 0x0F #define APDU_INS(x) \ @@ -220,13 +311,6 @@ struct parsed_operation_group { x; \ }) -#define STRCPY(buff, x) \ - ({ \ - _Static_assert(sizeof(buff) >= sizeof(x) && sizeof(*x) == sizeof(char), \ - "String won't fit in buffer"); \ - strcpy(buff, x); \ - }) - #define CUSTOM_MAX(a, b) \ ({ \ __typeof__(a) ____a_ = (a); \ From 897436609e761de10febdbbfcbd711d1781fc23e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 10:48:26 +0100 Subject: [PATCH 09/15] Doc: documents protocol.h file --- src/protocol.h | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/protocol.h b/src/protocol.h index f3238e57..5dc17c0b 100644 --- a/src/protocol.h +++ b/src/protocol.h @@ -29,16 +29,31 @@ #include "cx.h" #include "types.h" -#define MAGIC_BYTE_INVALID 0x00 -#define MAGIC_BYTE_UNSAFE_OP 0x03 -#define MAGIC_BYTE_BLOCK 0x11 -#define MAGIC_BYTE_PREATTESTATION 0x12 -#define MAGIC_BYTE_ATTESTATION 0x13 +/// Magic byte values +/// See: https://tezos.gitlab.io/user/key-management.html#signer-requests +#define MAGIC_BYTE_INVALID 0x00 /// No magic byte +#define MAGIC_BYTE_UNSAFE_OP 0x03 /// magic byte of an operation +#define MAGIC_BYTE_BLOCK 0x11 /// magic byte of a block +#define MAGIC_BYTE_PREATTESTATION 0x12 /// magic byte of a pre-attestation +#define MAGIC_BYTE_ATTESTATION 0x13 /// magic byte of an attestation +/** + * @brief Get the magic byte of a data + * + * @param data: data + * @param length: data length + * @return uint8_t: magic byte result + */ static inline uint8_t get_magic_byte(uint8_t const *const data, size_t const length) { return (data == NULL || length == 0) ? MAGIC_BYTE_INVALID : *data; } +/** + * @brief Reads unaligned big endian value + * + * @param type: type of the value + * @return in: data to read + */ #define READ_UNALIGNED_BIG_ENDIAN(type, in) \ ({ \ uint8_t const *bytes = (uint8_t const *) in; \ @@ -53,8 +68,12 @@ static inline uint8_t get_magic_byte(uint8_t const *const data, size_t const len res; \ }) -// Same as READ_UNALIGNED_BIG_ENDIAN but helps keep track of how many bytes -// have been read by adding sizeof(type) to the given counter. +/** + * @brief Same as READ_UNALIGNED_BIG_ENDIAN but helps keep track of + * how many bytes have been read by adding sizeof(type) to the + * given counter + * + */ #define CONSUME_UNALIGNED_BIG_ENDIAN(counter, type, addr) \ ({ \ counter += sizeof(type); \ From cc31bd3bd7dfe1e7782644e1afb6a19f0eb42602 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 10:49:02 +0100 Subject: [PATCH 10/15] Doc: documents baking_auth.(c|h) files --- src/baking_auth.c | 109 ++++++++++++++++++++++++++++++++++------------ src/baking_auth.h | 39 +++++++++++++++-- 2 files changed, 116 insertions(+), 32 deletions(-) diff --git a/src/baking_auth.c b/src/baking_auth.c index 5a17d5f1..948faef0 100644 --- a/src/baking_auth.c +++ b/src/baking_auth.c @@ -72,6 +72,14 @@ void authorize_baking(derivation_type_t const derivation_type, }); } +/** + * @brief Checks if a baking info pass all checks + * + * See `doc/signing.md#checks` + * + * @param baking_info: baking info + * @return bool: return true if it has passed checks + */ static bool is_level_authorized(parsed_baking_data_t const *const baking_info) { check_null(baking_info); if (!is_valid_level(baking_info->level)) { @@ -104,6 +112,13 @@ static bool is_level_authorized(parsed_baking_data_t const *const baking_info) { } } +/** + * @brief Checks if a key pass the checks + * + * @param derivation_type: curve of the key + * @param bip32_path: bip32 path of the key + * @return bool: return true if it has passed checks + */ bool is_path_authorized(derivation_type_t const derivation_type, bip32_path_t const *const bip32_path) { check_null(bip32_path); @@ -124,43 +139,46 @@ void guard_baking_authorized(parsed_baking_data_t const *const baking_info, } } +/** + * @brief Raw representation of block + * + * All information of the block are not parsed here. + * + * Except the fitness placed just after fitness size, the rest + * of the block content is not important for our checks + * + * Fitness = + * - tag_size(4) + tag(1) + + * - level_size(4) + level(4) + + * - locked_round_size(4) + locked_round(0|4) + + * - predecessor_round_size(4) + predecessor_round(4) + + * - current_round_size(4) + current_round(4) + * + */ struct block_wire { - uint8_t magic_byte; - uint32_t chain_id; - uint32_t level; - uint8_t proto; - uint8_t predecessor[32]; - uint64_t timestamp; - uint8_t validation_pass; - uint8_t operation_hash[32]; - uint32_t fitness_size; + uint8_t magic_byte; ///< magic bytes, should be 0x11 + uint32_t chain_id; ///< chain id of the block + uint32_t level; ///< height of the block, from the genesis block + uint8_t proto; ///< protocol number + uint8_t predecessor[32]; ///< hash of the preceding block + uint64_t timestamp; ///< timestamp at which the block have been created + uint8_t validation_pass; ///< number of validation passes + uint8_t operation_hash[32]; ///< hash of the operations + uint32_t fitness_size; ///< size of the fitness // ... beyond this we don't care } __attribute__((packed)); -struct consensus_op_wire { - uint8_t magic_byte; - uint32_t chain_id; - uint8_t branch[32]; - uint8_t tag; - uint16_t slot; - uint32_t level; - uint32_t round; - uint8_t block_payload_hash[32]; -} __attribute__((packed)); - -/** - * Fitness = - * - tag_size(4) + tag(1) + - * - level_size(4) + level(4) + - * - locked_round_size(4) + locked_round(0|4) + - * - predecessor_round_size(4) + predecessor_round(4) + - * - current_round_size(4) + current_round(4) - */ #define MINIMUM_FITNESS_SIZE 33 // When 'locked_round' == none #define MAXIMUM_FITNESS_SIZE 37 // When 'locked_round' != none #define TENDERBAKE_PROTO_FITNESS_VERSION 2 +/** + * @brief Get the protocol version from fitness + * + * @param fitness: fitness + * @return uint8_t: protocol version result + */ uint8_t get_proto_version(void const *const fitness) { // Each field is preceded by its size (uint32_t). // That's why we need to look at `sizeof(uint32_t)` bytes after @@ -171,6 +189,14 @@ uint8_t get_proto_version(void const *const fitness) { return READ_UNALIGNED_BIG_ENDIAN(uint8_t, fitness + sizeof(uint32_t)); } +/** + * @brief Parse a block + * + * @param out: baking data output + * @param data: input + * @param length: input length + * @return bool: returns false if it is invalid + */ bool parse_block(parsed_baking_data_t *const out, void const *const data, size_t const length) { if (length < sizeof(struct block_wire) + MINIMUM_FITNESS_SIZE) { return false; @@ -196,6 +222,33 @@ bool parse_block(parsed_baking_data_t *const out, void const *const data, size_t return true; } +/** + * @brief Raw representation of a consensus operation + * + * - Pre-attestation , tag: 20 + * - Attestation , tag: 21 + * - Attestation + DAL, tag: 23 + * + */ +struct consensus_op_wire { + uint8_t magic_byte; ///< magic bytes, should be 0x12, or 0x13 + uint32_t chain_id; ///< chain id of the block + uint8_t branch[32]; ///< block branch + uint8_t tag; ///< operation tag + uint16_t slot; ///< first slot of the baker + uint32_t level; ///< level of the related block + uint32_t round; ///< round of the related block + uint8_t block_payload_hash[32]; ///< hash of the related block +} __attribute__((packed)); + +/** + * @brief Parse a consensus operation + * + * @param out: baking data output + * @param data: input + * @param length: input length + * @return bool: returns false if it is invalid + */ bool parse_consensus_operation(parsed_baking_data_t *const out, void const *const data, size_t const length) { diff --git a/src/baking_auth.h b/src/baking_auth.h index 4eed3087..3f2f3168 100644 --- a/src/baking_auth.h +++ b/src/baking_auth.h @@ -30,16 +30,47 @@ #include #include +/** + * @brief Authorizes a key + * + * @param derivation_type: curve of the key + * @param bip32_path: bip32 path of the key + */ void authorize_baking(derivation_type_t const derivation_type, bip32_path_t const *const bip32_path); -void guard_baking_authorized(parsed_baking_data_t const *const baking_data, + +/** + * @brief Guards baking info and key pass required checks + * + * @param baking_info: baking info to check + * @param key: key to check + */ +void guard_baking_authorized(parsed_baking_data_t const *const baking_info, bip32_path_with_curve_t const *const key); -bool is_path_authorized(derivation_type_t const derivation_type, - bip32_path_t const *const bip32_path); + +/** + * @brief Checks if a level is valid + * + * @param level: level + * @return bool: if the level is valid + */ bool is_valid_level(level_t level); + +/** + * @brief Stores baking info into the NVRAM + * + * @param in: baking info + */ void write_high_water_mark(parsed_baking_data_t const *const in); -// Return false if it is invalid +/** + * @brief Parses a baking data + * + * @param out: baking data output + * @param data: input + * @param length: input length + * @return bool: returns false if it is invalid + */ bool parse_baking_data(parsed_baking_data_t *const out, void const *const data, size_t const length); From dde7f67d80404d5d8a1808238df8d7771203bff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Fri, 8 Mar 2024 16:25:59 +0100 Subject: [PATCH 11/15] Doc: documents keys.(c|h) files --- src/keys.c | 16 +++++++++ src/keys.h | 98 ++++++++++++++++++++++++++++++++++++++++++++++-------- 2 files changed, 101 insertions(+), 13 deletions(-) diff --git a/src/keys.c b/src/keys.c index 62d368db..642f7e00 100644 --- a/src/keys.c +++ b/src/keys.c @@ -55,6 +55,14 @@ size_t read_bip32_path(bip32_path_t *const out, uint8_t const *const in, size_t return ix; } +/** + * @brief Derivates private key from bip32 path and curve + * + * @param private_key: private key output + * @param derivation_type: curve + * @param bip32_path: bip32 path + * @return int: error, 0 if none + */ int crypto_derive_private_key(cx_ecfp_private_key_t *private_key, derivation_type_t const derivation_type, bip32_path_t const *const bip32_path) { @@ -94,6 +102,14 @@ int crypto_derive_private_key(cx_ecfp_private_key_t *private_key, return error; } +/** + * @brief Derivates public key from private and curve + * + * @param derivation_type: curve + * @param private_key: private key + * @param public_key public key output + * @return int: error, 0 if none + */ int crypto_init_public_key(derivation_type_t const derivation_type, cx_ecfp_private_key_t *private_key, cx_ecfp_public_key_t *public_key) { diff --git a/src/keys.h b/src/keys.h index abf655ee..01293f0f 100644 --- a/src/keys.h +++ b/src/keys.h @@ -30,26 +30,68 @@ #include "os_cx.h" #include "types.h" +/** + * @brief Raw representation of bip32 path + * + */ struct bip32_path_wire { - uint8_t length; - uint32_t components[0]; + uint8_t length; ///< length of the path + uint32_t components[0]; ///< pointer to the component array } __attribute__((packed)); -// throws +/** + * @brief Reads a bip32_path + * + * Can throw exception + * + * @param out: bip32_path output + * @param in: input data + * @param in_size: input size + * @return size_t: number of byte read + */ size_t read_bip32_path(bip32_path_t *const out, uint8_t const *const in, size_t const in_size); +/** + * @brief Generates a private/public key pair from a bip32 path and a curve + * + * @param key_pair: private/public key pair output + * @param derivation_type: curve + * @param bip32_path: bip32 path + * @return int: error, 0 if none + */ int generate_key_pair(key_pair_t *key_pair, derivation_type_t const derivation_type, bip32_path_t const *const bip32_path); -// Non-reentrant -void public_key_hash( - uint8_t *const hash_out, - size_t const hash_out_size, - cx_ecfp_public_key_t *const compressed_out, // pass NULL if this value is not desired - derivation_type_t const derivation_type, - cx_ecfp_public_key_t const *const restrict public_key); - +/** + * @brief Extract the public key hash from a public key and a curve + * + * Is non-reentrant + * + * @param hash_out: public key hash output + * @param hash_out_size: output size + * @param compressed_out: compressed public key output + * pass NULL if this value is not desired + * @param derivation_type: curve + * @param public_key: public key + */ +void public_key_hash(uint8_t *const hash_out, + size_t const hash_out_size, + cx_ecfp_public_key_t *const compressed_out, + derivation_type_t const derivation_type, + cx_ecfp_public_key_t const *const restrict public_key); + +/** + * @brief Signs a message with a key + * + * @param out: signature output + * @param out_size: output size + * @param derivation_type: key derivation_type + * @param key: key + * @param in: message input + * @param in_size: input size + * @return size_t: size of the signature + */ size_t sign(uint8_t *const out, size_t const out_size, derivation_type_t const derivation_type, @@ -57,7 +99,12 @@ size_t sign(uint8_t *const out, uint8_t const *const in, size_t const in_size); -// Read a curve code from wire-format and parse into `deviration_type`. +/** + * @brief Reads a curve code from wire-format and parse into `deviration_type` + * + * @param curve_code: curve code + * @return derivation_type_t: derivation_type result + */ static inline derivation_type_t parse_derivation_type(uint8_t const curve_code) { switch (curve_code) { case 0: @@ -73,7 +120,12 @@ static inline derivation_type_t parse_derivation_type(uint8_t const curve_code) } } -// Convert `derivation_type` to wire-format. +/** + * @brief Converts `derivation_type` to wire-format. + * + * @param derivation_type: curve + * @return uint8_t: curve code result + */ static inline uint8_t unparse_derivation_type(derivation_type_t const derivation_type) { switch (derivation_type) { case DERIVATION_TYPE_ED25519: @@ -89,6 +141,12 @@ static inline uint8_t unparse_derivation_type(derivation_type_t const derivation } } +/** + * @brief Converts `derivation_type` to `signature_type` + * + * @param derivation_type: derivation_type + * @return signature_type_t: signature_type result + */ static inline signature_type_t derivation_type_to_signature_type( derivation_type_t const derivation_type) { switch (derivation_type) { @@ -105,6 +163,12 @@ static inline signature_type_t derivation_type_to_signature_type( } } +/** + * @brief Converts `signature_type` to `cx_curve` + * + * @param signature_type: signature_type + * @return cx_curve_t: curve result + */ static inline cx_curve_t signature_type_to_cx_curve(signature_type_t const signature_type) { switch (signature_type) { case SIGNATURE_TYPE_SECP256K1: @@ -118,6 +182,14 @@ static inline cx_curve_t signature_type_to_cx_curve(signature_type_t const signa } } +/** + * @brief Generates a public key from a bip32 path and a curve + * + * @param public_key: public key output + * @param derivation_type: curve + * @param bip32_path: bip32 path + * @return int: error, 0 if none + */ int generate_public_key(cx_ecfp_public_key_t *public_key, derivation_type_t const derivation_type, bip32_path_t const *const bip32_path); From 5173e512323efde01e1c00b63737bd50fe56441a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 10:50:55 +0100 Subject: [PATCH 12/15] Doc: documents operations.(c|h) files --- src/operations.c | 88 ++++++++++++++++++++++++++++++++----- src/operations.h | 110 ++++++++++++++++++++++++++++++++++++----------- 2 files changed, 163 insertions(+), 35 deletions(-) diff --git a/src/operations.c b/src/operations.c index 99286c1e..50f39367 100644 --- a/src/operations.c +++ b/src/operations.c @@ -33,7 +33,10 @@ #define STEP_HARD_FAIL -2 -// Argument is to distinguish between different parse errors for debugging purposes only +/** + * @brief Raises an exception and force hard fail if continue to parse + * + */ __attribute__((noreturn)) static void parse_error(void) { global.apdu.u.sign.parse_state.op_step = STEP_HARD_FAIL; THROW(EXC_PARSE_ERROR); @@ -41,8 +44,14 @@ __attribute__((noreturn)) static void parse_error(void) { #define PARSE_ERROR() parse_error() -// Conversion/check functions +/// Conversion/check functions +/** + * @brief Get signature_type from a raw signature_type + * + * @param raw_signature_type: raw signature_type + * @return signature_type_t: signature_type result + */ static inline signature_type_t parse_raw_tezos_header_signature_type( raw_tezos_header_signature_type_t const *const raw_signature_type) { check_null(raw_signature_type); @@ -58,6 +67,14 @@ static inline signature_type_t parse_raw_tezos_header_signature_type( } } +/** + * @brief Extracts a compressed_pubkey and a contract from a key + * + * @param compressed_pubkey_out: compressed_pubkey output + * @param contract_out: contract output + * @param derivation_type: curve of the key + * @param bip32_path: bip32 path of the key + */ static inline void compute_pkh(cx_ecfp_public_key_t *const compressed_pubkey_out, parsed_contract_t *const contract_out, derivation_type_t const derivation_type, @@ -79,6 +96,13 @@ static inline void compute_pkh(cx_ecfp_public_key_t *const compressed_pubkey_out contract_out->originated = 0; } +/** + * @brief Parses implict contract + * + * @param out: implict contract output + * @param raw_signature_type: raw signature_type + * @param hash: input hash + */ static inline void parse_implicit(parsed_contract_t *const out, raw_tezos_header_signature_type_t const *const raw_signature_type, uint8_t const hash[HASH_SIZE]) { @@ -88,20 +112,30 @@ static inline void parse_implicit(parsed_contract_t *const out, memcpy(out->hash, hash, sizeof(out->hash)); } +/** + * @brief Helpers for sub parser + * + * Subparsers: no function here should be called anywhere in + * this file without using the CALL_SUBPARSER macro above. + * + */ #define CALL_SUBPARSER_LN(func, line, ...) \ if (func(__VA_ARGS__, line)) { \ return true; \ } #define CALL_SUBPARSER(func, ...) CALL_SUBPARSER_LN(func, __LINE__, __VA_ARGS__) -// Subparsers: no function here should be called anywhere in this file without using the -// CALL_SUBPARSER macro above. Verify this with -// /\s*( -// the only result should be the functiond definition. - #define NEXT_BYTE (byte) // TODO: this function cannot parse z values than would not fit in a uint64 +/** + * @brief Parses a Z number + * + * @param current_byte: the current read byte + * @param state: parsing state + * @param lineno: line number of the caller + * @return bool: if has finished to read the number + */ static inline bool parse_z(uint8_t current_byte, struct int_subparser_state *state, uint32_t lineno) { @@ -119,13 +153,21 @@ static inline bool parse_z(uint8_t current_byte, state->shift += 7; return current_byte & 0x80; // Return true if we need more bytes. } - #define PARSE_Z \ ({ \ CALL_SUBPARSER(parse_z, (byte), &(state)->subparser_state.integer); \ (state)->subparser_state.integer.value; \ }) +/** + * @brief Parses a wire type + * + * @param current_byte: the current read byte + * @param state: parsing state + * @param sizeof_type: size of the type + * @param lineno: line number of the caller + * @return bool: if has finished to read the type + */ static inline bool parse_next_type(uint8_t current_byte, struct nexttype_subparser_state *state, uint32_t sizeof_type, @@ -147,7 +189,6 @@ static inline bool parse_next_type(uint8_t current_byte, return state->fill_idx < sizeof_type; // Return true if we need more bytes. } - // do _NOT_ keep pointers to this data around. #define NEXT_TYPE(type) \ ({ \ @@ -157,6 +198,14 @@ static inline bool parse_next_type(uint8_t current_byte, // End of subparsers. +/** + * @brief Initialize the operation parser + * + * @param out: parsing output + * @param derivation_type: curve of the key + * @param bip32_path: bip32 path of the key + * @param state: parsing state + */ void parse_operations_init(struct parsed_operation_group *const out, derivation_type_t derivation_type, bip32_path_t const *const bip32_path, @@ -178,7 +227,7 @@ void parse_operations_init(struct parsed_operation_group *const out, state->tag = OPERATION_TAG_NONE; // This and the rest shouldn't be required. } -// Named steps in the top-level state machine +/// Named steps in the top-level state machine #define STEP_END_OF_MESSAGE -1 #define STEP_OP_TYPE_DISPATCH 10001 #define STEP_AFTER_MANAGER_FIELDS 10002 @@ -192,6 +241,14 @@ bool parse_operations_final(struct parse_state *const state, return state->op_step == STEP_END_OF_MESSAGE || state->op_step == 1; } +/** + * @brief Parse one bytes regarding the current parsing state + * + * @param byte: byte to read + * @param state: parsing state + * @param out: parsing output + * @return bool: returns true on success + */ static inline bool parse_byte(uint8_t byte, struct parse_state *const state, struct parsed_operation_group *const out) { @@ -371,6 +428,17 @@ static inline bool parse_byte(uint8_t byte, #define G global.apdu.u.sign +/** + * @brief Parses a group of operation + * + * Throws on parsing failure + * + * @param out: output + * @param data: input + * @param length: input length + * @param derivation_type: curve of the key + * @param bip32_path: bip32 path of the key + */ static void parse_operations_throws_parse_error(struct parsed_operation_group *const out, void const *const data, size_t length, diff --git a/src/operations.h b/src/operations.h index 75bb777c..de2008cf 100644 --- a/src/operations.h +++ b/src/operations.h @@ -32,66 +32,126 @@ #include "cx.h" #include "types.h" -// Wire format that gets parsed into `signature_type`. +/** + * @brief Wire format that gets parsed into `signature_type` + * + */ typedef struct { - uint8_t v; + uint8_t v; ///< value of the type of header signature } __attribute__((packed)) raw_tezos_header_signature_type_t; +/** + * @brief Wire representation of operation group header + * + */ struct operation_group_header { - uint8_t magic_byte; - uint8_t hash[32]; + uint8_t magic_byte; ///< magic bytes, should be 0x03 + uint8_t hash[32]; ///< hash of the operation } __attribute__((packed)); +/** + * @brief Wire representation of implicit contract + * + */ struct implicit_contract { - raw_tezos_header_signature_type_t signature_type; - uint8_t pkh[HASH_SIZE]; + raw_tezos_header_signature_type_t signature_type; ///< type of the contract signature + uint8_t pkh[HASH_SIZE]; ///< raw public key hash } __attribute__((packed)); +/** + * @brief Wire representation of delegation + * + */ struct delegation_contents { - raw_tezos_header_signature_type_t signature_type; - uint8_t hash[HASH_SIZE]; + raw_tezos_header_signature_type_t signature_type; ///< type of the delegate signature + uint8_t hash[HASH_SIZE]; ///< raw delegate } __attribute__((packed)); +/** + * @brief This structure represents the state of a Z parser + * + */ struct int_subparser_state { - uint32_t lineno; // Has to be in _all_ members of the subparser union. - uint64_t value; // Still need to fix this. - uint8_t shift; + uint32_t lineno; ///< line number + ///< Has to be in _all_ members of the subparser union. + uint64_t value; ///< Read value + /// Still need to fix this. + uint8_t shift; ///< Z shift }; +/** + * @brief This structure represents the state of a wire type parser + * + * Allows to read data using wire representation types + * + * Fills body using raw and an increasing fill_idx + * + */ struct nexttype_subparser_state { - uint32_t lineno; + uint32_t lineno; ///< line number + + /// union of all wire structure union { - raw_tezos_header_signature_type_t sigtype; + raw_tezos_header_signature_type_t sigtype; ///< wire signature_type - struct operation_group_header ogh; + struct operation_group_header ogh; ///< wire operation group header - struct implicit_contract ic; + struct implicit_contract ic; ///< wire implicit contract - struct delegation_contents dc; + struct delegation_contents dc; ///< wire delegation content - uint8_t raw[1]; + uint8_t raw[1]; ///< raw array to fill the body } body; - uint32_t fill_idx; + uint32_t fill_idx; ///< current fill index }; +/** + * @brief This structure represents the union of all subparsers + * + */ union subparser_state { - struct int_subparser_state integer; - struct nexttype_subparser_state nexttype; + struct int_subparser_state integer; ///< state of a n integer parser + struct nexttype_subparser_state nexttype; ///< state of a wire type parser }; +/** + * @brief This structure represents the parsing state + * + */ struct parse_state { - int16_t op_step; - union subparser_state subparser_state; - enum operation_tag tag; + int16_t op_step; ///< current parsing step + union subparser_state subparser_state; ///< state of subparser + enum operation_tag tag; ///< current operation tag }; -// Allows arbitrarily many "REVEAL" operations but only one operation of any other type, -// which is the one it puts into the group. +/** + * @brief Parses a group of operation + * + * Allows arbitrarily many "REVEAL" operations but only one + * operation of any other type, which is the one it puts into + * the group. + * + * Some checks are carried out during the parsing using a key using a key + * + * @param out: parsing output + * @param data: input + * @param length: input length + * @param curve: curve of the key + * @param bip32_path: bip32 path of the key + * @return bool: returns true on success + */ bool parse_operations(struct parsed_operation_group *const out, uint8_t const *const data, size_t length, derivation_type_t curve, bip32_path_t const *const bip32_path); +/** + * @brief Checks parsing has been completed successfully + * + * @param state: parsing state + * @param out: parsing output + * @return bool: returns true on success + */ bool parse_operations_final(struct parse_state *const state, struct parsed_operation_group *const out); From 9161ecec9b03a538d33589e5f075cbf2eeaeaa0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 10:53:06 +0100 Subject: [PATCH 13/15] Doc: documents to_string.(c|h) files --- src/to_string.c | 64 +++++++++++++++++++++++++++++++++++-------------- src/to_string.h | 62 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 105 insertions(+), 21 deletions(-) diff --git a/src/to_string.c b/src/to_string.c index 70372504..9c920ae7 100644 --- a/src/to_string.c +++ b/src/to_string.c @@ -28,16 +28,6 @@ #include -#define NO_CONTRACT_STRING "None" - -#ifdef HAVE_BAGL -#define NO_CONTRACT_NAME_STRING "Custom Delegate: please verify the address" -#endif - -#ifdef HAVE_NBGL -#define NO_CONTRACT_NAME_STRING "Custom Delegate:\nplease verify the address" -#endif - #define TEZOS_HASH_CHECKSUM_SIZE 4 #define TICKER_WITH_SPACE " XTZ" @@ -46,8 +36,6 @@ void pkh_to_string(char *const buff, size_t const buff_size, signature_type_t const signature_type, uint8_t const hash[HASH_SIZE]); - -// These functions output terminating null bytes, and return the ending offset. static size_t microtez_to_string(char *dest, uint64_t number); void pubkey_to_pkh_string(char *const out, @@ -73,6 +61,13 @@ void bip32_path_with_curve_to_pkh_string(char *const out, pubkey_to_pkh_string(out, out_size, key->derivation_type, &pubkey); } +/** + * @brief Computes the ckecsum of a hash + * + * @param out: result output + * @param data: hash input + * @param size: input size + */ void compute_hash_checksum(uint8_t out[TEZOS_HASH_CHECKSUM_SIZE], void const *const data, size_t size) { @@ -82,6 +77,14 @@ void compute_hash_checksum(uint8_t out[TEZOS_HASH_CHECKSUM_SIZE], memcpy(out, checksum, TEZOS_HASH_CHECKSUM_SIZE); } +/** + * @brief Converts a public key hash to string + * + * @param buff: result output + * @param buff_size: output size + * @param signature_type: curve of the key + * @param hash: public key hash + */ void pkh_to_string(char *const buff, size_t const buff_size, signature_type_t const signature_type, @@ -134,6 +137,13 @@ void pkh_to_string(char *const buff, } } +/** + * @brief Converts a chain id to string + * + * @param buff: result output + * @param buff_size: output size + * @param chain_id: chain id to convert + */ void chain_id_to_string(char *const buff, size_t const buff_size, chain_id_t const chain_id) { check_null(buff); if (buff_size < CHAIN_ID_BASE58_STRING_SIZE) { @@ -179,11 +189,20 @@ void chain_id_to_string_with_aliases(char *const out, } } -// These functions do not output terminating null bytes. - -// This function fills digits, potentially with all leading zeroes, from the end of the buffer -// backwards This is intended to be used with a temporary buffer of length MAX_INT_DIGITS Returns -// offset of where it stopped filling in +/// These functions do not output terminating null bytes. + +/** + * @brief Converts an uint64 number to string + * + * Fills digits, potentially with all leading zeroes, from the end of the buffer backwards + * + * This is intended to be used with a temporary buffer of length MAX_INT_DIGITS + * + * @param dest: result output + * @param number: number to convert + * @param leading_zeroes: if keep the leading 0 + * @return size_t: offset of where it stopped filling in + */ static inline size_t convert_number(char dest[MAX_INT_DIGITS], uint64_t number, bool leading_zeroes) { @@ -233,10 +252,19 @@ size_t number_to_string(char *const dest, uint64_t number) { return length; } -// Microtez are in millionths +/// Microtez are in millionths #define TEZ_SCALE 1000000 #define DECIMAL_DIGITS 6 +/** + * @brief Converts an uint64 number to microtez as string + * + * These functions output terminating null bytes, and return the ending offset. + * + * @param dest: output buffer + * @param number: number to convert + * @return size_t: size of the result + */ size_t microtez_to_string(char *const dest, uint64_t number) { check_null(dest); uint64_t whole_tez = number / TEZ_SCALE; diff --git a/src/to_string.h b/src/to_string.h index 60a0d765..dd0dbee5 100644 --- a/src/to_string.h +++ b/src/to_string.h @@ -30,27 +30,83 @@ #include "types.h" #include "ui.h" +/** + * @brief Converts a key to a public key hash string using its public key + * + * @param out: result output + * @param out_size: output size + * @param derivation_type: curve of the key + * @param public_key: public key of the key + */ void pubkey_to_pkh_string(char *const out, size_t const out_size, derivation_type_t const derivation_type, cx_ecfp_public_key_t const *const public_key); + +/** + * @brief Converts a key to a public key hash string using its bip32 path and curve + * + * @param out: result output + * @param out_size: output size + * @param key: bip32 path and curve of the key + */ void bip32_path_with_curve_to_pkh_string(char *const out, size_t const out_size, bip32_path_with_curve_t const *const key); + +/** + * @brief Converts a chain id to string + * + * Outputs its alias if it has one + * + * @param out: result output + * @param out_size: output size + * @param chain_id: chain id to convert + */ void chain_id_to_string_with_aliases(char *const out, size_t const out_size, chain_id_t const *const chain_id); -// dest must be at least MAX_INT_DIGITS +/** + * @brief Converts an uint64 number to string + * + * The size of `dest` must be at least `MAX_INT_DIGITS` + * + * @param dest: result output + * @param number: number to convert + * @return size_t: size of the result + */ size_t number_to_string(char *const dest, uint64_t number); -// These take their number parameter through a pointer and take a length +/** + * @brief Converts an uint32 number to string + * + * @param dest: result output + * @param buff_size: size of dest + * @param number: number to convert + */ void number_to_string_indirect32(char *const dest, size_t const buff_size, uint32_t const *const number); + +/** + * @brief Converts an uint64 number to microtez as string + * + * @param dest: result output + * @param buff_size: size of dest + * @param number: number to convert + */ void microtez_to_string_indirect(char *const dest, size_t const buff_size, uint64_t const *const number); -// `src` may be unrelocated pointer to rodata. +/** + * @brief Copies a string in a buffer + * + * `src` may be unrelocated pointer to rodata. + * + * @param dest: output buffer + * @param buff_size: size of the buffer + * @param src: input to copy + */ void copy_string(char *const dest, size_t const buff_size, char const *const src); From 5cf1be859fbe1bbfe1a4d962601f8207cdd5f1a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 11:48:34 +0100 Subject: [PATCH 14/15] Doc: documents global.h file --- src/globals.h | 176 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 125 insertions(+), 51 deletions(-) diff --git a/src/globals.h b/src/globals.h index 07c1ce5d..01d08da1 100644 --- a/src/globals.h +++ b/src/globals.h @@ -29,123 +29,183 @@ #include "operations.h" #include "ui.h" -// Zeros out all globals that can keep track of APDU instruction state. -// Notably this does *not* include UI state. +/** + * @brief Zeros out all globals that can keep track of APDU instruction state + * + * Notably this does *not* include UI state + * + */ void clear_apdu_globals(void); +/** + * @brief string_generation_callback for chains + * + * @param out: output buffer + * @param out_size: output size + * @param data: chain_id + */ void copy_chain(char *out, size_t out_size, void *data); + +/** + * @brief string_generation_callback for keys + * + * @param out: output buffer + * @param out_size: output size + * @param data: bip32 path curve key + */ void copy_key(char *out, size_t out_size, void *data); + +/** + * @brief string_generation_callback for high watermarks + * + * @param out: output buffer + * @param out_size: output size + * @param data: high watermark + */ void copy_hwm(char *out, size_t out_size, void *data); -// Zeros out all application-specific globals and SDK-specific UI/exchange buffers. + +/** + * @brief Zeros out all application-specific globals and SDK-specific UI/exchange buffers + * + */ void init_globals(void); -#define MAX_APDU_SIZE 235 // Maximum number of bytes in a single APDU +/// Maximum number of bytes in a single APDU +#define MAX_APDU_SIZE 235 -// Our buffer must accommodate any remainder from hashing and the next message at once. +/// Our buffer must accommodate any remainder from hashing and the next message at once. #define TEZOS_BUFSIZE (BLAKE2B_BLOCKBYTES + MAX_APDU_SIZE) #define PRIVATE_KEY_DATA_SIZE 64 +#define MAX_SIGNATURE_SIZE 100 -#define MAX_SIGNATURE_SIZE 100 - +/** + * @brief This structure represents the state needed to handle HMAC + * + */ typedef struct { - uint8_t signed_hmac_key[MAX_SIGNATURE_SIZE]; - uint8_t hashed_signed_hmac_key[CX_SHA512_SIZE]; - uint8_t hmac[CX_SHA256_SIZE]; + uint8_t signed_hmac_key[MAX_SIGNATURE_SIZE]; ///< buffer to hold the signed hmac key + uint8_t hashed_signed_hmac_key[CX_SHA512_SIZE]; ///< buffer to hold the hashed signed hmac key + uint8_t hmac[CX_SHA256_SIZE]; ///< buffer to hold the hmac result } apdu_hmac_state_t; +/** + * @brief This structure represents the state needed to hash messages + * + */ typedef struct { - cx_blake2b_t state; - bool initialized; + cx_blake2b_t state; ///< blake2b state + bool initialized; ///< if the state has already been initialized } blake2b_hash_state_t; +/** + * @brief This structure represents the state needed to sign messages + * + */ typedef struct { - uint8_t packet_index; // 0-index is the initial setup packet, 1 is first packet to hash, etc. + /// 0-index is the initial setup packet, 1 is first packet to hash, etc. + uint8_t packet_index; + /// state to hold the current parsed bakind data parsed_baking_data_t parsed_baking_data; + /// operation read, used for checks struct { - bool is_valid; - struct parsed_operation_group v; + bool is_valid; ///< if the parsed operation group is considered as valid + struct parsed_operation_group v; ///< current parsed operation group } maybe_ops; + /// buffer to hold the current message part and the previous message hash uint8_t message_data[TEZOS_BUFSIZE]; - size_t message_data_length; + size_t message_data_length; ///< length of message data - blake2b_hash_state_t hash_state; - uint8_t final_hash[SIGN_HASH_SIZE]; + blake2b_hash_state_t hash_state; ///< current blake2b hash state + uint8_t final_hash[SIGN_HASH_SIZE]; ///< buffer to hold hash of all the message - uint8_t magic_byte; - bool hash_only; - struct parse_state parse_state; + uint8_t magic_byte; ///< current magic byte read + struct parse_state parse_state; ///< current parser state } apdu_sign_state_t; -// Used to compute what we need to display on the screen. -// Title of the screen will be `title` field, and value of -// the screen will be generated by calling `callback_fn` and providing -// `data` as one of its parameter. +/** + * @brief This structure represents data used to compute what we need to display on the screen. + * + */ struct screen_data { - char *title; - string_generation_callback callback_fn; - void *data; + char *title; ///< title of the screen + string_generation_callback callback_fn; ///< callback to convert the data to string + void *data; ///< value to display on the screen }; -// State of the dynamic display. -// Used to keep track on whether we are displaying screens inside the stack, -// or outside the stack (for example confirmation screens). +/** + * @brief State of the dynamic display + * + * Used to keep track on whether we are displaying screens inside the stack, or outside the + * stack (for example confirmation screens) + * + */ enum e_state { STATIC_SCREEN, DYNAMIC_SCREEN, }; +/** + * @brief This structure holds all structure needed + * + */ typedef struct { + /// dynamic display state struct { struct screen_data screen_stack[MAX_SCREEN_STACK_SIZE]; - enum e_state current_state; // State of the dynamic display + enum e_state current_state; ///< State of the dynamic display - // Size of the screen stack + /// Size of the screen stack uint8_t screen_stack_size; - // Current index in the screen_stack. + /// Current index in the screen_stack. uint8_t formatter_index; - // Callback function if user accepted prompt. + /// Callback function if user accepted prompt. ui_callback_t ok_callback; - // Callback function if user rejected prompt. + /// Callback function if user rejected prompt. ui_callback_t cxl_callback; - // Title to be displayed on the screen. + /// Title to be displayed on the screen. char screen_title[PROMPT_WIDTH + 1]; - // Value to be displayed on the screen. + /// Value to be displayed on the screen. char screen_value[VALUE_WIDTH + 1]; - // Screensaver is on/off. + /// Screensaver is on/off. bool is_blank_screen; } dynamic_display; - apdu_handler handlers[INS_MAX + 1]; - bip32_path_with_curve_t path_with_curve; + apdu_handler handlers[INS_MAX + 1]; ///< all handlers + bip32_path_with_curve_t path_with_curve; ///< holds the bip32 path and curve of the current key + /// apdu handling state struct { union { - apdu_sign_state_t sign; + apdu_sign_state_t sign; ///< state used to handle signing + /// state used to handle reset struct { - level_t reset_level; + level_t reset_level; ///< requested reset level } baking; + /// state used to handle setup struct { - chain_id_t main_chain_id; + chain_id_t main_chain_id; ///< requested new main chain id + /// requested new HWM information struct { - level_t main; - level_t test; + level_t main; ///< level requested to be set on main HWM + level_t test; ///< level requested to be set on test HWM } hwm; } setup; - apdu_hmac_state_t hmac; + apdu_hmac_state_t hmac; ///< state used to handle hmac } u; + /// state used to store baking authorizing data struct { - nvram_data new_data; // Staging area for setting N_data + nvram_data new_data; ///< Staging area for setting N_data } baking_auth; } apdu; } globals_t; @@ -155,12 +215,26 @@ extern globals_t global; extern nvram_data const N_data_real; #define N_data (*(volatile nvram_data *) PIC(&N_data_real)) +/** + * @brief Selects a HWM for a given chain id depending on the ram + * + * Selects the main HWM of the ram if the main chain of the ram + * is not defined, or if the given chain matches the main chain + * of the ram. Selects the test HWM of the ram otherwise. + * + * @param chain_id: chain id + * @param ram: ram + * @return high_watermark_t*: selected HWM + */ high_watermark_t volatile *select_hwm_by_chain(chain_id_t const chain_id, nvram_data volatile *const ram); -// Properly updates NVRAM data to prevent any clobbering of data. -// 'out_param' defines the name of a pointer to the nvram_data struct -// that 'body' can change to apply updates. +/** + * @brief Properly updates NVRAM data to prevent any clobbering of data + * + * @param out_name: defines the name of a pointer to the nvram_data struct + * @param body: defines the code to apply updates + */ #define UPDATE_NVRAM(out_name, body) \ ({ \ nvram_data *const out_name = &global.apdu.baking_auth.new_data; \ From 876ce5596328eabff74c078e42756eeffdf2331c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Palmer?= Date: Mon, 11 Mar 2024 14:23:44 +0100 Subject: [PATCH 15/15] Doxygen: disable EXTRACT_ALL, which assumes that all entities are documented It enables to warn if the document is not documented and therefore fails for our CI. --- .doxygen/Doxyfile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.doxygen/Doxyfile b/.doxygen/Doxyfile index 1c8cfe4f..766774c8 100644 --- a/.doxygen/Doxyfile +++ b/.doxygen/Doxyfile @@ -518,7 +518,7 @@ TIMESTAMP = NO # normally produced when WARNINGS is set to YES. # The default value is: NO. -EXTRACT_ALL = YES +EXTRACT_ALL = NO # If the EXTRACT_PRIVATE tag is set to YES, all private members of a class will # be included in the documentation. @@ -877,7 +877,7 @@ WARN_IF_INCOMPLETE_DOC = YES # WARN_IF_INCOMPLETE_DOC # The default value is: NO. -WARN_NO_PARAMDOC = NO +WARN_NO_PARAMDOC = YES # If WARN_IF_UNDOC_ENUM_VAL option is set to YES, doxygen will warn about # undocumented enumeration values. If set to NO, doxygen will accept @@ -885,7 +885,7 @@ WARN_NO_PARAMDOC = NO # will automatically be disabled. # The default value is: NO. -WARN_IF_UNDOC_ENUM_VAL = NO +WARN_IF_UNDOC_ENUM_VAL = YES # If the WARN_AS_ERROR tag is set to YES then doxygen will immediately stop when # a warning is encountered. If the WARN_AS_ERROR tag is set to FAIL_ON_WARNINGS @@ -901,7 +901,7 @@ WARN_IF_UNDOC_ENUM_VAL = NO # Possible values are: NO, YES, FAIL_ON_WARNINGS and FAIL_ON_WARNINGS_PRINT. # The default value is: NO. -WARN_AS_ERROR = NO +WARN_AS_ERROR = YES # The WARN_FORMAT tag determines the format of the warning messages that doxygen # can produce. The string should contain the $file, $line, and $text tags, which