Code Review Request - Toy cryptocurrency miner

Chat / Discussions
, , ,
1

I’m taking a class this semester that has me coding little projects in Elixir. Since this is the first time I’m working with this kind of a programming model, I’d really appreciate some help and guidance about how to structure stuff better, I’d really love to learn more about elixir and erlang.

Here’s a link to the project.

Please note that I’m not asking for help in completing this, the project deadline is past and it has been submitted for grading. I want your help to improve it and learn from possible design mistakes.

For reference this is a link to the original prompt

Specifically, my questions would be:

  1. Is there is a better way to structure my process tree, and how do I supervise this whole tree correctly?
  2. How do I make this OTP compliant (for the sake of learning about OTP, I’m not even sure if this is the right way to do this)

Any other feedback, comments are most welcome!

Here is an excerpt from the project README about the architecture:

Architecture

  • There is a single globally named process that manages all worker nodes and itself called the MiningServer which is a GenServer with the name {:global, :mining_server}
    • This node is singluar in the network and responsible for distributing work among its children which are the process pool managers from remote nodes
  • Each node has a locally named process pool backed by the GenServer MinerPool, which recieves work messages from the master node MiningServer and translates them appropriately for its children to do work with. All commuincation is async with a callback to a master node’s listening process
  • The leaf in this tree of messages is a Miner node which does the real heavy lifting in computing the SHA256 of a range of numbers to find valid coin-strings

Drawn as a tree, the network looks like so (arrows indicate flow of messages)

                  MiningServer
                      |
                      V
------------------------------------------------------
|(Node1)     | (Node2)    | (Node 3)   ... | (Node n)
|->MinerPool |->MinerPool |->MinerPool     |->MinerPool
  |->Miner     |->Miner     |->Miner         |->Miner
  |->Miner     |->Miner     |->Miner         |->Miner
  ...          ...          ...              ...
  |->Miner     |->Miner     |->Miner         |->Miner
  ---------------------------------------------------
                      |-> MiningServer

PS: If this forum is not the right one for this kind of a topic, please direct me to one that may help in this regard! Really excited to learn about

1 Like
2

A few inconsequential things that I noticed:

Instead of:

def start_link do
  GenServer.start_link(__MODULE__, [])
end

def start_link(_) do
  GenServer.start_link(__MODULE__, [])
end

You can use default parameters and have the compiler create the two functions for you:

def start_link(_opts \\ [])

I also noticed a few places you are using using the __MODULE__ macro to call functions from the current module, like:

__MODULE__.add_workers(pid, workers)

__MODULE__ gets expanded at compile time to whichever module is currently being compiled, so the above is the same as just calling the function directly since you are already in the current modules scope:

add_workers(pid, workers)

The git repo is missing the Paras.MiningSupervisor supervisor so I’m not able to run it or work out exactly how your tree is set up currently. I don’t have too much experience in this area, but I could picture a supervision tree like such:

<Main Supervisor>
    MiningServer
    <Pools Supervisor>
        <Miner Pool Supervisor>[Node 1]
            MinerPool
            <Worker Supervisor>
                Miners...
        <Miner Pool Supervisor>[Node 2]
            MinerPool
            <Worker Supervisor>
                Miners...
        <Miner Pool Supervisor>[Node N]
            MinerPool
            <Worker Supervisor>
                Miners...

This way each of the individual workers in a pool are supervised together separately from the MinerPool they communicate with. You also have each pool in its own supervision tree that can run on each node, this is for dealing with MinerPool crashes. Then each pool in its entirety is supervised so they can crash independently from other pools. An finally you have the main supervisor managing the MiningServer and all of the pools.

I’m not totally confident this is the “right way”, so take this approach with a grain of salt. Hopefully someone else will chime in to clarify. I think generally when you have a pool of workers and some server module that manages them you want to put the workers under their own supervisor, and then have {ManagerServer, WorkerSupervisor} under their own supervisor.

1 Like
3

I had accidentally added mining_supervisor.ex to my gitignore file while excluding the binary. You can try building it now. Sorry for the confusion!

I’ll try re-implementing the supervision tree to match what you said. It makes more sense that each logical unit should have its own boss looking after the underlings. Thanks for taking the time to respond!