Delegate function always matches

[nathanjohnson320]

Was looking through the source to try and write an elm wrapper, noticed that this line: https://github.com/Highjhacker/Ark-Elixir/blob/master/lib/ark_elixir/delegate.ex#L76

will always match so this one https://github.com/Highjhacker/Ark-Elixir/blob/master/lib/ark_elixir/delegate.ex#L88

never gets hit.

Great library by the way 👍

[insanedefaults]

Here is an example of how this fails, testing against arkpool.

iex(1)> Ark_Elixir.Delegate.get_delegate "02b1d2ea7c265db66087789f571fceb8cc2b2d89e296ad966efb8ed51855f2ae0b"
%{"error" => "Delegate not found", "success" => false}

iex(2)> Ark_Elixir.Delegate.get_delegate "arkpool"                           
%{"delegate" => %{"address" => "ARAq9nhjCxwpWnGKDgxveAJSijNG8Y6dFQ",
    "approval" => 1.41, "missedblocks" => 290, "producedblocks" => 57510,
    "productivity" => 99.5,
    "publicKey" => "02b1d2ea7c265db66087789f571fceb8cc2b2d89e296ad966efb8ed51855f2ae0b",
    "rate" => 1, "username" => "arkpool", "vote" => "184815064113380"},
  "success" => true}

The maximum username size is 20 characters according to this line from ark-node.

The public key appears to always be 66 characters.

It will only accept a public key or username according to the Ark node swagger.

[Highjhacker]

Great catch guys :D ! Sorry for the delay, wasn't much around with the holidays. Gonna merge your PR. Great work !

[Highjhacker]

Hi, if you don't mind I would like your opinion on something I was thinking about ark_elixir.

It would be cool if it could be possible to switch between networks, for querying the dev network for example. So I've worked a bit on an implementation but I'm not sure about the usage, if it's enough in the functional spirit (I'm from Python so this may reflect on my code, I'm still learning a lot about all of this).

In the usage it would be something like

If you want to query all blocks, without any parameters, on the main network (because it will be the default one) you will type something like :

iex> Ark_Elixir.Block.gets_blocks # Nothing new here

If you want to query with the optionnal parameters (like limit) you will type something like :

iex> Ark_Elixir.Block.get_blocks([limit: 1]) # Nothing new here too

But if you want to query with the optionnal parameters (like limit) AND specify a network, it will be something like :

iex> Ark_Elixir.Block.get_blocks([limit: 1], "dev")

Till here I think it's not problematic, the next example is more a problem in my mind, but may be I'm wrong. And maybe there's a solution to that.

If you want to query without optional parameters BUT specify a network, the syntax is just ugly and it boggles me :

iex> Ark_Elixir.Block.get_blocks([], "dev") # Need to pass an empty list as first parameter

So, I would like to ask you what do you think about this ? Is the syntax just ugly and should I find an other way ? If you are interested I can create a new branch and push the new api.ex code and you could look at it.

---

De : Nathaniel Johnson notifications@github.com Envoyé : dimanche 24 décembre 2017 01:00 À : Highjhacker/Ark-Elixir Cc : Subscribed Objet : [Highjhacker/Ark-Elixir] Delegate function always matches (#1)

Was looking through the source to try and write an elm wrapper, noticed that this line: https://github.com/Highjhacker/Ark-Elixir/blob/master/lib/ark_elixir/delegate.ex#L76

[https://avatars1.githubusercontent.com/u/5347826?s=400&v=4]https://github.com/Highjhacker/Ark-Elixir/blob/master/lib/ark_elixir/delegate.ex#L76

Highjhacker/Ark-Elixirhttps://github.com/Highjhacker/Ark-Elixir/blob/master/lib/ark_elixir/delegate.ex#L76 github.com Ark-Elixir - Ark API Wrapper in Elixir

will always match so this one https://github.com/Highjhacker/Ark-Elixir/blob/master/lib/ark_elixir/delegate.ex#L88

[https://avatars1.githubusercontent.com/u/5347826?s=400&v=4]https://github.com/Highjhacker/Ark-Elixir/blob/master/lib/ark_elixir/delegate.ex#L88

Highjhacker/Ark-Elixirhttps://github.com/Highjhacker/Ark-Elixir/blob/master/lib/ark_elixir/delegate.ex#L88 github.com Ark-Elixir - Ark API Wrapper in Elixir

never gets hit.

Great library by the way 👍

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/Highjhacker/Ark-Elixir/issues/1, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AFGZ8vxmrdTxI_VGJlC0s4IdMr1JmcLwks5tDZQqgaJpZM4RLytq.

[nathanjohnson320]

@Highjhacker that is an interesting problem. Normally in elixir apps you would do the environment configuration in your dev/prod.exs files

config :ark_elixir,
    env: :dev

and then in Ark_Elixir.Api you would grab the environment with the standard get_env call:

Application.get_env(:ark_elixir, :env)

but I can see situations where you might want to do a call to the test net and the main net in the same environment so using the configuration stuff doesn't quite work (since it wouldn't be override-able)

In the elm-ark client I made the environments functions and they get passed through API calls like this:

mainNet
    |> getBalance "AUexKjGtgsSpVzPLs6jNMM6vJ6znEVTQWK"
    |> send ReceiveAccountBalance decodeBalance

but this adds a burden to the developer since they now have to always specify an environment and pass it in. Unlike your solution in which it has a default value which is passed in.

Another solution piggy backing off yours would be to have multiple clauses for the network with a default case passing in the production net:

def get_blocks(:dev, opts \\ []), do: Ark_Elixir.Api.get("api/blocks", opts, :dev)
def get_blocks(:main, opts \\ []), do: Ark_Elixir.Api.get("api/blocks", opts, :main)
def get_blocks(opts \\ []), do: Ark_Elixir.Api.get("api/blocks", opts, :main)

iex> Ark_Elixir.Block.get_blocks([limit: 1]) == Ark_Elixir.Block.get_blocks(:main, [limit: 1])

That keeps the API relatively clean but results in a lot of boilerplate code on the library itself.

I'd probably go with the last option since it gives you the most flexibility, but it would also be nice if you could add in the configuration piece so developers could configure it at the config.exs level.

[insanedefaults]

I'm working off of a few assumptions here about the problem and desired solution, so I'll list those out:

• Keep as close as possible to the api provided by the Ark team, perhaps forgoing a more traditional Elixir approach in exchange for a user experience more in-line with the rest of the Ark clients
• Having arguments that must be declared (regardless of necessity) when options are present is not desired

Have you considered not having a limit for Block.get_all and instead making two different functions out of it? Then you could simply use the config.exs option that @nathanjohnson320 presented and use it for the required arguments like version, port, and nethash in Ark_Elixir.API.get/2.

Perhaps something like this:

defmodule Ark_Elixir.Block do
  @moduledoc false

  defstruct id: nil

  @type t :: %__MODULE__{}
  @type index :: non_neg_integer
  @type amount :: pos_integer
  @type option :: {:nethash, String.t} | {:version, String.t} | {:port, String.t}

  @doc """
  Returns a list of all blocks.
  """
  @spec get_all([option]) :: [Block.t]
  def get_all(opts)

  @doc """
  Returns a list of the first `count` blocks.
  """
  @spec take(amount, [option]) :: [Block.t]
  def take(count, opts)

  @doc """
  Returns a list of `count` blocks from an `offset`.
  """
  @spec slice(index, amount, [option]) :: [Block.t]
  def slice(offset, count, opts)

  @doc """
  Returns a single block according to its `id`.
  """
  @spec fetch!(index, [option]) :: Block.t
  def fetch!(id, opts)

end

If you still want the network to be part of the function call, you could move the logic for version, port, and nethash out of Ark_Elixir.API.get/2 and into the functions as an opts keyword list. I have typespec'd this pseudocode for clarity.

---

As a small suggestion, it would also be nice to have a lazy version of the block listing functions. The blockchain can get rather long and the /api/blocks route will spit out the whole list eagerly. Having an extra set of lazy functions perhaps under Ark_Elixir.Block.Stream could be nice.

[Highjhacker]

Readed your answers and it seems really great and good practices, I'm gonna need to learn a bit more of Elixir haha ! I really like the idea of implementing somes functions like "slice" for querying the blocks.

I tried some things this morning before reading all of this and I think I have something who can works.

  def get(endpoint, opts \\ []) do
    case opts[:network] do
      "main" -> get_main(endpoint, opts)
      "dev" -> get_dev(endpoint, opts)
      _     -> get_main(endpoint, opts)
    end
  end

With that we can simply do something like

  Ark_Elixir.Block.get_blocks 

  Ark_Elixir.Block.get_blocks([limit: 2])

  Ark_Elixir.Block.get_blocks([limit: 2, network: "dev"]) 

  Ark_Elixir.Block.get_blocks([network: "dev", limit: 2])

  Ark_Elixir.Block.get_blocks([network: "dev"])

What do you think about this ? I think it's more user friendly but like I said I'm not an Elixir expert so maybe I miss something with my idea.

[insanedefaults]

If you would want to have everything set in the opts block, I think that interface would work fine! I would suggest also including offset in there if you're going to have limit. If someone is using limit, it's highly likely that they are also going to want to touch offset.

[Highjhacker]

Thanks for the input ! Well you can use the offset right now, you can do something like

    Ark_Elixir.Block.get_blocks([limit: 2, offset: 1, orderBy: "timestamp", network: "dev")

You can use every parameter listed on the swagger documentation, so for get_blocks you can pass : limit, orderBy, offset, generatorPublicKey, totalAmount, totalFee, reward, previousBlock, height in the opts parameter.

Or maybe I misunderstood your intention ?

[Highjhacker]

Pushed the changes on the ark_elixir_testing branch if you want to have a look. Gonna test it a bit before merging it to master. Let me know what you think about it !

[nathanjohnson320]

@Highjhacker can you open a PR so it makes commenting on it easier?

[Highjhacker]

You're right, gonna close this comment ! PR open