From ecaa5cda1b41592004b6d3f6ad3d6b153d85554e Mon Sep 17 00:00:00 2001 From: Peter Argue <89119817+peterargue@users.noreply.github.com> Date: Wed, 8 May 2024 12:22:27 -0700 Subject: [PATCH 1/9] [CI] Fix stale issue workflow --- .github/workflows/stale.yml | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/.github/workflows/stale.yml b/.github/workflows/stale.yml index a446dc86c50..8b3ad2cbe13 100644 --- a/.github/workflows/stale.yml +++ b/.github/workflows/stale.yml @@ -1,17 +1,19 @@ -# Number of days of inactivity before an issue becomes stale -daysUntilStale: 90 -# Number of days of inactivity before a stale issue is closed -daysUntilClose: 7 -# Issues with these labels will never be considered stale -exemptLabels: - - Preserve - - Idea -# Label to use when marking an issue as stale -staleLabel: Stale -# Comment to post when marking an issue as stale. Set to `false` to disable -markComment: > - This issue has been automatically marked as stale because it has not had - recent activity. It will be closed if no further activity occurs. Thank you - for your contributions. -# Comment to post when closing a stale issue. Set to `false` to disable -closeComment: false +name: 'Mark and close stale issues' +on: + schedule: + - cron: '30 1 * * *' + +jobs: + stale: + runs-on: ubuntu-latest + steps: + - uses: actions/stale@v9 + with: + days-before-issue-stale: 90 + days-before-issue-close: 7 + exempt-issue-labels: 'Preserve,Idea' + stale-issue-label: 'Stale' + stale-issue-message: > + This issue has been automatically marked as stale because it has not had + recent activity. It will be closed if no further activity occurs. Thank you + for your contributions. From 114eb0066a31ff23c28f87671b418d7ea86961b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 18 Jun 2024 11:48:45 -0700 Subject: [PATCH 2/9] add a flag which forces value comparison --- cmd/util/cmd/diff-states/cmd.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cmd/util/cmd/diff-states/cmd.go b/cmd/util/cmd/diff-states/cmd.go index ed1d13ef30a..414f7147d36 100644 --- a/cmd/util/cmd/diff-states/cmd.go +++ b/cmd/util/cmd/diff-states/cmd.go @@ -30,6 +30,7 @@ var ( flagStateCommitment1 string flagStateCommitment2 string flagRaw bool + flagAlwaysDiffValues bool flagNWorker int flagChain string ) @@ -113,6 +114,13 @@ func init() { "Raw or value", ) + Cmd.Flags().BoolVar( + &flagAlwaysDiffValues, + "always-diff-values", + false, + "always diff on value level. useful when trying to test iteration, by diffing same state.", + ) + Cmd.Flags().IntVar( &flagNWorker, "n-worker", @@ -294,6 +302,8 @@ func diffAccount( }) } + diffValues := flagAlwaysDiffValues + err = accountRegisters1.ForEach(func(owner, key string, value1 []byte) error { var value2 []byte value2, err = accountRegisters2.Get(owner, key) @@ -323,6 +333,10 @@ func diffAccount( return err } + diffValues = true + } + + if diffValues { address, err := common.BytesToAddress([]byte(owner)) if err != nil { return err From 0ad28ccd8d8eb2901af744f3705ef80a0db52b92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 18 Jun 2024 11:52:17 -0700 Subject: [PATCH 3/9] profile whole state extraction, not just migration a full profile is useful for e.g. profile-guided optimization --- cmd/util/cmd/execution-state-extract/cmd.go | 15 +++++++++++++++ .../execution_state_extract.go | 14 -------------- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/cmd/util/cmd/execution-state-extract/cmd.go b/cmd/util/cmd/execution-state-extract/cmd.go index f9701e503c3..b8920226a4a 100644 --- a/cmd/util/cmd/execution-state-extract/cmd.go +++ b/cmd/util/cmd/execution-state-extract/cmd.go @@ -5,6 +5,7 @@ import ( "fmt" "os" "path" + "runtime/pprof" "strings" "github.com/rs/zerolog/log" @@ -178,6 +179,20 @@ func init() { } func run(*cobra.Command, []string) { + if flagCPUProfile != "" { + f, err := os.Create(flagCPUProfile) + if err != nil { + log.Fatal().Err(err).Msg("could not create CPU profile") + } + + err = pprof.StartCPUProfile(f) + if err != nil { + log.Fatal().Err(err).Msg("could not start CPU profile") + } + + defer pprof.StopCPUProfile() + } + var stateCommitment flow.StateCommitment if len(flagBlockHash) > 0 && len(flagStateCommitment) > 0 { diff --git a/cmd/util/cmd/execution-state-extract/execution_state_extract.go b/cmd/util/cmd/execution-state-extract/execution_state_extract.go index 319f5fc057e..f53b53167c3 100644 --- a/cmd/util/cmd/execution-state-extract/execution_state_extract.go +++ b/cmd/util/cmd/execution-state-extract/execution_state_extract.go @@ -6,7 +6,6 @@ import ( "fmt" "math" "os" - "runtime/pprof" syncAtomic "sync/atomic" "time" @@ -325,19 +324,6 @@ func newMigration( nWorker int, ) ledger.Migration { return func(payloads []*ledger.Payload) ([]*ledger.Payload, error) { - if flagCPUProfile != "" { - f, err := os.Create(flagCPUProfile) - if err != nil { - logger.Fatal().Err(err).Msg("could not create CPU profile") - } - - err = pprof.StartCPUProfile(f) - if err != nil { - logger.Fatal().Err(err).Msg("could not start CPU profile") - } - - defer pprof.StopCPUProfile() - } if len(migrations) == 0 { return payloads, nil From 10c04d7296eec94f7b0ac48a0c7e7af35c9d2899 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 18 Jun 2024 11:53:59 -0700 Subject: [PATCH 4/9] expose encoding and decoding of contract names register --- fvm/environment/accounts.go | 43 +++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/fvm/environment/accounts.go b/fvm/environment/accounts.go index 01041f19a3f..b22ee29cb50 100644 --- a/fvm/environment/accounts.go +++ b/fvm/environment/accounts.go @@ -431,6 +431,20 @@ func (a *StatefulAccounts) setContract( return nil } +func EncodeContractNames(contractNames contractNames) ([]byte, error) { + var buf bytes.Buffer + cborEncoder := cbor.NewEncoder(&buf) + err := cborEncoder.Encode(contractNames) + if err != nil { + return nil, errors.NewEncodingFailuref( + err, + "cannot encode contract names: %s", + contractNames, + ) + } + return buf.Bytes(), nil +} + func (a *StatefulAccounts) setContractNames( contractNames contractNames, address flow.Address, @@ -443,16 +457,11 @@ func (a *StatefulAccounts) setContractNames( if !ok { return errors.NewAccountNotFoundError(address) } - var buf bytes.Buffer - cborEncoder := cbor.NewEncoder(&buf) - err = cborEncoder.Encode(contractNames) + + newContractNames, err := EncodeContractNames(contractNames) if err != nil { - return errors.NewEncodingFailuref( - err, - "cannot encode contract names: %s", - contractNames) + return err } - newContractNames := buf.Bytes() id := flow.ContractNamesRegisterID(address) prevContractNames, err := a.GetValue(id) @@ -607,20 +616,26 @@ func (a *StatefulAccounts) getContractNames( error, ) { // TODO return fatal error if can't fetch - encContractNames, err := a.GetValue(flow.ContractNamesRegisterID(address)) + encodedContractNames, err := a.GetValue(flow.ContractNamesRegisterID(address)) if err != nil { return nil, fmt.Errorf("cannot get deployed contract names: %w", err) } + + return DecodeContractNames(encodedContractNames) +} + +func DecodeContractNames(encodedContractNames []byte) ([]string, error) { identifiers := make([]string, 0) - if len(encContractNames) > 0 { - buf := bytes.NewReader(encContractNames) + if len(encodedContractNames) > 0 { + buf := bytes.NewReader(encodedContractNames) cborDecoder := cbor.NewDecoder(buf) - err = cborDecoder.Decode(&identifiers) + err := cborDecoder.Decode(&identifiers) if err != nil { return nil, fmt.Errorf( "cannot decode deployed contract names %x: %w", - encContractNames, - err) + encodedContractNames, + err, + ) } } return identifiers, nil From 81309672270436f17088b95b01191393c214e51e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 18 Jun 2024 11:54:31 -0700 Subject: [PATCH 5/9] get contract names from contract names register, avoid iteration over all registers --- .../cadence_values_migration_test.go | 10 ++++ .../migrations/contract_checking_migration.go | 53 ++++++++++++------- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/cmd/util/ledger/migrations/cadence_values_migration_test.go b/cmd/util/ledger/migrations/cadence_values_migration_test.go index 882952b9362..cb65054c35d 100644 --- a/cmd/util/ledger/migrations/cadence_values_migration_test.go +++ b/cmd/util/ledger/migrations/cadence_values_migration_test.go @@ -1274,6 +1274,16 @@ func TestProgramParsingError(t *testing.T) { ) require.NoError(t, err) + encodedContractNames, err := environment.EncodeContractNames([]string{contractName}) + require.NoError(t, err) + + err = registersByAccount.Set( + string(testAddress[:]), + flow.ContractNamesKey, + encodedContractNames, + ) + require.NoError(t, err) + // Migrate // TODO: EVM contract is not deployed in snapshot yet, so can't update it diff --git a/cmd/util/ledger/migrations/contract_checking_migration.go b/cmd/util/ledger/migrations/contract_checking_migration.go index d891ed5c966..668c644f70c 100644 --- a/cmd/util/ledger/migrations/contract_checking_migration.go +++ b/cmd/util/ledger/migrations/contract_checking_migration.go @@ -12,6 +12,7 @@ import ( "github.com/onflow/flow-go/cmd/util/ledger/reporters" "github.com/onflow/flow-go/cmd/util/ledger/util/registers" + "github.com/onflow/flow-go/fvm/environment" "github.com/onflow/flow-go/model/flow" ) @@ -50,35 +51,49 @@ func NewContractCheckingMigration( } contracts := make([]contract, 0, contractCountEstimate) - err = registersByAccount.ForEach(func(owner string, key string, value []byte) error { + err = registersByAccount.ForEachAccount(func(accountRegisters *registers.AccountRegisters) error { + owner := accountRegisters.Owner() - // Skip payloads that are not contract code - contractName := flow.KeyContractName(key) - if contractName == "" { - return nil + encodedContractNames, err := accountRegisters.Get(owner, flow.ContractNamesKey) + if err != nil { + return err } - address := common.Address([]byte(owner)) - code := value - location := common.AddressLocation{ - Address: address, - Name: contractName, + contractNames, err := environment.DecodeContractNames(encodedContractNames) + if err != nil { + return err } - contracts = append( - contracts, - contract{ - location: location, - code: code, - }, - ) + for _, contractName := range contractNames { + + contractKey := flow.ContractKey(contractName) + + code, err := accountRegisters.Get(owner, contractKey) + if err != nil { + return err + } - contractsForPrettyPrinting[location] = code + address := common.Address([]byte(owner)) + location := common.AddressLocation{ + Address: address, + Name: contractName, + } + + contracts = append( + contracts, + contract{ + location: location, + code: code, + }, + ) + + contractsForPrettyPrinting[location] = code + } return nil }) if err != nil { - return fmt.Errorf("failed to iterate over registers: %w", err) + return fmt.Errorf("failed to get contracts of accounts: %w", err) } sort.Slice(contracts, func(i, j int) bool { From db5fd33dfe5afafa6da4cc94e8bbbf5307472a1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 18 Jun 2024 12:50:56 -0700 Subject: [PATCH 6/9] log gathering of contracts --- cmd/util/ledger/migrations/contract_checking_migration.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cmd/util/ledger/migrations/contract_checking_migration.go b/cmd/util/ledger/migrations/contract_checking_migration.go index 668c644f70c..473c6679ed7 100644 --- a/cmd/util/ledger/migrations/contract_checking_migration.go +++ b/cmd/util/ledger/migrations/contract_checking_migration.go @@ -43,6 +43,8 @@ func NewContractCheckingMigration( // Gather all contracts + log.Info().Msg("Gathering contracts ...") + contractsForPrettyPrinting := make(map[common.Location][]byte, contractCountEstimate) type contract struct { @@ -102,6 +104,8 @@ func NewContractCheckingMigration( return a.location.ID() < b.location.ID() }) + log.Info().Msgf("Gathered all contracts (%d)", len(contracts)) + // Check all contracts for _, contract := range contracts { From f9c68391eb299688935a0ced918b6a0313ff75ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 18 Jun 2024 12:51:10 -0700 Subject: [PATCH 7/9] defer close of reporter --- cmd/util/ledger/migrations/contract_checking_migration.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/util/ledger/migrations/contract_checking_migration.go b/cmd/util/ledger/migrations/contract_checking_migration.go index 473c6679ed7..6683ee5569a 100644 --- a/cmd/util/ledger/migrations/contract_checking_migration.go +++ b/cmd/util/ledger/migrations/contract_checking_migration.go @@ -31,6 +31,7 @@ func NewContractCheckingMigration( return func(registersByAccount *registers.ByAccount) error { reporter := rwf.ReportWriter(contractCheckingReporterName) + defer reporter.Close() mr, err := NewInterpreterMigrationRuntime( registersByAccount, @@ -153,8 +154,6 @@ func NewContractCheckingMigration( } } - reporter.Close() - return nil } } From bc8a250186564a24e4e95d8d343ad77ecbf409a9 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Thu, 13 Jun 2024 14:55:56 -0700 Subject: [PATCH 8/9] log epoch height view --- network/p2p/cache/protocol_state_provider.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/network/p2p/cache/protocol_state_provider.go b/network/p2p/cache/protocol_state_provider.go index 6f7a4462b5b..cf782b46b5f 100644 --- a/network/p2p/cache/protocol_state_provider.go +++ b/network/p2p/cache/protocol_state_provider.go @@ -70,7 +70,10 @@ func NewProtocolStateIDCache( // and virtually latency free. However, we run data base queries and acquire locks here, // which is undesired. func (p *ProtocolStateIDCache) EpochTransition(newEpochCounter uint64, header *flow.Header) { - p.logger.Info().Uint64("newEpochCounter", newEpochCounter).Msg("epoch transition") + p.logger.Info().Uint64("newEpochCounter", newEpochCounter). + Uint64("height", header.Height). + Uint64("view", header.View). + Msg("epoch transition") p.update(header.ID()) } @@ -82,7 +85,10 @@ func (p *ProtocolStateIDCache) EpochTransition(newEpochCounter uint64, header *f // and virtually latency free. However, we run data base queries and acquire locks here, // which is undesired. func (p *ProtocolStateIDCache) EpochSetupPhaseStarted(currentEpochCounter uint64, header *flow.Header) { - p.logger.Info().Uint64("currentEpochCounter", currentEpochCounter).Msg("epoch setup phase started") + p.logger.Info().Uint64("currentEpochCounter", currentEpochCounter). + Uint64("height", header.Height). + Uint64("view", header.View). + Msg("epoch setup phase started") p.update(header.ID()) } @@ -94,7 +100,10 @@ func (p *ProtocolStateIDCache) EpochSetupPhaseStarted(currentEpochCounter uint64 // and virtually latency free. However, we run data base queries and acquire locks here, // which is undesired. func (p *ProtocolStateIDCache) EpochCommittedPhaseStarted(currentEpochCounter uint64, header *flow.Header) { - p.logger.Info().Uint64("currentEpochCounter", currentEpochCounter).Msg("epoch committed phase started") + p.logger.Info().Uint64("currentEpochCounter", currentEpochCounter). + Uint64("height", header.Height). + Uint64("view", header.View). + Msg("epoch committed phase started") p.update(header.ID()) } From 5b3dda96897aa4ccbf236bf834295d8d7cc7f871 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Tue, 18 Jun 2024 16:32:16 -0700 Subject: [PATCH 9/9] create protocol event logger --- cmd/scaffold.go | 5 +++ network/p2p/cache/protocol_state_provider.go | 12 ------ state/protocol/events/logger.go | 42 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 12 deletions(-) create mode 100644 state/protocol/events/logger.go diff --git a/cmd/scaffold.go b/cmd/scaffold.go index 734e8371b92..5db4e75c395 100644 --- a/cmd/scaffold.go +++ b/cmd/scaffold.go @@ -471,6 +471,11 @@ func (fnb *FlowNodeBuilder) EnqueueNetworkInit() { peerManagerFilters) }) + fnb.Module("epoch transition logger", func(node *NodeConfig) error { + node.ProtocolEvents.AddConsumer(events.NewEventLogger(node.Logger)) + return nil + }) + fnb.Module("network underlay dependency", func(node *NodeConfig) error { fnb.networkUnderlayDependable = module.NewProxiedReadyDoneAware() fnb.PeerManagerDependencies.Add(fnb.networkUnderlayDependable) diff --git a/network/p2p/cache/protocol_state_provider.go b/network/p2p/cache/protocol_state_provider.go index cf782b46b5f..5d4d5fa06e1 100644 --- a/network/p2p/cache/protocol_state_provider.go +++ b/network/p2p/cache/protocol_state_provider.go @@ -70,10 +70,6 @@ func NewProtocolStateIDCache( // and virtually latency free. However, we run data base queries and acquire locks here, // which is undesired. func (p *ProtocolStateIDCache) EpochTransition(newEpochCounter uint64, header *flow.Header) { - p.logger.Info().Uint64("newEpochCounter", newEpochCounter). - Uint64("height", header.Height). - Uint64("view", header.View). - Msg("epoch transition") p.update(header.ID()) } @@ -85,10 +81,6 @@ func (p *ProtocolStateIDCache) EpochTransition(newEpochCounter uint64, header *f // and virtually latency free. However, we run data base queries and acquire locks here, // which is undesired. func (p *ProtocolStateIDCache) EpochSetupPhaseStarted(currentEpochCounter uint64, header *flow.Header) { - p.logger.Info().Uint64("currentEpochCounter", currentEpochCounter). - Uint64("height", header.Height). - Uint64("view", header.View). - Msg("epoch setup phase started") p.update(header.ID()) } @@ -100,10 +92,6 @@ func (p *ProtocolStateIDCache) EpochSetupPhaseStarted(currentEpochCounter uint64 // and virtually latency free. However, we run data base queries and acquire locks here, // which is undesired. func (p *ProtocolStateIDCache) EpochCommittedPhaseStarted(currentEpochCounter uint64, header *flow.Header) { - p.logger.Info().Uint64("currentEpochCounter", currentEpochCounter). - Uint64("height", header.Height). - Uint64("view", header.View). - Msg("epoch committed phase started") p.update(header.ID()) } diff --git a/state/protocol/events/logger.go b/state/protocol/events/logger.go new file mode 100644 index 00000000000..942ba00e480 --- /dev/null +++ b/state/protocol/events/logger.go @@ -0,0 +1,42 @@ +package events + +import ( + "github.com/rs/zerolog" + + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/state/protocol" +) + +type EventLogger struct { + Noop // satisfy protocol events consumer interface + logger zerolog.Logger +} + +var _ protocol.Consumer = (*EventLogger)(nil) + +func NewEventLogger(logger zerolog.Logger) *EventLogger { + return &EventLogger{ + logger: logger.With().Str("module", "protocol_events_logger").Logger(), + } +} + +func (p EventLogger) EpochTransition(newEpochCounter uint64, header *flow.Header) { + p.logger.Info().Uint64("newEpochCounter", newEpochCounter). + Uint64("height", header.Height). + Uint64("view", header.View). + Msg("epoch transition") +} + +func (p EventLogger) EpochSetupPhaseStarted(currentEpochCounter uint64, header *flow.Header) { + p.logger.Info().Uint64("currentEpochCounter", currentEpochCounter). + Uint64("height", header.Height). + Uint64("view", header.View). + Msg("epoch setup phase started") +} + +func (p EventLogger) EpochCommittedPhaseStarted(currentEpochCounter uint64, header *flow.Header) { + p.logger.Info().Uint64("currentEpochCounter", currentEpochCounter). + Uint64("height", header.Height). + Uint64("view", header.View). + Msg("epoch committed phase started") +}