Merge pull request #6535 from oasisprotocol/peternose/trivial/rename-msg-execution

go/consensus/cometbft/api: Remove message execution from context

Author: peternose

.changelog/6535.trivial.md

@@ -25,13 +25,13 @@ func (md *messageDispatcher) Subscribe(kind any, ms api.MessageSubscriber) {
 }
 
 // Implements api.MessageDispatcher.
-func (md *messageDispatcher) Publish(ctx *api.Context, kind, msg any) (any, error) {
+func (md *messageDispatcher) Publish(ctx *api.Context, msg api.Message) (any, error) {
 	var (
 		result         any
 		errs           error
 		numSubscribers int
 	)
-	for _, ms := range md.subscriptions[kind] {
+	for _, ms := range md.subscriptions[msg.Kind] {
 		// Check whether the subscriber can be toggled.
 		if togMs, ok := ms.(api.TogglableMessageSubscriber); ok {
 			enabled, err := togMs.Enabled(ctx)
@@ -47,7 +47,7 @@ func (md *messageDispatcher) Publish(ctx *api.Context, kind, msg any) (any, erro
 		numSubscribers++
 
 		// Deliver the message.
-		if resp, err := ms.ExecuteMessage(ctx, kind, msg); err != nil {
+		if resp, err := ms.ExecuteMessage(ctx, msg); err != nil {
 			errs = errors.Join(errs, err)
 		} else {
 			switch {

go/consensus/cometbft/abci/messages.go

@@ -33,8 +33,8 @@ type testSubscriber struct {
 }
 
 // Implements api.MessageSubscriber.
-func (s *testSubscriber) ExecuteMessage(_ *api.Context, _, msg any) (any, error) {
-	switch m := msg.(type) {
+func (s *testSubscriber) ExecuteMessage(_ *api.Context, msg api.Message) (any, error) {
+	switch m := msg.Data.(type) {
 	case *testMessage:
 		s.msgs = append(s.msgs, m.foo)
 		if s.fail {
@@ -66,40 +66,58 @@ func TestMessageDispatcher(t *testing.T) {
 	md := newMessageDispatcher()
 
 	// Publish without subscribers should work.
-	res, err := md.Publish(ctx, testMessageA, &testMessage{foo: 42})
+	res, err := md.Publish(ctx, api.Message{
+		Kind: testMessageA,
+		Data: &testMessage{foo: 42},
+	})
 	require.Error(err, "Publish")
 	require.Equal(api.ErrNoSubscribers, err)
 	require.Nil(res, "Publish results should be empty")
 
 	// With a disabled subscriber should behave same as with no subscribers.
 	var ms testSubscriber
 	md.Subscribe(testMessageA, &ms)
-	res, err = md.Publish(ctx, testMessageA, &testMessage{foo: 42})
+	res, err = md.Publish(ctx, api.Message{
+		Kind: testMessageA,
+		Data: &testMessage{foo: 42},
+	})
 	require.Error(err, "Publish")
 	require.Equal(api.ErrNoSubscribers, err)
 	require.Nil(res, "Publish results should be empty")
 	require.Empty(ms.msgs, "no messages should be delivered when subscriber is disabled")
 
 	// With an enabled subscriber.
 	ms.enabled = true
-	res, err = md.Publish(ctx, testMessageA, &testMessage{foo: 42})
+	res, err = md.Publish(ctx, api.Message{
+		Kind: testMessageA,
+		Data: &testMessage{foo: 42},
+	})
 	require.NoError(err, "Publish")
 	require.EqualValues(int32(42), res, "correct publish message result")
 	require.EqualValues([]int32{42}, ms.msgs, "correct messages should be delivered")
 
-	res, err = md.Publish(ctx, testMessageA, &testMessage{foo: 43})
+	res, err = md.Publish(ctx, api.Message{
+		Kind: testMessageA,
+		Data: &testMessage{foo: 43},
+	})
 	require.NoError(err, "Publish")
 	require.EqualValues(int32(43), res, "correct publish message result")
 	require.EqualValues([]int32{42, 43}, ms.msgs, "correct messages should be delivered")
 
-	res, err = md.Publish(ctx, testMessageB, &testMessage{foo: 44})
+	res, err = md.Publish(ctx, api.Message{
+		Kind: testMessageB,
+		Data: &testMessage{foo: 44},
+	})
 	require.Error(err, "Publish")
 	require.Equal(api.ErrNoSubscribers, err)
 	require.Nil(res, "publish message results should be empty")
 	require.EqualValues([]int32{42, 43}, ms.msgs, "correct messages should be delivered")
 
 	// Returning an error.
-	res, err = md.Publish(ctx, testMessageA, &errorMessage{})
+	res, err = md.Publish(ctx, api.Message{
+		Kind: testMessageA,
+		Data: &errorMessage{},
+	})
 	require.Error(err, "Publish")
 	require.Nil(nil, res, "publish message results should be empty")
 	require.True(errors.Is(err, errTest), "returned error should be the correct one")
@@ -110,7 +128,10 @@ func TestMessageDispatcher(t *testing.T) {
 		noResult: true,
 	}
 	md.Subscribe(testMessageA, &ms2)
-	res, err = md.Publish(ctx, testMessageA, &testMessage{foo: 44})
+	res, err = md.Publish(ctx, api.Message{
+		Kind: testMessageA,
+		Data: &testMessage{foo: 44},
+	})
 	require.NoError(err, "Publish")
 	require.EqualValues(int32(44), res, "correct publish message result")
 	require.EqualValues([]int32{42, 43, 44}, ms.msgs, "correct messages should be delivered")
@@ -119,6 +140,9 @@ func TestMessageDispatcher(t *testing.T) {
 	// Multiple subscribers returning results.
 	ms2.noResult = false
 	require.Panics(func() {
-		_, _ = md.Publish(ctx, testMessageA, &testMessage{foo: 44})
+		_, _ = md.Publish(ctx, api.Message{
+			Kind: testMessageA,
+			Data: &testMessage{foo: 44},
+		})
 	}, "multiple subscribers returning results should panic")
 }

go/consensus/cometbft/abci/messages_test.go

@@ -945,7 +945,9 @@ func (mux *abciMux) finishInitialization() error {
 		ctx := mux.state.NewContext(api.ContextEndBlock)
 		defer ctx.Close()
 
-		if _, err := mux.md.Publish(ctx, api.MessageStateSyncCompleted, nil); err != nil {
+		if _, err := mux.md.Publish(ctx, api.Message{
+			Kind: api.MessageStateSyncCompleted,
+		}); err != nil {
 			mux.logger.Error("failed to dispatch state sync completed message",
 				"err", err,
 			)

go/consensus/cometbft/abci/mux.go

@@ -270,7 +270,9 @@ func (mux *abciMux) ApplySnapshotChunk(req types.RequestApplySnapshotChunk) type
 		ctx := mux.state.NewContext(api.ContextEndBlock)
 		defer ctx.Close()
 
-		if _, err = mux.md.Publish(ctx, api.MessageStateSyncCompleted, nil); err != nil {
+		if _, err = mux.md.Publish(ctx, api.Message{
+			Kind: api.MessageStateSyncCompleted,
+		}); err != nil {
 			mux.logger.Error("failed to dispatch state sync completed message",
 				"err", err,
 			)

go/consensus/cometbft/abci/snapshots.go

@@ -11,11 +11,11 @@ import (
 const maxSubcallDepth = 8
 
 // ExecuteMessage implements api.MessageSubscriber.
-func (mux *abciMux) ExecuteMessage(ctx *api.Context, kind, msg any) (any, error) {
-	switch kind {
+func (mux *abciMux) ExecuteMessage(ctx *api.Context, msg api.Message) (any, error) {
+	switch msg.Kind {
 	case api.MessageExecuteSubcall:
 		// Subcall execution request.
-		info, ok := msg.(*api.SubcallInfo)
+		info, ok := msg.Data.(*api.SubcallInfo)
 		if !ok {
 			return nil, fmt.Errorf("invalid subcall info")
 		}

go/consensus/cometbft/abci/subcall.go

@@ -12,10 +12,20 @@ import (
 // ErrNoSubscribers is the error returned when publishing a message that noone is subscribed to.
 var ErrNoSubscribers = errors.New("no subscribers to given message kind")
 
+// Message is a message sent between applications.
+type Message struct {
+	// Sender identifies the application that sent the message.
+	Sender string
+	// Kind specifies the receiver-defined message type.
+	Kind any
+	// Data contains the receiver-defined message data.
+	Data any
+}
+
 // MessageSubscriber is a message subscriber interface.
 type MessageSubscriber interface {
 	// ExecuteMessage executes a given message.
-	ExecuteMessage(ctx *Context, kind, msg any) (any, error)
+	ExecuteMessage(ctx *Context, msg Message) (any, error)
 }
 
 // TogglableMessageSubscriber is a message subscriber that can be disabled.
@@ -29,13 +39,12 @@ type MessageDispatcher interface {
 	// Subscribe subscribes a given message subscriber to messages of a specific kind.
 	Subscribe(kind any, ms MessageSubscriber)
 
-	// Publish publishes a message of a given kind by dispatching to all subscribers.
-	// Subscribers can return a result, but at most one subscriber should return a
-	// non-nil result to any published message. Panics in case more than one subscriber
-	// returns a non-nil result.
+	// Publish publishes a message to all subscribers./ Subscribers can return a result,
+	// but at most one subscriber should return a non-nil result to any published message.
+	// Panics in case more than one subscriber returns a non-nil result.
 	//
 	// In case there are no subscribers ErrNoSubscribers is returned.
-	Publish(ctx *Context, kind, msg any) (any, error)
+	Publish(ctx *Context, msg Message) (any, error)
 }
 
 // NoopMessageDispatcher is a no-op message dispatcher that performs no dispatch.
@@ -46,7 +55,7 @@ func (nd *NoopMessageDispatcher) Subscribe(any, MessageSubscriber) {
 }
 
 // Publish implements MessageDispatcher.
-func (nd *NoopMessageDispatcher) Publish(*Context, any, any) (any, error) {
+func (nd *NoopMessageDispatcher) Publish(*Context, Message) (any, error) {
 	return nil, nil
 }
 

go/consensus/cometbft/api/app.go

@@ -65,8 +65,7 @@ type Context struct {
 	mode        ContextMode
 	currentTime time.Time
 
-	isMessageExecution bool
-	isTransaction      bool
+	isTransaction bool
 
 	data           any
 	events         []types.Event
@@ -217,22 +216,21 @@ func (c *Context) CallDepth() int {
 
 // NewChild creates a new child context that shares state with the current context.
 //
-// If you want isolated state and events use NewTransaction instad.
+// If you want isolated state and events use NewTransaction instead.
 func (c *Context) NewChild() *Context {
 	cc := &Context{
-		parent:             c,
-		mode:               c.mode,
-		currentTime:        c.currentTime,
-		isMessageExecution: c.isMessageExecution,
-		gasAccountant:      c.gasAccountant,
-		txSigner:           c.txSigner,
-		callerAddress:      c.callerAddress,
-		appState:           c.appState,
-		state:              c.state,
-		lastHeight:         c.lastHeight,
-		blockCtx:           c.blockCtx,
-		initialHeight:      c.initialHeight,
-		logger:             c.logger,
+		parent:        c,
+		mode:          c.mode,
+		currentTime:   c.currentTime,
+		gasAccountant: c.gasAccountant,
+		txSigner:      c.txSigner,
+		callerAddress: c.callerAddress,
+		appState:      c.appState,
+		state:         c.state,
+		lastHeight:    c.lastHeight,
+		blockCtx:      c.blockCtx,
+		initialHeight: c.initialHeight,
+		logger:        c.logger,
 	}
 	cc.Context = context.WithValue(c.Context, contextKey{}, cc)
 	return cc
@@ -295,13 +293,6 @@ func (c *Context) WithSimulation() *Context {
 	return child
 }
 
-// WithMessageExecution creates a child context and sets the message execution flag.
-func (c *Context) WithMessageExecution() *Context {
-	child := c.NewChild()
-	child.isMessageExecution = true
-	return child
-}
-
 // IsInitChain returns true if this ia an init chain context.
 func (c *Context) IsInitChain() bool {
 	return c.mode == ContextInitChain
@@ -317,11 +308,6 @@ func (c *Context) IsSimulation() bool {
 	return c.mode == ContextSimulateTx
 }
 
-// IsMessageExecution returns true if this is a message execution context.
-func (c *Context) IsMessageExecution() bool {
-	return c.isMessageExecution
-}
-
 // EmitData emits data to be serialized as transaction output.
 //
 // Note: The use of this has mostly been replaced with EmitEvent, please

go/consensus/cometbft/api/context.go

@@ -84,11 +84,6 @@ func TestChildContext(t *testing.T) {
 	child.EmitEvent(NewEventBuilder("test").TypedAttribute(&FooEvent{Bar: []byte("bar")}))
 	child.Close()
 	require.Empty(ctx.GetEvents(), "events should not propagate in simulation mode")
-
-	child = ctx.WithMessageExecution()
-	require.True(child.IsMessageExecution(), "child should have message execution enabled")
-	require.False(ctx.IsMessageExecution(), "parent should not have message execution enabled")
-	child.Close()
 }
 
 func TestTransactionContext(t *testing.T) {