move effect hash computation to parser

Zondax · Jan 24, 2025 · 8363fe3 · 8363fe3
1 parent ecee434
commit 8363fe3
Show file tree

Hide file tree

Showing 8 changed files with 57 additions and 71 deletions.
diff --git a/app/src/common/parser.h b/app/src/common/parser.h
@@ -28,6 +28,9 @@ const char *parser_getMsgPackTypeDescription(uint8_t type);
 //// parses a tx buffer
 parser_error_t parser_parse(parser_context_t *ctx, const uint8_t *data, size_t dataLen, parser_tx_t *tx_obj);
 
+//// computes the effect hash
+parser_error_t parser_computeEffectHash(parser_context_t *ctx);
+
 //// verifies tx fields
 parser_error_t parser_validate(parser_context_t *ctx);
 

diff --git a/app/src/common/tx.c b/app/src/common/tx.c
@@ -85,6 +85,13 @@ const char *tx_parse() {
         return parser_getErrorDescription(err);
     }
 
+    err = parser_computeEffectHash(&ctx_parsed_tx);
+    CHECK_APP_CANARY()
+
+    if (err != parser_ok) {
+        return parser_getErrorDescription(err);
+    }
+
     err = parser_validate(&ctx_parsed_tx);
     CHECK_APP_CANARY()
 

diff --git a/app/src/crypto.c b/app/src/crypto.c
@@ -159,22 +159,10 @@ zxerr_t crypto_sign(parser_tx_t *tx_obj, uint8_t *signature, uint16_t signatureM
     nv_signature_init();
 
     zxerr_t error = zxerr_invalid_crypto_settings;
-
-    // compute parameters hash
-    CATCH_ZX_ERROR(compute_parameters_hash(&tx_obj->parameters_plan.data_bytes, &tx_obj->plan.parameters_hash));
-
+
     // compute spend key
     CATCH_ZX_ERROR(computeSpendKey(&keys));
 
-    // compute action hashes
-    for (uint16_t i = 0; i < tx_obj->plan.actions.qty; i++) {
-        CATCH_ZX_ERROR(
-            compute_action_hash(&tx_obj->actions_plan[i], &tx_obj->plan.memo.key, &tx_obj->plan.actions.hashes[i]));
-    }
-
-    // compute effect hash
-    CATCH_ZX_ERROR(compute_effect_hash(&tx_obj->plan, tx_obj->effect_hash, sizeof(tx_obj->effect_hash)));
-
     // Similar to what is done in:
     // https://github.com/penumbra-zone/penumbra/blob/main/crates/core/transaction/src/plan/auth.rs#L12
     uint8_t spend_signature[64] = {0};

diff --git a/app/src/parser.c b/app/src/parser.c
@@ -42,6 +42,8 @@
 #include "tx_metadata.h"
 #include "undelegate.h"
 #include "undelegate_claim.h"
+#include "parser_interface.h"
+
 static uint8_t action_idx = 0;
 
 parser_error_t parser_init_context(parser_context_t *ctx, const uint8_t *buffer, uint16_t bufferSize) {
@@ -65,6 +67,21 @@ parser_error_t parser_parse(parser_context_t *ctx, const uint8_t *data, size_t d
     return _read(ctx, tx_obj);
 }
 
+parser_error_t parser_computeEffectHash(parser_context_t *ctx) {
+    // compute parameters hash
+    CHECK_ERROR(compute_parameters_hash(&ctx->tx_obj->parameters_plan.data_bytes, &ctx->tx_obj->plan.parameters_hash));
+
+    // compute action hashes
+    for (uint16_t i = 0; i < ctx->tx_obj->plan.actions.qty; i++) {
+        CHECK_ERROR(compute_action_hash(&ctx->tx_obj->actions_plan[i], &ctx->tx_obj->plan.memo.key, &ctx->tx_obj->plan.actions.hashes[i]));
+    }
+
+    // compute effect hash
+    CHECK_ERROR(compute_effect_hash(&ctx->tx_obj->plan, ctx->tx_obj->effect_hash, sizeof(ctx->tx_obj->effect_hash)));
+
+    return parser_ok;
+}
+
 parser_error_t parser_validate(parser_context_t *ctx) {
     // Iterate through all items to check that all can be shown and are valid
     uint8_t numItems = 0;

diff --git a/app/src/parser_interface.c b/app/src/parser_interface.c
@@ -20,46 +20,45 @@
 #include "keys_def.h"
 #include "protobuf/penumbra/core/transaction/v1/transaction.pb.h"
 #include "rslib.h"
-#include "zxformat.h"
 
-zxerr_t compute_effect_hash(transaction_plan_t *plan, uint8_t *effect_hash, uint16_t effect_hash_len) {
-    if (plan == NULL || effect_hash == NULL) return zxerr_unknown;
+parser_error_t compute_effect_hash(transaction_plan_t *plan, uint8_t *effect_hash, uint16_t effect_hash_len) {
+    if (plan == NULL || effect_hash == NULL) return parser_unexpected_error;
 
     if (rs_compute_effect_hash(plan, effect_hash, effect_hash_len) != parser_ok) {
-        return zxerr_unknown;
+        return parser_unexpected_error;
     }
 
-    return zxerr_ok;
+    return parser_ok;
 }
 
-zxerr_t compute_parameters_hash(bytes_t *parameters_bytes, hash_t *output) {
-    if (parameters_bytes == NULL || output == NULL) return zxerr_unknown;
+parser_error_t compute_parameters_hash(bytes_t *parameters_bytes, hash_t *output) {
+    if (parameters_bytes == NULL || output == NULL) return parser_unexpected_error;
 
     if (rs_parameter_hash(parameters_bytes, (uint8_t *)output, 64) != parser_ok) {
-        return zxerr_unknown;
+        return parser_unexpected_error;
     }
 
-    return zxerr_ok;
+    return parser_ok;
 }
 
-zxerr_t compute_action_hash(action_t *action, bytes_t *memo_key, hash_t *output) {
-    if (action == NULL || output == NULL) return zxerr_unknown;
+parser_error_t compute_action_hash(action_t *action, bytes_t *memo_key, hash_t *output) {
+    if (action == NULL || output == NULL) return parser_unexpected_error;
 
     switch (action->action_type) {
         case penumbra_core_transaction_v1_ActionPlan_spend_tag:
             if (rs_spend_action_hash(&action->action.spend, (uint8_t *)output, 64) != parser_ok) {
-                return zxerr_encoding_failed;
+                return parser_unexpected_error;
             }
             break;
         case penumbra_core_transaction_v1_ActionPlan_output_tag:
             if (rs_output_action_hash(&action->action.output, memo_key, (uint8_t *)output, 64) != parser_ok) {
-                return zxerr_encoding_failed;
+                return parser_unexpected_error;
             }
             break;
 #if defined(FULL_APP)
         case penumbra_core_transaction_v1_ActionPlan_swap_tag:
             if (rs_swap_action_hash(&action->action.swap, (uint8_t *)output, 64) != parser_ok) {
-                return zxerr_encoding_failed;
+                return parser_unexpected_error;
             }
             break;
 #endif
@@ -71,33 +70,33 @@ zxerr_t compute_action_hash(action_t *action, bytes_t *memo_key, hash_t *output)
         case penumbra_core_transaction_v1_ActionPlan_action_dutch_auction_schedule_tag:
         case penumbra_core_transaction_v1_ActionPlan_action_dutch_auction_end_tag:
             if (rs_generic_action_hash(&action->action_data, action->action_type, (uint8_t *)output, 64) != parser_ok) {
-                return zxerr_encoding_failed;
+                return parser_unexpected_error;
             }
             break;
         case penumbra_core_transaction_v1_ActionPlan_undelegate_claim_tag:
             if (rs_undelegate_claim_action_hash(&action->action.undelegate_claim, (uint8_t *)output, 64) != parser_ok) {
-                return zxerr_encoding_failed;
+                return parser_unexpected_error;
             }
             break;
         case penumbra_core_transaction_v1_ActionPlan_delegator_vote_tag:
             if (rs_delegator_vote_action_hash(&action->action.delegator_vote, (uint8_t *)output, 64) != parser_ok) {
-                return zxerr_encoding_failed;
+                return parser_unexpected_error;
             }
             break;
         case penumbra_core_transaction_v1_ActionPlan_position_withdraw_tag:
             if (rs_position_withdraw_action_hash(&action->action.position_withdraw, (uint8_t *)output, 64) != parser_ok) {
-                return zxerr_encoding_failed;
+                return parser_unexpected_error;
             }
             break;
         case penumbra_core_transaction_v1_ActionPlan_action_dutch_auction_withdraw_tag:
             if (rs_action_dutch_auction_withdraw_action_hash(&action->action.action_dutch_auction_withdraw,
                                                              (uint8_t *)output, 64) != parser_ok) {
-                return zxerr_encoding_failed;
+                return parser_unexpected_error;
             }
             break;
         default:
-            return zxerr_unknown;
+            return parser_unexpected_error;
     }
 
-    return zxerr_ok;
+    return parser_ok;
 }
diff --git a/app/src/parser_interface.h b/app/src/parser_interface.h
@@ -25,12 +25,10 @@ extern "C" {
 
 #include "keys_def.h"
 #include "parser_common.h"
-#include "zxerror.h"
-#include "zxmacros.h"
 
-zxerr_t compute_parameters_hash(bytes_t *parameters_bytes, hash_t *output);
-zxerr_t compute_effect_hash(transaction_plan_t *plan, uint8_t *effect_hash, uint16_t effect_hash_len);
-zxerr_t compute_action_hash(action_t *action, bytes_t *memo_key, hash_t *output);
+parser_error_t compute_parameters_hash(bytes_t *parameters_bytes, hash_t *output);
+parser_error_t compute_effect_hash(transaction_plan_t *plan, uint8_t *effect_hash, uint16_t effect_hash_len);
+parser_error_t compute_action_hash(action_t *action, bytes_t *memo_key, hash_t *output);
 
 #ifdef __cplusplus
 }

diff --git a/tests/parser_impl.cpp b/tests/parser_impl.cpp
@@ -38,33 +38,16 @@ TEST(SCALE, ReadBytes) {
     uint8_t buffer[6000];
     auto bufferLen = parseHexString(
         buffer, sizeof(buffer),
-        "0a9201ba038e010a220a20c2ccae788b3e9972476a483dbff593a0739f64e2f45ee623fe72d0999224ce4310daa59499021a300a0a08eba1cca"
-        "fbcfea4f20812220a2029ea9c2f3371f6a487e7e95c247041f4a356f983eb064e5d2b3bcf322ca96a1022300a0a08faf9adeaae96c5c40d1222"
-        "0a2029ea9c2f3371f6a487e7e95c247041f4a356f983eb064e5d2b3bcf322ca96a10122708f8d50b1213656e77747372662d363739383432323"
-        "03230331a0c0a0a08e38f8c88c5c6a68c0b2a84010a600a520a50db52e218d7e10ab883b535d7f8989b1d75218ce9bdcb537adbff9da1ca60e2"
-        "c9e7bcbfde9dc8a4c119bae69aaedbfa17a4f5cf96a27453a2caec233e43e9a54e6db3abe48dff5c93b82b657052146d32120a764a43664b362"
-        "0612036122075726816d27699ac44b50fd78da53137b9ffa403c0000e6b3908f46b62d9b992");
+        "0abe020abb020aa8010a300a0a08caedc786f98cc3e50312220a2029ea9c2f3371f6a487e7e95c247041f4a356f983eb064e5d2b3bcf322ca96a10122042b70d0c3e32c65670ca200dbd3c3a9a9dd59ef896fee3579025b841050359431a520a50890bc98e3698aa4578e419b028da5672e627c280d8b06166f4c42d5366bccf1fcf3b296cd61e8d744a21f75f2fb697183e18595d8a79008539d8fb138b405db09db65cc42d54c0e772e5d42d5f20b52f109d94b78f8798391a20e5a37679976d378f1dabe0fd608211081bb9c43b1f53978f9cef43dbf7a3d0012220f229ec5a96f4445a12f4c314b6a789bb99391fecb49c9b91800c7c5c94ccb6012a20a9a2cf11e230952ddbf772551a29786b2d4fef65388b2f3ea22cec9efc0f54013220dfb0cd1034c290a673a6825cc6771d1c7b97c063b429ff089e890cb85175320c121f120f6b696b7a7262762d373935363239341a0c0a0a089ff59eb6b88a838f0b");
 
     err = parser_parse(&ctx, buffer, bufferLen, &tx_obj);
     ASSERT_EQ(err, parser_ok) << parser_getErrorDescription(err);
 
-    // compute parameters hash
-    zxerr = compute_parameters_hash(&tx_obj.parameters_plan.data_bytes, &tx_obj.plan.parameters_hash);
-    ASSERT_EQ(zxerr, zxerr_ok);
-
-    // compute action hashes
-    for (uint16_t i = 0; i < tx_obj.plan.actions.qty; i++) {
-        zxerr = compute_action_hash(&tx_obj.actions_plan[i], &tx_obj.plan.memo.key, &tx_obj.plan.actions.hashes[i]);
-        ASSERT_EQ(zxerr, zxerr_ok);
-    }
-
-    // compute effect hash
-    zxerr = compute_effect_hash(&tx_obj.plan, tx_obj.effect_hash, sizeof(tx_obj.effect_hash));
-    ASSERT_EQ(zxerr, zxerr_ok);
+    err = parser_computeEffectHash(&ctx);
+    ASSERT_EQ(err, parser_ok) << parser_getErrorDescription(err);
 
     std::string expected =
-        "91b5af928d5d5ac18f3b107eecd01daa6f5216576383ca0c2d245d604648365fa69aeb253e88bcee9b3228e5f7a63c78df2edfd4a3b61cfb0c6"
-        "2c6b77732b359";
+        "0e9fd733a3724555dd48a8ca91010b8d57b1291ee90784fc8cbfb2c152b1e762065a01b4fa9af5f0ee510ffcc2cb07987b2a4983e9481e6341ee94ddec213b4f";
     char actual[129];
     array_to_hexstr(actual, sizeof(actual), tx_obj.effect_hash, sizeof(tx_obj.effect_hash));
 

diff --git a/tests/plan_effect_hash.cpp b/tests/plan_effect_hash.cpp
@@ -93,17 +93,8 @@ void check_testcase_effect_hash(const testcase_effect_hash_t &tc, bool expert_mo
     err = parser_parse(&ctx, buffer, bufferLen, &tx_obj);
     ASSERT_EQ(err, parser_ok) << parser_getErrorDescription(err);
 
-    // compute parameters hash
-    zxerr = compute_parameters_hash(&tx_obj.parameters_plan.data_bytes, &tx_obj.plan.parameters_hash);
-    ASSERT_EQ(zxerr, zxerr_ok);
-
-    for (uint16_t i = 0; i < tx_obj.plan.actions.qty; i++) {
-        zxerr = compute_action_hash(&tx_obj.actions_plan[i], &tx_obj.plan.memo.key, &tx_obj.plan.actions.hashes[i]);
-        ASSERT_EQ(zxerr, zxerr_ok);
-    }
-
-    zxerr = compute_effect_hash(&tx_obj.plan, tx_obj.effect_hash, sizeof(tx_obj.effect_hash));
-    ASSERT_EQ(zxerr, zxerr_ok);
+    err = parser_computeEffectHash(&ctx);
+    ASSERT_EQ(err, parser_ok) << parser_getErrorDescription(err);
 
     std::string expected = tc.hash;
     char actual[129];