From 8984919f4fae22b557089067797ece547eb485b6 Mon Sep 17 00:00:00 2001
From: victor felder <victor@draft.li>
Date: Thu, 10 Oct 2024 14:58:53 +0200
Subject: [PATCH] fix(forge_operation): add error handling

---
 lib/forge_operation.ex | 104 ++++++++++++++++++++---------------------
 test/forge_test.exs    |   7 +--
 2 files changed, 52 insertions(+), 59 deletions(-)

diff --git a/lib/forge_operation.ex b/lib/forge_operation.ex
index 8291983..01d63e0 100644
--- a/lib/forge_operation.ex
+++ b/lib/forge_operation.ex
@@ -77,22 +77,21 @@ defmodule Tezex.ForgeOperation do
 
   @spec operation_group(map()) :: {:ok, nonempty_binary()} | {:error, nonempty_binary()}
   def operation_group(operation_group) do
-    case validate_required_keys(operation_group, ~w(branch contents)) do
-      :ok ->
-        operations =
-          Enum.map(operation_group["contents"], &operation/1)
-          |> Enum.map(fn {:ok, operation} -> operation end)
-
-        content =
-          [
-            Forge.forge_base58(operation_group["branch"]),
-            Enum.join(operations)
-          ]
-          |> IO.iodata_to_binary()
-          |> Base.encode16(case: :lower)
-
-        {:ok, content}
-
+    with :ok <- validate_required_keys(operation_group, ~w(branch contents)),
+         operations = Enum.map(operation_group["contents"], &operation/1),
+         nil <- Enum.find(operations, &(elem(&1, 0) == :error)) do
+      operations = Enum.map(operations, fn {:ok, operation} -> operation end)
+
+      content =
+        [
+          Forge.forge_base58(operation_group["branch"]),
+          Enum.join(operations)
+        ]
+        |> IO.iodata_to_binary()
+        |> Base.encode16(case: :lower)
+
+      {:ok, content}
+    else
       err ->
         err
     end
@@ -145,45 +144,42 @@ defmodule Tezex.ForgeOperation do
 
   @spec transaction(map()) :: {:ok, nonempty_binary()} | {:error, nonempty_binary()}
   def transaction(content) do
-    case validate_required_keys(
-           content,
-           ~w(kind source fee counter gas_limit storage_limit amount destination)
-         ) do
-      :ok ->
-        content =
-          [
-            forge_tag(@operation_tags[content["kind"]]),
-            Forge.forge_address(content["source"], :bytes, true),
-            Forge.forge_nat(String.to_integer(content["fee"])),
-            Forge.forge_nat(String.to_integer(content["counter"])),
-            Forge.forge_nat(String.to_integer(content["gas_limit"])),
-            Forge.forge_nat(String.to_integer(content["storage_limit"])),
-            Forge.forge_nat(String.to_integer(content["amount"])),
-            Forge.forge_address(content["destination"]),
-            if has_parameters(content) do
-              :ok =
-                validate_required_keys(
-                  content,
-                  ~w(parameters parameters.entrypoint parameters.value)
-                )
-
-              params = content["parameters"]
-
-              [
-                Forge.forge_bool(true),
-                entrypoint(params["entrypoint"]),
-                Forge.forge_array(Forge.forge_micheline(params["value"]))
-              ]
-            else
-              Forge.forge_bool(false)
-            end
-          ]
-          |> IO.iodata_to_binary()
-
-        {:ok, content}
+    with :ok <-
+           validate_required_keys(
+             content,
+             ~w(kind source fee counter gas_limit storage_limit amount destination)
+           ),
+         :ok <-
+           (has_parameters(content) &&
+              validate_required_keys(
+                content,
+                ~w(parameters parameters.entrypoint parameters.value)
+              )) || :ok do
+      content =
+        [
+          forge_tag(@operation_tags[content["kind"]]),
+          Forge.forge_address(content["source"], :bytes, true),
+          Forge.forge_nat(String.to_integer(content["fee"])),
+          Forge.forge_nat(String.to_integer(content["counter"])),
+          Forge.forge_nat(String.to_integer(content["gas_limit"])),
+          Forge.forge_nat(String.to_integer(content["storage_limit"])),
+          Forge.forge_nat(String.to_integer(content["amount"])),
+          Forge.forge_address(content["destination"]),
+          if has_parameters(content) do
+            params = content["parameters"]
+
+            [
+              Forge.forge_bool(true),
+              entrypoint(params["entrypoint"]),
+              Forge.forge_array(Forge.forge_micheline(params["value"]))
+            ]
+          else
+            Forge.forge_bool(false)
+          end
+        ]
+        |> IO.iodata_to_binary()
 
-      err ->
-        err
+      {:ok, content}
     end
   end
 
diff --git a/test/forge_test.exs b/test/forge_test.exs
index 9c55bcd..24c40c9 100644
--- a/test/forge_test.exs
+++ b/test/forge_test.exs
@@ -193,10 +193,7 @@ defmodule Tezex.ForgeTest do
       "signature" => nil
     }
 
-    assert catch_error(ForgeOperation.operation_group(opg)) ==
-             {
-               :badmatch,
-               {:error, "Operation content is missing required keys: parameters.entrypoint"}
-             }
+    assert {:error, "Operation content is missing required keys: parameters.entrypoint"} =
+             ForgeOperation.operation_group(opg)
   end
 end