feat(clients): executionGroups on endpoints + buildoor testing-build playbook

.github/workflows/_shared-check.yaml

@@ -30,7 +30,7 @@ jobs:
       run: if [ "$(gofmt -s -l . | wc -l)" -gt 0 ]; then exit 1; fi
 
     - name: Install staticcheck
-      run: go install honnef.co/go/tools/cmd/staticcheck@latest
+      run: go install honnef.co/go/tools/cmd/staticcheck@v0.7.0
     - name: Run staticcheck
       run: staticcheck ./...
 

docs/02-global-config.md

@@ -25,6 +25,20 @@ endpoints:
   - name: "node-1"
     executionUrl: "http://127.0.0.1:8545"
     consensusUrl: "http://127.0.0.1:5052"
+  # executionGroups puts an endpoint in spamoor client groups, so a scenario
+  # can direct its transactions at it with its client_group option.
+  #
+  # Membership is ADDITIVE: every endpoint is in "default", which is also the
+  # group used by every selection that names none - wallet funding and
+  # refills, contract deployments, and scenarios without client_group. Naming
+  # a group does not stop that traffic. Add "-default" to reserve an endpoint
+  # for the scenarios that ask for it, e.g. a builder's private tx intake that
+  # must hold only one scenario's transactions. Removal needs a spamoor
+  # version that supports it; assertoor fails at startup if it is ignored.
+  - name: "buildoor-intake"
+    executionUrl: "http://127.0.0.1:8085/rpc"
+    consensusUrl: "http://127.0.0.1:5052"
+    executionGroups: ["builder", "-default"]
 
 validatorNames:
   inventoryYaml: "./validator-names.yaml"

pkg/clients/clients.go

@@ -40,6 +40,21 @@ type ClientConfig struct {
 	ConsensusHeaders map[string]string `yaml:"consensusHeaders"`
 	ExecutionURL     string            `yaml:"executionUrl"`
 	ExecutionHeaders map[string]string `yaml:"executionHeaders"`
+	// ExecutionGroups are the spamoor client groups this execution endpoint
+	// belongs to, so a scenario can direct its transactions at specific
+	// endpoints via its client_group option.
+	//
+	// Membership is ADDITIVE: every client is in "default", which is also the
+	// group used by every selection that names none (wallet funding and
+	// refills, contract deployments, scenarios without client_group). Naming a
+	// group therefore does not stop that traffic. To reserve an endpoint —
+	// e.g. a builder's private tx intake that must hold only one scenario's
+	// transactions — also remove it from the default group with "-default".
+	// Removal requires a spamoor version that supports it; assertoor fails at
+	// startup if the linked one ignores the entry.
+	//
+	//	executionGroups: ["builder", "-default"]
+	ExecutionGroups []string `yaml:"executionGroups"`
 }
 
 func NewClientPool(logger logrus.FieldLogger) (*ClientPool, error) {
@@ -95,6 +110,7 @@ func (pool *ClientPool) AddClient(config *ClientConfig) error {
 		Name:    config.Name,
 		URL:     config.ExecutionURL,
 		Headers: config.ExecutionHeaders,
+		Groups:  config.ExecutionGroups,
 	})
 	if err != nil {
 		return fmt.Errorf("could not init consensus client: %w", err)

pkg/clients/execution/client.go

@@ -22,6 +22,7 @@ type ClientConfig struct {
 	URL     string
 	Name    string
 	Headers map[string]string
+	Groups  []string // spamoor client groups; empty = default
 }
 
 type Client struct {

pkg/txmgr/spamoor.go

@@ -45,7 +45,23 @@ func NewSpamoor(ctx context.Context, logger logrus.FieldLogger, executionPool *e
 		return nil, err
 	}
 
-	for i, client := range clientPool.GetAllClients() {
+	// InitClients skips options it could not turn into a client, so a short
+	// list would silently pair every later endpoint with another endpoint's
+	// client — wiring transactions to the wrong node and checking the wrong
+	// endpoint's groups. A client only fails to build on a malformed rpchost,
+	// which is a config error worth stopping for.
+	spamoorClients := clientPool.GetAllClients()
+	if len(spamoorClients) != len(endpoints) {
+		return nil, fmt.Errorf("spamoor initialized %d of %d execution endpoints; "+
+			"check the endpoint configuration (executionGroups, headers)",
+			len(spamoorClients), len(endpoints))
+	}
+
+	for i, client := range spamoorClients {
+		if groupErr := verifyClientGroups(endpoints[i].GetEndpointConfig(), client); groupErr != nil {
+			return nil, groupErr
+		}
+
 		s.clients[endpoints[i]] = client
 	}
 
@@ -94,6 +110,10 @@ func (s *Spamoor) getClientOptions(client *execution.Client) *spamoor.ClientOpti
 	rpcURL := client.GetEndpointConfig().URL
 	rpcURL = fmt.Sprintf("name(%s)%s", client.GetName(), rpcURL)
 
+	if groups := normalizedGroups(client.GetEndpointConfig()); len(groups) > 0 {
+		rpcURL = fmt.Sprintf("group(%s)%s", strings.Join(groups, ","), rpcURL)
+	}
+
 	if headers := client.GetEndpointConfig().Headers; len(headers) > 0 {
 		headerParts := make([]string, 0, len(headers))
 		for key, value := range headers {
@@ -269,3 +289,49 @@ func (s *Spamoor) NewWalletPoolByPrivkey(ctx context.Context, logger logrus.Fiel
 
 	return walletPool, nil
 }
+
+// verifyClientGroups checks that spamoor applied the endpoint's configured
+// client groups. Group membership decides which scenarios reach an endpoint,
+// so a silently ignored entry would send transactions somewhere the operator
+// deliberately excluded. A "-name" entry (removing a group, e.g. "-default"
+// to keep an endpoint out of the group-less selections) needs a spamoor
+// version that supports removal; older versions treat it as a literal group
+// name, which this check turns into a startup error instead of a surprise.
+func verifyClientGroups(config *execution.ClientConfig, client *spamoor.Client) error {
+	for _, group := range normalizedGroups(config) {
+		if remove, found := strings.CutPrefix(group, "-"); found {
+			if client.HasGroup(remove) {
+				return fmt.Errorf(
+					"endpoint %q: executionGroups asked to remove client group %q but the client is still in it "+
+						"(the linked spamoor does not support group removal)",
+					config.Name, remove)
+			}
+
+			continue
+		}
+
+		if !client.HasGroup(group) {
+			return fmt.Errorf("endpoint %q: executionGroups requested client group %q but the client is not in it",
+				config.Name, group)
+		}
+	}
+
+	return nil
+}
+
+// normalizedGroups trims the endpoint's configured client groups and drops
+// empty entries, matching what spamoor's own group(...) parser does. Without
+// it a stray space makes the two disagree: spamoor applies the trimmed name
+// while the check compares the raw one, which both fails startup spuriously
+// on an added group and, worse, passes a removal that never took effect.
+func normalizedGroups(config *execution.ClientConfig) []string {
+	groups := make([]string, 0, len(config.Groups))
+
+	for _, group := range config.Groups {
+		if group = strings.TrimSpace(group); group != "" {
+			groups = append(groups, group)
+		}
+	}
+
+	return groups
+}

pkg/txmgr/spamoor_groups_test.go

@@ -0,0 +1,64 @@
+package txmgr
+
+import (
+	"slices"
+	"testing"
+
+	"github.com/ethpandaops/assertoor/pkg/clients/execution"
+	"github.com/ethpandaops/spamoor/spamoor"
+)
+
+// spamoor trims group names in its own parser, so the check must trim too:
+// otherwise a stray space makes the two disagree and a removal that never
+// took effect passes verification — the silent under-isolation this guard
+// exists to prevent.
+func TestNormalizedGroupsMatchesSpamoorParsing(t *testing.T) {
+	config := &execution.ClientConfig{
+		Name:   "intake",
+		URL:    "http://localhost:8545",
+		Groups: []string{" builder ", "", "  ", "-default "},
+	}
+
+	got := normalizedGroups(config)
+	if want := []string{"builder", "-default"}; !slices.Equal(got, want) {
+		t.Fatalf("normalizedGroups = %v, want %v", got, want)
+	}
+
+	client, err := spamoor.NewClient(&spamoor.ClientOptions{
+		RpcHost: "group(" + got[0] + ")" + config.URL,
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if !client.HasGroup("builder") {
+		t.Error("spamoor did not apply the trimmed group name")
+	}
+
+	if err := verifyClientGroups(&execution.ClientConfig{Name: "intake", Groups: []string{" builder "}}, client); err != nil {
+		t.Errorf("verification must accept the same trimmed name spamoor applied: %v", err)
+	}
+}
+
+// A configured group the linked spamoor did not apply must stop startup.
+func TestVerifyClientGroupsRejectsAnUnappliedEntry(t *testing.T) {
+	client, err := spamoor.NewClient(&spamoor.ClientOptions{RpcHost: "group(builder)http://localhost:8545"})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if applied := verifyClientGroups(&execution.ClientConfig{Name: "intake", Groups: []string{"builder"}}, client); applied != nil {
+		t.Errorf("an applied group must verify: %v", applied)
+	}
+
+	// The pinned spamoor keeps the client in "default", so a removal that was
+	// ignored has to be an error rather than silent under-isolation.
+	ignored := verifyClientGroups(&execution.ClientConfig{Name: "intake", Groups: []string{"-default"}}, client)
+	if client.HasGroup("default") && ignored == nil {
+		t.Error("an ignored removal must fail verification")
+	}
+
+	if err := verifyClientGroups(&execution.ClientConfig{Name: "intake", Groups: []string{"absent"}}, client); err == nil {
+		t.Error("a group the client is not in must fail verification")
+	}
+}

playbooks/dev/buildoor-testing-build.yaml

@@ -0,0 +1,168 @@
+id: buildoor-testing-build
+name: "buildoor testing build: planned transactions land on chain exactly"
+description: |
+  Switches a buildoor to its testing build source (blocks built from the
+  builder's private tx intake through geth's testing_buildBlockV1), feeds the
+  intake with a spamoor scenario, and requires that every buildoor block after
+  the switch verified as an exact tx plan match: same transactions, same
+  order, same count. A single mismatch, missed payload or unknown block fails
+  the test.
+
+  Endpoint setup. While the builder is on the testing source its blocks are
+  the ONLY way a transaction gets included: anything sent to a normal EL sits
+  in the public txpool forever, because buildoor builds solely from its
+  intake. That includes spamoor's own wallet funding and refills, which use
+  the group-less "default" selection rather than this scenario's client_group.
+
+  So every endpoint spamoor may pick must be the intake. The simple form is to
+  give this playbook a config where the intake is the only execution endpoint
+  (it proxies all reads to the EL, so the client checks still work). It still
+  needs the group this test's clientGroup names, since a scenario selecting a
+  group no client is in gets no client at all:
+
+    endpoints:
+      - name: buildoor-intake
+        executionUrl: http://buildoor:8080/rpc
+        consensusUrl: http://beacon:5052
+        executionGroups: [builder]
+
+  With several execution endpoints, keep the intake in "default" and take the
+  others out of it, so the scenario reaches every node it needs while funding
+  can only land somewhere that gets built (needs a spamoor with group removal,
+  ethpandaops/spamoor#283):
+
+    endpoints:
+      - name: buildoor-intake
+        executionUrl: http://buildoor:8080/rpc
+        consensusUrl: http://beacon:5052
+        executionGroups: [builder]
+      - name: node-2
+        executionUrl: http://node2:8545
+        consensusUrl: http://beacon2:5052
+        executionGroups: ["-default"]
+
+  The buildoor geth must serve the testing namespace (--http.api ...,testing).
+version: 1.0.0
+tags: [buildoor, epbs, load, testing-build]
+timeout: 30m
+config:
+  buildoorUrl: "http://buildoor:8080"
+  spamoorPrivkey: ""
+  clientGroup: "builder"
+  minVerifiedBlocks: 6
+  minTxsPerBlock: 20
+  throughput: 300
+  baseFeeGwei: 100
+  # spamoor's scenarioYaml takes these in WEI (the --refill-* CLI flags take
+  # ETH, the YAML fields do not). Too small and the wallets are "funded" with
+  # dust, pass the balance check, and every transaction is then skipped for
+  # insufficient funds.
+  # Quoted: jq evaluates configVars as float64, so an unquoted 5e19 reaches
+  # spamoor as "5e+19" and fails its uint256 parse.
+  refillAmountWei: "50000000000000000000"   # 50 ETH
+  refillBalanceWei: "10000000000000000000"  # 10 ETH
+tasks:
+- name: check_clients_are_healthy
+  title: "Check if at least one client is ready"
+  timeout: 5m
+  config:
+    minClientCount: 1
+
+- name: check_http_json
+  title: "Switch buildoor to the testing build source"
+  timeout: 1m
+  configVars:
+    url: ".buildoorUrl + \"/api/config/testing\""
+  config:
+    method: POST
+    body:
+      source: testing
+      fill_gas_pct: 100
+      policy: fifo
+    expectStatus: 200
+    failOnCheckMiss: true
+    assertions:
+      - name: updated
+        query: ".status"
+        operator: eq
+        value: "updated"
+
+- name: run_task_background
+  title: "Feed the intake and verify buildoor's blocks"
+  config:
+    onBackgroundComplete: fail
+    backgroundTask:
+      name: run_spamoor_scenario
+      title: "spamoor eoatx into the builder intake"
+      configVars:
+        privateKey: "spamoorPrivkey"
+        scenarioYaml: ". as $v | {throughput: $v.throughput, max_pending: ($v.throughput * 10), max_wallets: 30, base_fee: $v.baseFeeGwei, tip_fee: 2, refill_amount: $v.refillAmountWei, refill_balance: $v.refillBalanceWei, rebroadcast: 30, client_group: $v.clientGroup}"
+      config:
+        scenarioName: eoatx
+    foregroundTask:
+      name: run_tasks
+      title: "Verify"
+      config:
+        tasks:
+        - name: check_consensus_block_proposals
+          title: "buildoor proposes ${minVerifiedBlocks} blocks with >= ${minTxsPerBlock} transactions"
+          timeout: 20m
+          configVars:
+            blockCount: "minVerifiedBlocks"
+            minTransactionCount: "minTxsPerBlock"
+          config:
+            checkLookback: 64
+            extraDataPattern: "^buildoor"
+
+        # Two checks on purpose. The tally lags the proposals it counts (the
+        # verifier fetches the block from the EL after inclusion), and an
+        # unsatisfied assertion is a FAILURE rather than a wait under
+        # failOnCheckMiss - so the count is polled without it, and only the
+        # must-be-zero counters fail fast.
+        - name: check_http_json
+          title: "Wait for ${minVerifiedBlocks} verified tx plans"
+          timeout: 5m
+          configVars:
+            url: ".buildoorUrl + \"/api/buildoor/tx-queue\""
+            assertions: >-
+              . as $v | [
+                {name: "enough-matches", query: ".plan_checks.match", operator: "gte", value: $v.minVerifiedBlocks}
+              ]
+          config:
+            pollInterval: 5s
+
+        - name: check_http_json
+          title: "No tx plan was mismatched, missed or unverifiable"
+          timeout: 1m
+          configVars:
+            url: ".buildoorUrl + \"/api/buildoor/tx-queue\""
+            # Cumulative counters, so this covers the whole run, not just now.
+            # Leading "." matters: assertoor prepends one to a configVars
+            # expression that lacks it, turning a bare [..] into an index.
+            assertions: >-
+              . | [
+                {name: "no-mismatch", query: ".plan_checks.mismatch", operator: "eq", value: 0},
+                {name: "no-missing-blocks", query: ".plan_checks.block_not_found", operator: "eq", value: 0},
+                {name: "no-missed-payloads", query: ".plan_checks.missed", operator: "eq", value: 0}
+              ]
+          config:
+            failOnCheckMiss: true
+
+        - name: check_consensus_forks
+          title: "No forks"
+          timeout: 1m
+          config:
+            minCheckEpochCount: 1
+            maxForkDistance: 1
+
+cleanupTasks:
+- name: check_http_json
+  title: "Switch buildoor back to the pool build source"
+  timeout: 1m
+  configVars:
+    url: ".buildoorUrl + \"/api/config/testing\""
+  config:
+    method: POST
+    body:
+      source: pool
+    expectStatus: 200