Merge pull request #62 from Zondax/fix/indefinite-len

support indefinite maps and update test vectors format
Zondax · May 11, 2021 · 0a02ac8 · 0a02ac8
2 parents 13896b0 + afb81a7
commit 0a02ac8
Showing 7 changed files with 87 additions and 40 deletions.
diff --git a/app/src/parser.c b/app/src/parser.c
@@ -118,12 +118,10 @@ __Z_INLINE parser_error_t print_textual(sender_t *sender,
 
     pageString(outVal, outValLen, buffer, pageIdx, pageCount);
 
-#if defined(TARGET_NANOS) || defined(TARGET_NANOX)
     // Remove trailing dashes
     if (outVal[17] == '-') outVal[17] = ' ';
     if (outVal[35] == '-') outVal[35] = ' ';
     if (outVal[53] == '-') outVal[53] = ' ';
-#endif
 
     return parser_ok;
 }
@@ -134,7 +132,7 @@ __Z_INLINE zxerr_t print_hexstring(char *out, uint16_t outLen, uint8_t *data, ui
     if (writtenBytes != dataLen*2) {
         return zxerr_out_of_bounds;
     }
-#if defined(TARGET_NANOS) || defined(TARGET_NANOX)
+
     // insert spaces to force alignment
     CHECK_ZXERR(inplace_insert_char(out, outLen, 8, ' '))
     CHECK_ZXERR(inplace_insert_char(out, outLen, 17, ' '))
@@ -143,7 +141,7 @@ __Z_INLINE zxerr_t print_hexstring(char *out, uint16_t outLen, uint8_t *data, ui
     CHECK_ZXERR(inplace_insert_char(out, outLen, 44, ' '))
     CHECK_ZXERR(inplace_insert_char(out, outLen, 53, ' '))
     CHECK_ZXERR(inplace_insert_char(out, outLen, 62, ' '))
-#endif
+
     return zxerr_ok;
 }
 

diff --git a/app/src/parser_impl.c b/app/src/parser_impl.c
@@ -149,10 +149,10 @@ const char *parser_getErrorDescription(parser_error_t err) {
 //    sender_pubkey [blob]
 //    sender_sig [blob]
 
-#define READ_INT64(MAP, FIELDNAME, V_OUTPUT) {                                             \
-    CborValue it;                                                                          \
-    CHECK_CBOR_MAP_ERR(cbor_value_map_find_value(MAP, FIELDNAME, &it));                    \
-    (V_OUTPUT) = _cbor_value_decode_int64_internal(&it);                                     \
+#define READ_INT64(MAP, FIELDNAME, V_OUTPUT) {                     \
+    CborValue it;                                                                           \
+    CHECK_CBOR_MAP_ERR(cbor_value_map_find_value(MAP, FIELDNAME, &it));                     \
+    CHECK_CBOR_MAP_ERR(cbor_value_get_raw_integer(&it, &V_OUTPUT)); \
 }
 
 #define READ_STRING(MAP, FIELDNAME, V_OUTPUT) {                                             \
@@ -167,6 +167,27 @@ const char *parser_getErrorDescription(parser_error_t err) {
     CHECK_CBOR_MAP_ERR(_cbor_value_copy_string(&it, (V_OUTPUT).data, &(V_OUTPUT).len, NULL)); \
 }
 
+parser_error_t try_read_nonce(CborValue *content_map, parser_tx_t *v){
+    size_t stringLen = 0;
+    CborValue it;
+
+    size_t *dataLen = &v->tx_fields.call.nonce.len;
+
+    MEMZERO(&v->tx_fields.call.nonce.data, sizeof(v->tx_fields.call.nonce.data));
+    CHECK_CBOR_MAP_ERR(cbor_value_map_find_value(content_map, "nonce", &it))
+    if(!cbor_value_is_valid(&it)){
+        v->tx_fields.call.has_nonce = false;
+        return parser_ok;
+    }
+    PARSER_ASSERT_OR_ERROR(cbor_value_is_byte_string(&it) || cbor_value_is_text_string(&it), parser_context_mismatch);
+    CHECK_CBOR_MAP_ERR(cbor_value_get_string_length(&it, &stringLen));
+    PARSER_ASSERT_OR_ERROR(stringLen < sizeof(v->tx_fields.call.nonce.data), parser_context_unexpected_size)
+    *dataLen = stringLen;
+    CHECK_CBOR_MAP_ERR(_cbor_value_copy_string(&it, v->tx_fields.call.nonce.data, dataLen, NULL));
+    v->tx_fields.call.has_nonce = true;
+    return parser_ok;
+}
+
 parser_error_t parsePaths(CborValue *content_map, state_read_t *stateRead) {
     CborValue it;                                                                           \
     CHECK_CBOR_MAP_ERR(cbor_value_map_find_value(content_map, "paths", &it));
@@ -255,20 +276,15 @@ parser_error_t readContent(CborValue *content_map, parser_tx_t *v) {
     READ_STRING(content_map, "request_type", v->request_type)
     size_t mapsize = 0;
     if (strcmp(v->request_type.data, "call") == 0) {
-        CHECK_CBOR_MAP_ERR(cbor_value_get_map_length(content_map, &mapsize))
-        PARSER_ASSERT_OR_ERROR(mapsize == 7 || mapsize == 6, parser_context_unexpected_size)
+//        CHECK_CBOR_MAP_ERR(cbor_value_get_map_length(content_map, &mapsize))
+//        PARSER_ASSERT_OR_ERROR(mapsize == 7 || mapsize == 6, parser_context_unexpected_size)
         v->txtype = token_transfer;
         // READ CALL
         call_t *fields = &v->tx_fields.call;
         READ_STRING(content_map, "sender", fields->sender)
         READ_STRING(content_map, "canister_id", fields->canister_id)
 
-        if (mapsize == 7) {
-            READ_STRING(content_map, "nonce", fields->nonce)
-            fields->has_nonce = true;
-        } else {
-            fields->has_nonce = false;
-        }
+        CHECK_PARSER_ERR(try_read_nonce(content_map, v));
 
         READ_STRING(content_map, "method_name", fields->method_name)
         READ_INT64(content_map, "ingress_expiry", fields->ingress_expiry)
@@ -429,7 +445,7 @@ uint8_t _getNumItems(const parser_context_t *c, const parser_tx_t *v) {
                 return 6;
             }
             uint8_t nonce = v->tx_fields.call.has_nonce ? 1 : 0;
-            itemCount = 7 + nonce;          //cbor contents + token transfer protobuf data
+            itemCount = 7 + nonce;
             break;
         }
         case state_transaction_read : {

diff --git a/app/src/parser_txdef.h b/app/src/parser_txdef.h
@@ -34,7 +34,7 @@ extern "C" {
 #define REQUEST_MAX_LEN 10
 #define METHOD_MAX_LEN 20
 #define NONCE_MAX_LEN 32
-#define ARG_MAX_LEN 100
+#define ARG_MAX_LEN 200
 #define PATH_MAX_LEN 40
 #define PATH_MAX_ARRAY 2
 

diff --git a/tests/cbor_parser.cpp b/tests/cbor_parser.cpp
@@ -234,7 +234,6 @@ namespace {
 
     TEST(CBORParserTest, TransactionStateRead) {
         uint8_t inBuffer[1000];
-        // FIXME: remove first byte
         const char *tmp = "d9d9f7a167636f6e74656e74a46e696e67726573735f6578706972791b167886d92efc388065706174687381824e726571756573745f7374617475735820564fd7aba0d5facd386adad8c095339be3ad9222389decf64e0bddee3cc11e466c726571756573745f747970656a726561645f73746174656673656e646572581dbd28a51aa219af2443896127d178f9b2de34215c948f3e265a0e083d02";
         auto inBufferLen = parseHexString(inBuffer, sizeof(inBuffer), tmp);
 
@@ -248,7 +247,6 @@ namespace {
 
     TEST(CBORParserTest, TokenTransfer) {
         uint8_t inBuffer[1000];
-        // FIXME: remove first byte
         const char *tmp = "d9d9f7a367636f6e74656e74a76c726571756573745f747970656463616c6c656e6f6e636550f5390d960c6e52f489155a4309da03da6e696e67726573735f6578706972791b1674c5e29ec9c2106673656e646572581d7bdd7f75eea6fcf58001e0dfb7d718b9e8f2c3b01e1ccec9ab305aad026b63616e69737465725f69644a000000000000000201016b6d6574686f645f6e616d656473656e646361726758560a0012050a0308e8071a0308890122220a2001010101010101010101010101010101010101010101010101010101010101012a220a2035548ec29e9d85305850e87a2d2642fe7214ff4bb36334070deafc3345c3b1276d73656e6465725f7075626b657958583056301006072a8648ce3d020106052b8104000a03420004e1142e1fbc940344d9161709196bb8bd151f94379c48dd507ab99a0776109128b94b5303cf2b2d28e25a779da175b62f8a975599b20c63d5193202640576ec5e6a73656e6465725f7369675840de5bccbb0a0173c432cd58ea4495d4d1e122d6ce04e31dcf63217f3d3a9b73130dc9bbf3b10e61c8db8bf8800bb4649e27786e5bc9418838c95864be28487a6a";
         auto inBufferLen = parseHexString(inBuffer, sizeof(inBuffer), tmp);
 

diff --git a/tests/common.cpp b/tests/common.cpp
@@ -58,7 +58,28 @@ std::vector<std::string> dumpUI(parser_context_t *ctx,
                 ss << parser_getErrorDescription(err);
             }
 
-            answer.push_back(ss.str());
+            auto output = ss.str();
+            if (output.back() == ' ') {
+                output = output.substr(0, output.size() - 1);
+            }
+
+            ///////////////////////////
+            // Temporary Workaround for test vector issues
+            if (output.find("From account[") != std::string::npos) {
+                output.insert(42,": ");
+            }
+            if (output.find("To account [") != std::string::npos) {
+                output.insert(41,": ");
+            }
+            if (output.find("Sender [") != std::string::npos) {
+                output.insert(37,": ");
+            }
+            if (output.find("Subaccount [") != std::string::npos) {
+                output.insert(41,": ");
+            }
+            ///////////////////////////
+
+            answer.push_back(output);
 
             pageIdx++;
         }

diff --git a/tests/manual.json b/tests/manual.json
@@ -6,24 +6,24 @@
     "valid": true,
     "output": [
       "0 | Transaction type : Send ICP",
-      "1 | From account[1/2] : 6956653cd25f069f308d4d8a61491d99fa82e27",
-      "1 | From account[2/2] : 2110f77a36bd10e06a28c82b0",
-      "2 | To account [1/2] : 35548ec29e9d85305850e87a2d2642fe7214ff4",
-      "2 | To account [2/2] : bb36334070deafc3345c3b127",
+      "1 | From account[1/2] : 6956653c d25f069f : 308d4d8a 61491d99",
+      "1 | From account[2/2] : fa82e272 110f77a3 : 6bd10e06 a28c82b0",
+      "2 | To account [1/2] : 35548ec2 9e9d8530 : 5850e87a 2d2642fe",
+      "2 | To account [2/2] : 7214ff4b b3633407 : 0deafc33 45c3b127",
       "3 | Payment (ICP) : 0.00001",
       "4 | Maximum fee (ICP) : 0.00000137",
       "5 | Memo : 0"
     ],
     "output_expert": [
       "0 | Transaction type : Send ICP",
-      "1 | Sender [1/2] : 5upke-tazvi-6ufqc-i3v6r-j4gpu-dpwti-",
-      "1 | Sender [2/2] : obhal-yb5xj-ue32x-ktkql-rqe",
-      "2 | Subaccount [1/2] : 010101010101010101010101010101010101010",
-      "2 | Subaccount [2/2] : 1010101010101010101010101",
-      "3 | From account[1/2] : 6956653cd25f069f308d4d8a61491d99fa82e27",
-      "3 | From account[2/2] : 2110f77a36bd10e06a28c82b0",
-      "4 | To account [1/2] : 35548ec29e9d85305850e87a2d2642fe7214ff4",
-      "4 | To account [2/2] : bb36334070deafc3345c3b127",
+      "1 | Sender [1/2] : 5upke-tazvi-6ufqc : i3v6r-j4gpu-dpwti",
+      "1 | Sender [2/2] : obhal-yb5xj-ue32x : ktkql-rqe",
+      "2 | Subaccount [1/2] : 01010101 01010101 : 01010101 01010101",
+      "2 | Subaccount [2/2] : 01010101 01010101 : 01010101 01010101",
+      "3 | From account[1/2] : 6956653c d25f069f : 308d4d8a 61491d99",
+      "3 | From account[2/2] : fa82e272 110f77a3 : 6bd10e06 a28c82b0",
+      "4 | To account [1/2] : 35548ec2 9e9d8530 : 5850e87a 2d2642fe",
+      "4 | To account [2/2] : 7214ff4b b3633407 : 0deafc33 45c3b127",
       "5 | Payment (ICP) : 0.00001",
       "6 | Maximum fee (ICP) : 0.00000137",
       "7 | Memo : 0"
@@ -39,10 +39,10 @@
     ],
     "output_expert": [
       "0 | Transaction type : Check status",
-      "1 | Sender [1/2] : 5upke-tazvi-6ufqc-i3v6r-j4gpu-dpwti-",
-      "1 | Sender [2/2] : obhal-yb5xj-ue32x-ktkql-rqe",
-      "2 | Request ID [1/2] : a740262068c4b22efed0cc67095fc9ce46c8831",
-      "2 | Request ID [2/2] : 82c09aa045b4c0396060105d2"
+      "1 | Sender [1/2] : 5upke-tazvi-6ufqc : i3v6r-j4gpu-dpwti",
+      "1 | Sender [2/2] : obhal-yb5xj-ue32x : ktkql-rqe",
+      "2 | Request ID [1/2] : a7402620 68c4b22e fed0cc67 095fc9ce",
+      "2 | Request ID [2/2] : 46c88318 2c09aa04 5b4c0396 060105d2"
     ]
   }
 ]
diff --git a/tests/ui_tests.cpp b/tests/ui_tests.cpp
@@ -48,6 +48,13 @@ class JsonTests : public ::testing::TestWithParam<testcase_t> {
     };
 };
 
+std::string CleanTestname(std::string s) {
+    s.erase(remove_if(s.begin(), s.end(), [](char v) -> bool {
+        return v == ':' || v == ' ' || v == '/' || v == '-' || v == '.' || v == '_' || v == '#';
+    }), s.end());
+    return s;
+}
+
 std::vector<testcase_t> GetJsonTestCases(const std::string &jsonFile) {
     auto answer = std::vector<testcase_t>();
 
@@ -78,11 +85,18 @@ std::vector<testcase_t> GetJsonTestCases(const std::string &jsonFile) {
             outputs_expert.push_back(s.asString());
         }
 
+        bool valid = true;
+        if (i.isMember("value")) {
+            valid = i["valid"].asBool();
+        }
+
+        auto name = CleanTestname(i["name"].asString());
+
         answer.push_back(testcase_t{
                 i["index"].asUInt64(),
-                i["name"].asString(),
+                name,
                 i["blob"].asString(),
-                i["valid"].asBool(),
+                valid,
                 outputs,
                 outputs_expert
         });
@@ -112,7 +126,7 @@ void check_testcase(const testcase_t &tc, bool expert_mode) {
     err = parser_validate(&ctx);
     ASSERT_EQ(err, parser_ok) << parser_getErrorDescription(err);
 
-    auto output = dumpUI(&ctx, 40, 40);
+    auto output = dumpUI(&ctx, 40, 37);
 
     std::cout << std::endl;
     for (const auto &i : output) {