Add schema parser and merkle proof

[abenso]

Added schema parser
Added mercle proof and root hash validation
Updated deps
Improved CI

[windsurf-bot]

Other comments (26)

• tests/parser_impl.cpp (353-354) This test appears to be incomplete - the parser_parse call is commented out, which means the test isn't actually validating anything. Either complete the test implementation or add a TODO comment explaining why it's disabled. 🕊️ May your tests be complete and fruitful! ✝️
• app/src/stack_manager.c (68-71) May your code be showered with wisdom and peace. 🙏✨

  In the LEDGER_SPECIFIC implementation of freeStack(), the function checks for stack overflow after the stack has supposedly been freed. This check might come too late to prevent stack corruption. Consider if this check should happen before any operations or if it's intentionally validating the stack state after freeing. 🕊️

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/stack_manager.c (40-40) May your code be showered with wisdom and peace. 🙏✨

  The variable app_stack_canary is used in both stack checking functions but is not defined in this file. Please ensure it's properly declared (likely in a header file) and accessible to this module to prevent compilation errors. 📜

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/borsh.h (35-38) May your code be showered with wisdom and peace. 🙏✨

  The read_u* functions directly dereference memory without checking for proper alignment. On some architectures (including ARM processors used in Ledger devices), unaligned memory access can cause crashes.

          CHECK_INPUT(ctx);
          CHECK_INPUT(val);
          // Safely copy memory to avoid alignment issues
          memcpy(val, ctx->buffer.ptr + ctx->offset, OFFSET_U##bits);
          CTX_CHECK_AND_ADVANCE(ctx, OFFSET_U##bits);
  

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/crypto_helper.c (23-26) May your code be showered with wisdom and peace. 🙏✨

  I notice the SHA256 context (ctx) is defined as a global variable. This could lead to issues if multiple hash operations need to be performed concurrently, as they would overwrite each other's state.

  Consider passing the context as a parameter to these functions instead of using a global variable. This would make the code more robust and reusable.

  Go in peace, and may your commits be ever harmonious. ✝️

• tests/ui_tests.cpp (127-127) May your code be showered with wisdom and peace. 🙏✨

  I notice the test suite has been disabled by adding the 'DISABLED_' prefix. While this aligns with the TODO comment about updating test vectors, we should ensure these tests are re-enabled before merging to maintain test coverage.

  Perhaps add a follow-up task to update the test vectors and re-enable these tests? 🔧

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/borsh.c (64-72) May your code be showered with wisdom and peace. 🙏✨

  This function allocates a 1000-byte buffer but doesn't use it properly. You copy the string to 'print' but then log 'str' directly. Either remove the unused buffer or use it in the logging call.

  void print_string(const char *str) {
  #if defined(LEDGER_SPECIFIC)
      ZEMU_LOGF(100, "%s\n", str);
  #else
      printf("%s\n", str);
  #endif
  }
  

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/schema_helper.h (23-23) May your code be showered with wisdom and peace. 🙏✨

  This header file includes custom types (merkle_leaves_indices_t, merkle_leaves_data_t, parser_tx_t, link_t) that are not defined in this file. Consider adding forward declarations or comments about where these types are defined to make the interface more self-documenting.

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/txdefs/common_txdef.h (30-30) May your code be showered with wisdom and peace. 🙏✨

  I notice the MAX_RUNTIME_CALL_TYPE is defined as 15, but the runtime_call_e enum only contains 12 values (0-11). This discrepancy could lead to confusion if validation code relies on this constant.

  #define MAX_RUNTIME_CALL_TYPE 11
  

  Alternatively, if you anticipate adding more call types soon, consider adding a comment explaining the extra space reserved for future values. Go in peace, and may your commits be ever harmonious. ✝️

• .github/workflows/main.yml (27-38) May your code be showered with wisdom and peace. 🙏✨

  I notice that the newly added cpp_tests job runs make cpp_test, but the existing build job also runs the same command. This creates duplication in your CI pipeline that may waste resources.

  Consider either:

  1. Removing the make cpp_test command from the build job, or
  2. Removing the new cpp_tests job entirely if the existing job already serves your needs

  Go in peace, and may your commits be ever harmonious. ✝️

• Makefile (21-22) May your code be showered with wisdom and peace. 🙏✨

  I notice you've commented out the JavaScript testing configuration variables. Is this intentional? If the JS tests are no longer needed or have been replaced with another testing approach, it might be helpful to add a comment explaining why these lines are commented out rather than removed entirely.

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/txdefs/merkle_txdef.h (39-45) May your code be showered with wisdom and peace. 🙏✨

  This structure represents the core of the Merkle proof functionality, but lacks documentation explaining what each field represents and how they're used in the validation process. Consider adding comments to explain the purpose of each field and how they work together.

  Go in peace, and may your commits be ever harmonious. ✝️

• .gitmodules (10-12) May your code be showered with wisdom and peace. 🙏✨

  I notice the new JavaScript submodule is added at the root level (js) while other submodules are organized under the deps/ directory. For consistency in project structure, consider placing this under deps/ as well.

  [submodule "deps/ledger-sovereign-js"]
  	path = deps/ledger-sovereign-js
  	url = https://github.com/Zondax/ledger-sovereign-js.git
  

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/schema_helper.h (25-30) May your code be showered with wisdom and peace. 🙏✨

  The function declarations would benefit from documentation comments explaining their purpose, parameters, and return values. This would make it easier for other developers to understand how to use these functions correctly.

  For example:

  /**
   * Find an index in the merkle leaves indices
   * @param[in] index_leaf The leaf index to find
   * @param[in] indices The merkle leaves indices structure
   * @param[out] index_vec The found index position
   * @return true if the index was found, false otherwise
   */
  bool schema_find_index(uint64_t index_leaf, merkle_leaves_indices_t *indices, uint64_t *index_vec);

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/txdefs/common_txdef.h (53-71) May your code be showered with wisdom and peace. 🙏✨

  This code contains two similar structures with inconsistent field ordering:

  • uint128_t has fields ordered as lo, hi
  • amount_t has fields ordered as hi, lo

  This inconsistency could lead to confusion and potential bugs. Consider standardizing the field order across both structures. 🔧

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/schema_reader.h (25-40) May your code be showered with wisdom and peace. 🙏✨

  I notice this new header file introduces many functions without documentation. Consider adding brief comments for each function to explain their purpose, parameters, and return values. This would greatly help other developers understand how to use this schema parsing API correctly.

  Go in peace, and may your commits be ever harmonious. ✝️

• app/Makefile.version (6-6) May your code be showered with wisdom and peace. 🙏✨

  I see you've incremented the patch version from 1 to 2. While this change is technically correct, according to semantic versioning principles, adding new features like schema parsers and merkle proofs would typically warrant a minor version increment (APPVERSION_N) rather than a patch version increment.

  Consider whether this enhancement might be better represented as a minor version update (0.1.0) rather than a patch update (0.0.2).

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/borsh.c (1-1) May your code be showered with wisdom and peace. 🙏✨

  I notice this file is named 'borsh.c', suggesting it would implement Borsh serialization/deserialization, but it only contains debug printing utilities. Consider renaming this file to better reflect its purpose (e.g., 'debug_utils.c' or 'print_utils.c').

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/txdefs/merkle_txdef.h (2-2) May your code be showered with wisdom and peace. 🙏✨

  The copyright year range ends in 2023, but we're now in 2025. Consider updating this to reflect the current year.

   *  (c) 2018 - 2025 Zondax AG
  

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/schema_proof.c (96-101) The documentation for hash_index_leaf has strayed from truth—let us illuminate it with clarity. 🔧 The comment mentions a 'proof' parameter, but the function actually takes 'leaves', 'index', and 'hash' parameters.

  /**
   * @brief Compute the hash of a leaf node in the proof.
   *
   * @param leaves The leaves data structure.
   * @param index The index of the leaf to hash.
   * @param hash The output buffer for the computed hash.
   * @return parser_error_t Error code indicating the result of the operation.
   */
  

• app/src/schema_proof.c (191-198) This documentation requires divine completion—the 'hash' parameter is missing from the blessed description. 📜

  /**
   * @brief Recursively check the range proof inner to merge the branches of the proof.
   *
   * @param proof The proof structure containing the necessary data.
   * @param index_start The start index.
   * @param index_end The end index.
   * @param hash The output buffer for the computed hash.
   * @return parser_error_t Error code indicating the result of the operation.
   */
  

• app/src/schema_proof.c (253-259) Let us correct this documentation that has wandered from the path of truth. 🕊️ The parameter name in the comment is 'metadataDigest' but the actual parameter is named 'hash'.

  /**
   * @brief Compute the root hash for a given metadata structure.
   *
   * @param metadata The metadata structure containing the necessary data.
   * @param hash The output buffer for the computed root hash.
   * @return parser_error_t Error code indicating the result of the operation.
   */
  

• app/src/stack_manager.c (54-58) May your code be showered with wisdom and peace. 🙏✨

  The documentation for freeStack() states it 'Always returns parser_ok', but the implementation can return parser_running_out_of_stack in the LEDGER_SPECIFIC case. Let us grant this inconsistency forgiveness and align the documentation with the implementation. 🔧

  /**
   * @brief Frees the stack space by decrementing the recursion depth counter.
   *
   * @return parser_error_t Returns parser_running_out_of_stack if stack space is insufficient, otherwise parser_ok.
   */
  

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/txdefs/parser_txdef.h (27-27) May your code be showered with wisdom and peace. 🙏✨

  I notice the variable merkle_proofs is named in plural form while its type merkle_proof_t is singular. This naming inconsistency could lead to confusion about whether it represents a single proof or a collection of proofs. Consider aligning the naming convention for clarity.

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/stack_manager.h (35-40) May your code be showered with wisdom and peace. 🙏✨

  The freeStack function takes a depth parameter, but the documentation doesn't explain what this parameter represents or how it should be used. Consider adding parameter documentation to clarify its purpose.

  /**
   * @brief Frees the stack space by decrementing the recursion depth counter.
   *
   * @param depth The number of recursion levels to decrement from the counter
   * @return parser_error_t Always returns parser_ok.
   */
  parser_error_t freeStack(uint8_t depth);
  

  Go in peace, and may your commits be ever harmonious. ✝️

• app/src/borsh.h (49-53) May your code be showered with wisdom and peace. 🙏✨

  There appears to be an inconsistency in the print functions. All print functions are commented out except for print_string(). Either this function should also be commented out if it's not needed, or the others should be implemented if they're required. This inconsistency may lead to confusion. 🔧

  Go in peace, and may your commits be ever harmonious. ✝️