Merge pull request #4465 from oasisprotocol/ptrus/feature/typed-attri…

…butes

go/consensus: use typed attributes in all services

.changelog/4034.breaking.md

@@ -0,0 +1,4 @@
+Use typed attributes in all consensus services
+
+Beacon, keymanager, registry, roothash and scheduler events are updated to use
+the typed event attribute API.

.changelog/4465.breaking.md

@@ -0,0 +1,7 @@
+consensus: base64 encode all tendermint event values
+
+Tendermint 0.35 requires all events to be actual strings and doesn't support
+binary data anymore. It also requires no dashes in attribute keys.
+
+Although oasis-core still uses Tendermint 0.34, events are updated now to
+avoid breaking changes later.

go/beacon/api/api.go

@@ -186,3 +186,25 @@ func (g *Genesis) SanityCheck() error {
 
 	return nil
 }
+
+// EpochEvent is the epoch event.
+type EpochEvent struct {
+	// Epoch is the new epoch.
+	Epoch EpochTime `json:"epoch,omitempty"`
+}
+
+// EventKind returns a string representation of this event's kind.
+func (ev *EpochEvent) EventKind() string {
+	return "epoch"
+}
+
+// BeaconEvent is the beacon event.
+type BeaconEvent struct {
+	// Beacon is the new beacon value.
+	Beacon []byte `json:"beacon,omitempty"`
+}
+
+// EventKind returns a string representation of this event's kind.
+func (ev *BeaconEvent) EventKind() string {
+	return "beacon"
+}

go/consensus/api/events/events.go

@@ -0,0 +1,57 @@
+package events
+
+import (
+	"bytes"
+	"encoding/base64"
+	"fmt"
+
+	"github.com/oasisprotocol/oasis-core/go/common/cbor"
+)
+
+// TypedAttribute is an interface implemented by types which can be transparently used as event
+// attributes with CBOR-marshalled value.
+type TypedAttribute interface {
+	// EventKind returns a string representation of this event's kind.
+	EventKind() string
+}
+
+// CustomTypedAttribute is an interface implemented by types which can be transparently used as event
+// attributes with custom value encoding.
+type CustomTypedAttribute interface {
+	TypedAttribute
+
+	// EventValue returns a string representation of this events value.
+	EventValue() string
+
+	// DecodeValue decodes the value encoded vy the EventValue.
+	DecodeValue(value string) error
+}
+
+// IsAttributeKind checks whether the given attribute key corresponds to the passed typed attribute.
+func IsAttributeKind(key []byte, kind TypedAttribute) bool {
+	return bytes.Equal(key, []byte(kind.EventKind()))
+}
+
+// DecodeValue decodes the attribute event value.
+func DecodeValue(value string, ev TypedAttribute) error {
+	// Use custom decode if this is a custom typed attribute.
+	if cta, ok := ev.(CustomTypedAttribute); ok {
+		return cta.DecodeValue(value)
+	}
+	// Otherwise assume default base64 encoded CBOR marshalled value.
+	decoded, err := base64.StdEncoding.DecodeString(value)
+	if err != nil {
+		return fmt.Errorf("invalid value: %w", err)
+	}
+	return cbor.Unmarshal(decoded, ev)
+}
+
+// EncodeValue encodes the attribute event value.
+func EncodeValue(ev TypedAttribute) []byte {
+	// Use custom decode if this is a custom typed attribute.
+	if cta, ok := ev.(CustomTypedAttribute); ok {
+		return []byte(cta.EventValue())
+	}
+	// Otherwise default to base64 encoded CBOR marshalled value.
+	return []byte(base64.StdEncoding.EncodeToString(cbor.Marshal(ev)))
+}

go/consensus/tendermint/api/api.go

@@ -2,7 +2,6 @@
 package api
 
 import (
-	"bytes"
 	"context"
 	"fmt"
 	"strings"
@@ -21,6 +20,7 @@ import (
 	"github.com/oasisprotocol/oasis-core/go/common/node"
 	"github.com/oasisprotocol/oasis-core/go/common/pubsub"
 	consensus "github.com/oasisprotocol/oasis-core/go/consensus/api"
+	"github.com/oasisprotocol/oasis-core/go/consensus/api/events"
 	"github.com/oasisprotocol/oasis-core/go/consensus/api/transaction"
 	"github.com/oasisprotocol/oasis-core/go/consensus/tendermint/crypto"
 	mkvsNode "github.com/oasisprotocol/oasis-core/go/storage/mkvs/node"
@@ -84,26 +84,14 @@ func NodeToP2PAddr(n *node.Node) (*tmp2p.NetAddress, error) {
 	return tmAddr, nil
 }
 
-// TypedAttribute is an interface implemented by types which can be transparently used as event
-// attributes.
-type TypedAttribute interface {
-	// EventKind returns a string representation of this event's kind.
-	EventKind() string
-}
-
-// IsAttributeKind checks whether the given attribute key corresponds to the passed typed attribute.
-func IsAttributeKind(key []byte, kind TypedAttribute) bool {
-	return bytes.Equal(key, []byte(kind.EventKind()))
-}
-
 // EventBuilder is a helper for constructing ABCI events.
 type EventBuilder struct {
 	app []byte
 	ev  types.Event
 }
 
-// Attribute appends a key/value pair to the event.
-func (bld *EventBuilder) Attribute(key, value []byte) *EventBuilder {
+// attribute appends a key/value pair to the event.
+func (bld *EventBuilder) attribute(key, value []byte) *EventBuilder {
 	bld.ev.Attributes = append(bld.ev.Attributes, types.EventAttribute{
 		Key:   key,
 		Value: value,
@@ -113,11 +101,8 @@ func (bld *EventBuilder) Attribute(key, value []byte) *EventBuilder {
 }
 
 // TypedAttribute appends a typed attribute to the event.
-//
-// The typed attribute is automatically converted to a key/value pair where its EventKind is used
-// as the key and a CBOR-marshalled value is used as value.
-func (bld *EventBuilder) TypedAttribute(value TypedAttribute) *EventBuilder {
-	return bld.Attribute([]byte(value.EventKind()), cbor.Marshal(value))
+func (bld *EventBuilder) TypedAttribute(value events.TypedAttribute) *EventBuilder {
+	return bld.attribute([]byte(value.EventKind()), events.EncodeValue(value))
 }
 
 // Dirty returns true iff the EventBuilder has attributes.
@@ -149,7 +134,7 @@ func NewEventBuilder(app string) *EventBuilder {
 // EventTypeForApp generates the ABCI event type for events belonging
 // to the specified App.
 func EventTypeForApp(eventApp string) string {
-	return "oasis-event-" + eventApp
+	return "oasis_event_" + eventApp
 }
 
 // QueryForApp generates a tmquery.Query for events belonging to the

go/consensus/tendermint/api/api_test.go

@@ -14,9 +14,9 @@ func TestServiceDescriptor(t *testing.T) {
 
 	q1 := tmquery.MustParse("a='b'")
 
-	sd := NewStaticServiceDescriptor("test", "test-type", []tmpubsub.Query{q1})
+	sd := NewStaticServiceDescriptor("test", "test_type", []tmpubsub.Query{q1})
 	require.Equal("test", sd.Name())
-	require.Equal("test-type", sd.EventType())
+	require.Equal("test_type", sd.EventType())
 	recvQ1 := <-sd.Queries()
 	require.EqualValues(q1, recvQ1, "received query should be correct")
 	_, ok := <-sd.Queries()

go/consensus/tendermint/api/context.go

@@ -8,9 +8,9 @@ import (
 
 	"github.com/tendermint/tendermint/abci/types"
 
-	"github.com/oasisprotocol/oasis-core/go/common/cbor"
 	"github.com/oasisprotocol/oasis-core/go/common/crypto/signature"
 	"github.com/oasisprotocol/oasis-core/go/common/logging"
+	"github.com/oasisprotocol/oasis-core/go/consensus/api/events"
 	staking "github.com/oasisprotocol/oasis-core/go/staking/api"
 	"github.com/oasisprotocol/oasis-core/go/storage/mkvs"
 )
@@ -329,8 +329,8 @@ func (c *Context) GetEvents() []types.Event {
 	return c.events
 }
 
-// HasEvent checks if a specific event has been emitted.
-func (c *Context) HasEvent(app string, key []byte) bool {
+// hasEvent checks if a specific event has been emitted.
+func (c *Context) hasEvent(app string, key []byte) bool {
 	evType := EventTypeForApp(app)
 
 	for _, ev := range c.events {
@@ -347,17 +347,17 @@ func (c *Context) HasEvent(app string, key []byte) bool {
 	return false
 }
 
-// HasTypedEvent checks if a specific typed event has been emitted.
-func (c *Context) HasTypedEvent(app string, kind TypedAttribute) bool {
-	return c.HasEvent(app, []byte(kind.EventKind()))
+// HasEvent checks if a specific event has been emitted.
+func (c *Context) HasEvent(app string, kind events.TypedAttribute) bool {
+	return c.hasEvent(app, []byte(kind.EventKind()))
 }
 
-// DecodeTypedEvent decodes the given raw event as a specific typed event.
-func (c *Context) DecodeTypedEvent(index int, ev TypedAttribute) error {
+// DecodeEvent decodes the given raw event as a specific typed event.
+func (c *Context) DecodeEvent(index int, ev events.TypedAttribute) error {
 	raw := c.events[index]
 	for _, pair := range raw.Attributes {
-		if bytes.Equal(pair.GetKey(), []byte(ev.EventKind())) {
-			return cbor.Unmarshal(pair.GetValue(), ev)
+		if events.IsAttributeKind(pair.GetKey(), ev) {
+			return events.DecodeValue(string(pair.GetValue()), ev)
 		}
 	}
 	return fmt.Errorf("incompatible event")

go/consensus/tendermint/api/context_test.go

@@ -16,6 +16,17 @@ func (k testBlockContextKey) NewDefault() interface{} {
 	return 42
 }
 
+// FooEvent is a test event.
+type FooEvent struct {
+	// Bar is the test event value.
+	Bar []byte
+}
+
+// EventKind returns a string representation of this event's kind.
+func (ev *FooEvent) EventKind() string {
+	return "foo"
+}
+
 func TestBlockContext(t *testing.T) {
 	require := require.New(t)
 
@@ -57,7 +68,7 @@ func TestChildContext(t *testing.T) {
 	require.EqualValues(ctx.BlockContext(), child.BlockContext(), "child.BlockContext should correspond to parent.BlockContext")
 
 	// Emitting an event should not propagate to the parent immediately.
-	child.EmitEvent(NewEventBuilder("test").Attribute([]byte("foo"), []byte("bar")))
+	child.EmitEvent(NewEventBuilder("test").TypedAttribute(&FooEvent{Bar: []byte("bar")}))
 	require.Len(child.GetEvents(), 1, "child event should be stored")
 	require.Len(ctx.GetEvents(), 0, "child event should not immediately propagate")
 	events := child.GetEvents()
@@ -70,7 +81,7 @@ func TestChildContext(t *testing.T) {
 	defer ctx.Close()
 
 	child = ctx.WithSimulation()
-	child.EmitEvent(NewEventBuilder("test").Attribute([]byte("foo"), []byte("bar")))
+	child.EmitEvent(NewEventBuilder("test").TypedAttribute(&FooEvent{Bar: []byte("bar")}))
 	child.Close()
 	require.Empty(ctx.GetEvents(), "events should not propagate in simulation mode")
 
@@ -91,7 +102,7 @@ func TestTransactionContext(t *testing.T) {
 	child := ctx.NewTransaction()
 
 	// Emitted events and state updates should not propagate to the parent unless committed.
-	child.EmitEvent(NewEventBuilder("test").Attribute([]byte("foo"), []byte("bar")))
+	child.EmitEvent(NewEventBuilder("test").TypedAttribute(&FooEvent{Bar: []byte("bar")}))
 	require.Len(child.GetEvents(), 1, "child event should be stored")
 	require.Len(ctx.GetEvents(), 0, "child event should not immediately propagate")
 
@@ -113,7 +124,7 @@ func TestTransactionContext(t *testing.T) {
 
 	child = ctx.NewTransaction()
 
-	child.EmitEvent(NewEventBuilder("test").Attribute([]byte("foo"), []byte("bar")))
+	child.EmitEvent(NewEventBuilder("test").TypedAttribute(&FooEvent{Bar: []byte("bar")}))
 	require.Len(child.GetEvents(), 1, "child event should be stored")
 	require.Len(ctx.GetEvents(), 0, "child event should not immediately propagate")
 	events := child.GetEvents()

go/consensus/tendermint/apps/beacon/api.go

@@ -27,12 +27,6 @@ var (
 	// beacon application.
 	QueryApp = api.QueryForApp(AppName)
 
-	// KeyEpoch is the ABCI event attribute for specifying the set epoch.
-	KeyEpoch = []byte("epoch")
-
-	// KeyBeacon is the ABCI event attribute key for the new beacons.
-	KeyBeacon = []byte("beacon")
-
 	// MethodSetEpoch is the method name for setting epochs.
 	MethodSetEpoch = transaction.NewMethodName(AppName, "SetEpoch", beacon.EpochTime(0))
 

go/consensus/tendermint/apps/beacon/beacon.go

@@ -10,7 +10,6 @@ import (
 	"golang.org/x/crypto/sha3"
 
 	beacon "github.com/oasisprotocol/oasis-core/go/beacon/api"
-	"github.com/oasisprotocol/oasis-core/go/common/cbor"
 	"github.com/oasisprotocol/oasis-core/go/consensus/api/transaction"
 	"github.com/oasisprotocol/oasis-core/go/consensus/tendermint/api"
 	beaconState "github.com/oasisprotocol/oasis-core/go/consensus/tendermint/apps/beacon/state"
@@ -90,7 +89,7 @@ func (app *beaconApplication) EndBlock(ctx *api.Context, req types.RequestEndBlo
 }
 
 func (app *beaconApplication) doEmitEpochEvent(ctx *api.Context, epoch beacon.EpochTime) {
-	ctx.EmitEvent(api.NewEventBuilder(app.Name()).Attribute(KeyEpoch, cbor.Marshal(epoch)))
+	ctx.EmitEvent(api.NewEventBuilder(app.Name()).TypedAttribute(&beacon.EpochEvent{Epoch: epoch}))
 }
 
 func (app *beaconApplication) scheduleEpochTransitionBlock(
@@ -111,17 +110,17 @@ func (app *beaconApplication) scheduleEpochTransitionBlock(
 	return nil
 }
 
-func (app *beaconApplication) onNewBeacon(ctx *api.Context, beacon []byte) error {
+func (app *beaconApplication) onNewBeacon(ctx *api.Context, value []byte) error {
 	state := beaconState.NewMutableState(ctx.State())
 
-	if err := state.SetBeacon(ctx, beacon); err != nil {
+	if err := state.SetBeacon(ctx, value); err != nil {
 		ctx.Logger().Error("onNewBeacon: failed to set beacon",
 			"err", err,
 		)
 		return fmt.Errorf("beacon: failed to set beacon: %w", err)
 	}
 
-	ctx.EmitEvent(api.NewEventBuilder(app.Name()).Attribute(KeyBeacon, beacon))
+	ctx.EmitEvent(api.NewEventBuilder(app.Name()).TypedAttribute(&beacon.BeaconEvent{Beacon: value}))
 
 	return nil
 }

go/consensus/tendermint/apps/keymanager/api.go

@@ -18,8 +18,4 @@ var (
 	// QueryApp is a query for filtering transactions processed by the
 	// key manager application.
 	QueryApp = api.QueryForApp(AppName)
-
-	// KeyStatusUpdate is an ABCI event attribute key for a key manager
-	// status update (value is a CBOR serialized key manager status).
-	KeyStatusUpdate = []byte("status")
 )

go/consensus/tendermint/apps/keymanager/genesis.go

@@ -9,7 +9,6 @@ import (
 	"github.com/tendermint/tendermint/abci/types"
 
 	"github.com/oasisprotocol/oasis-core/go/common"
-	"github.com/oasisprotocol/oasis-core/go/common/cbor"
 	tmapi "github.com/oasisprotocol/oasis-core/go/consensus/tendermint/api"
 	keymanagerState "github.com/oasisprotocol/oasis-core/go/consensus/tendermint/apps/keymanager/state"
 	genesis "github.com/oasisprotocol/oasis-core/go/genesis/api"
@@ -78,7 +77,9 @@ func (app *keymanagerApplication) InitChain(ctx *tmapi.Context, request types.Re
 	}
 
 	if len(toEmit) > 0 {
-		ctx.EmitEvent(tmapi.NewEventBuilder(app.Name()).Attribute(KeyStatusUpdate, cbor.Marshal(toEmit)))
+		ctx.EmitEvent(tmapi.NewEventBuilder(app.Name()).TypedAttribute(&keymanager.StatusUpdateEvent{
+			Statuses: toEmit,
+		}))
 	}
 
 	return nil

go/consensus/tendermint/apps/keymanager/keymanager.go

@@ -185,7 +185,9 @@ func (app *keymanagerApplication) onEpochChange(ctx *tmapi.Context, epoch beacon
 
 	// Emit the update event if required.
 	if len(toEmit) > 0 {
-		ctx.EmitEvent(tmapi.NewEventBuilder(app.Name()).Attribute(KeyStatusUpdate, cbor.Marshal(toEmit)))
+		ctx.EmitEvent(tmapi.NewEventBuilder(app.Name()).TypedAttribute(&api.StatusUpdateEvent{
+			Statuses: toEmit,
+		}))
 	}
 
 	return nil

go/consensus/tendermint/apps/keymanager/transactions.go

@@ -3,7 +3,6 @@ package keymanager
 import (
 	"fmt"
 
-	"github.com/oasisprotocol/oasis-core/go/common/cbor"
 	tmapi "github.com/oasisprotocol/oasis-core/go/consensus/tendermint/api"
 	keymanagerState "github.com/oasisprotocol/oasis-core/go/consensus/tendermint/apps/keymanager/state"
 	registryState "github.com/oasisprotocol/oasis-core/go/consensus/tendermint/apps/registry/state"
@@ -81,7 +80,9 @@ func (app *keymanagerApplication) updatePolicy(
 		panic(fmt.Errorf("failed to set keymanager status: %w", err))
 	}
 
-	ctx.EmitEvent(tmapi.NewEventBuilder(app.Name()).Attribute(KeyStatusUpdate, cbor.Marshal([]*api.Status{newStatus})))
+	ctx.EmitEvent(tmapi.NewEventBuilder(app.Name()).TypedAttribute(&api.StatusUpdateEvent{
+		Statuses: []*api.Status{newStatus},
+	}))
 
 	return nil
 }