Code Review Request - Toy cryptocurrency miner

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.

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!