Merge #4060

4060: Remove double counting of imports r=janezpodhostnik a=janezpodhostnik

closes: onflow/cadence#1684

This removes metering imports multiple times within the same "scope".

with this scenario:
```
A
B imports A
C imports A,B
D imports C,A, B  <- order is important
```

this is the loading process:

- scope D: count importing D
    - scope C: count importing C
        - count importing A
        - scope B: 
            - count importing  A (even though it has been seen in scope C it has not been seen in scope B so we need to count it)
    - don't count importing A it has already been seen in scope D
    - don't count importing B (and dependencies) it has already been seen in scope D

The reason for this is that any of those individual "scopes"/contracts/programs need to be loaded independently because they are cached independently.


Co-authored-by: Janez Podhostnik <[email protected]>

Author: bors[bot]

fvm/derived/derived_block_data.go

@@ -18,6 +18,7 @@ type DerivedTransaction interface {
 		*Program,
 		error,
 	)
+	GetProgram(location common.AddressLocation) (*Program, bool)
 
 	GetMeterParamOverrides(
 		txnState state.NestedTransaction,
@@ -185,6 +186,19 @@ func (transaction *DerivedTransactionData) GetOrComputeProgram(
 		programComputer)
 }
 
+// GetProgram returns the program for the given address location.
+// This does NOT apply reads/metering to any nested transaction.
+// Use with caution!
+func (transaction *DerivedTransactionData) GetProgram(
+	location common.AddressLocation,
+) (
+	*Program,
+	bool,
+) {
+	program, _, ok := transaction.programs.get(location)
+	return program, ok
+}
+
 func (transaction *DerivedTransactionData) AddInvalidator(
 	invalidator TransactionInvalidator,
 ) {

fvm/environment/programs.go

@@ -94,6 +94,24 @@ func (programs *Programs) getOrLoadAddressProgram(
 	load func() (*interpreter.Program, error),
 ) (*interpreter.Program, error) {
 
+	if programs.dependencyStack.top().ContainsLocation(location) {
+		// this dependency has already been seen in the current stack/scope
+		// this means that it is safe to just fetch it and not reapply
+		// state/metering changes
+		program, ok := programs.txnState.GetProgram(location)
+		if !ok {
+			// program should be in the cache, if it is not,
+			// this means there is an implementation error
+			return nil, errors.NewDerivedDataCacheImplementationFailure(
+				fmt.Errorf("expected program missing"+
+					" in cache for location: %s", location))
+		}
+		programs.dependencyStack.add(program.Dependencies)
+		programs.cacheHit()
+
+		return program.Program, nil
+	}
+
 	loader := newProgramLoader(load, programs.dependencyStack, location)
 	program, err := programs.txnState.GetOrComputeProgram(
 		programs.txnState,
@@ -201,12 +219,17 @@ func (loader *programLoader) Compute(
 		// This should never happen, as the program loader is only called once per
 		// program. The same loader is never reused. This is only here to make
 		// this more apparent.
-		panic("program loader called twice")
+		return nil,
+			errors.NewDerivedDataCacheImplementationFailure(
+				fmt.Errorf("program loader called twice"))
 	}
+
 	if loader.location != location {
 		// This should never happen, as the program loader constructed specifically
 		// to load one location once. This is only a sanity check.
-		panic("program loader called with unexpected location")
+		return nil,
+			errors.NewDerivedDataCacheImplementationFailure(
+				fmt.Errorf("program loader called with unexpected location"))
 	}
 
 	loader.called = true
@@ -339,7 +362,9 @@ func (s *dependencyStack) pop() (common.Location, derived.ProgramDependencies, e
 	if len(s.trackers) <= 1 {
 		return nil,
 			derived.NewProgramDependencies(),
-			fmt.Errorf("cannot pop the programs dependency stack, because it is empty")
+			errors.NewDerivedDataCacheImplementationFailure(
+				fmt.Errorf("cannot pop the programs" +
+					" dependency stack, because it is empty"))
 	}
 
 	// pop the last tracker
@@ -354,3 +379,14 @@ func (s *dependencyStack) pop() (common.Location, derived.ProgramDependencies, e
 
 	return tracker.location, tracker.dependencies, nil
 }
+
+// top returns the last dependencies on the stack without pop-ing them.
+func (s *dependencyStack) top() derived.ProgramDependencies {
+	l := len(s.trackers)
+	if l == 0 {
+		// This cannot happen, as the root of the stack is always present.
+		panic("Dependency stack unexpectedly empty")
+	}
+
+	return s.trackers[len(s.trackers)-1].dependencies
+}