From 13bba5223a63c80e6e096891081b8b435f8495eb Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Tue, 10 Oct 2023 23:03:30 -0500 Subject: [PATCH] pool: Use regular expressions to identify clients. This modifies that logic that identifies mining clients by their user agent to use regular expressions instead of requiring hard coded version numbers. It also adds tests to ensure proper matching functionality. The primary motivation is to allow the pool to continue to work with new patch versions of mining clients without requiring updates the pool software. However, it also provides the flexibility to adapt to specific client software that might not conform to semver semantics in the future if is desired. --- pool/client.go | 18 ++--- pool/client_test.go | 21 ++---- pool/minerid.go | 86 +++++++++++----------- pool/minerid_test.go | 165 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 220 insertions(+), 70 deletions(-) create mode 100644 pool/minerid_test.go diff --git a/pool/client.go b/pool/client.go index 701d7648..2ad8fce5 100644 --- a/pool/client.go +++ b/pool/client.go @@ -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 } @@ -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 } @@ -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) @@ -375,7 +375,7 @@ 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) @@ -383,8 +383,8 @@ func (c *Client) handleSubscribeRequest(req *Request, allowed bool) error { 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) @@ -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() @@ -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 { diff --git a/pool/client_test.go b/pool/client_test.go index 17b3863a..79265ee5 100644 --- a/pool/client_test.go +++ b/pool/client_test.go @@ -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() @@ -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) @@ -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) @@ -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 { @@ -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 { diff --git a/pool/minerid.go b/pool/minerid.go index 536c79c3..0ddbf6b3 100644 --- a/pool/minerid.go +++ b/pool/minerid.go @@ -6,60 +6,54 @@ package pool import ( "fmt" + "regexp" errs "github.com/decred/dcrpool/errors" ) -var ( - // These miner ids represent the expected identifications returned by - // supported miners in their mining.subscribe requests. - - cpuID = "cpuminer/1.0.0" - nhID = "NiceHash/1.0.0" -) - -// minerIDPair represents miner subscription identification pairing -// between the id and the miners that identify as. -type minerIDPair struct { - id string - miners map[int]string -} - -// newMinerIDPair creates a new miner ID pair. -func newMinerIDPair(id string, miners ...string) *minerIDPair { - set := make(map[int]string, len(miners)) - for id, entry := range miners { - set[id] = entry - } - sub := &minerIDPair{ - id: id, - miners: set, - } - return sub -} - -// generateMinerIDs creates the miner id pairings for all supported miners. -func generateMinerIDs() map[string]*minerIDPair { - ids := make(map[string]*minerIDPair) - cpu := newMinerIDPair(cpuID, CPU) - nicehash := newMinerIDPair(nhID, NiceHashValidator) - - ids[cpu.id] = cpu - ids[nicehash.id] = nicehash - return ids +// 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 ( - // minerIDs represents the minder id pairings for all supported miners. - minerIDs = generateMinerIDs() + // 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{ + cpuRE: {CPU}, + nhRE: {NiceHashValidator}, + } ) -// identifyMiner determines if the provided miner id is supported by the pool. -func identifyMiner(id string) (*minerIDPair, error) { - mID, ok := minerIDs[id] - if !ok { - msg := fmt.Sprintf("connected miner with id %s is unsupported", id) - return nil, errs.PoolError(errs.MinerUnknown, msg) +// 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 + } } - return mID, nil + + msg := fmt.Sprintf("connected miner with id %s is unsupported", userAgent) + return nil, errs.PoolError(errs.MinerUnknown, msg) } diff --git a/pool/minerid_test.go b/pool/minerid_test.go new file mode 100644 index 00000000..a0127258 --- /dev/null +++ b/pool/minerid_test.go @@ -0,0 +1,165 @@ +// Copyright (c) 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" + "testing" +) + +// TestNewUserAgentRE ensures the mining client user agent regular-expression +// matching logic works as intended. +func TestNewUserAgentRE(t *testing.T) { + // perRETest describes a test to run against the same regular expression. + type perRETest struct { + clientUA string // user agent string to test + wantMatch bool // expected match result + } + + // makePerRETests returns a series of tests for a variety of client UAs that + // are generated based on the provided parameters to help ensure the exact + // semantics that each test intends to test are actually what is being + // tested. + makePerRETests := func(client string, major, minor uint32) []perRETest { + p := fmt.Sprintf + pcmm := func(format string, a ...interface{}) string { + params := make([]interface{}, 0, len(a)+3) + params = append(params, client) + params = append(params, major) + params = append(params, minor) + params = append(params, a...) + return p(format, params...) + } + return []perRETest{ + // All patch versions including multi digit are allowed. + {pcmm("%s/%d.%d.0"), true}, + {pcmm("%s/%d.%d.1"), true}, + {pcmm("%s/%d.%d.2"), true}, + {pcmm("%s/%d.%d.3"), true}, + {pcmm("%s/%d.%d.4"), true}, + {pcmm("%s/%d.%d.5"), true}, + {pcmm("%s/%d.%d.6"), true}, + {pcmm("%s/%d.%d.7"), true}, + {pcmm("%s/%d.%d.8"), true}, + {pcmm("%s/%d.%d.9"), true}, + {pcmm("%s/%d.%d.10"), true}, + + // All valid prerelease and build metadata combinations are allowed. + {pcmm("%s/%d.%d.0-prerelease+meta"), true}, + {pcmm("%s/%d.%d.0+meta"), true}, + {pcmm("%s/%d.%d.0+meta-valid"), true}, + {pcmm("%s/%d.%d.0-alpha"), true}, + {pcmm("%s/%d.%d.0-beta"), true}, + {pcmm("%s/%d.%d.0-alpha.beta"), true}, + {pcmm("%s/%d.%d.0-alpha.beta.1"), true}, + {pcmm("%s/%d.%d.0-alpha.1"), true}, + {pcmm("%s/%d.%d.0-alpha0.valid"), true}, + {pcmm("%s/%d.%d.0-alpha.0valid"), true}, + {pcmm("%s/%d.%d.0-alpha.a.b-c-somethinglong+build.1-aef.1-its-okay"), true}, + {pcmm("%s/%d.%d.0-rc.1+build.1"), true}, + {pcmm("%s/%d.%d.0-rc.1+build.123"), true}, + {pcmm("%s/%d.%d.1-beta"), true}, + {pcmm("%s/%d.%d.10-DEV-SNAPSHOT"), true}, + {pcmm("%s/%d.%d.100-SNAPSHOT-123"), true}, + {pcmm("%s/%d.%d.0+build.1848"), true}, + {pcmm("%s/%d.%d.1-alpha.1227"), true}, + {pcmm("%s/%d.%d.0-alpha+beta"), true}, + {pcmm("%s/%d.%d.0-----RC-SNAPSHOT.12.9.1--.12+788"), true}, + {pcmm("%s/%d.%d.3----R-S.12.9.1--.12+meta"), true}, + {pcmm("%s/%d.%d.0----RC-SNAPSHOT.12.9.1--.12"), true}, + {pcmm("%s/%d.%d.0+0.build.1-rc.10000aaa-kk-0.1"), true}, + {pcmm("%s/%d.%d.0-0A.is.legal"), true}, + + // New minor revisions are not allowed. + {p("%s/%d.%d.0", client, major, minor+1), false}, + {p("%s/%d.%d.0", client, major, minor+2), false}, + {p("%s/%d.%d.0", client, major, minor+10), false}, + + // New major versions are not allowed. + {p("%s/%d.0.0", client, major+1), false}, + {p("%s/%d.0.0", client, major+2), false}, + {p("%s/%d.0.0", client, major+10), false}, + + // Prefixes not allowed. + {pcmm(" %s/%d.%d.0"), false}, + {pcmm("a%s/%d.%d.0"), false}, + {pcmm("1%s/%d.%d.0"), false}, + + // Invalid semantic versions not allowed. + {p("%s/%d", client, major), false}, + {pcmm("%s/%d.%d"), false}, + {pcmm("%s/%d.%d.0-0123"), false}, + {pcmm("%s/%d.%d.0-0123.0123"), false}, + {pcmm("%s/%d.%d.1+.123"), false}, + {pcmm("%s/%d.%d+invalid"), false}, + {pcmm("%s/%d.%d-invalid"), false}, + {pcmm("%s/%d.%d-invalid+invalid"), false}, + {pcmm("%s/%d.%d.0-alpha_beta"), false}, + {pcmm("%s/%d.%d.0-alpha.."), false}, + {pcmm("%s/%d.%d.0-alpha..1"), false}, + {pcmm("%s/%d.%d.0-alpha...1"), false}, + {pcmm("%s/%d.%d.0-alpha....1"), false}, + {pcmm("%s/%d.%d.0-alpha.....1"), false}, + {pcmm("%s/%d.%d.0-alpha......1"), false}, + {pcmm("%s/%d.%d.0-alpha.......1"), false}, + {pcmm("%s/%d.%d.0-alpha..1"), false}, + {pcmm("%s/0%d.%d.0"), false}, + {pcmm("%s/%d.0%d.0"), false}, + {pcmm("%s/%d.%d.00"), false}, + {pcmm("%s/%d.%d.0.DEV"), false}, + {pcmm("%s/%d.%d-SNAPSHOT"), false}, + {pcmm("%s/%d.%d.31.2.3----RC-SNAPSHOT.12.09.1--..12+788"), false}, + {pcmm("%s/%d.%d-RC-SNAPSHOT"), false}, + {pcmm("%s/-%d.%d.3-gamme+b7718"), false}, + {p("%s/+justmeta", client), false}, + {pcmm("%s/%d.%d.7+meta+meta"), false}, + {pcmm("%s/%d.%d.7-whatever+meta+meta"), false}, + } + } + + tests := []struct { + name string // test description + clientName string // required client name for regexp + major uint32 // required major ver of the client for regexp + minor uint32 // required minor ver of the client for regexp + }{{ + name: "cpuminer/1.0.x", + clientName: "cpuminer", + major: 1, + minor: 0, + }, { + name: "cpuminer/1.1.x", + clientName: "cpuminer", + major: 1, + minor: 1, + }, { + name: "cpuminer/2.4.x", + clientName: "cpuminer", + major: 2, + minor: 4, + }, { + name: "otherminer/10.17.x", + clientName: "otherminer", + major: 10, + minor: 17, + }} + + for _, test := range tests { + // Create the compiled regular expression as well as client UAs and + // expected results. + re := newUserAgentRE(test.clientName, test.major, test.minor) + perRETests := makePerRETests(test.clientName, test.major, test.minor) + + // Ensure all of the client UAs produce the expected match results. + for _, subTest := range perRETests { + gotMatch := re.MatchString(subTest.clientUA) + if gotMatch != subTest.wantMatch { + t.Errorf("%s: (ua: %q): unexpected match result -- got %v, want %v", + test.name, subTest.clientUA, gotMatch, subTest.wantMatch) + continue + } + } + } +}