Skip to content

Conversation

@Poseidon-G
Copy link
Collaborator

@Poseidon-G Poseidon-G commented Jul 6, 2025

Add durability for keygen event

@Poseidon-G Poseidon-G requested a review from Copilot July 6, 2025 09:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the old JetStream-based StreamPubsub abstraction with a new MessageBroker interface and its JetStream implementation, refactors event consumers/clients to use the broker, and updates application startup to run keygen and signing consumers concurrently.

  • Introduced MessageBroker and jetStreamBroker for publish/subscribe, removed StreamPubsub and jetstreamPubSub
  • Updated consumers and client initialization to use MessageBroker (CreateSubscription, PublishMessage)
  • Refactored cmd/mpcium/main.go to start keygen and signing consumers in parallel with error handling

Reviewed Changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/messaging/pubsub.go Removed StreamPubsub and jetstreamPubSub, simplified PublishWithReply
pkg/messaging/jetstream_borker.go New jetStreamBroker implementing MessageBroker with JetStream
pkg/eventconsumer/sign_consumer.go Switched from StreamPubsub to MessageBroker, updated reply logic
pkg/eventconsumer/keygen_consumer.go Switched to MessageBroker, added keygen consumer implementation
pkg/eventconsumer/event_consumer.go Unified reply handling via sendReplyToRemoveMsg
pkg/event/sign.go Removed deprecated MPCSigningEventTopic
pkg/event/keygen.go Added keygen stream and request topic constants
pkg/client/client.go Updated client to initialize and use MessageBroker
cmd/mpcium/main.go Replaced JetStreamPubSub with brokers, added concurrent consumer startup
Comments suppressed due to low confidence (4)

pkg/eventconsumer/keygen_consumer.go:40

  • The jsSub field is typed as messaging.Subscription, but CreateSubscription now returns messaging.MessageSubscription. Update the field type for consistency.
	jsSub messaging.Subscription

pkg/eventconsumer/sign_consumer.go:161

  • The constant MPCSignEvent is undefined (it was removed from pkg/event/sign.go). Replace this with an existing event constant or reintroduce it in the event package.
	if err := sc.pubsub.PublishWithReply(MPCSignEvent, replyInbox, msg.Data()); err != nil {

pkg/eventconsumer/keygen_consumer.go:153

  • This references signingResponseTimeout, which is undefined here. It should use keygenResponseTimeout defined at the top of this file.
	deadline := time.Now().Add(signingResponseTimeout)

pkg/eventconsumer/keygen_consumer.go:155

  • This uses signingPollingInterval, which does not exist in this context. It should be keygenPollingInterval.
		replyMsg, err := replySub.NextMsg(signingPollingInterval)

@Poseidon-G Poseidon-G force-pushed the feat/durable-keygen-request branch 3 times, most recently from e8c1215 to 6b66ae2 Compare July 6, 2025 09:40
@anhthii anhthii force-pushed the feat/durable-keygen-request branch from 6b66ae2 to 99522c3 Compare July 10, 2025 01:46
@anhthii
Copy link
Contributor

anhthii commented Jul 11, 2025

bugbot run

@anhthii anhthii changed the title Remove jetstream pubsub change to message broker Add durability for keygen event with jetstream Jul 11, 2025
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: NATS Reply Bug Sends Incorrect Data

The sendReplyToRemoveMsg function, introduced by this commit, incorrectly sends the original NATS message data (natMsg.Data) as a reply. This breaks the reply mechanism for key generation and signing operations, as consumers expecting the operation's result will now receive the original request instead.

pkg/eventconsumer/event_consumer.go#L488-L502

func (ec *eventConsumer) sendReplyToRemoveMsg(natMsg *nats.Msg) {
msg := natMsg.Data
if natMsg.Reply == "" {
logger.Warn("No reply inbox specified for sign success message", "msg", string(msg))
return
}
err := ec.pubsub.Publish(natMsg.Reply, msg)
if err != nil {
logger.Error("Failed to reply message", err, "reply", natMsg.Reply)
return
}
}

pkg/eventconsumer/event_consumer.go#L431-L435

onSuccess := func(data []byte) {
done()
ec.sendReplyToRemoveMsg(natMsg)
}

Fix in CursorFix in Web


BugBot free trial expires on July 22, 2025
You have used $0.00 of your $2.00 spend limit so far. Manage your spend limit in the Cursor dashboard.

Was this report helpful? Give feedback by reacting with 👍 or 👎

@anhthii anhthii merged commit 6ec2b18 into master Jul 12, 2025
26 checks passed
@anhthii anhthii deleted the feat/durable-keygen-request branch July 12, 2025 00:56
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.

3 participants