Merge pull request #119 from LedgerHQ/upgrade_swap

Upgrade swap code to better standards

Makefile

@@ -310,10 +310,10 @@ SDK_SOURCE_PATH  += lib_blewbxx lib_blewbxx_impl
 endif
 
 load: all
-	python -m ledgerblue.loadApp $(APP_LOAD_PARAMS)
+	python3 -m ledgerblue.loadApp $(APP_LOAD_PARAMS)
 
 delete:
-	python -m ledgerblue.deleteApp $(COMMON_DELETE_PARAMS)
+	python3 -m ledgerblue.deleteApp $(COMMON_DELETE_PARAMS)
 
 # import generic rules from the sdk
 include $(BOLOS_SDK)/Makefile.rules

src/handle_check_address.c

@@ -4,16 +4,20 @@
 #include "ethUtils.h"
 #include "string.h"
 
-#define ZERO(x) memset(x, 0, sizeof(x))
+#define ZERO(x) memset(&x, 0, sizeof(x))
 
-void handle_check_address(check_address_parameters_t* params, chain_config_t* chain_config) {
+static int os_strcmp(const char* s1, const char* s2) {
+    size_t size = strlen(s1) + 1;
+    return memcmp(s1, s2, size);
+}
+
+int handle_check_address(check_address_parameters_t* params, chain_config_t* chain_config) {
     PRINTF("Params on the address %d\n", (unsigned int) params);
     PRINTF("Address to check %s\n", params->address_to_check);
     PRINTF("Inside handle_check_address\n");
-    params->result = 0;
     if (params->address_to_check == 0) {
         PRINTF("Address to check == 0\n");
-        return;
+        return 0;
     }
 
     uint8_t i;
@@ -35,7 +39,7 @@ void handle_check_address(check_address_parameters_t* params, chain_config_t* ch
     if ((bip32PathLength < 0x01) || (bip32PathLength > MAX_BIP32_PATH) ||
         (bip32PathLength * 4 != params->address_parameters_length - 1)) {
         PRINTF("Invalid path\n");
-        return;
+        return 0;
     }
     for (i = 0; i < bip32PathLength; i++) {
         locals_union1.bip32Path[i] = U4BE(bip32_path_ptr, 0);
@@ -46,32 +50,29 @@ void handle_check_address(check_address_parameters_t* params, chain_config_t* ch
                                bip32PathLength,
                                locals_union2.privateKeyData,
                                NULL);
-    ZERO(&locals_union1);
+    ZERO(locals_union1);
     cx_ecfp_init_private_key(CX_CURVE_256K1,
                              locals_union2.privateKeyData,
                              32,
                              &locals_union1.privateKey);
-    ZERO(&locals_union2);
+    ZERO(locals_union2);
     cx_ecfp_generate_pair(CX_CURVE_256K1, &locals_union2.publicKey, &locals_union1.privateKey, 1);
-    ZERO(&locals_union1);
+    ZERO(locals_union1);
     getEthAddressStringFromKey(&locals_union2.publicKey,
                                (uint8_t*) locals_union1.address,
                                &local_sha3,
                                chain_config);
-    ZERO(&locals_union2);
+    ZERO(locals_union2);
 
     uint8_t offset_0x = 0;
     if (memcmp(params->address_to_check, "0x", 2) == 0) {
         offset_0x = 2;
     }
 
-    if ((strlen(locals_union1.address) != strlen(params->address_to_check + offset_0x)) ||
+    if (os_strcmp(locals_union1.address, params->address_to_check + offset_0x) != 0) {
-        memcmp(locals_union1.address,
+        PRINTF("Addresses don't match\n");
-               params->address_to_check + offset_0x,
+        return 0;
-               strlen(locals_union1.address)) != 0) {
-        PRINTF("Addresses doesn't match\n");
-        return;
     }
     PRINTF("Addresses  match\n");
-    params->result = 1;
+    return 1;
 }

src/handle_check_address.h

@@ -4,7 +4,7 @@
 #include "swap_lib_calls.h"
 #include "chainConfig.h"
 
-void handle_check_address(check_address_parameters_t* check_address_params,
+int handle_check_address(check_address_parameters_t* check_address_params,
-                          chain_config_t* chain_config);
+                         chain_config_t* chain_config);
 
 #endif  // _HANDLE_CHECK_ADDRESS_H_

src/handle_get_printable_amount.c

@@ -6,21 +6,20 @@
 #include "string.h"
 #include <stdint.h>
 
-void handle_get_printable_amount(get_printable_amount_parameters_t* params,
+int handle_get_printable_amount(get_printable_amount_parameters_t* params, chain_config_t* config) {
-                                 chain_config_t* config) {
     uint8_t decimals;
     char ticker[MAX_TICKER_LEN];
     memset(params->printable_amount, 0, sizeof(params->printable_amount));
     if (params->amount_length > 32) {
         PRINTF("Amount is too big, 32 bytes max but buffer has %u bytes", params->amount_length);
-        os_lib_end();
+        return 0;
     }
     if (!parse_swap_config(params->coin_configuration,
                            params->coin_configuration_length,
                            ticker,
                            &decimals)) {
         PRINTF("Error while parsing config\n");
-        os_lib_end();
+        return 0;
     }
 
     // If the amount is a fee, its value is nominated in ETH even if we're doing an ERC20 swap
@@ -38,4 +37,5 @@ void handle_get_printable_amount(get_printable_amount_parameters_t* params,
                    ticker,
                    params->printable_amount,
                    sizeof(params->printable_amount));
+    return 1;
 }

src/handle_get_printable_amount.h

@@ -4,7 +4,7 @@
 #include "swap_lib_calls.h"
 #include "chainConfig.h"
 
-void handle_get_printable_amount(get_printable_amount_parameters_t* get_printable_amount_params,
+int handle_get_printable_amount(get_printable_amount_parameters_t* get_printable_amount_params,
-                                 chain_config_t* config);
+                                chain_config_t* config);
 
 #endif  // _HANDLE_GET_PRINTABLE_AMOUNT_H_

src/handle_swap_sign_transaction.c

@@ -4,7 +4,7 @@
 #include "shared_context.h"
 #include "utils.h"
 
-void copy_transaction_parameters(create_transaction_parameters_t* sign_transaction_params,
+bool copy_transaction_parameters(create_transaction_parameters_t* sign_transaction_params,
                                  chain_config_t* config) {
     // first copy parameters to stack, and then to global data.
     // We need this "trick" as the input data position can overlap with app-ethereum globals
@@ -16,7 +16,7 @@ void copy_transaction_parameters(create_transaction_parameters_t* sign_transacti
     if ((stack_data.fullAddress[sizeof(stack_data.fullAddress) - 1] != '\0') ||
         (sign_transaction_params->amount_length > 32) ||
         (sign_transaction_params->fee_amount_length > 8)) {
-        os_lib_end();
+        return false;
     }
 
     uint8_t decimals;
@@ -26,7 +26,7 @@ void copy_transaction_parameters(create_transaction_parameters_t* sign_transacti
                            ticker,
                            &decimals)) {
         PRINTF("Error while parsing config\n");
-        os_lib_end();
+        return false;
     }
     amountToString(sign_transaction_params->amount,
                    sign_transaction_params->amount_length,
@@ -46,11 +46,10 @@ void copy_transaction_parameters(create_transaction_parameters_t* sign_transacti
                    sizeof(stack_data.maxFee));
 
     memcpy(&strings.common, &stack_data, sizeof(stack_data));
+    return true;
 }
 
-void handle_swap_sign_transaction(create_transaction_parameters_t* sign_transaction_params,
+void handle_swap_sign_transaction(chain_config_t* config) {
-                                  chain_config_t* config) {
-    copy_transaction_parameters(sign_transaction_params, config);
     chainConfig = config;
     reset_app_context();
     called_from_swap = true;

src/handle_swap_sign_transaction.h

@@ -4,7 +4,9 @@
 #include "swap_lib_calls.h"
 #include "chainConfig.h"
 
-void handle_swap_sign_transaction(create_transaction_parameters_t* get_printable_amount_params,
+bool copy_transaction_parameters(create_transaction_parameters_t* sign_transaction_params,
-                                  chain_config_t* config);
+                                 chain_config_t* config);
+
+void handle_swap_sign_transaction(chain_config_t* config);
 
 #endif  // _HANDLE_SWAP_SIGN_TRANSACTION_H_

src/main.c

@@ -680,8 +680,21 @@ void app_exit() {
     END_TRY_L(exit);
 }
 
-void coin_main_with_config(chain_config_t *config) {
+void init_coin_config(chain_config_t *coin_config) {
-    chainConfig = config;
+    memset(coin_config, 0, sizeof(chain_config_t));
+    strcpy(coin_config->coinName, CHAINID_COINNAME " ");
+    coin_config->chainId = CHAIN_ID;
+    coin_config->kind = CHAIN_KIND;
+}
+
+void coin_main(chain_config_t *coin_config) {
+    chain_config_t config;
+    if (coin_config == NULL) {
+        init_coin_config(&config);
+        chainConfig = &config;
+    } else {
+        chainConfig = coin_config;
+    }
     reset_app_context();
     tmpCtx.transactionContext.currentTokenIndex = 0;
 
@@ -734,53 +747,67 @@ void coin_main_with_config(chain_config_t *config) {
     app_exit();
 }
 
-void init_coin_config(chain_config_t *coin_config) {
+struct libargs_s {
-    memset(coin_config, 0, sizeof(chain_config_t));
+    unsigned int id;
-    strcpy(coin_config->coinName, CHAINID_COINNAME " ");
+    unsigned int command;
-    coin_config->chainId = CHAIN_ID;
+    chain_config_t *chain_config;
-    coin_config->kind = CHAIN_KIND;
+    union {
-}
+        check_address_parameters_t *check_address;
+        create_transaction_parameters_t *create_transaction;
+        get_printable_amount_parameters_t *get_printable_amount;
+    };
+};
 
-void coin_main() {
+static void library_main_helper(struct libargs_s *args) {
-    chain_config_t coin_config;
+    check_api_level(CX_COMPAT_APILEVEL);
-    init_coin_config(&coin_config);
+    PRINTF("Inside a library \n");
-    coin_main_with_config(&coin_config);
+    switch (args->command) {
-}
+        case CHECK_ADDRESS:
-
+            // ensure result is zero if an exception is thrown
-void library_main_with_config(chain_config_t *config,
+            args->check_address->result = 0;
-                              unsigned int command,
+            args->check_address->result =
-                              unsigned int *call_parameters) {
+                handle_check_address(args->check_address, args->chain_config);
-    BEGIN_TRY {
+            break;
-        TRY {
+        case SIGN_TRANSACTION:
-            check_api_level(CX_COMPAT_APILEVEL);
+            if (copy_transaction_parameters(args->create_transaction, args->chain_config)) {
-            PRINTF("Inside a library \n");
+                // never returns
-            switch (command) {
+                handle_swap_sign_transaction(args->chain_config);
-                case CHECK_ADDRESS:
-                    handle_check_address((check_address_parameters_t *) call_parameters, config);
-                    break;
-                case SIGN_TRANSACTION:
-                    handle_swap_sign_transaction(
-                        (create_transaction_parameters_t *) call_parameters,
-                        config);
-                    break;
-                case GET_PRINTABLE_AMOUNT:
-                    handle_get_printable_amount(
-                        (get_printable_amount_parameters_t *) call_parameters,
-                        config);
-                    break;
             }
-            os_lib_end();
+            break;
-        }
+        case GET_PRINTABLE_AMOUNT:
-        FINALLY {
+            // ensure result is zero if an exception is thrown
-        }
+            args->get_printable_amount->result = 0;
+            args->get_printable_amount->result =
+                handle_get_printable_amount(args->get_printable_amount, args->chain_config);
+            break;
+        default:
+            break;
     }
-    END_TRY;
 }
 
-void library_main(unsigned int call_id, unsigned int *call_parameters) {
+void library_main(struct libargs_s *args) {
     chain_config_t coin_config;
-    init_coin_config(&coin_config);
+    if (args->chain_config == NULL) {
-    library_main_with_config(&coin_config, call_id, call_parameters);
+        init_coin_config(&coin_config);
+        args->chain_config = &coin_config;
+    }
+    bool end = false;
+    /* This loop ensures that library_main_helper and os_lib_end are called
+     * within a try context, even if an exception is thrown */
+    while (1) {
+        BEGIN_TRY {
+            TRY {
+                if (!end) {
+                    library_main_helper(args);
+                }
+                os_lib_end();
+            }
+            FINALLY {
+                end = true;
+            }
+        }
+        END_TRY;
+    }
 }
 
 __attribute__((section(".boot"))) int main(int arg0) {
@@ -825,34 +852,26 @@ __attribute__((section(".boot"))) int main(int arg0) {
 
     if (!arg0) {
         // called from dashboard as standalone eth app
-        coin_main();
+        coin_main(NULL);
         return 0;
     }
 
-    if (((unsigned int *) arg0)[0] != 0x100) {
+    struct libargs_s *args = (struct libargs_s *) arg0;
+    if (args->id != 0x100) {
         app_exit();
         return 0;
     }
-    unsigned int command = ((unsigned int *) arg0)[1];
+    switch (args->command) {
-    chain_config_t *chain_config = ((unsigned int *) arg0)[2];
-    switch (command) {
         case RUN_APPLICATION:
             // coin application launched from dashboard
-            if (chain_config == NULL)
+            if (args->chain_config == NULL)
                 app_exit();
             else
-                coin_main_with_config(chain_config);
+                coin_main(args->chain_config);
             break;
         default:
-            if (chain_config == NULL)
+            // called as bitcoin or altcoin library
-                // Called as standalone eth library
+            library_main(args);
-                library_main(command, ((unsigned int *) arg0)[3]);  // called as bitcoin library
-            else
-                // Called as a library from an altcoin
-                library_main_with_config(chain_config,
-                                         command,
-                                         ((unsigned int *) arg0)[3]);  // called as coin library
-            break;
     }
 #endif
     return 0;

src/swap_lib_calls.h

@@ -36,6 +36,7 @@ typedef struct get_printable_amount_parameters_s {
     bool is_fee;
     // OUT
     char printable_amount[30];
+    int result;
 } get_printable_amount_parameters_t;
 
 typedef struct create_transaction_parameters_s {