From 70204dda3786dd86c5d1b5d89da6bddbb8375139 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 12 Apr 2024 14:00:05 -0700 Subject: [PATCH] only log failed staged updates if verbose logging is enabled. failures are always reported --- cmd/util/ledger/migrations/cadence.go | 26 +++- .../change_contract_code_migration.go | 51 +++---- .../change_contract_code_migration_test.go | 48 +++++-- .../migrations/staged_contracts_migration.go | 33 +++-- .../staged_contracts_migration_test.go | 132 +++++++++++++++--- 5 files changed, 210 insertions(+), 80 deletions(-) diff --git a/cmd/util/ledger/migrations/cadence.go b/cmd/util/ledger/migrations/cadence.go index 42d7a258210..1fb8389775b 100644 --- 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() stagedContractsMigration.RegisterContractUpdates(opts.StagedContracts) diff --git a/cmd/util/ledger/migrations/change_contract_code_migration.go b/cmd/util/ledger/migrations/change_contract_code_migration.go index c0329ce13c1..4be98050c5a 100644 --- 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 index ebe51fbe714..56de78c49f9 100644 --- a/cmd/util/ledger/migrations/change_contract_code_migration_test.go +++ b/cmd/util/ledger/migrations/change_contract_code_migration_test.go @@ -81,7 +81,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) @@ -107,7 +111,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) @@ -142,7 +150,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) @@ -187,7 +199,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) @@ -230,7 +246,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) @@ -282,7 +302,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) @@ -329,7 +353,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) @@ -378,7 +406,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) diff --git a/cmd/util/ledger/migrations/staged_contracts_migration.go b/cmd/util/ledger/migrations/staged_contracts_migration.go index 739767c05a6..7f49c5a5871 100644 --- a/cmd/util/ledger/migrations/staged_contracts_migration.go +++ b/cmd/util/ledger/migrations/staged_contracts_migration.go @@ -39,6 +39,7 @@ type StagedContractsMigration struct { contractAdditionHandler stdlib.AccountContractAdditionHandler contractNamesProvider stdlib.AccountContractNamesProvider reporter reporters.ReportWriter + verboseErrorOutput bool } type StagedContract struct { @@ -53,18 +54,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, } } @@ -277,13 +286,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(), diff --git a/cmd/util/ledger/migrations/staged_contracts_migration_test.go b/cmd/util/ledger/migrations/staged_contracts_migration_test.go index bb792b1c8a2..47fe464ddac 100644 --- a/cmd/util/ledger/migrations/staged_contracts_migration_test.go +++ b/cmd/util/ledger/migrations/staged_contracts_migration_test.go @@ -73,7 +73,12 @@ func TestStagedContractsMigration(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf) + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options) migration.RegisterContractUpdates(stagedContracts) err := migration.InitMigration(log, nil, 0) @@ -118,7 +123,12 @@ func TestStagedContractsMigration(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -166,7 +176,12 @@ func TestStagedContractsMigration(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -224,7 +239,13 @@ func TestStagedContractsMigration(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + const reporterName = "test" + migration := NewStagedContractsMigration("test", reporterName, log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -253,7 +274,7 @@ func TestStagedContractsMigration(t *testing.T) { // Second payload should have the updated code require.Equal(t, newCode2, string(payloads[1].Value())) - reportWriter := rwf.reportWriters["staged-contracts-migrator"] + reportWriter := rwf.reportWriters[reporterName] require.Len(t, reportWriter.entries, 2) assert.Equal( t, @@ -296,7 +317,13 @@ func TestStagedContractsMigration(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf) + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + const reporterName = "test" + migration := NewStagedContractsMigration("test", reporterName, log, rwf, options) migration.RegisterContractUpdates(stagedContracts) err := migration.InitMigration(log, nil, 0) @@ -339,7 +366,7 @@ func TestStagedContractsMigration(t *testing.T) { // No errors. require.Empty(t, logWriter.logs) - reportWriter := rwf.reportWriters["staged-contracts-migrator"] + reportWriter := rwf.reportWriters[reporterName] require.Len(t, reportWriter.entries, 1) assert.Equal( t, @@ -380,7 +407,12 @@ func TestStagedContractsMigration(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf) + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options) err := migration.InitMigration(log, nil, 0) require.NoError(t, err) @@ -430,7 +462,12 @@ func TestStagedContractsMigration(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf) + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options) err := migration.InitMigration(log, nil, 0) require.NoError(t, err) @@ -515,7 +552,12 @@ func TestStagedContractsWithImports(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf) + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options) migration.RegisterContractUpdates(stagedContracts) err := migration.InitMigration(log, nil, 0) @@ -592,7 +634,12 @@ func TestStagedContractsWithImports(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -685,7 +732,12 @@ func TestStagedContractsWithImports(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -783,7 +835,12 @@ func TestStagedContractsWithImports(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -1003,7 +1060,12 @@ func TestStagedContractsWithUpdateValidator(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -1096,7 +1158,12 @@ func TestStagedContractsWithUpdateValidator(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -1186,7 +1253,12 @@ func TestStagedContractsWithUpdateValidator(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -1288,7 +1360,12 @@ func TestStagedContractConformanceChanges(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -1403,7 +1480,12 @@ func TestStagedContractConformanceChanges(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -1621,7 +1703,12 @@ func TestStagedContractsUpdateValidationErrors(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts) @@ -1720,7 +1807,12 @@ func TestStagedContractsUpdateValidationErrors(t *testing.T) { rwf := &testReportWriterFactory{} - migration := NewStagedContractsMigration(chainID, log, rwf). + options := StagedContractsMigrationOptions{ + ChainID: chainID, + VerboseErrorOutput: true, + } + + migration := NewStagedContractsMigration("test", "test", log, rwf, options). WithContractUpdateValidation() migration.RegisterContractUpdates(stagedContracts)