Merge pull request #17 from oxen-io/oxen-tests

Oxen test fixes and improvements

.github/workflows/ci-workflow.yml

@@ -90,3 +90,40 @@ jobs:
         with:
           name: scan-build
           path: scan-build
+
+  job_test:
+    name: Test
+    needs: job_nanoS_build_debug
+    runs-on: ubuntu-latest
+
+    container:
+      image: ghcr.io/ledgerhq/speculos:latest
+      ports:
+        - 1234:1234
+        - 9999:9999
+        - 40000:40000
+        - 41000:41000
+        - 42000:42000
+        - 43000:43000
+      options: --entrypoint /bin/bash
+
+    steps:
+      - name: Clone
+        uses: actions/checkout@v2
+
+      - name: Download app binary
+        uses: actions/download-artifact@v2
+        with:
+          name: oxen-app-nanoS-debug
+          path: bin
+
+      - name: Run test
+        run: |
+          nohup bash -c "python /speculos/speculos.py bin/app.elf --seed \"abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about\" --apdu-port 9999 --api-port 5000 --display headless" > speculos.log 2<&1 &
+          pip install pytest
+          pytest
+      - name: Upload Speculos log
+        uses: actions/upload-artifact@v2
+        with:
+          name: speculos-log
+          path: speculos.log

Makefile

@@ -15,12 +15,7 @@ endif
 
 #Oxen /44'/240'
 APP_LOAD_PARAMS  =  --path "44'/240'" --curve secp256k1
-# OR MAYBE ? APP_LOAD_PARAMS=  --path "44'" --curve secp256k1 # based on boilerplate
-ifeq ($(TARGET_NAME), TARGET_NANOX)
-APP_LOAD_PARAMS+=--appFlags 0x200  # APPLICATION_FLAG_BOLOS_SETTINGS
-else
-APP_LOAD_PARAMS+=--appFlags 0x40
-endif
+APP_LOAD_PARAMS += --appFlags 0x240  # APPLICATION_FLAG_BOLOS_SETTINGS & global pin
 APP_LOAD_PARAMS += $(COMMON_LOAD_PARAMS)
 
 APPNAME = "Oxen"
@@ -163,13 +158,3 @@ dep/%.d: %.c Makefile
 
 listvariants:
 	@echo VARIANTS COIN oxen
-
-
-# We probably don't need this as the nanox default is 1500 and nanos default is 1024
-# ifeq ($(TARGET_NAME),TARGET_NANOS)
-# Rewrite the bolos sdk script to increase the stack size slightly
-
-# script.ld: $(BOLOS_SDK)/script.ld Makefile
-# 	sed -e 's/^STACK_SIZE\s*=\s*[0-9]\+;/STACK_SIZE = 712;/' $< >$@
-# endif
-

README.md

@@ -116,13 +116,20 @@ sudo docker build -t ledger-app-builder:latest .
 
 Compile the app in the container with
 ```
-sudo docker run --rm -ti -v "$(realpath .):/app" ledger-app-builder:latest
+sudo docker run --rm -ti -v "$(realpath .):/app" --privileged ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder:latest
 
-make CLANGPATH=/usr/lib/llvm-12/bin/ DEBUG=1 clean nanos
-make CLANGPATH=/usr/lib/llvm-12/bin/ DEBUG=1 clean nanox
+BOLOS_SDK=$NANOS_SDK make
+
+```
+
+Building for tests also requires DEBUG
+```
+BOLOS_SDK=$NANOS_SDK make DEBUG=1
 ```
 
-Then the binary will be available in the host system under `nanos/bin` or `nanox/bin`
+Specify the SDK desired to change what device its built for
+
+Then the binary will be available in the host system under `bin`
 
 ## Loading the app onto your Ledger Nano S
 
@@ -149,6 +156,23 @@ To remove the app from your Ledger:
 
     make delete_nanos
 
+## Tests
+
+Tests are run using pytest. Install with `pip install -U pytest`
+
+Tests require speculos running the app and it receives messages from the test suite. Assumes speculos is saved at `~/speculos`
+
+```
+nohup bash -c "python3 ~/speculos/speculos.py bin/app.elf --seed \"abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about\" --apdu-port 9999 --api-port 5000 --display headless" > speculos.log 2<&1 &
+pytest
+```
+
+to run speculos in a view you can see and in a separate terminal instead run
+
+```
+python3 ~/speculos/speculos.py bin/app.elf --seed "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" --apdu-port 9999 --api-port 5000
+```
+
 ## Useful links
 
 * Oxen client CLI - https://github.com/oxen-io/oxen-core/releases

src/oxen_api.h

@@ -36,6 +36,7 @@ int monero_dispatch(void);
 void clear_protocol(void);
 
 int monero_apdu_get_network(void);
+int monero_apdu_reset_network(void);  // Only allowed in a debug build
 int monero_apdu_put_key(void);
 int monero_apdu_get_key(void);
 int monero_apdu_display_address(void);

src/oxen_dispatch.c

@@ -57,8 +57,7 @@ int check_protocol(void) {
                 break;
             }
             // unknown protocol or hot protocol switch is not allowed
-            // FALL THROUGH
-
+            __attribute__((fallthrough));
         default:
             return SW_PROTOCOL_NOT_SUPPORTED;
     }
@@ -74,6 +73,7 @@ int check_ins_access(void) {
         case INS_LOCK_DISPLAY:
         case INS_RESET:
         case INS_GET_NETWORK:
+        case INS_RESET_NETWORK:
         case INS_PUT_KEY:
         case INS_GET_KEY:
         case INS_DISPLAY_ADDRESS:
@@ -86,7 +86,7 @@ int check_ins_access(void) {
         case INS_GEN_KEY_IMAGE:
         case INS_GEN_KEY_IMAGE_SIGNATURE:
         case INS_GEN_UNLOCK_SIGNATURE:
-        case INS_GEN_LNS_SIGNATURE:
+        case INS_GEN_ONS_SIGNATURE:
         case INS_SECRET_KEY_TO_PUBLIC_KEY:
         case INS_SECRET_KEY_ADD:
         case INS_GENERATE_KEYPAIR:
@@ -150,6 +150,9 @@ int monero_dispatch(void) {
         case INS_GET_NETWORK:
             sw = monero_apdu_get_network();
             break;
+        case INS_RESET_NETWORK:
+            sw = monero_apdu_reset_network();
+            break;
         /* --- KEYS --- */
         case INS_PUT_KEY:
             sw = monero_apdu_put_key();
@@ -241,7 +244,7 @@ int monero_dispatch(void) {
         case INS_OPEN_TX:
             // state machine check
             if (!(G_oxen_state.tx_state_ins == 0 ||
-                  G_oxen_state.tx_state_ins == INS_GEN_LNS_SIGNATURE)) {
+                  G_oxen_state.tx_state_ins == INS_GEN_ONS_SIGNATURE)) {
                 THROW(SW_COMMAND_NOT_ALLOWED);
             }
             // 2. command process
@@ -454,20 +457,20 @@ int monero_dispatch(void) {
             update_protocol();
             break;
 
-        /* --- LNS --- */
-        case INS_GEN_LNS_SIGNATURE:
+        /* --- ONS --- */
+        case INS_GEN_ONS_SIGNATURE:
             if (G_oxen_state.tx_in_progress) THROW(SW_COMMAND_NOT_ALLOWED);
             // Initialization: must not be in the middle of something else
             if (OXEN_IO_P_EQUALS(0, 0) && G_oxen_state.tx_state_ins == 0)
                 sw = oxen_apdu_generate_lns_hash();
             // [0,0]->[1,x] or [1,x]->[1,y] -- we receive data to hash in multiple parts
-            else if (G_oxen_state.tx_state_ins == INS_GEN_LNS_SIGNATURE &&
+            else if (G_oxen_state.tx_state_ins == INS_GEN_ONS_SIGNATURE &&
                      (OXEN_TX_STATE_P_EQUALS(0, 0) || G_oxen_state.tx_state_p1 == 1) &&
                      G_oxen_state.io_p1 == 1)
                 sw = oxen_apdu_generate_lns_hash();
             // [1,0] -> [2,0] gives the account indices and uses the hash built in the [1,x] steps
             // to make the signature
-            else if (OXEN_TX_STATE_INS_P_EQUALS(INS_GEN_LNS_SIGNATURE, 1, 0) &&
+            else if (OXEN_TX_STATE_INS_P_EQUALS(INS_GEN_ONS_SIGNATURE, 1, 0) &&
                      OXEN_IO_P_EQUALS(2, 0))
                 sw = oxen_apdu_generate_lns_signature();
             else

src/oxen_init.c

@@ -149,23 +149,23 @@ void oxen_install(unsigned char netId) {
     unsigned char c;
 
     // full reset data
-    nvm_write(N_oxen_state, NULL, sizeof(oxen_nv_state_t));
+    nvm_write((void*) N_oxen_state, NULL, sizeof(oxen_nv_state_t));
 
     // set mode key
     c = KEY_MODE_SEED;
-    nvm_write(&N_oxen_state->key_mode, &c, 1);
+    nvm_write((void*) &N_oxen_state->key_mode, &c, 1);
 
     // set net id
-    nvm_write(&N_oxen_state->network_id, &netId, 1);
+    nvm_write((void*) &N_oxen_state->network_id, &netId, 1);
 
     // write magic
-    nvm_write(N_oxen_state->magic, (void*) C_MAGIC, sizeof(C_MAGIC));
+    nvm_write((void*) N_oxen_state->magic, (void*) C_MAGIC, sizeof(C_MAGIC));
 
-#if DEBUG_HWDEVICE
+#ifdef DEBUG_HWDEVICE
     // Default into always-export-view-key mode when doing a debug build because it's annoying to
     // have to confirm the view key export every time when doing dev/debugging work.
     unsigned char always_export = VIEWKEY_EXPORT_ALWAYS_ALLOW;
-    nvm_write(&N_oxen_state->viewkey_export_mode, &always_export, 1);
+    nvm_write((void*) &N_oxen_state->viewkey_export_mode, &always_export, 1);
 #endif
 }
 
@@ -176,6 +176,7 @@ const char* const oxen_supported_client[] = {
     "8.",
     "9.",
     "10.",
+    "11.",
 };
 #define OXEN_SUPPORTED_CLIENT_SIZE (sizeof(oxen_supported_client) / sizeof(char*))
 

src/oxen_key.c

@@ -96,9 +96,18 @@ int monero_apdu_put_key(void) {
     unsigned char raw[32];
     unsigned char pub[32];
     unsigned char sec[32];
+    int address_size = 95;
+    switch (N_oxen_state->network_id) {
+        case TESTNET:
+        case DEVNET:
+            address_size = 97;
+            break;
+        default:
+            break;
+    }
 
     // option + priv/pub view key + priv/pub spend key + base58 address
-    if (G_oxen_state.io_length != (1 + 32 * 2 + 32 * 2 + 95)) {
+    if (G_oxen_state.io_length != (1 + 32 * 2 + 32 * 2 + address_size)) {
         THROW(SW_WRONG_LENGTH);
         return SW_WRONG_LENGTH;
     }
@@ -157,6 +166,27 @@ int monero_apdu_get_network(void) {
     return SW_OK;
 }
 
+int monero_apdu_reset_network(void) {
+#ifdef DEBUG_HWDEVICE
+    monero_io_discard(1);
+    // On (and only on) a debug build we allow this command to reset the wallet to a requested
+    // network (without any confirmation).  This is used for testing purposes (and is the only way
+    // to put the ledger into FAKECHAIN mode).
+    uint8_t nettype = G_oxen_state.io_p1;
+    if (
+#ifndef OXEN_ALPHA
+        nettype == MAINNET ||
+#endif
+        nettype == TESTNET || nettype == DEVNET || nettype == FAKECHAIN) {
+        oxen_install(nettype);
+        monero_init();
+        return SW_OK;
+    }
+#endif
+    // On a non-debug build this command always fails.
+    THROW(SW_COMMAND_NOT_ALLOWED);
+}
+
 /* ----------------------------------------------------------------------- */
 /* ---                                                                 --- */
 /* ----------------------------------------------------------------------- */
@@ -188,7 +218,7 @@ int monero_apdu_get_key(void) {
             }
             break;
 
-#if DEBUG_HWDEVICE
+#ifdef DEBUG_HWDEVICE
         // get info
         case 3: {
             unsigned int path[5];
@@ -549,10 +579,10 @@ int oxen_apdu_generate_unlock_signature(void) {
     return SW_OK;
 }
 
-// Generates an LNS hash
+// Generates an ONS hash
 int oxen_apdu_generate_lns_hash(void) {
     if (G_oxen_state.io_p1 == 0) {
-        // Confirm the LNS initialization with the user
+        // Confirm the ONS initialization with the user
         monero_io_discard(1);
         ui_menu_lns_validation_display();
         return 0;

src/oxen_monero.c

@@ -109,31 +109,10 @@ const unsigned char encoded_block_sizes[] = {0, 2, 3, 5, 6, 7, 9, 10, 11};
 
 static uint64_t uint_8be_to_64(const unsigned char* data, size_t size) {
     uint64_t res = 0;
-    switch (9 - size) {
-        case 1:
-            res |= *data++;
-        case 2:
-            res <<= 8;
-            res |= *data++;
-        case 3:
-            res <<= 8;
-            res |= *data++;
-        case 4:
-            res <<= 8;
-            res |= *data++;
-        case 5:
-            res <<= 8;
-            res |= *data++;
-        case 6:
-            res <<= 8;
-            res |= *data++;
-        case 7:
-            res <<= 8;
-            res |= *data++;
-        case 8:
-            res <<= 8;
-            res |= *data;
-            break;
+    for (size_t i = 0; i < 8; i++) {
+        if (i >= size) break;
+        res <<= 8;
+        res |= data[i];
     }
 
     return res;
@@ -175,6 +154,7 @@ unsigned char oxen_wallet_address(char* str_b58,
             break;
 #ifndef OXEN_ALPHA
         case MAINNET:
+        case FAKECHAIN:
             if (paymentID)
                 prefix = MAINNET_CRYPTONOTE_PUBLIC_INTEGRATED_ADDRESS_BASE58_PREFIX;
             else if (is_subbadress)

src/oxen_open_tx.c

@@ -52,7 +52,7 @@ int monero_apdu_open_tx(void) {
 
     if (txversion != 4) THROW(SW_WRONG_DATA_RANGE);
     if (!(txtype == TXTYPE_STANDARD || txtype == TXTYPE_UNLOCK || txtype == TXTYPE_STAKE ||
-          txtype == TXTYPE_LNS))
+          txtype == TXTYPE_ONS))
         THROW(SW_WRONG_DATA_RANGE);
 
     monero_io_discard(1);

src/oxen_prehash.c

@@ -39,9 +39,9 @@ int monero_apdu_clsag_prehash_init(void) {
             cx_keccak_init(&G_oxen_state.keccak_alt, 256);
         }
     }
-    // We always confirm fees for LNS because often this is the *only* confirmation for an LNS tx
+    // We always confirm fees for ONS because often this is the *only* confirmation for an ONS tx
     unsigned char confirm_fee_mode =
-        G_oxen_state.tx_type == TXTYPE_LNS ? CONFIRM_FEE_ALWAYS : N_oxen_state->confirm_fee_mode;
+        G_oxen_state.tx_type == TXTYPE_ONS ? CONFIRM_FEE_ALWAYS : N_oxen_state->confirm_fee_mode;
 
     oxen_hash_update(&G_oxen_state.keccak_alt,
                      G_oxen_state.io_buffer + G_oxen_state.io_offset,
@@ -71,7 +71,7 @@ int monero_apdu_clsag_prehash_init(void) {
         if (amount > 0) {
             // ask user
             oxen_currency_str(amount, G_oxen_state.ux_amount);
-            if (G_oxen_state.tx_type == TXTYPE_LNS)
+            if (G_oxen_state.tx_type == TXTYPE_ONS)
                 ui_menu_lns_fee_validation_display();
             else
                 ui_menu_fee_validation_display();
@@ -123,7 +123,7 @@ int monero_apdu_clsag_prehash_update(void) {
 
     if (G_oxen_state.tx_sig_mode == TRANSACTION_CREATE_REAL) {
         if (!is_change && G_oxen_state.tx_type != TXTYPE_STAKE &&
-            G_oxen_state.tx_type != TXTYPE_LNS) {
+            G_oxen_state.tx_type != TXTYPE_ONS) {
             // encode dest adress
             unsigned char pos =
                 oxen_wallet_address(G_oxen_state.ux_address, Aout, Bout, is_subaddress, NULL);
@@ -180,8 +180,8 @@ int monero_apdu_clsag_prehash_update(void) {
         if (amount) {
             oxen_currency_str(amount, G_oxen_state.ux_amount);
             if (!is_change) {
-                if (G_oxen_state.tx_type == TXTYPE_STAKE || G_oxen_state.tx_type == TXTYPE_LNS) {
-                    // If this is a stake or LNS tx then the non-change recipient must be ourself.
+                if (G_oxen_state.tx_type == TXTYPE_STAKE || G_oxen_state.tx_type == TXTYPE_ONS) {
+                    // If this is a stake or ONS tx then the non-change recipient must be ourself.
                     if (memcmp(Aout, G_oxen_state.view_pub, 32) ||
                         memcmp(Bout, G_oxen_state.spend_pub, 32))
                         monero_lock_and_throw(SW_SECURITY_INTERNAL);