Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unified nicehash+eth-proxy #417

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

hackmod
Copy link

@hackmod hackmod commented Dec 27, 2018

Nicehash(EthereumStratum/1.0.0) support based on Wolfgang Frisch's working prototype.

Author: Wolfgang Frisch <[email protected]>
Date:   Mon Sep 25 19:24:09 2017 +0200

    NiceHash support: working prototype

unified nicehash + ethproxy and cleanup by @hackmod

  • rebased and squashed
  • unified processShare*()
  • unified nicehash + ethproxy
  • height field added to support extended getWork()
  • support mining.extranonce.subscribe for nicehash
  • fixed crash and cleanup
  • remove DifficultyNiceHash using DiffFloatToInit()
  • remove all nicehash specific options
  • BUG: do not modify shareDiff to show hashrate correctly
  • check len(params)
  • Do not terminate connection if miner submits one duplicate share (by nicehashdev, manually applied by hackmod)

Note

it use modified version of the ethash lib. https://github.com/hackmod/ethereum-ethash

this work supported by Ethersocial Network (ESN)
see also Ethersocial/Proposals#15
cc: @kimmyeonghun @magnalucus

@duyk16
Copy link

duyk16 commented Dec 27, 2018

Your work is great 👍

@hackmod
Copy link
Author

hackmod commented Jan 14, 2019

@sammy007 // hello~
thanks for your great project!

if you have any interest or some free time to review this code, please let me know or I will make a temporal repo to support both nicehash(EthereumStratum/1.0.0) and EthereumStratum/2.0.0 for gangnam testnet. (I'm already prepared for the next PR to support EthereumStratum/2.0.0)

@aemet93
Copy link

aemet93 commented Jan 22, 2019

Hello @hackmod
I am trying to use your pull request, but no success. Have you tested it with real nicehash workers? Worker just connected and nothing more happened. It never creates session, submit work etc. Even if I tried to run your code without changes or merges to my code. It says Nicehash subscribe + ip and nothing till I shut down order. Then it says Client disconnected. Could you please help?
thank you in advance.

@hackmod
Copy link
Author

hackmod commented Feb 10, 2019

@aemet93 // sorry for late answer.
it works nicely with my local pool setup.
what is your setup or any error log?

@Ingham27
Copy link

@aemet93

type JSONRpcResp struct {
Id json.RawMessage json:"id"
Version string json:"jsonrpc,omitempty"
Result interface{} json:"result"
Error interface{} json:"error,omitempty"
}

delete omitempty in Error

type JSONRpcResp struct {
Id json.RawMessage json:"id"
Version string json:"jsonrpc,omitempty"
Result interface{} json:"result"
Error interface{} json:"error"
}

works

@sammy007
Copy link
Owner

sammy007 commented Feb 23, 2019

Could you squash? Using develop branch of course.

hackmod and others added 2 commits February 24, 2019 03:41
Author: Wolfgang Frisch <[email protected]>
Date:   Mon Sep 25 19:24:09 2017 +0200

    NiceHash support: working prototype

unified nicehash + ethproxy and cleanup by hackmod
 * rebased and squashed
 * unified processShare*()
 * unified nicehash + ethproxy
 * height field added to support extended getWork()
 * support mining.extranonce.subscribe for nicehash
 * fixed crash and cleanup
 * remove DifficultyNiceHash using DiffFloatToInit()
 * remove all nicehash specific options
 * BUG: do not modify shareDiff to show hashrate correctly
 * check len(params)
 * Do not terminate connection if miner submits one duplicate share (by nicehashdev, manually applied by hackmod)
 * fixed stale job bug using cached stale jobs.
 * BUG: old behavior always drop stale jobs
 * sendJob() again after stale share found.
 * cache stale jobs info.
 * use cached headHash/seedHash from stales cache
 * reuse Compute() result instead of calling Verfiy() again
 * confirm uniq Extranonce
 * set ExtranonceSub session flag to support non extranonce.subscribe capable miners for NiceHash
 * check the length of params
 * always calc actual difficulty
 * debug config option added to toggle some logs
 * disable some log conditionally

Co-authored-by: Wolfgang Frisch <[email protected]>
@mikeyb
Copy link

mikeyb commented Feb 23, 2019

FYI, you should be able to squash on merge. Just hit the dropdown arrow on the merge button

@sammy007
Copy link
Owner

I just wanna look into it and it's easier to read it all at once.

@neptunix
Copy link

Hi there! I've tried to run it with nicehash and the first thing i've encountered was nonce size mismatch:

	// 4/1/2019 12:07:46 AM2019/03/31 21:07:46 Malformed PoW result from [email protected]
	//  nonce: 0xeee00d4b765f
	//  hash1: 0xfffb566c75d59b8bbaf7a4bfaa4e41c83ea1498ef3202f671b38fa61d1bde016
	//  hash2: 0x271ec813d6c4f8c53d526488cb51abc749bbb818dd6fcad76a80a334ccc7c7c1

it indeed does not match the regex (it's len is 12 instead of 16).
var noncePattern = regexp.MustCompile("^0x[0-9a-f]{16}$")

If I skip that check, I get the following error:

		4/1/2019 12:41:08 AM2019/03/31 21:41:08 share difficulty too low, 0.000000 < 260546931191, from [email protected]

And that's probably related to nonce length. Maybe I should add leading 0x0 to the nonce value?

@hackmod
Copy link
Author

hackmod commented Mar 31, 2019

Hi!

extranonce (randomHex(4)) + nonce(12 len hex string) makes total 16 bytes len.

(extranonce)
185febe#diff-40ea75f4bd2f5f500a0cc37a432246faR551

(nonce = extranonce + param[2](miner nonce from miner))
185febe#diff-40ea75f4bd2f5f500a0cc37a432246faR265

I guess the extranonce was missing or not properly prepended at pool side.

Hi there! I've tried to run it with nicehash and the first thing i've encountered was nonce size mismatch:

	// 4/1/2019 12:07:46 AM2019/03/31 21:07:46 Malformed PoW result from [email protected]
	//  nonce: 0xeee00d4b765f
	//  hash1: 0xfffb566c75d59b8bbaf7a4bfaa4e41c83ea1498ef3202f671b38fa61d1bde016
	//  hash2: 0x271ec813d6c4f8c53d526488cb51abc749bbb818dd6fcad76a80a334ccc7c7c1

it indeed does not match the regex (it's len is 12 instead of 16).
var noncePattern = regexp.MustCompile("^0x[0-9a-f]{16}$")

If I skip that check, I get the following error:

		4/1/2019 12:41:08 AM2019/03/31 21:41:08 share difficulty too low, 0.000000 < 260546931191, from [email protected]

And that's probably related to nonce length. Maybe I should add leading 0x0 to the nonce value?

@neptunix
Copy link

neptunix commented Apr 1, 2019

Hi!

Hi, thanks for the reply!

I guess the extranonce was missing or not properly prepended at pool side.

I've dumped the beginning of the communication, could you please give me a hint if something looks wrong here...

4/2/2019 12:13:15 AM004★ new client from 10.42.198.17:59524
4/2/2019 12:13:15 AM004✓ connected to backend
4/2/2019 12:13:15 AM004→ {"id":1,"method":"mining.subscribe","params":["NiceHash/1.0.0","EthereumStratum/1.0.0"]}<0a>
4/2/2019 12:13:15 AM004← {"id":1,"result":[["mining.notify","b9374b251df5b4ed","EthereumStratum/1.0.0"],"3968"],"error":null}<0a>
4/2/2019 12:13:15 AM004→ {"id":2,"method":"mining.authorize","params":["0x3F390F01C44aFe91A40cE9C6Fd03bf7436A0ddC0","x"]}<0a>
4/2/2019 12:13:15 AM004← {"id":2,"result":true,"error":null}<0a>
4/2/2019 12:13:15 AM004← {"method":"mining.set_difficulty","params":[1.1641354547009541],"id":null}<0a>
4/2/2019 12:13:15 AM004← {"method":"mining.notify","params":["32dbbb46","fffb566c75d59b8bbaf7a4bfaa4e41c83ea1498ef3202f671b38fa61d1bde016","7af88f0ffa1444f34a34b7ed37f03030a0c99bd2df499bb50cfae807da6f2033",true],"id":null}<0a>
4/2/2019 12:13:24 AM004→ {"id":3,"method":"mining.submit","params":["0x3F390F01C44aFe91A40cE9C6Fd03bf7436A0ddC0","32dbbb46","4ec000558d7a"]}<0a>
4/2/2019 12:13:25 AM004← {"id":3,"result":false,"error":null}<0a>
4/2/2019 12:13:26 AM004→ {"id":4,"method":"mining.submit","params":["0x3F390F01C44aFe91A40cE9C6Fd03bf7436A0ddC0","32dbbb46","4ec0046b64b5"]}<0a>
4/2/2019 12:13:26 AM004← {"id":4,"result":false,"error":null}<0a>

Backend skipped the wrong nonce length (len 12) and failed with the incorrect difficulty error in this case

3968 - is the extra nonce here, right?

@hackmod
Copy link
Author

hackmod commented Apr 1, 2019

Hi!

Hi, thanks for the reply!

I guess the extranonce was missing or not properly prepended at pool side.

I've dumped the beginning of the communication, could you please give me a hint if something looks wrong here...

4/2/2019 12:13:15 AM004★ new client from 10.42.198.17:59524
4/2/2019 12:13:15 AM004✓ connected to backend
4/2/2019 12:13:15 AM004→ {"id":1,"method":"mining.subscribe","params":["NiceHash/1.0.0","EthereumStratum/1.0.0"]}<0a>
4/2/2019 12:13:15 AM004← {"id":1,"result":[["mining.notify","b9374b251df5b4ed","EthereumStratum/1.0.0"],"3968"],"error":null}<0a>
4/2/2019 12:13:15 AM004→ {"id":2,"method":"mining.authorize","params":["0x3F390F01C44aFe91A40cE9C6Fd03bf7436A0ddC0","x"]}<0a>
4/2/2019 12:13:15 AM004← {"id":2,"result":true,"error":null}<0a>
4/2/2019 12:13:15 AM004← {"method":"mining.set_difficulty","params":[1.1641354547009541],"id":null}<0a>
4/2/2019 12:13:15 AM004← {"method":"mining.notify","params":["32dbbb46","fffb566c75d59b8bbaf7a4bfaa4e41c83ea1498ef3202f671b38fa61d1bde016","7af88f0ffa1444f34a34b7ed37f03030a0c99bd2df499bb50cfae807da6f2033",true],"id":null}<0a>
4/2/2019 12:13:24 AM004→ {"id":3,"method":"mining.submit","params":["0x3F390F01C44aFe91A40cE9C6Fd03bf7436A0ddC0","32dbbb46","4ec000558d7a"]}<0a>
4/2/2019 12:13:25 AM004← {"id":3,"result":false,"error":null}<0a>
4/2/2019 12:13:26 AM004→ {"id":4,"method":"mining.submit","params":["0x3F390F01C44aFe91A40cE9C6Fd03bf7436A0ddC0","32dbbb46","4ec0046b64b5"]}<0a>
4/2/2019 12:13:26 AM004← {"id":4,"result":false,"error":null}<0a>

Backend skipped the wrong nonce length (len 12) and failed with the incorrect difficulty error in this case

3968 - is the extra nonce here, right?

yes right. the extranonce seems correctly responsed by pool server.

{"id":3,"method":"mining.submit","params":["0x3F390F01C44aFe91A40cE9C6Fd03bf7436A0ddC0","32dbbb46","4ec000558d7a"]}
so, for this case, pool server got 3968 + 4ec000558d7a (parms[2]) as 16 bytes len accepted nonce.

this PR already handle it. (I guess you are trying to make your own fix or pool server)

@neptunix
Copy link

neptunix commented Apr 2, 2019

Thanks, @hackmod.

As I see from the code

			// check Extranonce subscription.
			extranonce := cs.Extranonce
			if !cs.ExtranonceSub { extranonce = "" }
			nonce := extranonce + params[2]

the extranonce is not concatenated because cs.ExtranonceSub is false.
and it's set in mining.extranonce.subscribe which was not called by nicehash in my case

case "mining.extranonce.subscribe":
			var params []string
			err := json.Unmarshal(req.Params, &params)
			if err != nil {
				return errors.New("invalid params")
			}
			if len(params) == 0 {
				if err := cs.sendStratumResult(req.Id, true); err != nil {
					return err
				}
				cs.ExtranonceSub = true
				req := JSONStratumReq{
					Id:     nil,
					Method: "mining.set_extranonce",
					Params: []interface{}{
						cs.Extranonce,
					},
				}
				return cs.sendTCPReq(req)
			}
			return cs.sendStratumError(req.Id, []string{
				"20",
				"Not supported.",
			})

Do I miss something else?

this PR already handle it. (I guess you are trying to make your own fix or pool server)
Yes, I'm trying to run the pool with the code from your PR.

Thanks

@hackmod
Copy link
Author

hackmod commented Apr 2, 2019

Thanks, @hackmod.

As I see from the code

			// check Extranonce subscription.
			extranonce := cs.Extranonce
			if !cs.ExtranonceSub { extranonce = "" }
			nonce := extranonce + params[2]

the extranonce is not concatenated because cs.ExtranonceSub is false.
and it's set in mining.extranonce.subscribe which was not called by nicehash in my case

claymore + nicehash mode or ethminer + nicehash mode always call mining.extranonce.subscribe.

how about to apply the following fix?

diff --git a/proxy/stratum.go b/proxy/stratum.go
index fda127c..10fb42a 100644
--- a/proxy/stratum.go
+++ b/proxy/stratum.go
@@ -165,6 +165,7 @@ func (cs *Session) handleTCPMessage(s *ProxyServer, req *StratumReq) error {
                        return cs.sendStratumError(req.Id, "unsupported stratum version")
                }

+               cs.ExtranonceSub = true
                cs.setStratumMode("EthereumStratum/1.0.0")
                log.Println("Nicehash subscribe", cs.ip)
                result := cs.getNotificationResponse(s)
case "mining.extranonce.subscribe":
			var params []string
			err := json.Unmarshal(req.Params, &params)
			if err != nil {
				return errors.New("invalid params")
			}
			if len(params) == 0 {
				if err := cs.sendStratumResult(req.Id, true); err != nil {
					return err
				}
				cs.ExtranonceSub = true
				req := JSONStratumReq{
					Id:     nil,
					Method: "mining.set_extranonce",
					Params: []interface{}{
						cs.Extranonce,
					},
				}
				return cs.sendTCPReq(req)
			}
			return cs.sendStratumError(req.Id, []string{
				"20",
				"Not supported.",
			})

Do I miss something else?

this PR already handle it. (I guess you are trying to make your own fix or pool server)

Yes, I'm trying to run the pool with the code from your PR.

Thanks

ExtranonceSub := false by default to supprt non extranonce.subscribe capable miners for NiceHash.

Please see EthersocialNetwork@e2f191a

@neptunix
Copy link

neptunix commented Apr 2, 2019

Thank you. I got the point.
I think I'd better do it that way:
if (the nonce length == 12 && extranonce is enabled ) => concatenate the values

@StayClassic
Copy link

any update on this?

@aemet93
Copy link

aemet93 commented Jun 20, 2019

Hello all again.
Everything works with this fixes, but we have another problem, which we cannot identify. It is delta.
I have checked everything many times, all share way from nicehash to our pool, verification and till database. We have stable -15% delta. All recommendations from nicehash support, such as connection, stable uptime etc. was complied. Our pool have very little amount of stale shares, it is much more with regular miners (not from nicehash), and it is ok. Strange thing, that nicehash does not see any rejection from our side, but surely says that speed at pool 15% less than we paying. Will be much thankful for any ideas, tips, and advices, because it becomes real problem for us. Thank you very much.

Screen Shot 2019-06-20 at 15 35 10

Screen Shot 2019-06-20 at 15 35 30

@korjavin
Copy link

We have seen this delta before with another implementation of NH protocol.

@aemet93
Copy link

aemet93 commented Jun 21, 2019

Also, maybe anybody know what is this "old stratum" version? I got this message when try to mine on nanopool (this pool works good and doesn't have delta).
Screen Shot 2019-06-21 at 14 51 55

@aemet93
Copy link

aemet93 commented Jun 24, 2019

If anyone interested, continue debugging this stuff, I assume that not all shares comes to pool. I have logged all shares, and for 100% share pool respond to NH as true. Speed calculation is obviously based on shares amount, so looks like NH send more shares than we see on pool. Now most interesting question is where is this lost shares?.. I will give a report here, if will find something interesting.

@hackmod
Copy link
Author

hackmod commented Jun 25, 2019

this is the original unified nicehash-v1 branch https://github.com/EthersocialNetwork/open-ethereum-pool/tree/unified-nicehash-v1-original

I guess, lost shares may be caused by the following code. (original NH fix always drop these shares)
EthersocialNetwork@254d409

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

Successfully merging this pull request may close these issues.

9 participants