simplify handlers, rename cancel function, start switch to standard l…

…ibrary io functions

src/app_main.c

@@ -109,6 +109,7 @@
 #include <stdint.h>
 #include <io.h>
 #include <ux.h>
+#include <parser.h>
 
 #include "blake2b.h"
 #include "sia.h"
@@ -197,18 +198,12 @@ void ui_menu_about(void) {
 
 #endif
 
-// io_exchange_with_code is a helper function for sending response APDUs from
-// button handlers. Note that the IO_RETURN_AFTER_TX flag is set. 'tx' is the
-// conventional name for the size of the response APDU, i.e. the write-offset
-// within G_io_apdu_buffer.
 void io_exchange_with_code(uint16_t code, uint16_t tx) {
-    G_io_apdu_buffer[tx++] = code >> 8;
-    G_io_apdu_buffer[tx++] = code & 0xFF;
-    io_exchange(CHANNEL_APDU | IO_RETURN_AFTER_TX, tx);
+    io_send_sw(code);
 }
 
 unsigned int io_reject(void) {
-    io_exchange_with_code(SW_USER_REJECTED, 0);
+    io_send_sw(SW_USER_REJECTED);
     // Return to the main screen.
     ui_idle();
     return 0;
@@ -226,12 +221,7 @@ unsigned int io_reject(void) {
 // out-parameters that will control the behavior of the next io_exchange call
 // in sia_main. It's common to set *flags |= IO_ASYNC_REPLY, but tx is
 // typically unused unless the handler is immediately sending a response APDU.
-typedef void handler_fn_t(uint8_t p1,
-                          uint8_t p2,
-                          uint8_t *dataBuffer,
-                          uint16_t dataLength,
-                          volatile unsigned int *flags,
-                          volatile unsigned int *tx);
+typedef void handler_fn_t(uint8_t p1, uint8_t p2, uint8_t *dataBuffer, uint16_t dataLength);
 
 handler_fn_t handleGetVersion;
 handler_fn_t handleGetPublicKey;
@@ -281,18 +271,8 @@ void app_main() {
 
     ui_idle();
 
-    // This is the main loop that reads and writes APDUs. It receives request
-    // APDUs from the computer, looks up the corresponding command handler, and
-    // calls it on the APDU payload. Then it loops around and calls io_exchange
-    // again. The handler may set the 'flags' and 'tx' variables, which affect the
-    // subsequent io_exchange call. The handler may also throw an exception, which
-    // will be caught, converted to an error code, appended to the response APDU,
-    // and sent in the next io_exchange call.
-    volatile unsigned int rx = 0;
-    volatile unsigned int tx = 0;
-    volatile unsigned int flags = 0;
-
-    // Exchange APDUs until EXCEPTION_IO_RESET is thrown.
+    int input_len = 0;
+    command_t cmd = {0};
     for (;;) {
         volatile unsigned short sw = 0;
 
@@ -306,21 +286,21 @@ void app_main() {
         // "true" main function defined at the bottom of this file.
         BEGIN_TRY {
             TRY {
-                rx = tx;
-                tx = 0;  // ensure no race in CATCH_OTHER if io_exchange throws an error
-                rx = io_exchange(CHANNEL_APDU | flags, rx);
-                flags = 0;
-
-                // No APDU received; trigger a reset.
-                if (rx == 0) {
-                    THROW(EXCEPTION_IO_RESET);
+                // Read command into G_io_apdu_buffer
+                if ((input_len = io_recv_command()) < 0) {
+                    PRINTF("Failed to receive");
+                    return;
                 }
-                // Malformed APDU.
-                if (G_io_apdu_buffer[OFFSET_CLA] != CLA) {
-                    THROW(0x6E00);
+
+                // Parse command into CLA, INS, P1/P2, LC, and data
+                if (!apdu_parser(&cmd, G_io_apdu_buffer, input_len)) {
+                    PRINTF("Invalid command length");
+                    io_send_sw(SW_INVALID_PARAM);
+                    continue;
                 }
+
                 // Lookup and call the requested command handler.
-                handler_fn_t *handlerFn = lookupHandler(G_io_apdu_buffer[OFFSET_INS]);
+                handler_fn_t *handlerFn = lookupHandler(cmd.ins);
                 if (!handlerFn) {
                     THROW(0x6D00);
                 }
@@ -333,12 +313,7 @@ void app_main() {
                 ux_layout_paging_reset();
 #endif
 
-                handlerFn(G_io_apdu_buffer[OFFSET_P1],
-                          G_io_apdu_buffer[OFFSET_P2],
-                          G_io_apdu_buffer + OFFSET_CDATA,
-                          G_io_apdu_buffer[OFFSET_LC],
-                          &flags,
-                          &tx);
+                handlerFn(cmd.p1, cmd.p2, cmd.data, cmd.lc);
             }
             CATCH(EXCEPTION_IO_RESET) {
                 THROW(EXCEPTION_IO_RESET);
@@ -365,8 +340,7 @@ void app_main() {
                         sw = 0x6800 | (e & 0x7FF);
                         break;
                 }
-                G_io_apdu_buffer[tx++] = sw >> 8;
-                G_io_apdu_buffer[tx++] = sw & 0xFF;
+                io_send_sw(sw);
             }
             FINALLY {
             }

src/calcTxnHash.c

@@ -26,6 +26,7 @@
 #include <stdint.h>
 #include <string.h>
 #include <ux.h>
+#include <io.h>
 
 #include "blake2b.h"
 #include "sia.h"
@@ -184,7 +185,7 @@ static void fmtTxnElem(void) {
         }
         default: {
             // This should never happen.
-            io_exchange_with_code(SW_DEVELOPER_ERR, 0);
+            io_send_sw(SW_DEVELOPER_ERR);
             ui_idle();
             break;
         }
@@ -199,12 +200,7 @@ static void zero_ctx(void) {
 // SigHash of the transaction, and optionally signs the hash using a specified
 // key. The transaction is processed in a streaming fashion and displayed
 // piece-wise to the user.
-void handleCalcTxnHash(uint8_t p1,
-                       uint8_t p2,
-                       uint8_t *dataBuffer,
-                       uint16_t dataLength,
-                       volatile unsigned int *flags,
-                       volatile unsigned int *tx __attribute__((unused))) {
+void handleCalcTxnHash(uint8_t p1, uint8_t p2, uint8_t *dataBuffer, uint16_t dataLength) {
     if ((p1 != P1_FIRST && p1 != P1_MORE) || (p2 != P2_DISPLAY_HASH && p2 != P2_SIGN_HASH)) {
         THROW(SW_INVALID_PARAM);
     }
@@ -265,7 +261,6 @@ void handleCalcTxnHash(uint8_t p1,
             THROW(SW_OK);
             break;
         case TXN_STATE_FINISHED:
-            *flags |= IO_ASYNCH_REPLY;
             fmtTxnElem();
             ux_flow_init(0, ux_show_txn_elem_flow, NULL);
             break;

src/calcTxnHash_nbgl.c

@@ -76,7 +76,7 @@ static void fmtTxnElem(void) {
 
         default:
             // This should never happen.
-            io_exchange_with_code(SW_DEVELOPER_ERR, 0);
+            io_send_sw(SW_DEVELOPER_ERR);
             ui_idle();
             break;
     }
@@ -97,7 +97,7 @@ static void confirm_callback(bool confirm) {
             nbgl_useCaseStatus("TRANSACTION HASHED", true, ui_idle);
         }
     } else {
-        io_exchange_with_code(SW_USER_REJECTED, 0);
+        io_send_sw(SW_USER_REJECTED);
         nbgl_useCaseStatus("Transaction Rejected", false, ui_idle);
     }
 }
@@ -174,12 +174,7 @@ static void zero_ctx(void) {
 // SigHash of the transaction, and optionally signs the hash using a specified
 // key. The transaction is processed in a streaming fashion and displayed
 // piece-wise to the user.
-void handleCalcTxnHash(uint8_t p1,
-                       uint8_t p2,
-                       uint8_t *dataBuffer,
-                       uint16_t dataLength,
-                       volatile unsigned int *flags,
-                       volatile unsigned int *tx __attribute__((unused))) {
+void handleCalcTxnHash(uint8_t p1, uint8_t p2, uint8_t *dataBuffer, uint16_t dataLength) {
     if ((p1 != P1_FIRST && p1 != P1_MORE) || (p2 != P2_DISPLAY_HASH && p2 != P2_SIGN_HASH)) {
         THROW(SW_INVALID_PARAM);
     }
@@ -237,7 +232,6 @@ void handleCalcTxnHash(uint8_t p1,
             THROW(SW_OK);
             break;
         case TXN_STATE_FINISHED:
-            *flags |= IO_ASYNCH_REPLY;
             nbgl_useCaseReviewStart(&C_stax_app_sia,
                                     (ctx->sign) ? "Sign Transaction" : "Hash Transaction",
                                     NULL,

src/getPublicKey.c

@@ -36,7 +36,7 @@
 // Get a pointer to getPublicKey's state variables.
 static getPublicKeyContext_t* ctx = &global.getPublicKeyContext;
 
-static unsigned int io_seproxyhal_touch_pk_ok(void);
+static unsigned int send_pubkey(void);
 
 #ifdef HAVE_BAGL
 // Allows scrolling through the address/public key
@@ -51,10 +51,7 @@ UX_STEP_NOCB(ux_approve_pk_flow_1_step,
              bn,
              {global.getPublicKeyContext.typeStr, global.getPublicKeyContext.keyStr});
 
-UX_STEP_VALID(ux_approve_pk_flow_2_step,
-              pb,
-              io_seproxyhal_touch_pk_ok(),
-              {&C_icon_validate_14, "Approve"});
+UX_STEP_VALID(ux_approve_pk_flow_2_step, pb, send_pubkey(), {&C_icon_validate_14, "Approve"});
 
 UX_STEP_VALID(ux_approve_pk_flow_3_step, pb, io_reject(), {&C_icon_crossmark, "Reject"});
 
@@ -78,7 +75,7 @@ static void cancel_status(void) {
 
 static void confirm_address_rejection(void) {
     // display a status page and go back to main
-    io_exchange_with_code(SW_USER_REJECTED, 0);
+    io_send_sw(SW_USER_REJECTED);
     cancel_status();
 }
 
@@ -95,12 +92,12 @@ static void review_choice(bool confirm) {
 }
 
 static void continue_review(void) {
-    io_seproxyhal_touch_pk_ok();
+    send_pubkey();
     nbgl_useCaseAddressConfirmation(ctx->fullStr, review_choice);
 }
 #endif
 
-static unsigned int io_seproxyhal_touch_pk_ok(void) {
+static unsigned int send_pubkey(void) {
     uint8_t publicKey[65] = {0};
 
     // The response APDU will contain multiple objects, which means we need to
@@ -137,15 +134,9 @@ static unsigned int io_seproxyhal_touch_pk_ok(void) {
     return 0;
 }
 
-void handleGetPublicKey(uint8_t p1,
-                        uint8_t p2,
-                        uint8_t* buffer,
-                        uint16_t len,
-                        /* out */ volatile unsigned int* flags,
-                        /* out */ volatile unsigned int* tx) {
+void handleGetPublicKey(uint8_t p1, uint8_t p2, uint8_t* buffer, uint16_t len) {
     UNUSED(p1);
     UNUSED(len);
-    UNUSED(tx);
 
     if ((p2 != P2_DISPLAY_ADDRESS) && (p2 != P2_DISPLAY_PUBKEY)) {
         // Although THROW is technically a general-purpose exception
@@ -185,6 +176,4 @@ void handleGetPublicKey(uint8_t p1,
                             continue_review,
                             confirm_address_rejection);
 #endif
-
-    *flags |= IO_ASYNCH_REPLY;
 }

src/getVersion.c

@@ -1,3 +1,4 @@
+#include <io.h>
 #include <os.h>
 #include <os_io_seproxyhal.h>
 #include <stdbool.h>
@@ -6,17 +7,20 @@
 #include "blake2b.h"
 #include "sia.h"
 #include "sia_ux.h"
+#include <buffer.h>
 
 // handleGetVersion is the entry point for the getVersion command. It
 // unconditionally sends the app version.
 void handleGetVersion(uint8_t p1 __attribute__((unused)),
                       uint8_t p2 __attribute__((unused)),
                       uint8_t *dataBuffer __attribute__((unused)),
-                      uint16_t dataLength __attribute__((unused)),
-                      volatile unsigned int *flags __attribute__((unused)),
-                      volatile unsigned int *tx __attribute__((unused))) {
-    G_io_apdu_buffer[0] = APPVERSION[0] - '0';
-    G_io_apdu_buffer[1] = APPVERSION[2] - '0';
-    G_io_apdu_buffer[2] = APPVERSION[4] - '0';
-    io_exchange_with_code(SW_OK, 3);
+                      uint16_t dataLength __attribute__((unused))) {
+    static const uint8_t appVersion[3] = {APPVERSION[0] - '0',
+                                          APPVERSION[2] - '0',
+                                          APPVERSION[4] - '0'};
+
+    buffer_t buffer = {0};
+    buffer.ptr = appVersion;
+    buffer.size = sizeof(appVersion);
+    io_send_response_buffers(&buffer, 1, SW_OK);
 }

src/signHash.c

@@ -21,6 +21,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <string.h>
+#include <io.h>
 
 #include "blake2b.h"
 #include "sia.h"
@@ -57,10 +58,7 @@ UX_STEP_VALID(ux_approve_hash_flow_2_step,
               io_seproxyhal_touch_hash_ok(),
               {&C_icon_validate_14, "Approve"});
 
-UX_STEP_VALID(ux_approve_hash_flow_3_step,
-              pb,
-              io_reject(),
-              {&C_icon_crossmark, "Reject"});
+UX_STEP_VALID(ux_approve_hash_flow_3_step, pb, io_reject(), {&C_icon_crossmark, "Reject"});
 
 // Flow for the signing hash menu:
 // #1 screen: the hash repeated for confirmation
@@ -78,17 +76,15 @@ static void io_seproxyhal_touch_hash_ok_void(void) {
 
 static void sign_rejection(void) {
     // display a status page and go back to main
-    io_exchange_with_code(SW_USER_REJECTED, 0);
+    io_send_sw(SW_USER_REJECTED);
     nbgl_useCaseStatus("Signing Cancelled", false, ui_idle);
 }
 #endif
 
 void handleSignHash(uint8_t p1 __attribute__((unused)),
                     uint8_t p2 __attribute__((unused)),
                     uint8_t *buffer,
-                    uint16_t len,
-                    /* out */ volatile unsigned int *flags,
-                    /* out */ volatile unsigned int *tx __attribute__((unused))) {
+                    uint16_t len) {
     if (len != sizeof(uint32_t) + SIA_HASH_SIZE) {
         THROW(SW_INVALID_PARAM);
     }
@@ -115,8 +111,6 @@ void handleSignHash(uint8_t p1 __attribute__((unused)),
                             io_seproxyhal_touch_hash_ok_void,
                             sign_rejection);
 #endif
-
-    *flags |= IO_ASYNCH_REPLY;
 }
 
 // Now that we've seen the individual pieces, we can construct a full picture

src/txn.h

@@ -13,9 +13,9 @@
 
 // macros for converting raw bytes to uint64_t
 #define U8BE(buf, off) \
-    (((uint64_t)(U4BE(buf, off)) << 32) | ((uint64_t)(U4BE(buf, off + 4)) & 0xFFFFFFFF))
+    (((uint64_t) (U4BE(buf, off)) << 32) | ((uint64_t) (U4BE(buf, off + 4)) & 0xFFFFFFFF))
 #define U8LE(buf, off) \
-    (((uint64_t)(U4LE(buf, off + 4)) << 32) | ((uint64_t)(U4LE(buf, off)) & 0xFFFFFFFF))
+    (((uint64_t) (U4LE(buf, off + 4)) << 32) | ((uint64_t) (U4LE(buf, off)) & 0xFFFFFFFF))
 
 // txnDecoderState_e indicates a transaction decoder status
 typedef enum {