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.

Sign up for GitHub

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

Jump to bottom

pool: Use regular expressions to identify clients. #406

Merged
merged 2 commits into from
Oct 13, 2023
Merged

pool: Use regular expressions to identify clients. #406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
44438b1
pool: Rename miner_id.go to minerid.go.
davecgh Oct 11, 2023
13bba52
pool: Use regular expressions to identify clients.
davecgh Oct 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions pool/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,9 @@ func (c *Client) handleAuthorizeRequest(req *Request, allowed bool) error {
// monitor periodically checks the miner details set against expected
// incoming submission tally and upgrades the miner if possible when the
// submission tallies exceed the expected number by 30 percent.
func (c *Client) monitor(idx int, pair *minerIDPair, monitorCycle time.Duration, maxTries uint32) {
func (c *Client) monitor(idx int, clients []string, monitorCycle time.Duration, maxTries uint32) {
var subs, tries uint32
if len(pair.miners) <= 1 {
if len(clients) <= 1 {
// Nothing to do if there are no more miner ids to upgrade to.
return
}
Expand All @@ -308,7 +308,7 @@ func (c *Client) monitor(idx int, pair *minerIDPair, monitorCycle time.Duration,

select {
case <-ticker.C:
if idx == len(pair.miners)-1 {
if idx == len(clients)-1 {
// No more miner upgrades possible.
return
}
Expand All @@ -335,7 +335,7 @@ func (c *Client) monitor(idx int, pair *minerIDPair, monitorCycle time.Duration,
// Update the miner's details and send a new mining.set_difficulty
// message to the client.
c.mtx.Lock()
miner := pair.miners[idx]
miner := clients[idx]
newID := fmt.Sprintf("%v/%v", c.extraNonce1, miner)
log.Infof("upgrading %s to %s", c.id, newID)
info, err := c.cfg.FetchMinerDifficulty(miner)
Expand Down Expand Up @@ -375,16 +375,16 @@ func (c *Client) handleSubscribeRequest(req *Request, allowed bool) error {
return errs.PoolError(errs.LimitExceeded, err.Error())
}

mid, nid, err := ParseSubscribeRequest(req)
userAgent, nid, err := ParseSubscribeRequest(req)
if err != nil {
sErr := NewStratumError(Unknown, err)
resp := SubscribeResponse(*req.ID, "", "", 0, sErr)
c.sendMessage(resp)
return err
}

// Identify the miner and fetch needed mining information for it.
idPair, err := identifyMiner(mid)
// Identify the mining client and fetch needed mining information for it.
clients, err := identifyMiningClients(userAgent)
if err != nil {
sErr := NewStratumError(Unknown, err)
resp := SubscribeResponse(*req.ID, "", "", 0, sErr)
Expand All @@ -394,7 +394,7 @@ func (c *Client) handleSubscribeRequest(req *Request, allowed bool) error {

c.mtx.Lock()
minerIdx := 0
miner := idPair.miners[minerIdx]
miner := clients[minerIdx]
info, err := c.cfg.FetchMinerDifficulty(miner)
if err != nil {
c.mtx.Unlock()
Expand All @@ -413,7 +413,7 @@ func (c *Client) handleSubscribeRequest(req *Request, allowed bool) error {
nid = fmt.Sprintf("mn%v", c.extraNonce1)
}

go c.monitor(minerIdx, idPair, c.cfg.MonitorCycle, c.cfg.MaxUpgradeTries)
go c.monitor(minerIdx, clients, c.cfg.MonitorCycle, c.cfg.MaxUpgradeTries)

var resp *Response
switch miner {
Expand Down
21 changes: 6 additions & 15 deletions pool/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,9 @@ var (
MaxUpgradeTries: 5,
RollWorkCycle: rollWorkCycle,
}
userAgent = func(miner, version string) string {
return fmt.Sprintf("%s/%s", miner, version)
}
)

func splitMinerID(id string) (string, string) {
const separator = "/"
split := strings.Split(id, separator)
return split[0], split[1]
}
const cpuUserAgent = "cpuminer/1.0.0"

func setCurrentWork(work string) {
currentWorkMtx.Lock()
Expand Down Expand Up @@ -380,8 +373,7 @@ func testClientMessageHandling(t *testing.T) {
}

id++
cpu, cpuVersion := splitMinerID(cpuID)
r = SubscribeRequest(&id, userAgent(cpu, cpuVersion), "")
r = SubscribeRequest(&id, cpuUserAgent, "")
err = sE.Encode(r)
if err != nil {
t.Fatalf("[Encode] unexpected error: %v", err)
Expand Down Expand Up @@ -916,8 +908,7 @@ func testClientTimeRolledWork(t *testing.T) {
// Ensure a CPU client receives a valid non-error response when
// a valid subscribe request is sent.
id++
cpu, cpuVersion := splitMinerID(cpuID)
r = SubscribeRequest(&id, userAgent(cpu, cpuVersion), "")
r = SubscribeRequest(&id, cpuUserAgent, "")
err = sE.Encode(r)
if err != nil {
t.Fatalf("[Encode] unexpected error: %v", err)
Expand Down Expand Up @@ -1055,12 +1046,12 @@ func testClientUpgrades(t *testing.T) {
}

const minerIdx = 0
idPair := newMinerIDPair(cpuID, CPU, clientCPU2)
clients := []string{CPU, clientCPU2}

// Trigger a client upgrade.
atomic.StoreInt64(&client.submissions, 50)

go client.monitor(minerIdx, idPair, cfg.MonitorCycle, cfg.MaxUpgradeTries)
go client.monitor(minerIdx, clients, cfg.MonitorCycle, cfg.MaxUpgradeTries)
time.Sleep(cfg.MonitorCycle + (cfg.MonitorCycle / 2))

if fetchMiner(client) != clientCPU2 {
Expand Down Expand Up @@ -1088,7 +1079,7 @@ func testClientUpgrades(t *testing.T) {

atomic.StoreInt64(&client.submissions, 2)

go client.monitor(minerIdx, idPair, cfg.MonitorCycle, cfg.MaxUpgradeTries)
go client.monitor(minerIdx, clients, cfg.MonitorCycle, cfg.MaxUpgradeTries)
time.Sleep(cfg.MonitorCycle + (cfg.MonitorCycle / 2))

if fetchMiner(client) == CPU {
Expand Down
65 changes: 0 additions & 65 deletions pool/miner_id.go
View file
Open in desktop

This file was deleted.

59 changes: 59 additions & 0 deletions pool/minerid.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright (c) 2020-2023 The Decred developers
// Use of this source code is governed by an ISC
// license that can be found in the LICENSE file.

package pool

import (
"fmt"
"regexp"

errs "github.com/decred/dcrpool/errors"
)

// newUserAgentRE returns a compiled regular expression that matches a user
// agent with the provided client name, major version, and minor version as well
// as any patch, pre-release, and build metadata suffix that are valid per the
// semantic versioning 2.0.0 spec.
//
// For reference, user agents are expected to be of the form "name/version"
// where the name is a string and the version follows the semantic versioning
// 2.0.0 spec.
func newUserAgentRE(clientName string, clientMajor, clientMinor uint32) *regexp.Regexp {
// semverBuildAndMetadataSuffixRE is a regular expression to match the
// optional pre-release and build metadata portions of a semantic version
// 2.0 string.
const semverBuildAndMetadataSuffixRE = `(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-]` +
`[0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?` +
`(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?`

return regexp.MustCompile(fmt.Sprintf(`^%s\/%d\.%d\.(0|[1-9]\d*)%s$`,
clientName, clientMajor, clientMinor, semverBuildAndMetadataSuffixRE))
}

var (
// These regular expressions are used to identify the expected mining
// clients by the user agents in their mining.subscribe requests.
cpuRE = newUserAgentRE("cpuminer", 1, 0)
nhRE = newUserAgentRE("NiceHash", 1, 0)

// miningClients maps regular expressions to the supported mining client IDs
// for all user agents that match the regular expression.
miningClients = map[*regexp.Regexp][]string{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
miningClients = map[*regexp.Regexp][]string{
miningClients = map[*regexp.Regexp]string{

I think this change would serve to simplify things quite a bit. Maybe there is a reason for it, but I don't see why a single ID would need to be mapping to multiple miner types. We can add support for that later if it ever becomes necessary.

Copy link
Member Author

@davecgh davecgh Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left it that way because there is infrastructure for upgrading based on the hash rate. There is nothing forcing two different devices to use a different user agent, and as a result, you can have multiple devices with wildly different hashrates identifying themselves the same.

There are tests which cover the upgrading as well (see testClientUpgrades).

Copy link
Member Author

@davecgh davecgh Oct 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, I think that ideally the pool really generally shouldn't even be assigning hash rates and instead should figure it out dynamically. It would be similar to how the client upgrade code is doing for devices with the same UA, but instead it would allow it to become more or less difficult depending on share submission rate.

Then, it could assign some initial hash rates per UA and even better would be for the protocol to allow clients to specify it themselves (which is unfortunately not part of stratum). Naturally, it would need to be sanity checked and only serve as an initial starting point for the aforementioned dynamic behavior.

However, that is a fair amount of work that doesn't seem worth it given the goal at the moment is primarily getting into shape for BLAKE3 and gominer as opposed to adding new features.

cpuRE: {CPU},
nhRE: {NiceHashValidator},
}
)

// identifyMiningClients returns the possible mining client IDs for a given user agent
// or an error when the user agent is not supported.
func identifyMiningClients(userAgent string) ([]string, error) {
for re, clients := range miningClients {
if re.MatchString(userAgent) {
return clients, nil
}
}

msg := fmt.Sprintf("connected miner with id %s is unsupported", userAgent)
return nil, errs.PoolError(errs.MinerUnknown, msg)
}
Loading
Loading