Merge pull request #5957 from onflow/janez/fix-dependency-check

Fix FVM call to checkDependencies

engine/execution/computation/computer/computer.go

@@ -132,6 +132,9 @@ func SystemChunkContext(vmCtx fvm.Context) fvm.Context {
 		fvm.WithMemoryAndInteractionLimitsDisabled(),
 		// only the system transaction is allowed to call the block entropy provider
 		fvm.WithRandomSourceHistoryCallAllowed(true),
+
+		// never enable the dependency check for the system transaction
+		fvm.WithDependencyCheckEnabled(false),
 	)
 }
 

fvm/environment/system_contracts.go

@@ -300,6 +300,8 @@ var checkDependenciesSpec = ContractFunctionSpec{
 	},
 }
 
+// CheckDependencies calls the checkDependencies function on the service account.
+// The inputs should be deterministically sorted.
 func (sys *SystemContracts) CheckDependencies(
 	dependencies []common.AddressLocation,
 	authorizers []flow.Address,

fvm/fvm_test.go

@@ -3368,6 +3368,7 @@ func TestDependencyCheck(t *testing.T) {
 
 			deployServiceContractDependencyCheck(fmt.Sprintf(`
 				access(all) fun checkDependencies(_ dependenciesAddresses: [Address], _ dependenciesNames: [String], _ authorizers: [Address]) {
+					// tis is so that is doesnt trigger on the contract update transaction
 					if authorizers.length == 1 && authorizers[0] == %s {
 						return
 					}
@@ -3396,14 +3397,15 @@ func TestDependencyCheck(t *testing.T) {
 			)
 
 			deployServiceContractDependencyCheck(fmt.Sprintf(`
-				pub event TestEvent(_ dependenciesNames: [String])
+				pub event TestEvent(_ dependenciesNames: [String], _ authorizers: [Address])
 				
 				access(all) fun checkDependencies(_ dependenciesAddresses: [Address], _ dependenciesNames: [String], _ authorizers: [Address]) {
+					// tis is so that is doesnt trigger on the contract update transaction
 					if authorizers.length == 1 && authorizers[0] == %s {
 						return
 					}
 
-					emit TestEvent(dependenciesNames)
+					emit TestEvent(dependenciesNames, authorizers)
 				}
 			`, chain.ServiceAddress().HexWithPrefix()))
 
@@ -3432,8 +3434,10 @@ func TestDependencyCheck(t *testing.T) {
 					require.NoError(t, err)
 
 					dependencies := payload.(cadence.Event).Fields[0].(cadence.Array).Values
+					require.Equal(t, []cadence.Value{cadence.String("A"), cadence.String("B"), cadence.String("C"), cadence.String("D")}, dependencies)
 
-					require.ElementsMatch(t, []cadence.Value{cadence.String("D"), cadence.String("C"), cadence.String("B"), cadence.String("A")}, dependencies)
+					authorizers := payload.(cadence.Event).Fields[1].(cadence.Array).Values
+					require.Equal(t, []cadence.Value{cadence.BytesToAddress(accounts[0].Bytes()), cadence.BytesToAddress(chain.ServiceAddress().Bytes())}, authorizers)
 				},
 			)
 		}))

fvm/transactionInvoker.go

@@ -1,15 +1,18 @@
 package fvm
 
 import (
+	"bytes"
 	"fmt"
 	"strconv"
+	"strings"
 
 	"github.com/onflow/cadence/runtime"
 	"github.com/onflow/cadence/runtime/common"
 	"github.com/onflow/cadence/runtime/interpreter"
 	"github.com/rs/zerolog"
 	"go.opentelemetry.io/otel/attribute"
 	otelTrace "go.opentelemetry.io/otel/trace"
+	"golang.org/x/exp/slices"
 
 	"github.com/onflow/flow-go/fvm/environment"
 	"github.com/onflow/flow-go/fvm/errors"
@@ -504,13 +507,16 @@ func (executor *transactionExecutor) commit(
 }
 
 func (executor *transactionExecutor) checkDependencies() error {
-
 	if !executor.ctx.DependencyCheckEnabled {
 		return nil
 	}
 
-	deps, err := executor.env.GetProgramDependencies()
+	// This should not happen, but if it does, we should halt
+	if executor.proc.Transaction == nil {
+		return nil
+	}
 
+	deps, err := executor.env.GetProgramDependencies()
 	if err != nil {
 		return fmt.Errorf("failed to get program dependencies: %w", err)
 	}
@@ -521,18 +527,36 @@ func (executor *transactionExecutor) checkDependencies() error {
 			dependencies = append(dependencies, addressLoc)
 		}
 	}
+	if len(dependencies) == 0 {
+		// this is an unlikely case, but we don't need to check dependencies
+		// if there are none
+		return nil
+	}
+	// sort the dependencies so the execution order is deterministic
+	slices.SortFunc(
+		dependencies,
+		func(a common.AddressLocation, b common.AddressLocation) int {
+			return strings.Compare(a.ID(), b.ID())
+		})
 
 	authorizerSet := make(map[flow.Address]struct{}, len(executor.proc.Transaction.Authorizers))
 	for _, authorizer := range executor.proc.Transaction.Authorizers {
 		authorizerSet[authorizer] = struct{}{}
 	}
 
+	// add the payer as well
 	authorizerSet[executor.proc.Transaction.Payer] = struct{}{}
 
 	auths := make([]flow.Address, 0, len(authorizerSet))
 	for auth := range authorizerSet {
 		auths = append(auths, auth)
 	}
+	// sort the authorizers so the execution order is deterministic
+	slices.SortFunc(
+		auths,
+		func(a flow.Address, b flow.Address) int {
+			return bytes.Compare(a[:], b[:])
+		})
 
 	_, err = executor.env.CheckDependencies(
 		dependencies,