Skip to content

chore: Remove RuntimeConfig dependency from client.New - #273

Merged
meling merged 6 commits into
mainfrom
copilot/refactor-client-id-usage
Nov 15, 2025
Merged

meling merged 6 commits into
mainfrom
copilot/refactor-client-id-usage

Conversation

Copilot AI commented Nov 15, 2025 •

Copy link
Copy Markdown
Contributor

client.New() required a RuntimeConfig parameter solely to extract the client ID. Additionally, clients reused hotstuff.ID despite being distinct from replica IDs.

Changes

  • client/client.go: Added client.ID type; replaced config *core.RuntimeConfig field with id ID; updated New() signature
  • internal/proto/orchestrationpb/client_opts.go: Added ClientID() helper to convert ClientOpts.ID to client.ID
  • internal/orchestration/worker.go: Changed clients map from map[hotstuff.ID] to map[client.ID]; removed RuntimeConfig instantiation; pass opts.ClientID() directly to client.New()

Before

runtimeCfg := core.NewRuntimeConfig(hotstuff.ID(opts.GetID()), nil)
cli := client.New(eventLoop, logger, runtimeCfg, c)
w.clients[hotstuff.ID(opts.GetID())] = cli

After

cli := client.New(eventLoop, logger, opts.ClientID(), c)
w.clients[opts.ClientID()] = cli
Original prompt

This section details on the original issue you should resolve

<issue_title>chore: client.New requires RuntimeConfig only to get the client's ID</issue_title>
<issue_description>Clients are created in startClients from internal/orchestration/worker.go:

		runtimeCfg := core.NewRuntimeConfig(hotstuff.ID(opts.GetID()), nil)
		cli := client.New(
			eventLoop,
			logger,
			runtimeCfg,
			c,
		)

In client.sendCommands() we do this:

		cmd := &clientpb.Command{
			ClientID:       uint32(c.config.ID()),
			SequenceNumber: num,
			Data:           data[:n],
		}

This is the only usage of the c.config field. We should instead just pass the client.ID in New. See below for more details.

Moreover, there is this map in worker.go:

	clients  map[hotstuff.ID]*client.Client

We should not use the same ID for hotstuff replicas and clients. We should create a new ID type in the client package (client/client.go):

type ID uint32

Along with a helper method in a new file internal/proto/orchestrationpb/client_opts.go:

package orchestrationpb

import (
	"github.com/relab/hotstuff/client"
)

func (x *ClientOpts) ClientID() client.ID {
	return client.ID(x.GetID())
}

We can then use opts.ClientID() directly instead of passing the RuntimeConfig as shown above. That is, we should have this function in the client package:

// New returns a new Client.
func New(
	eventLoop *eventloop.EventLoop,
	logger logging.Logger,
	id ID,
	conf Config,
) (client *Client) {

And use it like this from worker.go:

		cli := client.New(
			eventLoop,
			logger,
			opts.ClientID(),
			c,
		)
```</issue_description>

## Comments on the Issue (you are @copilot in this section)

<comments>
</comments>

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Refactor client creation to use client ID directly chore: Remove RuntimeConfig dependency from client.New Nov 15, 2025
Copilot AI requested a review from meling November 15, 2025 14:51
@meling
meling marked this pull request as ready for review November 15, 2025 16:09
Copilot AI and others added 6 commits November 16, 2025 00:21
Co-authored-by: meling <810999+meling@users.noreply.github.com>
These simplifies usage in the orchestration worker.
This will simplify usage in the orchestration worker (next commit).
This selectively enables metrics only for client and replica based
on the type of ID passed to the Enable function; thus, we do not
enable client metrics logging on replicas and vice versa.

This also updates the Worker to use appropriate ID types depending
on whether it is a client or replica we are creating/starting/stopping.

This also updates the documation for the various metrics types.
@meling
meling force-pushed the copilot/refactor-client-id-usage branch from ada2901 to 93bfbaf Compare November 15, 2025 23:21
@meling
meling merged commit 48b6137 into main Nov 15, 2025
4 checks passed
@meling
meling deleted the copilot/refactor-client-id-usage branch November 15, 2025 23:55
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.

chore: client.New requires RuntimeConfig only to get the client's ID

2 participants