Skip to content
This repository has been archived by the owner on Apr 24, 2022. It is now read-only.

Farm mode (getWork) deliberately looses valid shares. Lowered efficiency. #1213

Closed
AndreaLanfranchi opened this issue Jun 4, 2018 · 8 comments

Comments

@AndreaLanfranchi
Copy link
Collaborator

AndreaLanfranchi commented Jun 4, 2018

For first time readers ! This issue is only related to farm mode (getWork) ie using eth-proxy or connecting to pools still using getWork mode. Stratum mode is not affected.

Logic inside EthGetWorkClient is wrong by design due to a misinterpretation about --farm-recheck argument value.

Summary : all work is performed inside WorkLoop routine which is paused at every cycle by the amount of milliseconds defined in --farm-recheck cli argument (which defaults to 500 ms)

Problem : WorkLoop does not limit to check if new work is available from source but also sends found solution(s) on next loop. This causes two issues :

  1. Solution found may stay in waiting state for submission up to --farm-recheck ms value (thus increasing the probability for it to become stale)
  2. Before the --farm-recheck period expires another solution may be found and the latter overwrites the previous one which has not been submitted yet

This leads to increased stale share ratio and possibly lowers the miner efficiency (as valid shares may never get submitted). To mitigate this users may enter insanely low values for --farm-recheck which leads to a huge overhead while hammering source with get_work and submissions of hashrates.

/CC @chfast

@AndreaLanfranchi AndreaLanfranchi changed the title Farm mode (get_work) deliberately looses valid shares. Lowered efficiency. Farm mode (getWork) deliberately looses valid shares. Lowered efficiency. Jun 4, 2018
@chfast
Copy link
Contributor

chfast commented Jun 4, 2018

Great analysis!

How important this feature is nowadays? Are there any pool depending on this protocol? For sure this is the only way to communicate with some Ethereum clients like geth and aleth (cpp-ethereum). I think parity implemented stratum.

Personally, I'd just deprecate the "getwork" protocol and inform the user that this is not optimal to use it.
For Ethereum clients I'd rather implement stratum server there.

@AndreaLanfranchi
Copy link
Collaborator Author

AndreaLanfranchi commented Jun 4, 2018

How important this feature is nowadays ?

I think it's still the only option for solo mining where you connect to a geth daemon.
Nevertheless there are some pools still offering this connection mode

  • Dwarfpool getWork port 80
  • Nanopool getWork port 8888

In addition there are still many users convinced (and thus using) that eth-proxy is efficient when more than one rig connected through a poor network link.

We should decide wether amend the logic or completely drop getWork mode : in this last case we could also get rid completely of the json-rpccpp external project (already removed in API server) and finally won't see --farm-recheck put randomly in stratum mode.

But, I reiterate, it seems like getWork mode might be useful for (not so) few people.

@AndreaLanfranchi
Copy link
Collaborator Author

Opinions from @smurfy and @jean-m-cyr welcome too.

@AndreaLanfranchi
Copy link
Collaborator Author

Another addition ... afaik EthOS ships bundled with eth-proxy and binds ethminer to it (one instance per GPU).

@chfast
Copy link
Contributor

chfast commented Jun 4, 2018

Dwarfpool getWork port 80
Nanopool getWork port 8888

Both pools have also stratum support. I don't think we should care about this case.

The eth-proxy only supports getwork to communicate with the workers?

Can more abstraction in PoolConnection help here? I mean can we abstract the logic of when solution are delivered from the protocol used? That could help both getwork and code quality in general.

@AndreaLanfranchi
Copy link
Collaborator Author

The eth-proxy only supports getwork to communicate with the workers?

Yes. As they state eth-proxy it's a bridge eth_getwork => stratum
Noticeable that it's last update it's 2 years old.

Can more abstraction in PoolConnection help here?

I believe not. PoolConnection has already a good level of abstraction. It's the implementation in the derived class which sucks. For instance those two methods do nothing else than setting two variables overwriting previous values.

void EthGetworkClient::submitHashrate(string const & rate)
{
	// Store the rate in temp var. Will be handled in workLoop
	m_currentHashrateToSubmit = rate;
}

void EthGetworkClient::submitSolution(Solution solution)
{
	// Store the solution in temp var. Will be handled in workLoop
	m_solutionToSubmit = solution;
}

SubmitSolution, at least, should send immediatley the solution instead of waiting for workloop.

@jean-m-cyr
Copy link
Contributor

GetWork is definitely necessary to bind to a geth node.

AndreaLanfranchi added a commit that referenced this issue Jun 4, 2018
Addresses #1213

- Solution submit is immediate upon arrival and is no influenced by
--farm-recheck value
- Submission of hashrate is conditional
- Renamed m_report_stratum_hashrate to m_report_hashrate as it's valid
for both modes

Now even sligtly higher --farm-recheck values may be applied to mitigate
overhead.
chfast pushed a commit that referenced this issue Jun 5, 2018
Addresses #1213

- Solution submit is immediate upon arrival and is no influenced by
--farm-recheck value
- Submission of hashrate is conditional
- Renamed m_report_stratum_hashrate to m_report_hashrate as it's valid
for both modes

Now even sligtly higher --farm-recheck values may be applied to mitigate
overhead.
@AndreaLanfranchi
Copy link
Collaborator Author

Solved.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants