Merge pull request #5665 from onflow/bastian/verbose-log-staged-updat…

…e-failures
onflow · Apr 12, 2024 · f218386 · f218386
2 parents a349738 + 8b748b2
commit f218386
Show file tree

Hide file tree

Showing 5 changed files with 217 additions and 81 deletions.
diff --git a/cmd/util/ledger/migrations/cadence.go b/cmd/util/ledger/migrations/cadence.go
@@ -303,18 +303,30 @@ func NewCadence1ContractsMigrations(
 	opts Options,
 ) []NamedMigration {
 
+	stagedContractsMigrationOptions := StagedContractsMigrationOptions{
+		ChainID:            opts.ChainID,
+		VerboseErrorOutput: opts.VerboseErrorOutput,
+	}
+
+	systemContractsMigrationOptions := SystemContractsMigrationOptions{
+		StagedContractsMigrationOptions: stagedContractsMigrationOptions,
+		EVM:                             opts.EVMContractChange,
+		Burner:                          opts.BurnerContractChange,
+	}
+
 	systemContractsMigration := NewSystemContractsMigration(
-		opts.ChainID,
 		log,
 		rwf,
-		SystemContractChangesOptions{
-			EVM:    opts.EVMContractChange,
-			Burner: opts.BurnerContractChange,
-		},
+		systemContractsMigrationOptions,
 	)
 
-	stagedContractsMigration := NewStagedContractsMigration(opts.ChainID, log, rwf).
-		WithContractUpdateValidation().
+	stagedContractsMigration := NewStagedContractsMigration(
+		"StagedContractsMigration",
+		"staged-contracts-migrator",
+		log,
+		rwf,
+		stagedContractsMigrationOptions,
+	).WithContractUpdateValidation().
 		WithStagedContractUpdates(opts.StagedContracts)
 
 	toAccountBasedMigration := func(migration AccountBasedMigration) ledger.Migration {

diff --git a/cmd/util/ledger/migrations/change_contract_code_migration.go b/cmd/util/ledger/migrations/change_contract_code_migration.go
@@ -13,29 +13,6 @@ import (
 	"github.com/onflow/flow-go/model/flow"
 )
 
-type ChangeContractCodeMigration struct {
-	*StagedContractsMigration
-}
-
-var _ AccountBasedMigration = (*ChangeContractCodeMigration)(nil)
-
-func NewChangeContractCodeMigration(
-	chainID flow.ChainID,
-	log zerolog.Logger,
-	rwf reporters.ReportWriterFactory,
-) *ChangeContractCodeMigration {
-	return &ChangeContractCodeMigration{
-		StagedContractsMigration: &StagedContractsMigration{
-			name:                "ChangeContractCodeMigration",
-			log:                 log,
-			chainID:             chainID,
-			stagedContracts:     map[common.Address]map[flow.RegisterID]Contract{},
-			contractsByLocation: map[common.Location][]byte{},
-			reporter:            rwf.ReportWriter("system-contracts-migrator"),
-		},
-	}
-}
-
 func NewSystemContractChange(
 	systemContract systemcontracts.SystemContract,
 	newContractCode []byte,
@@ -64,11 +41,6 @@ const (
 	BurnerContractChangeUpdate
 )
 
-type SystemContractChangesOptions struct {
-	EVM    EVMContractChange
-	Burner BurnerContractChange
-}
-
 func BurnerAddressForChain(chainID flow.ChainID) flow.Address {
 
 	systemContracts := systemcontracts.SystemContractsForChain(chainID)
@@ -87,7 +59,7 @@ func BurnerAddressForChain(chainID flow.ChainID) flow.Address {
 	}
 }
 
-func SystemContractChanges(chainID flow.ChainID, options SystemContractChangesOptions) []StagedContract {
+func SystemContractChanges(chainID flow.ChainID, options SystemContractsMigrationOptions) []StagedContract {
 	systemContracts := systemcontracts.SystemContractsForChain(chainID)
 
 	env := systemContracts.AsTemplateEnv()
@@ -276,14 +248,25 @@ func SystemContractChanges(chainID flow.ChainID, options SystemContractChangesOp
 	return contractChanges
 }
 
+type SystemContractsMigrationOptions struct {
+	StagedContractsMigrationOptions
+	EVM    EVMContractChange
+	Burner BurnerContractChange
+}
+
 func NewSystemContractsMigration(
-	chainID flow.ChainID,
 	log zerolog.Logger,
 	rwf reporters.ReportWriterFactory,
-	options SystemContractChangesOptions,
-) *ChangeContractCodeMigration {
-	migration := NewChangeContractCodeMigration(chainID, log, rwf)
-	for _, change := range SystemContractChanges(chainID, options) {
+	options SystemContractsMigrationOptions,
+) *StagedContractsMigration {
+	migration := NewStagedContractsMigration(
+		"SystemContractsMigration",
+		"system-contracts-migrator",
+		log,
+		rwf,
+		options.StagedContractsMigrationOptions,
+	)
+	for _, change := range SystemContractChanges(options.ChainID, options) {
 		migration.registerContractChange(change)
 	}
 	return migration

diff --git a/cmd/util/ledger/migrations/change_contract_code_migration_test.go b/cmd/util/ledger/migrations/change_contract_code_migration_test.go
@@ -91,7 +91,11 @@ func TestChangeContractCodeMigration(t *testing.T) {
 
 		rwf := &testReportWriterFactory{}
 
-		migration := migrations.NewChangeContractCodeMigration(flow.Emulator, log, rwf)
+		options := migrations.StagedContractsMigrationOptions{
+			ChainID:            flow.Emulator,
+			VerboseErrorOutput: true,
+		}
+		migration := migrations.NewStagedContractsMigration("test", "test", log, rwf, options)
 
 		err := migration.InitMigration(log, nil, 0)
 		require.NoError(t, err)
@@ -117,7 +121,11 @@ func TestChangeContractCodeMigration(t *testing.T) {
 
 		rwf := &testReportWriterFactory{}
 
-		migration := migrations.NewChangeContractCodeMigration(flow.Emulator, log, rwf)
+		options := migrations.StagedContractsMigrationOptions{
+			ChainID:            flow.Emulator,
+			VerboseErrorOutput: true,
+		}
+		migration := migrations.NewStagedContractsMigration("test", "test", log, rwf, options)
 
 		err := migration.InitMigration(log, nil, 0)
 		require.NoError(t, err)
@@ -152,7 +160,11 @@ func TestChangeContractCodeMigration(t *testing.T) {
 
 		rwf := &testReportWriterFactory{}
 
-		migration := migrations.NewChangeContractCodeMigration(flow.Emulator, log, rwf).
+		options := migrations.StagedContractsMigrationOptions{
+			ChainID:            flow.Emulator,
+			VerboseErrorOutput: true,
+		}
+		migration := migrations.NewStagedContractsMigration("test", "test", log, rwf, options).
 			WithStagedContractUpdates([]migrations.StagedContract{
 				{
 					Address: common.Address(address1),
@@ -196,7 +208,11 @@ func TestChangeContractCodeMigration(t *testing.T) {
 
 		rwf := &testReportWriterFactory{}
 
-		migration := migrations.NewChangeContractCodeMigration(flow.Emulator, log, rwf).
+		options := migrations.StagedContractsMigrationOptions{
+			ChainID:            flow.Emulator,
+			VerboseErrorOutput: true,
+		}
+		migration := migrations.NewStagedContractsMigration("test", "test", log, rwf, options).
 			WithStagedContractUpdates([]migrations.StagedContract{
 				{
 					Address: common.Address(address1),
@@ -238,7 +254,11 @@ func TestChangeContractCodeMigration(t *testing.T) {
 
 		rwf := &testReportWriterFactory{}
 
-		migration := migrations.NewChangeContractCodeMigration(flow.Emulator, log, rwf).
+		options := migrations.StagedContractsMigrationOptions{
+			ChainID:            flow.Emulator,
+			VerboseErrorOutput: true,
+		}
+		migration := migrations.NewStagedContractsMigration("test", "test", log, rwf, options).
 			WithStagedContractUpdates([]migrations.StagedContract{
 				{
 					Address: common.Address(address1),
@@ -287,7 +307,11 @@ func TestChangeContractCodeMigration(t *testing.T) {
 
 		rwf := &testReportWriterFactory{}
 
-		migration := migrations.NewChangeContractCodeMigration(flow.Emulator, log, rwf).
+		options := migrations.StagedContractsMigrationOptions{
+			ChainID:            flow.Emulator,
+			VerboseErrorOutput: true,
+		}
+		migration := migrations.NewStagedContractsMigration("test", "test", log, rwf, options).
 			WithStagedContractUpdates([]migrations.StagedContract{
 				{
 					Address: common.Address(address1),
@@ -333,7 +357,11 @@ func TestChangeContractCodeMigration(t *testing.T) {
 
 		rwf := &testReportWriterFactory{}
 
-		migration := migrations.NewChangeContractCodeMigration(flow.Emulator, log, rwf).
+		options := migrations.StagedContractsMigrationOptions{
+			ChainID:            flow.Emulator,
+			VerboseErrorOutput: true,
+		}
+		migration := migrations.NewStagedContractsMigration("test", "test", log, rwf, options).
 			WithStagedContractUpdates([]migrations.StagedContract{
 				{
 					Address: common.Address(address1),
@@ -379,7 +407,11 @@ func TestChangeContractCodeMigration(t *testing.T) {
 
 		rwf := &testReportWriterFactory{}
 
-		migration := migrations.NewChangeContractCodeMigration(flow.Emulator, log, rwf).
+		options := migrations.StagedContractsMigrationOptions{
+			ChainID:            flow.Emulator,
+			VerboseErrorOutput: true,
+		}
+		migration := migrations.NewStagedContractsMigration("test", "test", log, rwf, options).
 			WithStagedContractUpdates([]migrations.StagedContract{
 				{
 					Address: common.Address(address2),

diff --git a/cmd/util/ledger/migrations/staged_contracts_migration.go b/cmd/util/ledger/migrations/staged_contracts_migration.go
@@ -40,6 +40,7 @@ type StagedContractsMigration struct {
 	contractAdditionHandler        stdlib.AccountContractAdditionHandler
 	contractNamesProvider          stdlib.AccountContractNamesProvider
 	reporter                       reporters.ReportWriter
+	verboseErrorOutput             bool
 }
 
 type StagedContract struct {
@@ -54,18 +55,26 @@ type Contract struct {
 
 var _ AccountBasedMigration = &StagedContractsMigration{}
 
+type StagedContractsMigrationOptions struct {
+	ChainID            flow.ChainID
+	VerboseErrorOutput bool
+}
+
 func NewStagedContractsMigration(
-	chainID flow.ChainID,
+	name string,
+	reporterName string,
 	log zerolog.Logger,
 	rwf reporters.ReportWriterFactory,
+	options StagedContractsMigrationOptions,
 ) *StagedContractsMigration {
 	return &StagedContractsMigration{
-		name:                "StagedContractsMigration",
+		name:                name,
 		log:                 log,
-		chainID:             chainID,
+		chainID:             options.ChainID,
 		stagedContracts:     map[common.Address]map[flow.RegisterID]Contract{},
 		contractsByLocation: map[common.Location][]byte{},
-		reporter:            rwf.ReportWriter("staged-contracts-migrator"),
+		reporter:            rwf.ReportWriter(reporterName),
+		verboseErrorOutput:  options.VerboseErrorOutput,
 	}
 }
 
@@ -464,13 +473,15 @@ func (m *StagedContractsMigration) MigrateAccount(
 				errorDetails = err.Error()
 			}
 
-			m.log.Error().
-				Msgf(
-					"failed to update contract %s in account %s: %s",
-					name,
-					address.HexWithPrefix(),
-					errorDetails,
-				)
+			if m.verboseErrorOutput {
+				m.log.Error().
+					Msgf(
+						"failed to update contract %s in account %s: %s",
+						name,
+						address.HexWithPrefix(),
+						errorDetails,
+					)
+			}
 
 			m.reporter.Write(contractUpdateFailed{
 				AccountAddressHex: address.HexWithPrefix(),