[Flow EVM] improvements to COA proof verification

fvm/evm/handler/handler.go

@@ -12,7 +12,6 @@ import (
 	"github.com/onflow/flow-go/fvm/environment"
 	fvmErrors "github.com/onflow/flow-go/fvm/errors"
 	"github.com/onflow/flow-go/fvm/evm/handler/coa"
-	"github.com/onflow/flow-go/fvm/evm/precompiles"
 	"github.com/onflow/flow-go/fvm/evm/types"
 	"github.com/onflow/flow-go/model/flow"
 )
@@ -52,24 +51,10 @@ func NewContractHandler(
 		addressAllocator:   addressAllocator,
 		backend:            backend,
 		emulator:           emulator,
-		precompiles:        getPrecompiles(evmContractAddress, addressAllocator, backend),
+		precompiles:        preparePrecompiles(evmContractAddress, addressAllocator, backend),
 	}
 }
 
-func getPrecompiles(
-	evmContractAddress flow.Address,
-	addressAllocator types.AddressAllocator,
-	backend types.Backend,
-) []types.Precompile {
-	archAddress := addressAllocator.AllocatePrecompileAddress(1)
-	archContract := precompiles.ArchContract(
-		archAddress,
-		backend.GetCurrentBlockHeight,
-		COAOwnershipProofValidator(evmContractAddress, backend),
-	)
-	return []types.Precompile{archContract}
-}
-
 // DeployCOA deploys a cadence-owned-account and returns the address
 func (h *ContractHandler) DeployCOA(uuid uint64) types.Address {
 	addr, err := h.deployCOA(uuid)

fvm/evm/handler/precompiles.go

@@ -0,0 +1,71 @@
+package handler
+
+import (
+	"fmt"
+
+	"github.com/onflow/cadence"
+	"github.com/onflow/cadence/runtime/sema"
+
+	"github.com/onflow/flow-go/fvm/environment"
+	"github.com/onflow/flow-go/fvm/evm/precompiles"
+	"github.com/onflow/flow-go/fvm/evm/types"
+	"github.com/onflow/flow-go/model/flow"
+)
+
+func preparePrecompiles(
+	evmContractAddress flow.Address,
+	addressAllocator types.AddressAllocator,
+	backend types.Backend,
+) []types.Precompile {
+	archAddress := addressAllocator.AllocatePrecompileAddress(1)
+	archContract := precompiles.ArchContract(
+		archAddress,
+		blockHeightProvider(backend),
+		coaOwnershipProofValidator(evmContractAddress, backend),
+	)
+	return []types.Precompile{archContract}
+}
+
+func blockHeightProvider(backend types.Backend) func() (uint64, error) {
+	return func() (uint64, error) {
+		h, err := backend.GetCurrentBlockHeight()
+		if types.IsAFatalError(err) || types.IsABackendError(err) {
+			panic(err)
+		}
+		return h, err
+	}
+}
+
+func coaOwnershipProofValidator(contractAddress flow.Address, backend types.Backend) func(proof *types.COAOwnershipProofInContext) (bool, error) {
+	return func(proof *types.COAOwnershipProofInContext) (bool, error) {
+		value, err := backend.Invoke(
+			environment.ContractFunctionSpec{
+				AddressFromChain: func(_ flow.Chain) flow.Address {
+					return contractAddress
+				},
+				LocationName: "EVM",
+				FunctionName: "validateCOAOwnershipProof",
+				ArgumentTypes: []sema.Type{
+					types.FlowAddressSemaType,
+					types.PublicPathSemaType,
+					types.SignedDataSemaType,
+					types.KeyIndicesSemaType,
+					types.SignaturesSemaType,
+					types.AddressBytesSemaType,
+				},
+			},
+			proof.ToCadenceValues(),
+		)
+		if err != nil {
+			if types.IsAFatalError(err) || types.IsABackendError(err) {
+				panic(err)
+			}
+			return false, err
+		}
+		data, ok := value.(cadence.Struct)
+		if !ok || len(data.Fields) == 0 {
+			return false, fmt.Errorf("invalid output data received from validateCOAOwnershipProof")
+		}
+		return bool(data.Fields[0].(cadence.Bool)), nil
+	}
+}

fvm/evm/stdlib/contract.cdc

@@ -233,6 +233,21 @@ contract EVM {
         return InternalEVM.decodeABI(types: types, data: data)
     }
 
+    /// ValidationResult returns the result of COA ownership proof validation
+    access(all)
+    struct ValidationResult {
+        access(all)
+        let isValid: Bool
+
+        access(all)
+        let problem: String?
+
+        init(isValid: Bool, problem: String?) {
+            self.isValid = isValid
+            self.problem = problem
+        }
+    }
+
     /// validateCOAOwnershipProof validates a COA ownership proof
     access(all)
     fun validateCOAOwnershipProof(
@@ -242,12 +257,16 @@ contract EVM {
         keyIndices: [UInt64],
         signatures: [[UInt8]],
         evmAddress: [UInt8; 20]
-    ) {
+    ): ValidationResult {
 
         // make signature set first 
         // check number of signatures matches number of key indices
-        assert(keyIndices.length == signatures.length,
-               message: "key indices size doesn't match the signatures")
+        if keyIndices.length != signatures.length {
+            return ValidationResult(
+                isValid: false,
+                problem: "key indices size doesn't match the signatures"
+            )
+        }
 
         var signatureSet: [Crypto.KeyListSignature] = []
         var idx = 0 
@@ -278,18 +297,40 @@ contract EVM {
             signatureSet: signatureSet,
             signedData: signedData
         )
-        assert(isValid, message: "signatures not valid")
+
+        if !isValid{
+            return ValidationResult(
+                isValid: false,
+                problem: "the given signatures are not valid or provide enough weight" 
+            )
+        }
 
         let coaRef = acc.getCapability(path)
             .borrow<&EVM.CadenceOwnedAccount{EVM.Addressable}>()
-            ?? panic("could not borrow coa resource addressable capability")
+
+        if coaRef == nil {
+             return ValidationResult(
+                 isValid: false,
+                 problem: "could not borrow bridge account's resource"
+             )
+        }
 
         // verify evm address matching
         var i = 0
-        for item in coaRef.address().bytes {
-            assert(item == evmAddress[i], message: "evm address mismatch")
+        var addr = coaRef!.address()
+        for item in addr.bytes {
+            if item != evmAddress[i] {
+                return ValidationResult(
+                    isValid: false,
+                    problem: "EVM address mismatch"
+                )
+            }
             i = i +1
         }
-
+
+        return ValidationResult(
+        	isValid: true,
+        	problem: nil
+        )
     }
 }

fvm/evm/stdlib/contract_test.go

@@ -3680,7 +3680,6 @@ func TestEVMAccountBalanceForABIOnlyContract(t *testing.T) {
 }
 
 func TestEVMValidateCOAOwnershipProof(t *testing.T) {
-
 	t.Parallel()
 
 	contractsAddress := flow.BytesToAddress([]byte{0x1})