Merge pull request #5845 from onflow/jord/5828-allow-service-events-d…

…efault

Enable service events for all collection context

engine/execution/computation/computer/computer.go

@@ -126,7 +126,6 @@ func SystemChunkContext(vmCtx fvm.Context) fvm.Context {
 		fvm.WithAuthorizationChecksEnabled(false),
 		fvm.WithSequenceNumberCheckAndIncrementEnabled(false),
 		fvm.WithTransactionFeesEnabled(false),
-		fvm.WithServiceEventCollectionEnabled(),
 		fvm.WithEventCollectionSizeLimit(SystemChunkEventCollectionMaxSize),
 		fvm.WithMemoryAndInteractionLimitsDisabled(),
 		// only the system transaction is allowed to call the block entropy provider

engine/execution/computation/computer/computer_test.go

@@ -588,7 +588,6 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) {
 	t.Run(
 		"service events are emitted", func(t *testing.T) {
 			execCtx := fvm.NewContext(
-				fvm.WithServiceEventCollectionEnabled(),
 				fvm.WithAuthorizationChecksEnabled(false),
 				fvm.WithSequenceNumberCheckAndIncrementEnabled(false),
 			)

fvm/context.go

@@ -67,7 +67,7 @@ func newContext(ctx Context, opts ...Option) Context {
 }
 
 func defaultContext() Context {
-	return Context{
+	ctx := Context{
 		DisableMemoryAndInteractionLimits: false,
 		ComputationLimit:                  DefaultComputationLimit,
 		MemoryLimit:                       DefaultMemoryLimit,
@@ -77,6 +77,7 @@ func defaultContext() Context {
 		TransactionExecutorParams:         DefaultTransactionExecutorParams(),
 		EnvironmentParams:                 environment.DefaultEnvironmentParams(),
 	}
+	return ctx
 }
 
 // An Option sets a configuration parameter for a virtual machine context.
@@ -185,14 +186,6 @@ func WithBlockHeader(header *flow.Header) Option {
 	}
 }
 
-// WithServiceEventCollectionEnabled enables service event collection
-func WithServiceEventCollectionEnabled() Option {
-	return func(ctx Context) Context {
-		ctx.ServiceEventCollectionEnabled = true
-		return ctx
-	}
-}
-
 // WithBlocks sets the block storage provider for a virtual machine context.
 //
 // The VM uses the block storage provider to provide historical block information to

fvm/environment/event_emitter.go

@@ -19,16 +19,14 @@ const (
 )
 
 type EventEmitterParams struct {
-	ServiceEventCollectionEnabled bool
-	EventCollectionByteSizeLimit  uint64
-	EventEncoder                  EventEncoder
+	EventCollectionByteSizeLimit uint64
+	EventEncoder                 EventEncoder
 }
 
 func DefaultEventEmitterParams() EventEmitterParams {
 	return EventEmitterParams{
-		ServiceEventCollectionEnabled: false,
-		EventCollectionByteSizeLimit:  DefaultEventCollectionByteSizeLimit,
-		EventEncoder:                  NewCadenceEventEncoder(),
+		EventCollectionByteSizeLimit: DefaultEventCollectionByteSizeLimit,
+		EventEncoder:                 NewCadenceEventEncoder(),
 	}
 }
 
@@ -191,27 +189,24 @@ func (emitter *eventEmitter) EmitEvent(event cadence.Event) error {
 	// TODO: to set limit to maximum when it is service account and get rid of this flag
 	isServiceAccount := emitter.payer == emitter.chain.ServiceAddress()
 
-	if emitter.ServiceEventCollectionEnabled {
-		ok, err := IsServiceEvent(eventType, emitter.chain.ChainID())
-		if err != nil {
-			return fmt.Errorf("unable to check service event: %w", err)
-		}
-		if ok {
-			eventEmitError := emitter.eventCollection.AppendServiceEvent(
-				emitter.chain,
-				flowEvent,
-				uint64(payloadSize))
-
-			// skip limit if payer is service account
-			// TODO skip only limit-related errors
-			if !isServiceAccount && eventEmitError != nil {
-				return eventEmitError
-			}
+	isServiceEvent, err := IsServiceEvent(eventType, emitter.chain.ChainID())
+	if err != nil {
+		return fmt.Errorf("unable to check service event: %w", err)
+	}
+	if isServiceEvent {
+		eventEmitError := emitter.eventCollection.AppendServiceEvent(
+			emitter.chain,
+			flowEvent,
+			uint64(payloadSize))
+
+		// skip limit if payer is service account
+		// TODO skip only limit-related errors
+		if !isServiceAccount && eventEmitError != nil {
+			return eventEmitError
 		}
-		// We don't return and append the service event into event collection
-		// as well.
 	}
 
+	// Regardless of whether it is a service event, add to eventCollection
 	eventEmitError := emitter.eventCollection.AppendEvent(flowEvent, uint64(payloadSize))
 	// skip limit if payer is service account
 	if !isServiceAccount {
@@ -295,8 +290,11 @@ func (collection *EventCollection) TotalEventCounter() uint32 {
 	return collection.eventCounter
 }
 
-// IsServiceEvent determines whether or not an emitted Cadence event is
-// considered a service event for the given chain.
+// IsServiceEvent determines whether an emitted Cadence event is considered a service event for the given chain.
+// An event is a service event if it is defined in the `systemcontracts` package allow-list.
+// Note that we have *removed* the prior constraint that service events can only be
+// emitted in the system chunk. Now a system smart contract can emit service events
+// as part of any transaction.
 func IsServiceEvent(eventType flow.EventType, chain flow.ChainID) (bool, error) {
 
 	// retrieve the service event information for this chain

fvm/environment/event_emitter_test.go

@@ -175,9 +175,8 @@ func createTestEventEmitterWithLimit(chain flow.ChainID, address flow.Address, e
 			},
 		},
 		environment.EventEmitterParams{
-			ServiceEventCollectionEnabled: false,
-			EventCollectionByteSizeLimit:  eventEmitLimit,
-			EventEncoder:                  environment.NewCadenceEventEncoder(),
+			EventCollectionByteSizeLimit: eventEmitLimit,
+			EventEncoder:                 environment.NewCadenceEventEncoder(),
 		},
 	)
 }