Merge pull request #5596 from onflow/supun/improve-contract-errors

onflow · Mar 28, 2024 · 11040f7 · 11040f7
2 parents b6a64cc + d27e701
commit 11040f7
Show file tree

Hide file tree

Showing 2 changed files with 184 additions and 1 deletion.
diff --git a/cmd/util/ledger/migrations/staged_contracts_migration.go b/cmd/util/ledger/migrations/staged_contracts_migration.go
@@ -261,7 +261,7 @@ func (m *StagedContractsMigration) MigrateAccount(
 				Name:    name,
 				Address: address,
 			}
-			printErr := errorPrinter.PrettyPrintError(err, location, nil)
+			printErr := errorPrinter.PrettyPrintError(err, location, m.contractsByLocation)
 
 			var errorDetails string
 			if printErr == nil {

diff --git a/cmd/util/ledger/migrations/staged_contracts_migration_test.go b/cmd/util/ledger/migrations/staged_contracts_migration_test.go
@@ -2,6 +2,7 @@ package migrations
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"testing"
@@ -1505,3 +1506,185 @@ func TestConcurrentContractUpdate(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, allPayloads, 5)
 }
+
+func TestStagedContractsUpdateValidationErrors(t *testing.T) {
+	t.Parallel()
+
+	chainID := flow.Emulator
+	systemContracts := systemcontracts.SystemContractsForChain(chainID)
+
+	addressGenerator := chainID.Chain().NewAddressGenerator()
+
+	address, err := addressGenerator.NextAddress()
+	require.NoError(t, err)
+
+	t.Run("field mismatch", func(t *testing.T) {
+		t.Parallel()
+
+		oldCodeA := `
+            access(all) contract Test {
+                access(all) var a: Int
+                init() {
+                    self.a = 0
+                }
+            }
+        `
+
+		newCodeA := `
+            access(all) contract Test {
+                access(all) var a: String
+                init() {
+                    self.a = "hello"
+                }
+            }
+        `
+
+		stagedContracts := []StagedContract{
+			{
+				Contract: Contract{
+					Name: "A",
+					Code: []byte(newCodeA),
+				},
+				Address: common.Address(address),
+			},
+		}
+
+		logWriter := &logWriter{}
+		log := zerolog.New(logWriter)
+
+		migration := NewStagedContractsMigration(chainID, log).
+			WithContractUpdateValidation()
+
+		migration.RegisterContractUpdates(stagedContracts)
+
+		payloads := []*ledger.Payload{
+			newContractPayload(common.Address(address), "A", []byte(oldCodeA)),
+		}
+
+		err = migration.InitMigration(log, payloads, 0)
+		require.NoError(t, err)
+
+		_, err = migration.MigrateAccount(
+			context.Background(),
+			common.Address(address),
+			payloads,
+		)
+		require.NoError(t, err)
+
+		err = migration.Close()
+		require.NoError(t, err)
+
+		require.Len(t, logWriter.logs, 1)
+
+		var jsonObject map[string]any
+
+		err := json.Unmarshal([]byte(logWriter.logs[0]), &jsonObject)
+		require.NoError(t, err)
+
+		assert.Equal(
+			t,
+			"failed to update contract A in account 0xf8d6e0586b0a20c7: error: mismatching field `a` in `Test`\n"+
+				" --> f8d6e0586b0a20c7.A:3:35\n"+
+				"  |\n"+
+				"3 |                 access(all) var a: String\n"+
+				"  |                                    ^^^^^^ incompatible type annotations. expected `Int`, found `String`\n",
+			jsonObject["message"],
+		)
+	})
+
+	t.Run("field mismatch with entitlements", func(t *testing.T) {
+		t.Parallel()
+
+		nftAddress := common.Address(systemContracts.NonFungibleToken.Address)
+
+		oldCodeA := fmt.Sprintf(`
+            import NonFungibleToken from %s
+
+            access(all) contract Test {
+                access(all) var a: Capability<&{NonFungibleToken.Provider}>?
+                init() {
+                    self.a = nil
+                }
+            }
+        `,
+			nftAddress.HexWithPrefix(),
+		)
+
+		newCodeA := fmt.Sprintf(`
+            import NonFungibleToken from %s
+
+            access(all) contract Test {
+                access(all) var a: Capability<auth(NonFungibleToken.E1) &{NonFungibleToken.Provider}>?
+                init() {
+                    self.a = nil
+                }
+            }
+        `,
+			nftAddress.HexWithPrefix(),
+		)
+
+		nftContract := `
+            access(all) contract NonFungibleToken {
+
+                access(all) entitlement E1
+                access(all) entitlement E2
+
+                access(all) resource interface Provider {
+                    access(E1) fun foo()
+                    access(E2) fun bar()
+                }
+		    }
+        `
+
+		stagedContracts := []StagedContract{
+			{
+				Contract: Contract{
+					Name: "A",
+					Code: []byte(newCodeA),
+				},
+				Address: common.Address(address),
+			},
+		}
+
+		logWriter := &logWriter{}
+		log := zerolog.New(logWriter)
+
+		migration := NewStagedContractsMigration(chainID, log).
+			WithContractUpdateValidation()
+
+		migration.RegisterContractUpdates(stagedContracts)
+
+		contractACode := newContractPayload(common.Address(address), "A", []byte(oldCodeA))
+		nftCode := newContractPayload(nftAddress, "NonFungibleToken", []byte(nftContract))
+
+		accountPayloads := []*ledger.Payload{contractACode}
+		allPayloads := []*ledger.Payload{contractACode, nftCode}
+
+		err = migration.InitMigration(log, allPayloads, 0)
+		require.NoError(t, err)
+
+		_, err = migration.MigrateAccount(
+			context.Background(),
+			common.Address(address),
+			accountPayloads,
+		)
+		require.NoError(t, err)
+
+		require.Len(t, logWriter.logs, 1)
+
+		var jsonObject map[string]any
+		err := json.Unmarshal([]byte(logWriter.logs[0]), &jsonObject)
+		require.NoError(t, err)
+
+		assert.Equal(
+			t,
+			"failed to update contract A in account 0xf8d6e0586b0a20c7: error: mismatching field `a` in `Test`\n"+
+				" --> f8d6e0586b0a20c7.A:5:35\n"+
+				"  |\n"+
+				"5 |                 access(all) var a: Capability<auth(NonFungibleToken.E1) &{NonFungibleToken.Provider}>?\n"+
+				"  |                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ mismatching authorization:"+
+				" the entitlements migration would only grant this value `NonFungibleToken.E1, NonFungibleToken.E2`, but the annotation present is `NonFungibleToken.E1`\n",
+			jsonObject["message"],
+		)
+	})
+}