sammy007 / open-ethereum-pool

Open Ethereum Mining Pool
GNU General Public License v3.0
1.4k stars 1.12k forks source link

Your stratum implementation does not fit my business requirements #42

Closed kenshirothefist closed 8 years ago

kenshirothefist commented 8 years ago

You seem to be using awkward and broken stratum implementation. Is your stratum implementation (https://github.com/sammy007/open-ethereum-pool/blob/master/docs/STRATUM.md) meant to be compatible with Dwarf's eth-proxy? It seems to be something in between ... Can you standardize it or if possible include this stratum implementation?

sammy007 commented 8 years ago

Thanks for kind words. Since there is not IETF RFC of "stratum" I reverse-engineered eth-proxy's proto - the one and only "stratum" on a moment of pool release.

sammy007 commented 8 years ago

Can you standardize it

What does it mean? There is a description included that you linked yourself.

UPDATE: yes, it's compatible with dwarf and not in between.

kenshirothefist commented 8 years ago

Well, at NiceHash we evaluated all "stratum" variants, identified several weakness and proposed proper stratum for Ethereum (and Ethereum altcoins) ... this is what this: https://github.com/nicehash/Specifications/blob/master/EthereumStratum_NiceHash_v1.0.0.txt is for (please read "I. Introduction" ... you'll see that it makes sense).

Genoil and Claymore both now supports this stratum implementation and it would be great if your pool would also support it.

btw: sorry for the "harsh not-so kind words" in issue report, but it really is a headache with all these stratum implementations and it makes so much issues in the whole Ethereum ecosystem ... anyway, keep up the good work and hopefully you can implement new stratum.

sammy007 commented 8 years ago

I have no idea why you are rising this question again, because I am well-informed about your proposal via email.

Surprise, someone is working on yet another stratum implementation.

Regarding your spec, I know about it, maybe I will implement it. Meanwhile both genoil and claymore's blob support my implementation of dwarf.

I don't work for free especially in crypto projects for someone's business benefits, currently only nicehash multipool has high interest in subject for obvious reasons.

kenshirothefist commented 8 years ago

btw: any idea why your pool would assign insanely high diff to workers (for example 26147370000.00000000)?

pool assigns difficulty specified in config file, if it's 4,000,000,000 it will be 4,000,000,000.

fluffypony commented 8 years ago

xkcd

kenshirothefist commented 8 years ago

btw: any idea why your pool would assign insanely high diff to workers (for example 26147370000.00000000)?

This was related to an issue where pool would not send full hex data ... missing zeros at the begging ... anyway, pools based on this open-source pool are now compatible with NiceHash.

sammy007 commented 8 years ago

Do you have a canonical hex and int? I will write a test case. I use common.ToHex from ethereum repo.

LeChuckDE commented 8 years ago

Sammy, I was trying to contact nicehash, but they are not answered, so my decision is to NOT implement a stratum protocol for free where only nicehash profits on. have a working implementation. Only will implement Stratum V2, V1 eth-proxy-mode (your implementation) works like a charm, but get sometimes reconnect issues with genoil and claymore miner, but think that is a problem of submit-hashrate from miner, V2 will fix that. My question was only to get a free testing. But now answer no implementation ...

Nicehash, try to support one of the supported Stratum Protocols and not build a own one, your are not big enough to rule the crypto-world. Or pay for implementation !

bye the way Nicehash DON'T SUPPORT this stratum implementation, yesterday get 2 request why mining not possible, identified it was nicehash rented hashpower that will not connect ...

kenshirothefist commented 8 years ago

Nicehash, try to support one of the supported Stratum Protocols and not build a own one, your are not big enough to rule the crypto-world. Or pay for implementation !

NiceHash supports several stratum variants:

0- the "early stratum" (it was used by ethpool, ethermine, coinotron, mph, nanopool) 1.1- the eth-proxy (aka dwarfpool) style (used by dwarfpool and some other pools) 1.2- the eth-proxy-sammy-reversed-engineered (used by all pools, based on this open-source sw that we're talking here) 2- the standardized EthereumStratum/1.0.0: this is the preferred one, supported by coinotron, hitchpool, etc.)

@LeChuckDE, please take a few minutes and read "I. Introduction" section in the specifications. You'll see that the specification that we introduced is NOT yet another specification, but it is:

  1. an actual, RFC-like documented specification
  2. strictly follows original stratum documentation https://slushpool.com/help/#!/manual/stratum-protocol
  3. fixes several issues in previous Ethereum stratum implementations

bye the way Nicehash DON'T SUPPORT this stratum implementation, yesterday get 2 request why mining not possible, identified it was nicehash rented hashpower that will not connect ...

We added support for the eth-proxy-sammy-reversed-engineered variant only approximate 10 hours ago ... so you might want to try again.

Also, @LeChuckDE if you didn't get answer from our support team then please write again tu support@nicehash.com, we'll make sure you'll get all the support needed.

nicehashdev commented 8 years ago

We got this reply from you, when we asked you the reasons for pool terminating connection:

It disconnect on duplicate share, on any malformed request, on stratum timeout, if limits are enforced, if bad shares threshold is reached. In other words it disconnects if there is any response from "stratum" which contains "error" field.

So, if we send a share, that you consider duplicate, you will terminate connection, rather than send response back, that share is invalid? This is very strange behaviour... especially considering that all workers are working on same job and duplicates will happen, it is inevitable. Unless you use EthereumStratum protocol, which prevents miners from ever mining duplicate work.

nicehashdev commented 8 years ago

I quickly analysed your code and yes, it seems like you really drop con if there is any kind of invalid share... Mind changing that to just 'report back invalid share', but keep connection?

LeChuckDE commented 8 years ago

@nicehashdev nope it is configureable at how many % bad shares they client will disconnected and blacklisted for time N ... this is:

  1. A DDOS Protection (Maleformate Share-Flood-Attack)
  2. A Protection against multible submit share attack

normal protection scenario that is implemeted in so mutch stratum implementations. All other renting-plattforms work.

sammy007 commented 8 years ago

This is very strange behaviour...

If miner is malicious and trying to flood pool with duplicate shares in order to screw other miners and get free coins it must be disconnected and banned. If I ban miner via firewall and don't disconnect him ban will not take effect.

sammy007 commented 8 years ago

I quickly analysed your code and yes, it seems like you really drop con if there is any kind of invalid share...

Only if threshold reached. This is signal that miner is malicious or broken. Such miners must be banned and disconnected.

LeChuckDE commented 8 years ago

@sammy007 ,nicehash never think about pool security, only at keep it easy for cloudminers ... this is why there is a big nope to implement nicehash stratum protocol.

Edit: yes it is the correct Title now !

sammy007 commented 8 years ago

Original title: Awkward and broken stratum implementation

I just edited title to clarify real problem of this ticket.

sammy007 commented 8 years ago

And also, the policy module born from my 2 years experience of running pool when we suffered from all kind of attacks like several duplicate shares vulnerabilities, constant bad share flood which make CPU load 100%, etc. Better safe than regret, if you think this is bad, disable policies or even alter handlers.go to not return any errors during share processing and just return nil. There is already a userbase unfortunately consist of not such experienced hardened admins and I want them to be a little bit safe, if you know what you do, alter behaviour and go with it.

sammy007 commented 8 years ago

I see someone implemented NH stratum version, I will merge it to mainline if it works, will take some time because I am busy and can't dedicate 24h for testing it and mining thousands of testnet blocks.

nicehashdev commented 8 years ago

sammy007, we went through all kind of attacks you just described, 2 years ago already. We have like a special system for handling all kind of rogue behaviour of miners and even pools. I understand your concerns. We had same concerns regarding DaggerHashimoto when we checked existing mining protocols (like this one, which is used by your pool). And you know what we found out? That miners may cheat and submit duplicate shares... This was a no-go for us. And we didn't implement it, but rather create new specifications that would eliminate this problem. We did not create specifications to fit our business, but rather made "stratum" that deserves being called stratum, because stratum that does not prevent duplicate work by it's design cannot be called stratum and original developer of stratum (slush) would be disgraced to what extent the name of his protocol is used for.

I tripple checked your code. I am not familiar with GO, but tracing your code, here some error is set https://github.com/sammy007/open-ethereum-pool/blob/master/proxy/stratum.go#L56 if share is being rejected, thus connection gets dropped. I do not see connection being dropped only if X rogue shares are being received, but if any invalid share is being received.

sammy007 commented 8 years ago

(like this one, which is used by your pool)

So you claim that miners can submit duplicate shares on this pool?

Every PoW result is logged and each new submission is checked if exists in a log.

sammy007 commented 8 years ago

I tripple checked your code. I am not familiar with GO, but I tracing your code, here some error is set https://github.com/sammy007/open-ethereum-pool/blob/master/proxy/stratum.go#L56 if share is being rejected

Usually on invalid share this err is nil and only not nil when policy action applied.

nicehashdev commented 8 years ago

****So you claim that miners can submit duplicate shares on this pool?

No, I am saying that specifications allow this - there is no mechanism to prevent it, because each miner is working on same job. With proper stratum, this is not possible, because each miner gets own unique work space.

As for code; here is sending back something, that is not nil, right? https://github.com/sammy007/open-ethereum-pool/blob/master/proxy/stratum.go#L169 And this is called in case of invalid share considering this: https://github.com/sammy007/open-ethereum-pool/blob/master/proxy/stratum.go#L133 and this: https://github.com/sammy007/open-ethereum-pool/blob/master/proxy/handlers.go#L77

nicehashdev commented 8 years ago

Yep, I submitted a pull...

sammy007 commented 8 years ago

there is no mechanism to prevent it

Not true. I just described mechanism to prevent it.

nicehashdev commented 8 years ago

Specification mechanism :) There has to be one, if it is possible for one to exist. Else, we can say these are bad specifications.

sammy007 commented 8 years ago

As for code; here is sending back something, that is not nil, right?

error only if policy hit or an attempt to submit duplicate, under normal circumstances if bad share submitted there is just false, nil.

sammy007 commented 8 years ago

Ok, but PR will be pending for a while cuz I have now working environment right now to run and test everything.

I wonder what is probability of duplicate to arrive under normal mining process.

nicehashdev commented 8 years ago

But we can see how often these duplicates happen... We had an arguing at ethereum forum already; people were claiming that miners under normal circumstances (not trying to cheat intentionally) do not submit duplicates and that chances of it happening are so slim that there is higher possibility of universe suddenly dying... well... looks like this is not true as all pools using your code are constantly terminating connection to our service, especially when orders reach higher speeds.

sammy007 commented 8 years ago

I still claim that people is trying to screw pool.

nicehashdev commented 8 years ago

Of course, some will always try. They try to screw us constantly from day one, but first line of defence is always good specifications that cannot be cheated by definition.

sammy007 commented 8 years ago

good specifications that cannot be cheated by definition

There is no much difference, you validate local duplicates to the miner, other spec requires global submission validations.

nicehashdev commented 8 years ago

Oh there is... the question is not where you validate share, but whether specifications give each miner unique work space or not. And if miners do mine same work... then your pools is also less efficient and by not rewarding miners for accidental duplicate shares, you are putting this unefficiency to miners; for example, a miner with 100mhs would actually be efficient only for 98mhs.

sammy007 commented 8 years ago

The nonce is wide enough for mid-size pools. I have stats from several pools where stats for several thousands blocks are 100% shares/diff which is excellent efficiency.

I am not against your variant, as I said I will prolly merge it, but not this week. I took the popular one proto and implemented because it was the one and only at the moment of pool publishing and there was not miner with another impl.

LeChuckDE commented 8 years ago

@sammy007 . @nicehashdev have implement the push manually at exp.coinpool.biz (expanse), etc.coinpool.biz (Ethereum Classic) and eth.coinpool.biz (Ethereum HF) feel free to test my deposit is in transit and don't look like that i am able to test it in next hrs ...

would prefer to use eth.coinpool.biz because at time no miner connected to it.

sammy007 commented 8 years ago

@LeChuckDE not now, not this week actually. BTW can you add extradata to HF how I ask in wiki?

LeChuckDE commented 8 years ago

@sammy007

extraData: have done on all nodes i am running (ETH HF). also on my testpool at home (100/50 MBit connection Down/Up) NH-Pull-Request: No, you missunderstood me, meaned not to merge the pull. I have implement it by hand on all the named pools from me and recompiled it. Test with testrig looks good and also etc and exp pool have working members.

I am amazed about the restart of open-ethereum-pool daemon (using upstart), a restart of pool daemon is so fast, clients stay connected, no interruption of mining proccess ! (no reconnect msg at client side, only pool log shows the restart and reconnect, reconnect could cause switch to next pool or DAG Rebuild, bad for miners that have not implemented build DAG in Ram)

sammy007 commented 8 years ago

clients stay connected

Not sure this is correct and not an illusion. If log show reconnect - disconnect happened. it's very weird and complicated to make graceful restarts, usually mining node should work for weeks without restart so I didn't bother myself trying to implement that.

LeChuckDE commented 8 years ago

was the thing that i have seen now at first restart with testrig connected. using genoil modified miner (own DaggerHashimoto Kernel, max optomized for HD7850)

LeChuckDE commented 8 years ago

@nicehashdev tested now for nearly 24 hrs works without interruption ... some free hash for spending 0.04 BTC in testing stratum nh pull request ?

account: mark.ernst.me ---- at ---- gmail.com

ghost commented 8 years ago

@nicehashdev everything I do, I can't get nicehash to work with epool. It works for 15 minites and cancles the order, Unfortunately i have to tell miners it's not supported.

LeChuckDE commented 8 years ago

@etherninja it works, but needs tweeking. Does over 48 hrs. testing with own payed nh hashpower 250 mh/s - 2 gh/s, 3 gh/s from nicehash directly. But they didn't held there commitment to support the testing(refund/free hashpower). So i have canceled all support of nicehash for ethereum, ethereumclassic and expanse. They tested successfully, but then no more response nor compensation.

in my opinion, nothing more or less then scammers ... only own profit counts, support for devs ? na for what, they will only make money. let other pay the debs for testing. they want to get implemented, but support for it ??? nope

never again do business with them.

kenshirothefist commented 8 years ago

@LeChuckDE Well, Mark, we said:

we would support your effort to make the pools better

then we continued testing on your ETH pool, after we helped you make it working correctly, we got this:

This ETHEREUM HARDFORK Pool is closed for ever!

http://eth.coinpool.biz/

Also, your other pools are not doing that well, too:

http://etc.coinpool.biz/ http://exp.coinpool.biz/

So, please, stop spamming this thread and posting irrelevant and misleading information.

Oh, and here is the compensation of the 0.05 BTC that you spent testing configuring your pools: 9f1162b4582e1e62e094f52eedd28cf10a745b89f0c01edcede21406864a90d1 ... hopefully you can spend it well and get at least some miners on your pools ;)

sammy007 commented 8 years ago

Advertisement removed. Go push your service somewhere else.