From a79c9b84f91169996e6939f54c864a47273509c2 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Fri, 11 Oct 2024 11:02:42 -0700 Subject: [PATCH 01/13] Expose version boundary to cadence interface --- fvm/environment/current_version_boundary.go | 135 ++++++++++++++++++ fvm/environment/derived_data_invalidator.go | 49 +++++++ .../derived_data_invalidator_test.go | 6 +- fvm/environment/env.go | 2 + fvm/environment/facade_env.go | 10 ++ fvm/environment/meter.go | 1 + .../mock/current_version_boundary.go | 55 +++++++ fvm/environment/mock/environment.go | 30 ++++ fvm/environment/system_contracts.go | 14 ++ fvm/storage/derived/derived_block_data.go | 52 ++++++- fvm/storage/derived/invalidator.go | 7 + fvm/systemcontracts/system_contracts.go | 1 + fvm/transactionInvoker.go | 6 +- model/convert/service_event.go | 135 ++++++++++-------- .../indexer/indexer_core.go | 8 +- module/state_synchronization/indexer/util.go | 23 ++- module/trace/constants.go | 3 +- 17 files changed, 464 insertions(+), 73 deletions(-) create mode 100644 fvm/environment/current_version_boundary.go create mode 100644 fvm/environment/mock/current_version_boundary.go diff --git a/fvm/environment/current_version_boundary.go b/fvm/environment/current_version_boundary.go new file mode 100644 index 00000000000..7bdb7dd51bd --- /dev/null +++ b/fvm/environment/current_version_boundary.go @@ -0,0 +1,135 @@ +package environment + +import ( + "context" + "github.com/onflow/flow-go/fvm/storage" + "github.com/onflow/flow-go/fvm/storage/state" + "github.com/onflow/flow-go/fvm/tracing" + "github.com/onflow/flow-go/model/convert" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/module/trace" +) + +type CurrentVersionBoundary interface { + CurrentVersionBoundary() (flow.VersionBoundary, error) +} + +type ParseRestrictedCurrentVersionBoundary struct { + txnState state.NestedTransactionPreparer + impl CurrentVersionBoundary +} + +func NewParseRestrictedCurrentVersionBoundary( + txnState state.NestedTransactionPreparer, + impl CurrentVersionBoundary, +) CurrentVersionBoundary { + return ParseRestrictedCurrentVersionBoundary{ + txnState: txnState, + impl: impl, + } +} + +func (p ParseRestrictedCurrentVersionBoundary) CurrentVersionBoundary() (flow.VersionBoundary, error) { + return parseRestrict1Ret( + p.txnState, + trace.FVMEnvRandom, + p.impl.CurrentVersionBoundary) +} + +type currentVersionBoundary struct { + tracer tracing.TracerSpan + meter Meter + txnState storage.TransactionPreparer + envParams EnvironmentParams +} + +func NewCurrentVersionBoundary( + tracer tracing.TracerSpan, + meter Meter, + txnState storage.TransactionPreparer, + envParams EnvironmentParams, +) CurrentVersionBoundary { + return currentVersionBoundary{ + tracer: tracer, + meter: meter, + txnState: txnState, + envParams: envParams, + } +} + +func (c currentVersionBoundary) CurrentVersionBoundary() (flow.VersionBoundary, error) { + tracerSpan := c.tracer.StartExtensiveTracingChildSpan( + trace.FVMEnvGetCurrentVersionBoundary) + defer tracerSpan.End() + + err := c.meter.MeterComputation(ComputationKindGetCurrentVersionBoundary, 1) + if err != nil { + // TODO: return a better error + return flow.VersionBoundary{}, err + } + + value, _, err := c.txnState.GetCurrentVersionBoundary( + c.txnState, + NewCurrentVersionBoundaryComputer( + tracerSpan, + c.envParams, + c.txnState, + ), + ) + if err != nil { + // TODO: return a better error + return flow.VersionBoundary{}, err + } + + return value, nil +} + +var _ CurrentVersionBoundary = (*currentVersionBoundary)(nil) + +type CurrentVersionBoundaryComputer struct { + tracerSpan tracing.TracerSpan + envParams EnvironmentParams + txnState storage.TransactionPreparer +} + +func NewCurrentVersionBoundaryComputer( + tracerSpan tracing.TracerSpan, + envParams EnvironmentParams, + txnState storage.TransactionPreparer, +) CurrentVersionBoundaryComputer { + return CurrentVersionBoundaryComputer{ + tracerSpan: tracerSpan, + envParams: envParams, + txnState: txnState, + } +} + +func (computer CurrentVersionBoundaryComputer) Compute( + _ state.NestedTransactionPreparer, + _ struct{}, +) ( + flow.VersionBoundary, + error, +) { + env := NewScriptEnv( + context.Background(), + computer.tracerSpan, + computer.envParams, + computer.txnState) + + value, err := env.GetCurrentVersionBoundary() + + if err != nil { + // TODO: return a better error + return flow.VersionBoundary{}, err + } + + boundary, err := convert.VersionBoundary(value) + + if err != nil { + // TODO: return a better error + return flow.VersionBoundary{}, err + } + + return boundary, nil +} diff --git a/fvm/environment/derived_data_invalidator.go b/fvm/environment/derived_data_invalidator.go index 8427ffe2f76..2419fb21267 100644 --- a/fvm/environment/derived_data_invalidator.go +++ b/fvm/environment/derived_data_invalidator.go @@ -5,6 +5,7 @@ import ( "github.com/onflow/flow-go/fvm/storage/derived" "github.com/onflow/flow-go/fvm/storage/snapshot" + "github.com/onflow/flow-go/model/flow" ) type ContractUpdate struct { @@ -26,6 +27,8 @@ type DerivedDataInvalidator struct { ContractUpdates MeterParamOverridesUpdated bool + + CurrentVersionBoundaryUpdated bool } var _ derived.TransactionInvalidator = DerivedDataInvalidator{} @@ -34,12 +37,16 @@ func NewDerivedDataInvalidator( contractUpdates ContractUpdates, executionSnapshot *snapshot.ExecutionSnapshot, meterStateRead *snapshot.ExecutionSnapshot, + serviceAddress flow.Address, ) DerivedDataInvalidator { return DerivedDataInvalidator{ ContractUpdates: contractUpdates, MeterParamOverridesUpdated: meterParamOverridesUpdated( executionSnapshot, meterStateRead), + CurrentVersionBoundaryUpdated: currentVersionBoundaryUpdated( + serviceAddress, + meterStateRead), } } @@ -69,6 +76,26 @@ func meterParamOverridesUpdated( return false } +// currentVersionBoundaryUpdated returns true if the current version boundary +// should be invalidated. Currently, this will trigger on any change to the +// service account, which will trigger at least once per block. This is ok for now as +// the meterParamOverrides also trigger on every block. +func currentVersionBoundaryUpdated( + serviceAddress flow.Address, + executionSnapshot *snapshot.ExecutionSnapshot, +) bool { + serviceAccount := string(serviceAddress.Bytes()) + + for registerId := range executionSnapshot.WriteSet { + // The meter param override values are stored in the service account. + if registerId.Owner == serviceAccount { + return true + } + } + + return false +} + func (invalidator DerivedDataInvalidator) ProgramInvalidator() derived.ProgramInvalidator { return ProgramInvalidator{invalidator} } @@ -77,6 +104,10 @@ func (invalidator DerivedDataInvalidator) MeterParamOverridesInvalidator() deriv return MeterParamOverridesInvalidator{invalidator} } +func (invalidator DerivedDataInvalidator) CurrentVersionBoundaryInvalidator() derived.CurrentVersionBoundaryInvalidator { + return CurrentVersionBoundaryInvalidator{invalidator} +} + type ProgramInvalidator struct { DerivedDataInvalidator } @@ -139,3 +170,21 @@ func (invalidator MeterParamOverridesInvalidator) ShouldInvalidateEntry( ) bool { return invalidator.MeterParamOverridesUpdated } + +type CurrentVersionBoundaryInvalidator struct { + DerivedDataInvalidator +} + +func (invalidator CurrentVersionBoundaryInvalidator) ShouldInvalidateEntries() bool { + return invalidator.CurrentVersionBoundaryUpdated +} + +func (invalidator CurrentVersionBoundaryInvalidator) ShouldInvalidateEntry( + _ struct{}, + _ flow.VersionBoundary, + _ *snapshot.ExecutionSnapshot, +) bool { + // If the meter params are updated + // the current version boundary derived data table becomes invalid. + return invalidator.MeterParamOverridesUpdated || invalidator.CurrentVersionBoundaryUpdated +} diff --git a/fvm/environment/derived_data_invalidator_test.go b/fvm/environment/derived_data_invalidator_test.go index 50ce6dca4e3..41be4af8f44 100644 --- a/fvm/environment/derived_data_invalidator_test.go +++ b/fvm/environment/derived_data_invalidator_test.go @@ -1,6 +1,7 @@ package environment_test import ( + "github.com/onflow/flow-go/fvm/systemcontracts" "testing" "github.com/onflow/cadence/runtime/common" @@ -296,10 +297,13 @@ func TestMeterParamOverridesUpdated(t *testing.T) { }, } + sc := systemcontracts.SystemContractsForChain(flow.Testnet) + invalidator := environment.NewDerivedDataInvalidator( environment.ContractUpdates{}, snapshot, - meterStateRead) + meterStateRead, + sc.FlowServiceAccount.Address) require.Equal(t, expected, invalidator.MeterParamOverridesUpdated) } diff --git a/fvm/environment/env.go b/fvm/environment/env.go index 5b01728111c..06c2d9f01ea 100644 --- a/fvm/environment/env.go +++ b/fvm/environment/env.go @@ -62,6 +62,8 @@ type Environment interface { error, ) + GetCurrentVersionBoundary() (cadence.Value, error) + // AccountInfo GetAccount(address flow.Address) (*flow.Account, error) GetAccountKeys(address flow.Address) ([]flow.AccountPublicKey, error) diff --git a/fvm/environment/facade_env.go b/fvm/environment/facade_env.go index ba1f2d21189..dbd2bf96a53 100644 --- a/fvm/environment/facade_env.go +++ b/fvm/environment/facade_env.go @@ -37,6 +37,7 @@ type facadeEnvironment struct { ValueStore *SystemContracts + CurrentVersionBoundary UUIDGenerator AccountLocalIDGenerator @@ -103,6 +104,12 @@ func newFacadeEnvironment( ), SystemContracts: systemContracts, + CurrentVersionBoundary: NewCurrentVersionBoundary( + tracer, + meter, + txnState, + params, + ), UUIDGenerator: NewUUIDGenerator( tracer, @@ -293,6 +300,9 @@ func (env *facadeEnvironment) addParseRestrictedChecks() { env.RandomSourceHistoryProvider = NewParseRestrictedRandomSourceHistoryProvider( env.txnState, env.RandomSourceHistoryProvider) + env.CurrentVersionBoundary = NewParseRestrictedCurrentVersionBoundary( + env.txnState, + env.CurrentVersionBoundary) env.UUIDGenerator = NewParseRestrictedUUIDGenerator( env.txnState, env.UUIDGenerator) diff --git a/fvm/environment/meter.go b/fvm/environment/meter.go index ad7c28c7eb3..456294b51f5 100644 --- a/fvm/environment/meter.go +++ b/fvm/environment/meter.go @@ -55,6 +55,7 @@ const ( _ ComputationKindEVMEncodeABI ComputationKindEVMDecodeABI + ComputationKindGetCurrentVersionBoundary ) // MainnetExecutionEffortWeights are the execution effort weights as they are diff --git a/fvm/environment/mock/current_version_boundary.go b/fvm/environment/mock/current_version_boundary.go new file mode 100644 index 00000000000..7bdaaa9f93b --- /dev/null +++ b/fvm/environment/mock/current_version_boundary.go @@ -0,0 +1,55 @@ +// Code generated by mockery v2.43.2. DO NOT EDIT. + +package mock + +import ( + flow "github.com/onflow/flow-go/model/flow" + mock "github.com/stretchr/testify/mock" +) + +// CurrentVersionBoundary is an autogenerated mock type for the CurrentVersionBoundary type +type CurrentVersionBoundary struct { + mock.Mock +} + +// CurrentVersionBoundary provides a mock function with given fields: +func (_m *CurrentVersionBoundary) CurrentVersionBoundary() (flow.VersionBoundary, error) { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for CurrentVersionBoundary") + } + + var r0 flow.VersionBoundary + var r1 error + if rf, ok := ret.Get(0).(func() (flow.VersionBoundary, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() flow.VersionBoundary); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(flow.VersionBoundary) + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewCurrentVersionBoundary creates a new instance of CurrentVersionBoundary. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewCurrentVersionBoundary(t interface { + mock.TestingT + Cleanup(func()) +}) *CurrentVersionBoundary { + mock := &CurrentVersionBoundary{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/fvm/environment/mock/environment.go b/fvm/environment/mock/environment.go index ab6fd164c30..7f3293a1cf3 100644 --- a/fvm/environment/mock/environment.go +++ b/fvm/environment/mock/environment.go @@ -907,6 +907,36 @@ func (_m *Environment) GetCurrentBlockHeight() (uint64, error) { return r0, r1 } +// GetCurrentVersionBoundary provides a mock function with given fields: +func (_m *Environment) GetCurrentVersionBoundary() (cadence.Value, error) { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for GetCurrentVersionBoundary") + } + + var r0 cadence.Value + var r1 error + if rf, ok := ret.Get(0).(func() (cadence.Value, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() cadence.Value); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(cadence.Value) + } + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetInterpreterSharedState provides a mock function with given fields: func (_m *Environment) GetInterpreterSharedState() *interpreter.SharedState { ret := _m.Called() diff --git a/fvm/environment/system_contracts.go b/fvm/environment/system_contracts.go index cd7c9db9339..f4ae51a8839 100644 --- a/fvm/environment/system_contracts.go +++ b/fvm/environment/system_contracts.go @@ -312,3 +312,17 @@ func (sys *SystemContracts) AccountsStorageCapacity( }, ) } + +var getCurrentVersionBoundarySpec = ContractFunctionSpec{ + AddressFromChain: ServiceAddress, + LocationName: systemcontracts.ContractNameNodeVersionBeacon, + FunctionName: systemcontracts.ContractVersionBeacon_getCurrentVersionBoundary, + ArgumentTypes: []sema.Type{}, +} + +func (sys *SystemContracts) GetCurrentVersionBoundary() (cadence.Value, error) { + return sys.Invoke( + getCurrentVersionBoundarySpec, + []cadence.Value{}, + ) +} diff --git a/fvm/storage/derived/derived_block_data.go b/fvm/storage/derived/derived_block_data.go index 450d09cd9d5..53140ce38ba 100644 --- a/fvm/storage/derived/derived_block_data.go +++ b/fvm/storage/derived/derived_block_data.go @@ -3,13 +3,13 @@ package derived import ( "fmt" - "github.com/onflow/flow-go/fvm/storage/snapshot" - "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/interpreter" "github.com/onflow/flow-go/fvm/storage/logical" + "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/fvm/storage/state" + "github.com/onflow/flow-go/model/flow" ) type DerivedTransactionPreparer interface { @@ -32,6 +32,15 @@ type DerivedTransactionPreparer interface { error, ) + GetCurrentVersionBoundary( + txnState state.NestedTransactionPreparer, + getCurrentVersionBoundaryComputer ValueComputer[struct{}, flow.VersionBoundary], + ) ( + flow.VersionBoundary, + *snapshot.ExecutionSnapshot, + error, + ) + AddInvalidator(invalidator TransactionInvalidator) } @@ -47,6 +56,8 @@ type DerivedBlockData struct { programs *DerivedDataTable[common.AddressLocation, *Program] meterParamOverrides *DerivedDataTable[struct{}, MeterParamOverrides] + + currentVersionBoundary *DerivedDataTable[struct{}, flow.VersionBoundary] } // DerivedTransactionData is the derived data scratch space for a single @@ -60,6 +71,11 @@ type DerivedTransactionData struct { // There's only a single entry in this table. For simplicity, we'll use // struct{} as the entry's key. meterParamOverrides *TableTransaction[struct{}, MeterParamOverrides] + + // The currently effective version boundary + // There's only a single entry in this table. For simplicity, we'll use + // struct{} as the entry's key. + currentVersionBoundary *TableTransaction[struct{}, flow.VersionBoundary] } func NewEmptyDerivedBlockData( @@ -74,6 +90,10 @@ func NewEmptyDerivedBlockData( struct{}, MeterParamOverrides, ](initialSnapshotTime), + currentVersionBoundary: NewEmptyTable[ + struct{}, + flow.VersionBoundary, + ](initialSnapshotTime), } } @@ -119,9 +139,17 @@ func (block *DerivedBlockData) NewDerivedTransactionData( return nil, err } + txnCurrentVersionBoundary, err := block.currentVersionBoundary.NewTableTransaction( + snapshotTime, + executionTime) + if err != nil { + return nil, err + } + return &DerivedTransactionData{ - programs: txnPrograms, - meterParamOverrides: txnMeterParamOverrides, + programs: txnPrograms, + meterParamOverrides: txnMeterParamOverrides, + currentVersionBoundary: txnCurrentVersionBoundary, }, nil } @@ -180,6 +208,8 @@ func (transaction *DerivedTransactionData) AddInvalidator( transaction.programs.AddInvalidator(invalidator.ProgramInvalidator()) transaction.meterParamOverrides.AddInvalidator( invalidator.MeterParamOverridesInvalidator()) + transaction.currentVersionBoundary.AddInvalidator( + invalidator.CurrentVersionBoundaryInvalidator()) } func (transaction *DerivedTransactionData) GetMeterParamOverrides( @@ -196,6 +226,20 @@ func (transaction *DerivedTransactionData) GetMeterParamOverrides( getMeterParamOverrides) } +func (transaction *DerivedTransactionData) GetCurrentVersionBoundary( + txnState state.NestedTransactionPreparer, + getCurrentVersionBoundaryComputer ValueComputer[struct{}, flow.VersionBoundary], +) ( + flow.VersionBoundary, + *snapshot.ExecutionSnapshot, + error, +) { + return transaction.currentVersionBoundary.GetWithStateOrCompute( + txnState, + struct{}{}, + getCurrentVersionBoundaryComputer) +} + func (transaction *DerivedTransactionData) Validate() error { err := transaction.programs.Validate() if err != nil { diff --git a/fvm/storage/derived/invalidator.go b/fvm/storage/derived/invalidator.go index 63e3ee290d5..9667a8d4e3c 100644 --- a/fvm/storage/derived/invalidator.go +++ b/fvm/storage/derived/invalidator.go @@ -2,6 +2,7 @@ package derived import ( "github.com/onflow/cadence/runtime/common" + "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/fvm/meter" ) @@ -22,7 +23,13 @@ type MeterParamOverridesInvalidator TableInvalidator[ MeterParamOverrides, ] +type CurrentVersionBoundaryInvalidator TableInvalidator[ + struct{}, + flow.VersionBoundary, +] + type TransactionInvalidator interface { ProgramInvalidator() ProgramInvalidator MeterParamOverridesInvalidator() MeterParamOverridesInvalidator + CurrentVersionBoundaryInvalidator() CurrentVersionBoundaryInvalidator } diff --git a/fvm/systemcontracts/system_contracts.go b/fvm/systemcontracts/system_contracts.go index cc8a84a0b62..ace2fd49637 100644 --- a/fvm/systemcontracts/system_contracts.go +++ b/fvm/systemcontracts/system_contracts.go @@ -64,6 +64,7 @@ const ( ContractStorageFeesFunction_calculateAccountCapacity = "calculateAccountCapacity" ContractStorageFeesFunction_getAccountsCapacityForTransactionStorageCheck = "getAccountsCapacityForTransactionStorageCheck" ContractStorageFeesFunction_defaultTokenAvailableBalance = "defaultTokenAvailableBalance" + ContractVersionBeacon_getCurrentVersionBoundary = "getCurrentVersionBoundary" // These are the account indexes of system contracts as deployed by the default bootstrapping. // On long-running networks some of these contracts might have been deployed after bootstrapping, diff --git a/fvm/transactionInvoker.go b/fvm/transactionInvoker.go index ef3d34b658b..8f84f3de5b5 100644 --- a/fvm/transactionInvoker.go +++ b/fvm/transactionInvoker.go @@ -2,6 +2,7 @@ package fvm import ( "fmt" + "github.com/onflow/flow-go/fvm/systemcontracts" "strconv" "github.com/onflow/cadence/runtime" @@ -400,10 +401,13 @@ func (executor *transactionExecutor) normalExecution() ( return } + sc := systemcontracts.SystemContractsForChain(executor.ctx.Chain.ChainID()) invalidator = environment.NewDerivedDataInvalidator( contractUpdates, bodySnapshot, - executor.meterStateRead) + executor.meterStateRead, + sc.FlowServiceAccount.Address, + ) // Check if all account storage limits are ok // diff --git a/model/convert/service_event.go b/model/convert/service_event.go index f8edf54aeae..dfe0a4d1e83 100644 --- a/model/convert/service_event.go +++ b/model/convert/service_event.go @@ -1170,69 +1170,12 @@ func convertVersionBoundaries(array cadence.Array) ( boundaries := make([]flow.VersionBoundary, len(array.Values)) for i, cadenceVal := range array.Values { - boundary, err := DecodeCadenceValue( - fmt.Sprintf(".Values[%d]", i), - cadenceVal, - func(structVal cadence.Struct) ( - flow.VersionBoundary, - error, - ) { - if structVal.Type() == nil { - return flow.VersionBoundary{}, fmt.Errorf("VersionBoundary struct doesn't have type") - } - - fields := cadence.FieldsMappedByName(structVal) - - const expectedFieldCount = 2 - if len(fields) < expectedFieldCount { - return flow.VersionBoundary{}, fmt.Errorf( - "incorrect number of fields (%d != %d)", - len(fields), - expectedFieldCount, - ) - } - - blockHeightValue, err := getField[cadence.Value](fields, "blockHeight") - if err != nil { - return flow.VersionBoundary{}, fmt.Errorf("failed to decode VersionBoundary struct: %w", err) - } - - versionValue, err := getField[cadence.Value](fields, "version") - if err != nil { - return flow.VersionBoundary{}, fmt.Errorf("failed to decode VersionBoundary struct: %w", err) - } - - height, err := DecodeCadenceValue( - ".blockHeight", - blockHeightValue, - func(cadenceVal cadence.UInt64) ( - uint64, - error, - ) { - return uint64(cadenceVal), nil - }, - ) - if err != nil { - return flow.VersionBoundary{}, err - } - - version, err := DecodeCadenceValue( - ".version", - versionValue, - convertSemverVersion, - ) - if err != nil { - return flow.VersionBoundary{}, err - } - - return flow.VersionBoundary{ - BlockHeight: height, - Version: version, - }, nil - }, - ) + boundary, err := VersionBoundary(cadenceVal) if err != nil { - return nil, err + return nil, decodeError{ + location: fmt.Sprintf(".Values[%d]", i), + err: err, + } } boundaries[i] = boundary } @@ -1240,6 +1183,74 @@ func convertVersionBoundaries(array cadence.Array) ( return boundaries, nil } +func VersionBoundary(value cadence.Value) ( + flow.VersionBoundary, + error, +) { + boundary, err := DecodeCadenceValue( + "VersionBoundary", + value, + func(structVal cadence.Struct) ( + flow.VersionBoundary, + error, + ) { + if structVal.Type() == nil { + return flow.VersionBoundary{}, fmt.Errorf("VersionBoundary struct doesn't have type") + } + + fields := cadence.FieldsMappedByName(structVal) + + const expectedFieldCount = 2 + if len(fields) < expectedFieldCount { + return flow.VersionBoundary{}, fmt.Errorf( + "incorrect number of fields (%d != %d)", + len(fields), + expectedFieldCount, + ) + } + + blockHeightValue, err := getField[cadence.Value](fields, "blockHeight") + if err != nil { + return flow.VersionBoundary{}, fmt.Errorf("failed to decode VersionBoundary struct: %w", err) + } + + versionValue, err := getField[cadence.Value](fields, "version") + if err != nil { + return flow.VersionBoundary{}, fmt.Errorf("failed to decode VersionBoundary struct: %w", err) + } + + height, err := DecodeCadenceValue( + ".blockHeight", + blockHeightValue, + func(cadenceVal cadence.UInt64) ( + uint64, + error, + ) { + return uint64(cadenceVal), nil + }, + ) + if err != nil { + return flow.VersionBoundary{}, err + } + + version, err := DecodeCadenceValue( + ".version", + versionValue, + convertSemverVersion, + ) + if err != nil { + return flow.VersionBoundary{}, err + } + + return flow.VersionBoundary{ + BlockHeight: height, + Version: version, + }, nil + }, + ) + return boundary, err +} + func convertSemverVersion(structVal cadence.Struct) ( string, error, diff --git a/module/state_synchronization/indexer/indexer_core.go b/module/state_synchronization/indexer/indexer_core.go index aede5d6ac4f..8e5bed0780e 100644 --- a/module/state_synchronization/indexer/indexer_core.go +++ b/module/state_synchronization/indexer/indexer_core.go @@ -287,11 +287,17 @@ func (c *IndexerCore) updateProgramCache(header *flow.Header, events []flow.Even tx.AddInvalidator(&accessInvalidator{ programs: &programInvalidator{ - invalidated: updatedContracts, + invalidated: updatedContracts, + invalidateAll: hasAuthorizedTransaction(collections, c.serviceAddress), }, meterParamOverrides: &meterParamOverridesInvalidator{ invalidateAll: hasAuthorizedTransaction(collections, c.serviceAddress), }, + currentVersionBoundary: ¤tVersionBoundaryInvalidator{ + invalidateAll: hasAuthorizedTransaction(collections, c.serviceAddress), + // this will eventually be different from invalidateAll + invalidate: hasAuthorizedTransaction(collections, c.serviceAddress), + }, }) err = tx.Commit() diff --git a/module/state_synchronization/indexer/util.go b/module/state_synchronization/indexer/util.go index e070c6b4ace..72fa47b9ac8 100644 --- a/module/state_synchronization/indexer/util.go +++ b/module/state_synchronization/indexer/util.go @@ -91,8 +91,9 @@ var _ derived.TransactionInvalidator = (*accessInvalidator)(nil) // accessInvalidator is a derived.TransactionInvalidator that invalidates programs and meter param overrides. type accessInvalidator struct { - programs *programInvalidator - meterParamOverrides *meterParamOverridesInvalidator + programs *programInvalidator + meterParamOverrides *meterParamOverridesInvalidator + currentVersionBoundary *currentVersionBoundaryInvalidator } func (inv *accessInvalidator) ProgramInvalidator() derived.ProgramInvalidator { @@ -103,6 +104,10 @@ func (inv *accessInvalidator) MeterParamOverridesInvalidator() derived.MeterPara return inv.meterParamOverrides } +func (inv *accessInvalidator) CurrentVersionBoundaryInvalidator() derived.CurrentVersionBoundaryInvalidator { + return inv.currentVersionBoundary +} + var _ derived.ProgramInvalidator = (*programInvalidator)(nil) // programInvalidator is a derived.ProgramInvalidator that invalidates all programs or a specific set of programs. @@ -135,3 +140,17 @@ func (inv *meterParamOverridesInvalidator) ShouldInvalidateEntries() bool { func (inv *meterParamOverridesInvalidator) ShouldInvalidateEntry(_ struct{}, _ derived.MeterParamOverrides, _ *snapshot.ExecutionSnapshot) bool { return inv.invalidateAll } + +// currentVersionBoundaryInvalidator is a derived.CurrentVersionBoundaryInvalidator that invalidates current version boundary. +type currentVersionBoundaryInvalidator struct { + invalidateAll bool + invalidate bool +} + +func (inv *currentVersionBoundaryInvalidator) ShouldInvalidateEntries() bool { + return inv.invalidate +} + +func (inv *currentVersionBoundaryInvalidator) ShouldInvalidateEntry(_ struct{}, _ flow.VersionBoundary, _ *snapshot.ExecutionSnapshot) bool { + return inv.invalidate +} diff --git a/module/trace/constants.go b/module/trace/constants.go index 9e8ab96f3ad..004f21b2045 100644 --- a/module/trace/constants.go +++ b/module/trace/constants.go @@ -183,13 +183,12 @@ const ( FVMEnvGetBlockAtHeight SpanName = "fvm.env.getBlockAtHeight" FVMEnvRandom SpanName = "fvm.env.unsafeRandom" FVMEnvRandomSourceHistoryProvider SpanName = "fvm.env.randomSourceHistoryProvider" + FVMEnvGetCurrentVersionBoundary SpanName = "fvm.env.getCurrentVersionBoundary" FVMEnvCreateAccount SpanName = "fvm.env.createAccount" FVMEnvAddAccountKey SpanName = "fvm.env.addAccountKey" - FVMEnvAddEncodedAccountKey SpanName = "fvm.env.addEncodedAccountKey" FVMEnvAccountKeysCount SpanName = "fvm.env.accountKeysCount" FVMEnvGetAccountKey SpanName = "fvm.env.getAccountKey" FVMEnvRevokeAccountKey SpanName = "fvm.env.revokeAccountKey" - FVMEnvRevokeEncodedAccountKey SpanName = "fvm.env.revokeEncodedAccountKey" FVMEnvUpdateAccountContractCode SpanName = "fvm.env.updateAccountContractCode" FVMEnvGetAccountContractCode SpanName = "fvm.env.getAccountContractCode" FVMEnvRemoveAccountContractCode SpanName = "fvm.env.removeAccountContractCode" From f82f503e7cfb0c033b5f66666252ff2f07a5c79f Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Tue, 22 Oct 2024 17:49:44 +0200 Subject: [PATCH 02/13] cleanup and test --- fvm/environment/current_version_boundary.go | 135 ----------- .../derived_data_invalidator_test.go | 3 +- fvm/environment/facade_env.go | 8 +- fvm/environment/meter.go | 2 +- fvm/environment/minimum_required_version.go | 190 ++++++++++++++++ .../mock/current_version_boundary.go | 55 ----- .../mock/minimum_required_version.go | 52 +++++ fvm/fvm_test.go | 215 ++++++++++++++++++ fvm/storage/derived/derived_block_data.go | 6 +- fvm/storage/derived/invalidator.go | 1 + fvm/transactionInvoker.go | 3 +- module/trace/constants.go | 2 +- 12 files changed, 471 insertions(+), 201 deletions(-) delete mode 100644 fvm/environment/current_version_boundary.go create mode 100644 fvm/environment/minimum_required_version.go delete mode 100644 fvm/environment/mock/current_version_boundary.go create mode 100644 fvm/environment/mock/minimum_required_version.go diff --git a/fvm/environment/current_version_boundary.go b/fvm/environment/current_version_boundary.go deleted file mode 100644 index 7bdb7dd51bd..00000000000 --- a/fvm/environment/current_version_boundary.go +++ /dev/null @@ -1,135 +0,0 @@ -package environment - -import ( - "context" - "github.com/onflow/flow-go/fvm/storage" - "github.com/onflow/flow-go/fvm/storage/state" - "github.com/onflow/flow-go/fvm/tracing" - "github.com/onflow/flow-go/model/convert" - "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/module/trace" -) - -type CurrentVersionBoundary interface { - CurrentVersionBoundary() (flow.VersionBoundary, error) -} - -type ParseRestrictedCurrentVersionBoundary struct { - txnState state.NestedTransactionPreparer - impl CurrentVersionBoundary -} - -func NewParseRestrictedCurrentVersionBoundary( - txnState state.NestedTransactionPreparer, - impl CurrentVersionBoundary, -) CurrentVersionBoundary { - return ParseRestrictedCurrentVersionBoundary{ - txnState: txnState, - impl: impl, - } -} - -func (p ParseRestrictedCurrentVersionBoundary) CurrentVersionBoundary() (flow.VersionBoundary, error) { - return parseRestrict1Ret( - p.txnState, - trace.FVMEnvRandom, - p.impl.CurrentVersionBoundary) -} - -type currentVersionBoundary struct { - tracer tracing.TracerSpan - meter Meter - txnState storage.TransactionPreparer - envParams EnvironmentParams -} - -func NewCurrentVersionBoundary( - tracer tracing.TracerSpan, - meter Meter, - txnState storage.TransactionPreparer, - envParams EnvironmentParams, -) CurrentVersionBoundary { - return currentVersionBoundary{ - tracer: tracer, - meter: meter, - txnState: txnState, - envParams: envParams, - } -} - -func (c currentVersionBoundary) CurrentVersionBoundary() (flow.VersionBoundary, error) { - tracerSpan := c.tracer.StartExtensiveTracingChildSpan( - trace.FVMEnvGetCurrentVersionBoundary) - defer tracerSpan.End() - - err := c.meter.MeterComputation(ComputationKindGetCurrentVersionBoundary, 1) - if err != nil { - // TODO: return a better error - return flow.VersionBoundary{}, err - } - - value, _, err := c.txnState.GetCurrentVersionBoundary( - c.txnState, - NewCurrentVersionBoundaryComputer( - tracerSpan, - c.envParams, - c.txnState, - ), - ) - if err != nil { - // TODO: return a better error - return flow.VersionBoundary{}, err - } - - return value, nil -} - -var _ CurrentVersionBoundary = (*currentVersionBoundary)(nil) - -type CurrentVersionBoundaryComputer struct { - tracerSpan tracing.TracerSpan - envParams EnvironmentParams - txnState storage.TransactionPreparer -} - -func NewCurrentVersionBoundaryComputer( - tracerSpan tracing.TracerSpan, - envParams EnvironmentParams, - txnState storage.TransactionPreparer, -) CurrentVersionBoundaryComputer { - return CurrentVersionBoundaryComputer{ - tracerSpan: tracerSpan, - envParams: envParams, - txnState: txnState, - } -} - -func (computer CurrentVersionBoundaryComputer) Compute( - _ state.NestedTransactionPreparer, - _ struct{}, -) ( - flow.VersionBoundary, - error, -) { - env := NewScriptEnv( - context.Background(), - computer.tracerSpan, - computer.envParams, - computer.txnState) - - value, err := env.GetCurrentVersionBoundary() - - if err != nil { - // TODO: return a better error - return flow.VersionBoundary{}, err - } - - boundary, err := convert.VersionBoundary(value) - - if err != nil { - // TODO: return a better error - return flow.VersionBoundary{}, err - } - - return boundary, nil -} diff --git a/fvm/environment/derived_data_invalidator_test.go b/fvm/environment/derived_data_invalidator_test.go index 41be4af8f44..7e45e39eebc 100644 --- a/fvm/environment/derived_data_invalidator_test.go +++ b/fvm/environment/derived_data_invalidator_test.go @@ -1,9 +1,10 @@ package environment_test import ( - "github.com/onflow/flow-go/fvm/systemcontracts" "testing" + "github.com/onflow/flow-go/fvm/systemcontracts" + "github.com/onflow/cadence/runtime/common" "github.com/stretchr/testify/require" diff --git a/fvm/environment/facade_env.go b/fvm/environment/facade_env.go index dbd2bf96a53..c9ff2e4931a 100644 --- a/fvm/environment/facade_env.go +++ b/fvm/environment/facade_env.go @@ -37,7 +37,7 @@ type facadeEnvironment struct { ValueStore *SystemContracts - CurrentVersionBoundary + MinimumRequiredVersion UUIDGenerator AccountLocalIDGenerator @@ -104,7 +104,7 @@ func newFacadeEnvironment( ), SystemContracts: systemContracts, - CurrentVersionBoundary: NewCurrentVersionBoundary( + MinimumRequiredVersion: NewMinimumRequiredVersion( tracer, meter, txnState, @@ -300,9 +300,9 @@ func (env *facadeEnvironment) addParseRestrictedChecks() { env.RandomSourceHistoryProvider = NewParseRestrictedRandomSourceHistoryProvider( env.txnState, env.RandomSourceHistoryProvider) - env.CurrentVersionBoundary = NewParseRestrictedCurrentVersionBoundary( + env.MinimumRequiredVersion = NewParseRestrictedMinimumRequiredVersion( env.txnState, - env.CurrentVersionBoundary) + env.MinimumRequiredVersion) env.UUIDGenerator = NewParseRestrictedUUIDGenerator( env.txnState, env.UUIDGenerator) diff --git a/fvm/environment/meter.go b/fvm/environment/meter.go index 456294b51f5..91adef249d8 100644 --- a/fvm/environment/meter.go +++ b/fvm/environment/meter.go @@ -55,7 +55,7 @@ const ( _ ComputationKindEVMEncodeABI ComputationKindEVMDecodeABI - ComputationKindGetCurrentVersionBoundary + ComputationKindMinimumRequiredVersion ) // MainnetExecutionEffortWeights are the execution effort weights as they are diff --git a/fvm/environment/minimum_required_version.go b/fvm/environment/minimum_required_version.go new file mode 100644 index 00000000000..136fd9dbb5d --- /dev/null +++ b/fvm/environment/minimum_required_version.go @@ -0,0 +1,190 @@ +package environment + +import ( + "context" + "fmt" + + "github.com/coreos/go-semver/semver" + + "github.com/onflow/flow-go/fvm/storage" + "github.com/onflow/flow-go/fvm/storage/state" + "github.com/onflow/flow-go/fvm/tracing" + "github.com/onflow/flow-go/model/convert" + "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/module/trace" +) + +// MinimumRequiredVersion returns the minimum required cadence version for the current environment +// in semver format. +type MinimumRequiredVersion interface { + MinimumRequiredVersion() (string, error) +} + +type ParseRestrictedMinimumRequiredVersion struct { + txnState state.NestedTransactionPreparer + impl MinimumRequiredVersion +} + +func NewParseRestrictedMinimumRequiredVersion( + txnState state.NestedTransactionPreparer, + impl MinimumRequiredVersion, +) MinimumRequiredVersion { + return ParseRestrictedMinimumRequiredVersion{ + txnState: txnState, + impl: impl, + } +} + +func (p ParseRestrictedMinimumRequiredVersion) MinimumRequiredVersion() (string, error) { + return parseRestrict1Ret( + p.txnState, + trace.FVMEnvRandom, + p.impl.MinimumRequiredVersion) +} + +type minimumRequiredVersion struct { + tracer tracing.TracerSpan + meter Meter + + txnState storage.TransactionPreparer + envParams EnvironmentParams +} + +func NewMinimumRequiredVersion( + tracer tracing.TracerSpan, + meter Meter, + txnState storage.TransactionPreparer, + envParams EnvironmentParams, +) MinimumRequiredVersion { + return minimumRequiredVersion{ + tracer: tracer, + meter: meter, + + txnState: txnState, + envParams: envParams, + } +} + +func (c minimumRequiredVersion) MinimumRequiredVersion() (string, error) { + tracerSpan := c.tracer.StartExtensiveTracingChildSpan( + trace.FVMEnvMinimumRequiredVersion) + defer tracerSpan.End() + + err := c.meter.MeterComputation(ComputationKindMinimumRequiredVersion, 1) + if err != nil { + return "", fmt.Errorf("get MinimumRequiredVersion failed: %w", err) + } + + value, err := c.txnState.GetCurrentVersionBoundary( + c.txnState, + NewCurrentVersionBoundaryComputer( + tracerSpan, + c.envParams, + c.txnState, + ), + ) + if err != nil { + return "", fmt.Errorf("get MinimumRequiredVersion failed: %w", err) + } + + version, err := c.mapToCadenceVersion(value.Version) + + if err != nil { + return "", fmt.Errorf("get MinimumRequiredVersion failed: %w", err) + } + + return version, nil +} + +func (c minimumRequiredVersion) mapToCadenceVersion(version string) (string, error) { + semVer, err := semver.NewVersion(version) + if err != nil { + // TODO: return a better error + return "", fmt.Errorf("failed to map FVM version to cadence version: %w", err) + } + + // return 0.0.0 if there is no mapping for the version + var cadenceVersion = semver.Version{} + + greaterThanOrEqualTo := func(version semver.Version, versionToCompare semver.Version) bool { + return version.Compare(versionToCompare) >= 0 + } + + for _, entry := range fvmToCadenceVersionMapping { + if greaterThanOrEqualTo(*semVer, entry.FlowGoVersion) { + cadenceVersion = entry.CadenceVersion + } else { + break + } + } + + return cadenceVersion.String(), nil +} + +type VersionMapEntry struct { + FlowGoVersion semver.Version + CadenceVersion semver.Version +} + +// FVMToCadenceVersionMapping is a hardcoded mapping between FVM versions and Cadence versions. +// Entries are only needed for cadence versions where cadence intends to switch behaviour +// based on the version. +// This should be ordered. +var fvmToCadenceVersionMapping = []VersionMapEntry{ + { + FlowGoVersion: *semver.New("0.37.0"), + CadenceVersion: *semver.New("1.0.0"), + }, +} + +func SetFVMToCadenceVersionMappingForTestingOnly(mapping []VersionMapEntry) { + fvmToCadenceVersionMapping = mapping +} + +var _ MinimumRequiredVersion = (*minimumRequiredVersion)(nil) + +type CurrentVersionBoundaryComputer struct { + tracerSpan tracing.TracerSpan + envParams EnvironmentParams + txnState storage.TransactionPreparer +} + +func NewCurrentVersionBoundaryComputer( + tracerSpan tracing.TracerSpan, + envParams EnvironmentParams, + txnState storage.TransactionPreparer, +) CurrentVersionBoundaryComputer { + return CurrentVersionBoundaryComputer{ + tracerSpan: tracerSpan, + envParams: envParams, + txnState: txnState, + } +} + +func (computer CurrentVersionBoundaryComputer) Compute( + _ state.NestedTransactionPreparer, + _ struct{}, +) ( + flow.VersionBoundary, + error, +) { + env := NewScriptEnv( + context.Background(), + computer.tracerSpan, + computer.envParams, + computer.txnState) + + value, err := env.GetCurrentVersionBoundary() + + if err != nil { + return flow.VersionBoundary{}, fmt.Errorf("could not get current version boundary: %w", err) + } + + boundary, err := convert.VersionBoundary(value) + + if err != nil { + return flow.VersionBoundary{}, fmt.Errorf("could not parse current version boundary: %w", err) + } + + return boundary, nil +} diff --git a/fvm/environment/mock/current_version_boundary.go b/fvm/environment/mock/current_version_boundary.go deleted file mode 100644 index 7bdaaa9f93b..00000000000 --- a/fvm/environment/mock/current_version_boundary.go +++ /dev/null @@ -1,55 +0,0 @@ -// Code generated by mockery v2.43.2. DO NOT EDIT. - -package mock - -import ( - flow "github.com/onflow/flow-go/model/flow" - mock "github.com/stretchr/testify/mock" -) - -// CurrentVersionBoundary is an autogenerated mock type for the CurrentVersionBoundary type -type CurrentVersionBoundary struct { - mock.Mock -} - -// CurrentVersionBoundary provides a mock function with given fields: -func (_m *CurrentVersionBoundary) CurrentVersionBoundary() (flow.VersionBoundary, error) { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for CurrentVersionBoundary") - } - - var r0 flow.VersionBoundary - var r1 error - if rf, ok := ret.Get(0).(func() (flow.VersionBoundary, error)); ok { - return rf() - } - if rf, ok := ret.Get(0).(func() flow.VersionBoundary); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(flow.VersionBoundary) - } - - if rf, ok := ret.Get(1).(func() error); ok { - r1 = rf() - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// NewCurrentVersionBoundary creates a new instance of CurrentVersionBoundary. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. -func NewCurrentVersionBoundary(t interface { - mock.TestingT - Cleanup(func()) -}) *CurrentVersionBoundary { - mock := &CurrentVersionBoundary{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock -} diff --git a/fvm/environment/mock/minimum_required_version.go b/fvm/environment/mock/minimum_required_version.go new file mode 100644 index 00000000000..3894d135148 --- /dev/null +++ b/fvm/environment/mock/minimum_required_version.go @@ -0,0 +1,52 @@ +// Code generated by mockery v2.43.2. DO NOT EDIT. + +package mock + +import mock "github.com/stretchr/testify/mock" + +// MinimumRequiredVersion is an autogenerated mock type for the MinimumRequiredVersion type +type MinimumRequiredVersion struct { + mock.Mock +} + +// MinimumRequiredVersion provides a mock function with given fields: +func (_m *MinimumRequiredVersion) MinimumRequiredVersion() (string, error) { + ret := _m.Called() + + if len(ret) == 0 { + panic("no return value specified for MinimumRequiredVersion") + } + + var r0 string + var r1 error + if rf, ok := ret.Get(0).(func() (string, error)); ok { + return rf() + } + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + if rf, ok := ret.Get(1).(func() error); ok { + r1 = rf() + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// NewMinimumRequiredVersion creates a new instance of MinimumRequiredVersion. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// The first argument is typically a *testing.T value. +func NewMinimumRequiredVersion(t interface { + mock.TestingT + Cleanup(func()) +}) *MinimumRequiredVersion { + mock := &MinimumRequiredVersion{} + mock.Mock.Test(t) + + t.Cleanup(func() { mock.AssertExpectations(t) }) + + return mock +} diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 7526f5b3837..5cb46bd9279 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -8,6 +8,14 @@ import ( "math" "strings" "testing" + "time" + + "github.com/coreos/go-semver/semver" + "github.com/onflow/flow-core-contracts/lib/go/templates" + + "github.com/onflow/flow-go/cmd/util/ledger/util" + "github.com/onflow/flow-go/fvm/storage" + "github.com/onflow/flow-go/fvm/storage/state" stdlib2 "github.com/onflow/cadence/runtime/stdlib" @@ -3229,3 +3237,210 @@ func TestAccountCapabilitiesPublishEntitledRejection(t *testing.T) { }), ) } + +func Test_MinimumRequiredVersion(t *testing.T) { + + chain := flow.Emulator.Chain() + sc := systemcontracts.SystemContractsForChain(chain.ChainID()) + tracer := tracing.NewTracerSpan() + + getVersion := func(snapshotTree snapshot.SnapshotTree) string { + blockDatabase := storage.NewBlockDatabase( + snapshotTree, + 0, + nil) + + txnState, err := blockDatabase.NewTransaction(0, state.DefaultParameters()) + require.NoError(t, err) + + envParams := environment.DefaultEnvironmentParams() + envParams.Chain = chain + + mrv := environment.NewMinimumRequiredVersion(tracer, util.NopMeter{}, txnState, envParams) + + v, err := mrv.MinimumRequiredVersion() + + require.NoError(t, err) + return v + } + + insertVersionBoundary := func(newVersion semver.Version, currentHeight, insertHeight uint64, ctx fvm.Context, snapshotTree snapshot.SnapshotTree, vm fvm.VM, txIndex uint32) snapshot.SnapshotTree { + setVersionBoundaryScript := templates.GenerateSetVersionBoundaryScript(sc.AsTemplateEnv()) + tx := flow.NewTransactionBody(). + SetScript(setVersionBoundaryScript). + SetProposalKey(sc.FlowServiceAccount.Address, 0, 0). + AddAuthorizer(sc.FlowServiceAccount.Address). + SetPayer(sc.FlowServiceAccount.Address) + + tx. + AddArgument(jsoncdc.MustEncode(cadence.UInt8(newVersion.Major))). + AddArgument(jsoncdc.MustEncode(cadence.UInt8(newVersion.Minor))). + AddArgument(jsoncdc.MustEncode(cadence.UInt8(newVersion.Patch))). + AddArgument(jsoncdc.MustEncode(cadence.String(newVersion.PreRelease))) + + tx.AddArgument(jsoncdc.MustEncode(cadence.UInt64(insertHeight))) + + startHeader := flow.Header{ + Height: currentHeight, + ChainID: chain.ChainID(), + Timestamp: time.Now().UTC(), + } + + blocks := new(envMock.Blocks) + ctxWithBlock := fvm.NewContextFromParent( + ctx, + fvm.WithBlockHeader(&startHeader), + fvm.WithBlocks(blocks), + ) + + executionSnapshot, output, err := vm.Run( + ctxWithBlock, + fvm.Transaction(tx, txIndex), + snapshotTree) + + require.NoError(t, err) + require.NoError(t, output.Err) + return snapshotTree.Append(executionSnapshot) + } + + runSystemTxToUpdateNodeVersionBeaconContract := func(atHeight uint64, ctx fvm.Context, snapshotTree snapshot.SnapshotTree, vm fvm.VM, txIndex uint32) snapshot.SnapshotTree { + txBody := flow.NewTransactionBody(). + SetScript([]byte(fmt.Sprintf(` + import NodeVersionBeacon from %s + + transaction { + prepare(serviceAccount: auth(BorrowValue) &Account) { + + let versionBeaconHeartbeat = serviceAccount.storage + .borrow<&NodeVersionBeacon.Heartbeat>(from: NodeVersionBeacon.HeartbeatStoragePath) + ?? panic("Couldn't borrow NodeVersionBeacon.Heartbeat Resource") + versionBeaconHeartbeat.heartbeat() + } + } + `, + sc.NodeVersionBeacon.Address.HexWithPrefix()))). + SetProposalKey(sc.FlowServiceAccount.Address, 0, 0). + AddAuthorizer(sc.FlowServiceAccount.Address). + SetPayer(sc.FlowServiceAccount.Address) + + endHeader := flow.Header{ + Height: atHeight, + ChainID: chain.ChainID(), + Timestamp: time.Now().UTC(), + } + + blocks := new(envMock.Blocks) + ctxWithBlock := fvm.NewContextFromParent(ctx, + fvm.WithBlockHeader(&endHeader), + fvm.WithBlocks(blocks), + ) + + executionSnapshot, output, err := vm.Run( + ctxWithBlock, + fvm.Transaction(txBody, txIndex), + snapshotTree) + + require.NoError(t, err) + require.NoError(t, output.Err) + + return snapshotTree.Append(executionSnapshot) + } + + t.Run("minimum required version", newVMTest(). + withContextOptions( + fvm.WithChain(chain), + fvm.WithAuthorizationChecksEnabled(false), + fvm.WithSequenceNumberCheckAndIncrementEnabled(false), + ). + run(func( + t *testing.T, + vm fvm.VM, + chain flow.Chain, + ctx fvm.Context, + snapshotTree snapshot.SnapshotTree, + ) { + // default version is empty + require.Equal(t, semver.Version{}.String(), getVersion(snapshotTree)) + + // define mapping for flow go version to cadence version + flowVersion1 := semver.Version{ + Major: 1, + Minor: 2, + Patch: 3, + PreRelease: "rc.1", + } + cadenceVersion1 := semver.Version{ + Major: 2, + Minor: 1, + Patch: 3, + PreRelease: "rc.2", + } + flowVersion2 := semver.Version{ + Major: 2, + Minor: 2, + Patch: 3, + PreRelease: "rc.1", + } + cadenceVersion2 := semver.Version{ + Major: 12, + Minor: 0, + Patch: 0, + PreRelease: "", + } + environment.SetFVMToCadenceVersionMappingForTestingOnly( + []environment.VersionMapEntry{ + { + FlowGoVersion: flowVersion1, + CadenceVersion: cadenceVersion1, + }, + { + FlowGoVersion: flowVersion2, + CadenceVersion: cadenceVersion2, + }, + }) + + h0 := uint64(100) // starting height + hv1 := uint64(2000) // version boundary height + hv2 := uint64(4000) // version boundary height + + txIndex := uint32(0) + + // insert version boundary 1 + snapshotTree = insertVersionBoundary(flowVersion1, h0, hv1, ctx, snapshotTree, vm, txIndex) + txIndex += 1 + + // so far no change: + require.Equal(t, semver.Version{}.String(), getVersion(snapshotTree)) + + // system transaction needs to run to update the flowVersion on chain + snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv1-1, ctx, snapshotTree, vm, txIndex) + txIndex += 1 + + // no change: + require.Equal(t, semver.Version{}.String(), getVersion(snapshotTree)) + + // system transaction needs to run to update the flowVersion on chain + snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv1, ctx, snapshotTree, vm, txIndex) + txIndex += 1 + + // switch to cadence version 1 + require.Equal(t, cadenceVersion1.String(), getVersion(snapshotTree)) + + // system transaction needs to run to update the flowVersion on chain + snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv1+1, ctx, snapshotTree, vm, txIndex) + txIndex += 1 + + // still cadence version 1 + require.Equal(t, cadenceVersion1.String(), getVersion(snapshotTree)) + + // insert version boundary 2 + snapshotTree = insertVersionBoundary(flowVersion2, hv1+1, hv2, ctx, snapshotTree, vm, txIndex) + txIndex += 1 + + // system transaction needs to run to update the flowVersion on chain + snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv2, ctx, snapshotTree, vm, txIndex) + + // switch cadence version 2 + require.Equal(t, cadenceVersion2.String(), getVersion(snapshotTree)) + })) +} diff --git a/fvm/storage/derived/derived_block_data.go b/fvm/storage/derived/derived_block_data.go index 53140ce38ba..35fb0d6c63f 100644 --- a/fvm/storage/derived/derived_block_data.go +++ b/fvm/storage/derived/derived_block_data.go @@ -37,7 +37,6 @@ type DerivedTransactionPreparer interface { getCurrentVersionBoundaryComputer ValueComputer[struct{}, flow.VersionBoundary], ) ( flow.VersionBoundary, - *snapshot.ExecutionSnapshot, error, ) @@ -231,13 +230,14 @@ func (transaction *DerivedTransactionData) GetCurrentVersionBoundary( getCurrentVersionBoundaryComputer ValueComputer[struct{}, flow.VersionBoundary], ) ( flow.VersionBoundary, - *snapshot.ExecutionSnapshot, error, ) { - return transaction.currentVersionBoundary.GetWithStateOrCompute( + vb, _, err := transaction.currentVersionBoundary.GetWithStateOrCompute( txnState, struct{}{}, getCurrentVersionBoundaryComputer) + + return vb, err } func (transaction *DerivedTransactionData) Validate() error { diff --git a/fvm/storage/derived/invalidator.go b/fvm/storage/derived/invalidator.go index 9667a8d4e3c..027d573176b 100644 --- a/fvm/storage/derived/invalidator.go +++ b/fvm/storage/derived/invalidator.go @@ -2,6 +2,7 @@ package derived import ( "github.com/onflow/cadence/runtime/common" + "github.com/onflow/flow-go/model/flow" "github.com/onflow/flow-go/fvm/meter" diff --git a/fvm/transactionInvoker.go b/fvm/transactionInvoker.go index 8f84f3de5b5..af9d500ae67 100644 --- a/fvm/transactionInvoker.go +++ b/fvm/transactionInvoker.go @@ -2,9 +2,10 @@ package fvm import ( "fmt" - "github.com/onflow/flow-go/fvm/systemcontracts" "strconv" + "github.com/onflow/flow-go/fvm/systemcontracts" + "github.com/onflow/cadence/runtime" "github.com/onflow/cadence/runtime/common" "github.com/rs/zerolog" diff --git a/module/trace/constants.go b/module/trace/constants.go index 004f21b2045..f011c1064b0 100644 --- a/module/trace/constants.go +++ b/module/trace/constants.go @@ -183,7 +183,7 @@ const ( FVMEnvGetBlockAtHeight SpanName = "fvm.env.getBlockAtHeight" FVMEnvRandom SpanName = "fvm.env.unsafeRandom" FVMEnvRandomSourceHistoryProvider SpanName = "fvm.env.randomSourceHistoryProvider" - FVMEnvGetCurrentVersionBoundary SpanName = "fvm.env.getCurrentVersionBoundary" + FVMEnvMinimumRequiredVersion SpanName = "fvm.env.minimumRequiredVersion" FVMEnvCreateAccount SpanName = "fvm.env.createAccount" FVMEnvAddAccountKey SpanName = "fvm.env.addAccountKey" FVMEnvAccountKeysCount SpanName = "fvm.env.accountKeysCount" From f36aafea0c409d089191ea64528a17592e007be8 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Tue, 22 Oct 2024 17:55:47 +0200 Subject: [PATCH 03/13] fix invalidation in indexer --- module/state_synchronization/indexer/util.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/module/state_synchronization/indexer/util.go b/module/state_synchronization/indexer/util.go index 72fa47b9ac8..4b284de3eb5 100644 --- a/module/state_synchronization/indexer/util.go +++ b/module/state_synchronization/indexer/util.go @@ -148,9 +148,9 @@ type currentVersionBoundaryInvalidator struct { } func (inv *currentVersionBoundaryInvalidator) ShouldInvalidateEntries() bool { - return inv.invalidate + return inv.invalidate || inv.invalidateAll } func (inv *currentVersionBoundaryInvalidator) ShouldInvalidateEntry(_ struct{}, _ flow.VersionBoundary, _ *snapshot.ExecutionSnapshot) bool { - return inv.invalidate + return inv.invalidate || inv.invalidateAll } From fe18d5ae6488d888b3e804781cbebb62238c6a4a Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Wed, 23 Oct 2024 20:30:49 +0200 Subject: [PATCH 04/13] switch to a different aproach --- fvm/environment/derived_data_invalidator.go | 75 ++------- .../derived_data_invalidator_test.go | 17 +-- fvm/environment/facade_env.go | 6 - fvm/environment/minimum_required_version.go | 144 ++---------------- fvm/environment/mock/environment.go | 2 +- fvm/executionParameters.go | 115 ++++++++++---- fvm/fvm_test.go | 35 +++-- fvm/meter/meter.go | 7 + fvm/script.go | 5 +- fvm/storage/derived/derived_block_data.go | 83 +++------- fvm/storage/derived/invalidator.go | 21 ++- fvm/storage/state/execution_state.go | 18 ++- fvm/storage/state/transaction_state.go | 11 +- fvm/transactionInvoker.go | 24 ++- module/state_synchronization/indexer/util.go | 6 +- 15 files changed, 218 insertions(+), 351 deletions(-) diff --git a/fvm/environment/derived_data_invalidator.go b/fvm/environment/derived_data_invalidator.go index 0a31acc5b4b..77bb9270f17 100644 --- a/fvm/environment/derived_data_invalidator.go +++ b/fvm/environment/derived_data_invalidator.go @@ -5,7 +5,6 @@ import ( "github.com/onflow/flow-go/fvm/storage/derived" "github.com/onflow/flow-go/fvm/storage/snapshot" - "github.com/onflow/flow-go/model/flow" ) type ContractUpdate struct { @@ -26,7 +25,7 @@ func (u ContractUpdates) Any() bool { type DerivedDataInvalidator struct { ContractUpdates - MeterParamOverridesUpdated bool + ExecutionParametersUpdated bool CurrentVersionBoundaryUpdated bool } @@ -37,23 +36,19 @@ func NewDerivedDataInvalidator( contractUpdates ContractUpdates, executionSnapshot *snapshot.ExecutionSnapshot, meterStateRead *snapshot.ExecutionSnapshot, - serviceAddress flow.Address, ) DerivedDataInvalidator { return DerivedDataInvalidator{ ContractUpdates: contractUpdates, - MeterParamOverridesUpdated: meterParamOverridesUpdated( + ExecutionParametersUpdated: executionParametersUpdated( executionSnapshot, meterStateRead), - CurrentVersionBoundaryUpdated: currentVersionBoundaryUpdated( - serviceAddress, - meterStateRead), } } -// meterParamOverridesUpdated returns true if the meter param overrides have been updated +// executionParametersUpdated returns true if the meter param overrides have been updated // this is done by checking if the registers needed to compute the meter param overrides // have been touched in the execution snapshot -func meterParamOverridesUpdated( +func executionParametersUpdated( executionSnapshot *snapshot.ExecutionSnapshot, meterStateRead *snapshot.ExecutionSnapshot, ) bool { @@ -76,36 +71,12 @@ func meterParamOverridesUpdated( return false } -// currentVersionBoundaryUpdated returns true if the current version boundary -// should be invalidated. Currently, this will trigger on any change to the -// service account, which will trigger at least once per block. This is ok for now as -// the meterParamOverrides also trigger on every block. -func currentVersionBoundaryUpdated( - serviceAddress flow.Address, - executionSnapshot *snapshot.ExecutionSnapshot, -) bool { - serviceAccount := string(serviceAddress.Bytes()) - - for registerId := range executionSnapshot.WriteSet { - // The meter param override values are stored in the service account. - if registerId.Owner == serviceAccount { - return true - } - } - - return false -} - func (invalidator DerivedDataInvalidator) ProgramInvalidator() derived.ProgramInvalidator { return ProgramInvalidator{invalidator} } -func (invalidator DerivedDataInvalidator) MeterParamOverridesInvalidator() derived.MeterParamOverridesInvalidator { - return MeterParamOverridesInvalidator{invalidator} -} - -func (invalidator DerivedDataInvalidator) CurrentVersionBoundaryInvalidator() derived.CurrentVersionBoundaryInvalidator { - return CurrentVersionBoundaryInvalidator{invalidator} +func (invalidator DerivedDataInvalidator) ExecutionParametersInvalidator() derived.ExecutionParametersInvalidator { + return ExecutionParametersInvalidator{invalidator} } type ProgramInvalidator struct { @@ -113,7 +84,7 @@ type ProgramInvalidator struct { } func (invalidator ProgramInvalidator) ShouldInvalidateEntries() bool { - return invalidator.MeterParamOverridesUpdated || + return invalidator.ExecutionParametersUpdated || invalidator.ContractUpdates.Any() } @@ -122,7 +93,7 @@ func (invalidator ProgramInvalidator) ShouldInvalidateEntry( program *derived.Program, _ *snapshot.ExecutionSnapshot, ) bool { - if invalidator.MeterParamOverridesUpdated { + if invalidator.ExecutionParametersUpdated { // if meter parameters changed we need to invalidate all programs return true } @@ -155,36 +126,18 @@ func (invalidator ProgramInvalidator) ShouldInvalidateEntry( return false } -type MeterParamOverridesInvalidator struct { - DerivedDataInvalidator -} - -func (invalidator MeterParamOverridesInvalidator) ShouldInvalidateEntries() bool { - return invalidator.MeterParamOverridesUpdated -} - -func (invalidator MeterParamOverridesInvalidator) ShouldInvalidateEntry( - _ struct{}, - _ derived.MeterParamOverrides, - _ *snapshot.ExecutionSnapshot, -) bool { - return invalidator.MeterParamOverridesUpdated -} - -type CurrentVersionBoundaryInvalidator struct { +type ExecutionParametersInvalidator struct { DerivedDataInvalidator } -func (invalidator CurrentVersionBoundaryInvalidator) ShouldInvalidateEntries() bool { - return invalidator.CurrentVersionBoundaryUpdated +func (invalidator ExecutionParametersInvalidator) ShouldInvalidateEntries() bool { + return invalidator.ExecutionParametersUpdated } -func (invalidator CurrentVersionBoundaryInvalidator) ShouldInvalidateEntry( +func (invalidator ExecutionParametersInvalidator) ShouldInvalidateEntry( _ struct{}, - _ flow.VersionBoundary, + _ derived.StateExecutionParameters, _ *snapshot.ExecutionSnapshot, ) bool { - // If the meter params are updated - // the current version boundary derived data table becomes invalid. - return invalidator.MeterParamOverridesUpdated || invalidator.CurrentVersionBoundaryUpdated + return invalidator.ExecutionParametersUpdated } diff --git a/fvm/environment/derived_data_invalidator_test.go b/fvm/environment/derived_data_invalidator_test.go index 91d9240d0ce..0abf50f644d 100644 --- a/fvm/environment/derived_data_invalidator_test.go +++ b/fvm/environment/derived_data_invalidator_test.go @@ -84,7 +84,7 @@ func TestDerivedDataProgramInvalidator(t *testing.T) { }) t.Run("meter parameters invalidator invalidates all entries", func(t *testing.T) { invalidator := environment.DerivedDataInvalidator{ - MeterParamOverridesUpdated: true, + ExecutionParametersUpdated: true, }.ProgramInvalidator() require.True(t, invalidator.ShouldInvalidateEntries()) @@ -209,7 +209,7 @@ func TestDerivedDataProgramInvalidator(t *testing.T) { func TestMeterParamOverridesInvalidator(t *testing.T) { invalidator := environment.DerivedDataInvalidator{}. - MeterParamOverridesInvalidator() + ExecutionParametersInvalidator() require.False(t, invalidator.ShouldInvalidateEntries()) require.False(t, invalidator.ShouldInvalidateEntry( @@ -219,8 +219,8 @@ func TestMeterParamOverridesInvalidator(t *testing.T) { invalidator = environment.DerivedDataInvalidator{ ContractUpdates: environment.ContractUpdates{}, - MeterParamOverridesUpdated: true, - }.MeterParamOverridesInvalidator() + ExecutionParametersUpdated: true, + }.ExecutionParametersInvalidator() require.True(t, invalidator.ShouldInvalidateEntries()) require.True(t, invalidator.ShouldInvalidateEntry( @@ -267,7 +267,7 @@ func TestMeterParamOverridesUpdated(t *testing.T) { txnState, err := blockDatabase.NewTransaction(0, state.DefaultParameters()) require.NoError(t, err) - computer := fvm.NewMeterParamOverridesComputer(ctx, txnState) + computer := fvm.NewExecutionParametersComputer( overrides, err := computer.Compute(txnState, struct{}{}) require.NoError(t, err) @@ -298,14 +298,11 @@ func TestMeterParamOverridesUpdated(t *testing.T) { }, } - sc := systemcontracts.SystemContractsForChain(flow.Testnet) - invalidator := environment.NewDerivedDataInvalidator( environment.ContractUpdates{}, snapshot, - meterStateRead, - sc.FlowServiceAccount.Address) - require.Equal(t, expected, invalidator.MeterParamOverridesUpdated) + meterStateRead) + require.Equal(t, expected, invalidator.ExecutionParametersUpdated) } executionSnapshot, err = txnState.FinalizeMainTransaction() diff --git a/fvm/environment/facade_env.go b/fvm/environment/facade_env.go index ca4d771c3e7..7366dd4a89e 100644 --- a/fvm/environment/facade_env.go +++ b/fvm/environment/facade_env.go @@ -105,10 +105,7 @@ func newFacadeEnvironment( SystemContracts: systemContracts, MinimumRequiredVersion: NewMinimumRequiredVersion( - tracer, - meter, txnState, - params, ), UUIDGenerator: NewUUIDGenerator( @@ -300,9 +297,6 @@ func (env *facadeEnvironment) addParseRestrictedChecks() { env.RandomSourceHistoryProvider = NewParseRestrictedRandomSourceHistoryProvider( env.txnState, env.RandomSourceHistoryProvider) - env.MinimumRequiredVersion = NewParseRestrictedMinimumRequiredVersion( - env.txnState, - env.MinimumRequiredVersion) env.UUIDGenerator = NewParseRestrictedUUIDGenerator( env.txnState, env.UUIDGenerator) diff --git a/fvm/environment/minimum_required_version.go b/fvm/environment/minimum_required_version.go index 136fd9dbb5d..f4b269c39b8 100644 --- a/fvm/environment/minimum_required_version.go +++ b/fvm/environment/minimum_required_version.go @@ -1,17 +1,9 @@ package environment import ( - "context" - "fmt" - "github.com/coreos/go-semver/semver" - "github.com/onflow/flow-go/fvm/storage" "github.com/onflow/flow-go/fvm/storage/state" - "github.com/onflow/flow-go/fvm/tracing" - "github.com/onflow/flow-go/model/convert" - "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/module/trace" ) // MinimumRequiredVersion returns the minimum required cadence version for the current environment @@ -20,89 +12,27 @@ type MinimumRequiredVersion interface { MinimumRequiredVersion() (string, error) } -type ParseRestrictedMinimumRequiredVersion struct { - txnState state.NestedTransactionPreparer - impl MinimumRequiredVersion -} - -func NewParseRestrictedMinimumRequiredVersion( - txnState state.NestedTransactionPreparer, - impl MinimumRequiredVersion, -) MinimumRequiredVersion { - return ParseRestrictedMinimumRequiredVersion{ - txnState: txnState, - impl: impl, - } -} - -func (p ParseRestrictedMinimumRequiredVersion) MinimumRequiredVersion() (string, error) { - return parseRestrict1Ret( - p.txnState, - trace.FVMEnvRandom, - p.impl.MinimumRequiredVersion) -} - type minimumRequiredVersion struct { - tracer tracing.TracerSpan - meter Meter - - txnState storage.TransactionPreparer - envParams EnvironmentParams + txnPreparer state.NestedTransactionPreparer } func NewMinimumRequiredVersion( - tracer tracing.TracerSpan, - meter Meter, - txnState storage.TransactionPreparer, - envParams EnvironmentParams, + txnPreparer state.NestedTransactionPreparer, ) MinimumRequiredVersion { return minimumRequiredVersion{ - tracer: tracer, - meter: meter, - - txnState: txnState, - envParams: envParams, + txnPreparer: txnPreparer, } } func (c minimumRequiredVersion) MinimumRequiredVersion() (string, error) { - tracerSpan := c.tracer.StartExtensiveTracingChildSpan( - trace.FVMEnvMinimumRequiredVersion) - defer tracerSpan.End() - - err := c.meter.MeterComputation(ComputationKindMinimumRequiredVersion, 1) - if err != nil { - return "", fmt.Errorf("get MinimumRequiredVersion failed: %w", err) - } + executionParameters := c.txnPreparer.ExecutionParameters() - value, err := c.txnState.GetCurrentVersionBoundary( - c.txnState, - NewCurrentVersionBoundaryComputer( - tracerSpan, - c.envParams, - c.txnState, - ), - ) - if err != nil { - return "", fmt.Errorf("get MinimumRequiredVersion failed: %w", err) - } - - version, err := c.mapToCadenceVersion(value.Version) - - if err != nil { - return "", fmt.Errorf("get MinimumRequiredVersion failed: %w", err) - } + cadenceVersion := mapToCadenceVersion(executionParameters.ExecutionVersion) - return version, nil + return cadenceVersion.String(), nil } -func (c minimumRequiredVersion) mapToCadenceVersion(version string) (string, error) { - semVer, err := semver.NewVersion(version) - if err != nil { - // TODO: return a better error - return "", fmt.Errorf("failed to map FVM version to cadence version: %w", err) - } - +func mapToCadenceVersion(version semver.Version) semver.Version { // return 0.0.0 if there is no mapping for the version var cadenceVersion = semver.Version{} @@ -111,14 +41,14 @@ func (c minimumRequiredVersion) mapToCadenceVersion(version string) (string, err } for _, entry := range fvmToCadenceVersionMapping { - if greaterThanOrEqualTo(*semVer, entry.FlowGoVersion) { + if greaterThanOrEqualTo(version, entry.FlowGoVersion) { cadenceVersion = entry.CadenceVersion } else { break } } - return cadenceVersion.String(), nil + return cadenceVersion } type VersionMapEntry struct { @@ -130,11 +60,13 @@ type VersionMapEntry struct { // Entries are only needed for cadence versions where cadence intends to switch behaviour // based on the version. // This should be ordered. + var fvmToCadenceVersionMapping = []VersionMapEntry{ - { - FlowGoVersion: *semver.New("0.37.0"), - CadenceVersion: *semver.New("1.0.0"), - }, + // Leaving this example in, so it's easier to understand + //{ + // FlowGoVersion: *semver.New("0.37.0"), + // CadenceVersion: *semver.New("1.0.0"), + //}, } func SetFVMToCadenceVersionMappingForTestingOnly(mapping []VersionMapEntry) { @@ -142,49 +74,3 @@ func SetFVMToCadenceVersionMappingForTestingOnly(mapping []VersionMapEntry) { } var _ MinimumRequiredVersion = (*minimumRequiredVersion)(nil) - -type CurrentVersionBoundaryComputer struct { - tracerSpan tracing.TracerSpan - envParams EnvironmentParams - txnState storage.TransactionPreparer -} - -func NewCurrentVersionBoundaryComputer( - tracerSpan tracing.TracerSpan, - envParams EnvironmentParams, - txnState storage.TransactionPreparer, -) CurrentVersionBoundaryComputer { - return CurrentVersionBoundaryComputer{ - tracerSpan: tracerSpan, - envParams: envParams, - txnState: txnState, - } -} - -func (computer CurrentVersionBoundaryComputer) Compute( - _ state.NestedTransactionPreparer, - _ struct{}, -) ( - flow.VersionBoundary, - error, -) { - env := NewScriptEnv( - context.Background(), - computer.tracerSpan, - computer.envParams, - computer.txnState) - - value, err := env.GetCurrentVersionBoundary() - - if err != nil { - return flow.VersionBoundary{}, fmt.Errorf("could not get current version boundary: %w", err) - } - - boundary, err := convert.VersionBoundary(value) - - if err != nil { - return flow.VersionBoundary{}, fmt.Errorf("could not parse current version boundary: %w", err) - } - - return boundary, nil -} diff --git a/fvm/environment/mock/environment.go b/fvm/environment/mock/environment.go index 445dcbd49d8..65ef356b4a3 100644 --- a/fvm/environment/mock/environment.go +++ b/fvm/environment/mock/environment.go @@ -912,7 +912,7 @@ func (_m *Environment) GetCurrentVersionBoundary() (cadence.Value, error) { ret := _m.Called() if len(ret) == 0 { - panic("no return value specified for GetCurrentVersionBoundary") + panic("no return value specified for getMinimuRequiredVersion") } var r0 cadence.Value diff --git a/fvm/executionParameters.go b/fvm/executionParameters.go index 34e3f4594cb..8ec72ba9e8e 100644 --- a/fvm/executionParameters.go +++ b/fvm/executionParameters.go @@ -3,6 +3,9 @@ package fvm import ( "context" "fmt" + "github.com/coreos/go-semver/semver" + "github.com/onflow/flow-go/model/convert" + "github.com/rs/zerolog" "math" "github.com/onflow/flow-go/fvm/storage/snapshot" @@ -52,75 +55,82 @@ func getBasicMeterParameters( return params } -// getBodyMeterParameters returns the set of meter parameters used for -// transaction/script body execution. -func getBodyMeterParameters( +// getExecutionParameters returns the set of meter parameters used for +// transaction/script body execution and the minimum required version as defined by the +// NodeVersionBeacon contract. +func getExecutionParameters( + log zerolog.Logger, ctx Context, proc Procedure, txnState storage.TransactionPreparer, -) ( - meter.MeterParameters, - *snapshot.ExecutionSnapshot, - error, -) { - procParams := getBasicMeterParameters(ctx, proc) +) (meter.ExecutionParameters, *snapshot.ExecutionSnapshot, error) { + meterParams := getBasicMeterParameters(ctx, proc) - overrides, meterStateRead, err := txnState.GetMeterParamOverrides( + executionParams, executionParamsStateRead, err := txnState.GetStateExecutionParameters( txnState, - NewMeterParamOverridesComputer(ctx, txnState)) + NewExecutionParametersComputer(log, ctx, txnState)) if err != nil { - return procParams, nil, err + return meter.ExecutionParameters{ + MeterParameters: meterParams, + ExecutionVersion: semver.Version{}, + }, nil, err } - if overrides.ComputationWeights != nil { - procParams = procParams.WithComputationWeights( - overrides.ComputationWeights) + if executionParams.ComputationWeights != nil { + meterParams = meterParams.WithComputationWeights( + executionParams.ComputationWeights) } - if overrides.MemoryWeights != nil { - procParams = procParams.WithMemoryWeights(overrides.MemoryWeights) + if executionParams.MemoryWeights != nil { + meterParams = meterParams.WithMemoryWeights(executionParams.MemoryWeights) } - if overrides.MemoryLimit != nil { - procParams = procParams.WithMemoryLimit(*overrides.MemoryLimit) + if executionParams.MemoryLimit != nil { + meterParams = meterParams.WithMemoryLimit(*executionParams.MemoryLimit) } // NOTE: The memory limit (and interaction limit) may be overridden by the // environment. We need to ignore the override in that case. if proc.ShouldDisableMemoryAndInteractionLimits(ctx) { - procParams = procParams.WithMemoryLimit(math.MaxUint64). + meterParams = meterParams.WithMemoryLimit(math.MaxUint64). WithStorageInteractionLimit(math.MaxUint64) } - return procParams, meterStateRead, nil + return meter.ExecutionParameters{ + MeterParameters: meterParams, + ExecutionVersion: executionParams.ExecutionVersion, + }, executionParamsStateRead, nil } -type MeterParamOverridesComputer struct { +type ExecutionParametersComputer struct { + log zerolog.Logger ctx Context txnState storage.TransactionPreparer } -func NewMeterParamOverridesComputer( +func NewExecutionParametersComputer( + log zerolog.Logger, ctx Context, txnState storage.TransactionPreparer, -) MeterParamOverridesComputer { - return MeterParamOverridesComputer{ +) ExecutionParametersComputer { + return ExecutionParametersComputer{ + log: log, ctx: ctx, txnState: txnState, } } -func (computer MeterParamOverridesComputer) Compute( +func (computer ExecutionParametersComputer) Compute( _ state.NestedTransactionPreparer, _ struct{}, ) ( - derived.MeterParamOverrides, + derived.StateExecutionParameters, error, ) { - var overrides derived.MeterParamOverrides + var overrides derived.StateExecutionParameters var err error computer.txnState.RunWithAllLimitsDisabled(func() { - overrides, err = computer.getMeterParamOverrides() + overrides, err = computer.getExecutionParameters() }) if err != nil { @@ -132,8 +142,8 @@ func (computer MeterParamOverridesComputer) Compute( return overrides, nil } -func (computer MeterParamOverridesComputer) getMeterParamOverrides() ( - derived.MeterParamOverrides, +func (computer ExecutionParametersComputer) getExecutionParameters() ( + derived.StateExecutionParameters, error, ) { // Check that the service account exists because all the settings are @@ -147,7 +157,7 @@ func (computer MeterParamOverridesComputer) getMeterParamOverrides() ( computer.ctx.EnvironmentParams, computer.txnState) - overrides := derived.MeterParamOverrides{} + overrides := derived.StateExecutionParameters{} // set the property if no error, but if the error is a fatal error then // return it @@ -204,6 +214,15 @@ func (computer MeterParamOverridesComputer) getMeterParamOverrides() ( return overrides, err } + executionVersion, err := GetMinimumExecutionVersion(computer.log, env) + err = setIfOk( + "execution version", + err, + func() { overrides.ExecutionVersion = executionVersion }) + if err != nil { + return overrides, err + } + return overrides, nil } @@ -336,3 +355,35 @@ func GetExecutionMemoryLimit( return uint64(memoryLimitRaw), nil } + +func GetMinimumExecutionVersion( + log zerolog.Logger, + env environment.Environment, +) (semver.Version, error) { + + value, err := env.GetCurrentVersionBoundary() + + if err != nil { + return semver.Version{}, fmt.Errorf("could not get current version boundary: %w", err) + } + + boundary, err := convert.VersionBoundary(value) + + if err != nil { + return semver.Version{}, fmt.Errorf("could not parse current version boundary: %w", err) + } + + semVer, err := semver.NewVersion(boundary.Version) + if err != nil { + // This could be problematic, if the version is not a valid semver version. The NodeVersionBeacon should prevent + // this, but it could have bugs. + // Erroring here gives us no way to recover as no transactions would work anymore, + // instead return the version as 0.0.0 and log the error, allowing us to recover. + // this would mean that any if-statements that were relying on a higher version would fail, + // but that is preferable to all transactions halting. + log.Error().Err(err).Msg("could not parse version boundary. Version boundary as defined in the NodeVersionBeacon contract is not a valid semver version!") + return semver.Version{}, nil + } + + return *semVer, nil +} diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 5e014ca7811..65832fd48c5 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "encoding/hex" "fmt" + "github.com/rs/zerolog" "math" "strings" "testing" @@ -13,7 +14,6 @@ import ( "github.com/coreos/go-semver/semver" "github.com/onflow/flow-core-contracts/lib/go/templates" - "github.com/onflow/flow-go/cmd/util/ledger/util" "github.com/onflow/flow-go/fvm/storage" "github.com/onflow/flow-go/fvm/storage/state" @@ -3242,9 +3242,9 @@ func Test_MinimumRequiredVersion(t *testing.T) { chain := flow.Emulator.Chain() sc := systemcontracts.SystemContractsForChain(chain.ChainID()) - tracer := tracing.NewTracerSpan() + log := zerolog.New(zerolog.NewTestWriter(t)) - getVersion := func(snapshotTree snapshot.SnapshotTree) string { + getVersion := func(ctx fvm.Context, snapshotTree snapshot.SnapshotTree) string { blockDatabase := storage.NewBlockDatabase( snapshotTree, 0, @@ -3253,14 +3253,25 @@ func Test_MinimumRequiredVersion(t *testing.T) { txnState, err := blockDatabase.NewTransaction(0, state.DefaultParameters()) require.NoError(t, err) - envParams := environment.DefaultEnvironmentParams() - envParams.Chain = chain + executionParams, _, err := txnState.GetStateExecutionParameters( + txnState, + fvm.NewExecutionParametersComputer(log, ctx, txnState)) - mrv := environment.NewMinimumRequiredVersion(tracer, util.NopMeter{}, txnState, envParams) + // this will set the parameters to the txnState. + // this is done at the beginning of a transaction/script + txnId, err := txnState.BeginNestedTransactionWithMeterParams( + meter.ExecutionParameters{ + ExecutionVersion: executionParams.ExecutionVersion, + }) + + mrv := environment.NewMinimumRequiredVersion(txnState) v, err := mrv.MinimumRequiredVersion() require.NoError(t, err) + _, err = txnState.CommitNestedTransaction(txnId) + require.NoError(t, err) + return v } @@ -3360,7 +3371,7 @@ func Test_MinimumRequiredVersion(t *testing.T) { snapshotTree snapshot.SnapshotTree, ) { // default version is empty - require.Equal(t, semver.Version{}.String(), getVersion(snapshotTree)) + require.Equal(t, semver.Version{}.String(), getVersion(ctx, snapshotTree)) // define mapping for flow go version to cadence version flowVersion1 := semver.Version{ @@ -3410,28 +3421,28 @@ func Test_MinimumRequiredVersion(t *testing.T) { txIndex += 1 // so far no change: - require.Equal(t, semver.Version{}.String(), getVersion(snapshotTree)) + require.Equal(t, semver.Version{}.String(), getVersion(ctx, snapshotTree)) // system transaction needs to run to update the flowVersion on chain snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv1-1, ctx, snapshotTree, vm, txIndex) txIndex += 1 // no change: - require.Equal(t, semver.Version{}.String(), getVersion(snapshotTree)) + require.Equal(t, semver.Version{}.String(), getVersion(ctx, snapshotTree)) // system transaction needs to run to update the flowVersion on chain snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv1, ctx, snapshotTree, vm, txIndex) txIndex += 1 // switch to cadence version 1 - require.Equal(t, cadenceVersion1.String(), getVersion(snapshotTree)) + require.Equal(t, cadenceVersion1.String(), getVersion(ctx, snapshotTree)) // system transaction needs to run to update the flowVersion on chain snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv1+1, ctx, snapshotTree, vm, txIndex) txIndex += 1 // still cadence version 1 - require.Equal(t, cadenceVersion1.String(), getVersion(snapshotTree)) + require.Equal(t, cadenceVersion1.String(), getVersion(ctx, snapshotTree)) // insert version boundary 2 snapshotTree = insertVersionBoundary(flowVersion2, hv1+1, hv2, ctx, snapshotTree, vm, txIndex) @@ -3441,6 +3452,6 @@ func Test_MinimumRequiredVersion(t *testing.T) { snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv2, ctx, snapshotTree, vm, txIndex) // switch cadence version 2 - require.Equal(t, cadenceVersion2.String(), getVersion(snapshotTree)) + require.Equal(t, cadenceVersion2.String(), getVersion(ctx, snapshotTree)) })) } diff --git a/fvm/meter/meter.go b/fvm/meter/meter.go index 2654a03aaa7..6d6be4ffae6 100644 --- a/fvm/meter/meter.go +++ b/fvm/meter/meter.go @@ -1,5 +1,7 @@ package meter +import "github.com/coreos/go-semver/semver" + type MeterParameters struct { ComputationMeterParameters MemoryMeterParameters @@ -7,6 +9,11 @@ type MeterParameters struct { InteractionMeterParameters } +type ExecutionParameters struct { + MeterParameters + ExecutionVersion semver.Version +} + func DefaultParameters() MeterParameters { return MeterParameters{ ComputationMeterParameters: DefaultComputationMeterParameters(), diff --git a/fvm/script.go b/fvm/script.go index 9a5a2551f4d..4d8a86323b8 100644 --- a/fvm/script.go +++ b/fvm/script.go @@ -173,7 +173,8 @@ func (executor *scriptExecutor) Execute() error { } func (executor *scriptExecutor) execute() error { - meterParams, _, err := getBodyMeterParameters( + executionParams, _, err := getExecutionParameters( + executor.env.Logger(), executor.ctx, executor.proc, executor.txnState) @@ -182,7 +183,7 @@ func (executor *scriptExecutor) execute() error { } txnId, err := executor.txnState.BeginNestedTransactionWithMeterParams( - meterParams) + executionParams) if err != nil { return err } diff --git a/fvm/storage/derived/derived_block_data.go b/fvm/storage/derived/derived_block_data.go index 4c42e4ae882..bc0c44bfa7b 100644 --- a/fvm/storage/derived/derived_block_data.go +++ b/fvm/storage/derived/derived_block_data.go @@ -2,14 +2,12 @@ package derived import ( "fmt" - "github.com/onflow/cadence/common" "github.com/onflow/cadence/interpreter" "github.com/onflow/flow-go/fvm/storage/logical" "github.com/onflow/flow-go/fvm/storage/snapshot" "github.com/onflow/flow-go/fvm/storage/state" - "github.com/onflow/flow-go/model/flow" ) type DerivedTransactionPreparer interface { @@ -23,23 +21,16 @@ type DerivedTransactionPreparer interface { ) GetProgram(location common.AddressLocation) (*Program, bool) - GetMeterParamOverrides( + // GetStateExecutionParameters returns parameters needed for execution from the state. + GetStateExecutionParameters( txnState state.NestedTransactionPreparer, - getMeterParamOverrides ValueComputer[struct{}, MeterParamOverrides], + getMeterParamOverrides ValueComputer[struct{}, StateExecutionParameters], ) ( - MeterParamOverrides, + StateExecutionParameters, *snapshot.ExecutionSnapshot, error, ) - GetCurrentVersionBoundary( - txnState state.NestedTransactionPreparer, - getCurrentVersionBoundaryComputer ValueComputer[struct{}, flow.VersionBoundary], - ) ( - flow.VersionBoundary, - error, - ) - AddInvalidator(invalidator TransactionInvalidator) } @@ -54,9 +45,7 @@ type Program struct { type DerivedBlockData struct { programs *DerivedDataTable[common.AddressLocation, *Program] - meterParamOverrides *DerivedDataTable[struct{}, MeterParamOverrides] - - currentVersionBoundary *DerivedDataTable[struct{}, flow.VersionBoundary] + meterParamOverrides *DerivedDataTable[struct{}, StateExecutionParameters] } // DerivedTransactionData is the derived data scratch space for a single @@ -69,12 +58,7 @@ type DerivedTransactionData struct { // There's only a single entry in this table. For simplicity, we'll use // struct{} as the entry's key. - meterParamOverrides *TableTransaction[struct{}, MeterParamOverrides] - - // The currently effective version boundary - // There's only a single entry in this table. For simplicity, we'll use - // struct{} as the entry's key. - currentVersionBoundary *TableTransaction[struct{}, flow.VersionBoundary] + executionParameters *TableTransaction[struct{}, StateExecutionParameters] } func NewEmptyDerivedBlockData( @@ -87,11 +71,7 @@ func NewEmptyDerivedBlockData( ](initialSnapshotTime), meterParamOverrides: NewEmptyTable[ struct{}, - MeterParamOverrides, - ](initialSnapshotTime), - currentVersionBoundary: NewEmptyTable[ - struct{}, - flow.VersionBoundary, + StateExecutionParameters, ](initialSnapshotTime), } } @@ -106,14 +86,14 @@ func (block *DerivedBlockData) NewChildDerivedBlockData() *DerivedBlockData { func (block *DerivedBlockData) NewSnapshotReadDerivedTransactionData() *DerivedTransactionData { return &DerivedTransactionData{ programs: block.programs.NewSnapshotReadTableTransaction(), - meterParamOverrides: block.meterParamOverrides.NewSnapshotReadTableTransaction(), + executionParameters: block.meterParamOverrides.NewSnapshotReadTableTransaction(), } } func (block *DerivedBlockData) NewCachingSnapshotReadDerivedTransactionData() *DerivedTransactionData { return &DerivedTransactionData{ programs: block.programs.NewCachingSnapshotReadTableTransaction(), - meterParamOverrides: block.meterParamOverrides.NewCachingSnapshotReadTableTransaction(), + executionParameters: block.meterParamOverrides.NewCachingSnapshotReadTableTransaction(), } } @@ -138,17 +118,9 @@ func (block *DerivedBlockData) NewDerivedTransactionData( return nil, err } - txnCurrentVersionBoundary, err := block.currentVersionBoundary.NewTableTransaction( - snapshotTime, - executionTime) - if err != nil { - return nil, err - } - return &DerivedTransactionData{ - programs: txnPrograms, - meterParamOverrides: txnMeterParamOverrides, - currentVersionBoundary: txnCurrentVersionBoundary, + programs: txnPrograms, + executionParameters: txnMeterParamOverrides, }, nil } @@ -205,48 +177,31 @@ func (transaction *DerivedTransactionData) AddInvalidator( } transaction.programs.AddInvalidator(invalidator.ProgramInvalidator()) - transaction.meterParamOverrides.AddInvalidator( - invalidator.MeterParamOverridesInvalidator()) - transaction.currentVersionBoundary.AddInvalidator( - invalidator.CurrentVersionBoundaryInvalidator()) + transaction.executionParameters.AddInvalidator( + invalidator.ExecutionParametersInvalidator()) } -func (transaction *DerivedTransactionData) GetMeterParamOverrides( +func (transaction *DerivedTransactionData) GetStateExecutionParameters( txnState state.NestedTransactionPreparer, - getMeterParamOverrides ValueComputer[struct{}, MeterParamOverrides], + getMeterParamOverrides ValueComputer[struct{}, StateExecutionParameters], ) ( - MeterParamOverrides, + StateExecutionParameters, *snapshot.ExecutionSnapshot, error, ) { - return transaction.meterParamOverrides.GetWithStateOrCompute( + return transaction.executionParameters.GetWithStateOrCompute( txnState, struct{}{}, getMeterParamOverrides) } -func (transaction *DerivedTransactionData) GetCurrentVersionBoundary( - txnState state.NestedTransactionPreparer, - getCurrentVersionBoundaryComputer ValueComputer[struct{}, flow.VersionBoundary], -) ( - flow.VersionBoundary, - error, -) { - vb, _, err := transaction.currentVersionBoundary.GetWithStateOrCompute( - txnState, - struct{}{}, - getCurrentVersionBoundaryComputer) - - return vb, err -} - func (transaction *DerivedTransactionData) Validate() error { err := transaction.programs.Validate() if err != nil { return fmt.Errorf("programs validate failed: %w", err) } - err = transaction.meterParamOverrides.Validate() + err = transaction.executionParameters.Validate() if err != nil { return fmt.Errorf("meter param overrides validate failed: %w", err) } @@ -260,7 +215,7 @@ func (transaction *DerivedTransactionData) Commit() error { return fmt.Errorf("programs commit failed: %w", err) } - err = transaction.meterParamOverrides.Commit() + err = transaction.executionParameters.Commit() if err != nil { return fmt.Errorf("meter param overrides commit failed: %w", err) } diff --git a/fvm/storage/derived/invalidator.go b/fvm/storage/derived/invalidator.go index 9c99a02200f..9467c169bdc 100644 --- a/fvm/storage/derived/invalidator.go +++ b/fvm/storage/derived/invalidator.go @@ -1,10 +1,9 @@ package derived import ( + "github.com/coreos/go-semver/semver" "github.com/onflow/cadence/common" - "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/fvm/meter" ) @@ -14,23 +13,23 @@ type MeterParamOverrides struct { MemoryLimit *uint64 // nil indicates no override } +// StateExecutionParameters are parameters needed for execution defined in the execution state. +type StateExecutionParameters struct { + MeterParamOverrides + ExecutionVersion semver.Version +} + type ProgramInvalidator TableInvalidator[ common.AddressLocation, *Program, ] -type MeterParamOverridesInvalidator TableInvalidator[ - struct{}, - MeterParamOverrides, -] - -type CurrentVersionBoundaryInvalidator TableInvalidator[ +type ExecutionParametersInvalidator TableInvalidator[ struct{}, - flow.VersionBoundary, + StateExecutionParameters, ] type TransactionInvalidator interface { ProgramInvalidator() ProgramInvalidator - MeterParamOverridesInvalidator() MeterParamOverridesInvalidator - CurrentVersionBoundaryInvalidator() CurrentVersionBoundaryInvalidator + ExecutionParametersInvalidator() ExecutionParametersInvalidator } diff --git a/fvm/storage/state/execution_state.go b/fvm/storage/state/execution_state.go index ca67bb0a0c0..56503405135 100644 --- a/fvm/storage/state/execution_state.go +++ b/fvm/storage/state/execution_state.go @@ -2,6 +2,7 @@ package state import ( "fmt" + "github.com/coreos/go-semver/semver" "github.com/onflow/cadence/common" "github.com/onflow/crypto/hash" @@ -27,7 +28,8 @@ type ExecutionState struct { finalized bool *spockState - meter *meter.Meter + meter *meter.Meter + executionVersion semver.Version // NOTE: parent and child state shares the same limits controller *limitsController @@ -129,19 +131,20 @@ func NewExecutionStateWithSpockStateHasher( // NewChildWithMeterParams generates a new child state using the provide meter // parameters. func (state *ExecutionState) NewChildWithMeterParams( - params meter.MeterParameters, + params meter.ExecutionParameters, ) *ExecutionState { return &ExecutionState{ finalized: false, spockState: state.spockState.NewChild(), - meter: meter.NewMeter(params), + meter: meter.NewMeter(params.MeterParameters), + executionVersion: params.ExecutionVersion, limitsController: state.limitsController, } } // NewChild generates a new child state using the parent's meter parameters. func (state *ExecutionState) NewChild() *ExecutionState { - return state.NewChildWithMeterParams(state.meter.MeterParameters) + return state.NewChildWithMeterParams(state.ExecutionParameters()) } // InteractionUsed returns the amount of ledger interaction (total ledger byte read + total ledger byte written) @@ -337,6 +340,13 @@ func (state *ExecutionState) checkSize( return nil } +func (state *ExecutionState) ExecutionParameters() meter.ExecutionParameters { + return meter.ExecutionParameters{ + MeterParameters: state.meter.MeterParameters, + ExecutionVersion: state.executionVersion, + } +} + func (state *ExecutionState) readSetSize() int { return state.spockState.readSetSize() } diff --git a/fvm/storage/state/transaction_state.go b/fvm/storage/state/transaction_state.go index 4175a9d4941..21b210db0b4 100644 --- a/fvm/storage/state/transaction_state.go +++ b/fvm/storage/state/transaction_state.go @@ -44,6 +44,9 @@ type Meter interface { type NestedTransactionPreparer interface { Meter + // ExecutionParameters returns the execution parameters + ExecutionParameters() meter.ExecutionParameters + // NumNestedTransactions returns the number of uncommitted nested // transactions. Note that the main transaction is not considered a // nested transaction. @@ -83,7 +86,7 @@ type NestedTransactionPreparer interface { // the provided meter parameters. This returns error if the current nested // transaction is program restricted. BeginNestedTransactionWithMeterParams( - params meter.MeterParameters, + params meter.ExecutionParameters, ) ( NestedTransactionId, error, @@ -199,6 +202,10 @@ func (txnState *transactionState) current() nestedTransactionStackFrame { return txnState.nestedTransactions[txnState.NumNestedTransactions()] } +func (txnState *transactionState) ExecutionParameters() meter.ExecutionParameters { + return txnState.current().ExecutionParameters() +} + func (txnState *transactionState) NumNestedTransactions() int { return len(txnState.nestedTransactions) - 1 } @@ -266,7 +273,7 @@ func (txnState *transactionState) BeginNestedTransaction() ( } func (txnState *transactionState) BeginNestedTransactionWithMeterParams( - params meter.MeterParameters, + params meter.ExecutionParameters, ) ( NestedTransactionId, error, diff --git a/fvm/transactionInvoker.go b/fvm/transactionInvoker.go index bb5b44d2861..b7ab79c19be 100644 --- a/fvm/transactionInvoker.go +++ b/fvm/transactionInvoker.go @@ -4,8 +4,6 @@ import ( "fmt" "strconv" - "github.com/onflow/flow-go/fvm/systemcontracts" - "github.com/onflow/cadence/common" "github.com/onflow/cadence/runtime" "github.com/rs/zerolog" @@ -70,7 +68,7 @@ type transactionExecutor struct { // the state reads needed to compute the metering parameters // this is used to invalidate the metering parameters if a transaction // writes to any of those registers - meterStateRead *snapshot.ExecutionSnapshot + executionStateRead *snapshot.ExecutionSnapshot cadenceRuntime *reusableRuntime.ReusableCadenceRuntime txnBodyExecutor runtime.Executor @@ -201,27 +199,27 @@ func (executor *transactionExecutor) preprocessTransactionBody() error { return err } } - // get meter parameters - meterParams, meterStateRead, err := getBodyMeterParameters( + executionParameters, executionStateRead, err := getExecutionParameters( + executor.env.Logger(), executor.ctx, executor.proc, executor.txnState) if err != nil { - return fmt.Errorf("error getting meter parameters: %w", err) + return fmt.Errorf("error getting execution parameters: %w", err) } - if len(meterStateRead.WriteSet) != 0 { + if len(executionStateRead.WriteSet) != 0 { // this should never happen // and indicates an implementation error - panic("getting metering parameters should not write to registers") + panic("getting execution parameters should not write to registers") } - // we need to save the meter state read for invalidation purposes - executor.meterStateRead = meterStateRead + // we need to save the execution state read for invalidation purposes + executor.executionStateRead = executionStateRead txnId, err := executor.txnState.BeginNestedTransactionWithMeterParams( - meterParams) + executionParameters) if err != nil { return err } @@ -402,12 +400,10 @@ func (executor *transactionExecutor) normalExecution() ( return } - sc := systemcontracts.SystemContractsForChain(executor.ctx.Chain.ChainID()) invalidator = environment.NewDerivedDataInvalidator( contractUpdates, bodySnapshot, - executor.meterStateRead, - sc.FlowServiceAccount.Address, + executor.executionStateRead, ) // Check if all account storage limits are ok diff --git a/module/state_synchronization/indexer/util.go b/module/state_synchronization/indexer/util.go index b61ec1e65f5..5c52dba0c5c 100644 --- a/module/state_synchronization/indexer/util.go +++ b/module/state_synchronization/indexer/util.go @@ -100,7 +100,7 @@ func (inv *accessInvalidator) ProgramInvalidator() derived.ProgramInvalidator { return inv.programs } -func (inv *accessInvalidator) MeterParamOverridesInvalidator() derived.MeterParamOverridesInvalidator { +func (inv *accessInvalidator) ExecutionParametersInvalidator() derived.ExecutionParametersInvalidator { return inv.meterParamOverrides } @@ -126,9 +126,9 @@ func (inv *programInvalidator) ShouldInvalidateEntry(location common.AddressLoca return inv.invalidateAll || ok } -var _ derived.MeterParamOverridesInvalidator = (*meterParamOverridesInvalidator)(nil) +var _ derived.ExecutionParametersInvalidator = (*meterParamOverridesInvalidator)(nil) -// meterParamOverridesInvalidator is a derived.MeterParamOverridesInvalidator that invalidates meter param overrides. +// meterParamOverridesInvalidator is a derived.ExecutionParametersInvalidator that invalidates meter param overrides. type meterParamOverridesInvalidator struct { invalidateAll bool } From 4166e0a5e9f73a9e0287ae50b2e77177ec086539 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Wed, 23 Oct 2024 21:38:46 +0200 Subject: [PATCH 05/13] fix tests --- .../computation/computer/computer_test.go | 9 +++-- fvm/context.go | 9 +++++ fvm/environment/derived_data_invalidator.go | 2 -- .../derived_data_invalidator_test.go | 10 +++--- fvm/environment/mock/environment.go | 2 +- fvm/environment/programs_test.go | 8 +++-- fvm/executionParameters.go | 6 +++- fvm/storage/state/transaction_state_test.go | 8 +++-- fvm/transactionInvoker.go | 3 ++ .../indexer/indexer_core.go | 7 +--- module/state_synchronization/indexer/util.go | 35 +++++-------------- 11 files changed, 52 insertions(+), 47 deletions(-) diff --git a/engine/execution/computation/computer/computer_test.go b/engine/execution/computation/computer/computer_test.go index 4eade023273..0d7899100ad 100644 --- a/engine/execution/computation/computer/computer_test.go +++ b/engine/execution/computation/computer/computer_test.go @@ -705,6 +705,7 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) { }, ), ), + fvm.WithReadVersionFromNodeVersionBeacon(false), ) vm := fvm.NewVirtualMachine() @@ -816,7 +817,9 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) { runtime.Config{}, func(_ runtime.Config) runtime.Runtime { return rt - }))) + })), + fvm.WithReadVersionFromNodeVersionBeacon(false), + ) vm := fvm.NewVirtualMachine() @@ -929,7 +932,9 @@ func TestBlockExecutor_ExecuteBlock(t *testing.T) { runtime.Config{}, func(_ runtime.Config) runtime.Runtime { return rt - }))) + })), + fvm.WithReadVersionFromNodeVersionBeacon(false), + ) vm := fvm.NewVirtualMachine() diff --git a/fvm/context.go b/fvm/context.go index 4007f22286f..fd198633b54 100644 --- a/fvm/context.go +++ b/fvm/context.go @@ -392,3 +392,12 @@ func WithEVMTracer(tracer debug.EVMTracer) Option { return ctx } } + +// WithReadVersionFromNodeVersionBeacon sets whether the version from the node version beacon should be read +// this should only be disabled for testing +func WithReadVersionFromNodeVersionBeacon(enabled bool) Option { + return func(ctx Context) Context { + ctx.ReadVersionFromNodeVersionBeacon = enabled + return ctx + } +} diff --git a/fvm/environment/derived_data_invalidator.go b/fvm/environment/derived_data_invalidator.go index 77bb9270f17..754a1e37513 100644 --- a/fvm/environment/derived_data_invalidator.go +++ b/fvm/environment/derived_data_invalidator.go @@ -26,8 +26,6 @@ type DerivedDataInvalidator struct { ContractUpdates ExecutionParametersUpdated bool - - CurrentVersionBoundaryUpdated bool } var _ derived.TransactionInvalidator = DerivedDataInvalidator{} diff --git a/fvm/environment/derived_data_invalidator_test.go b/fvm/environment/derived_data_invalidator_test.go index 0abf50f644d..147c03fb57a 100644 --- a/fvm/environment/derived_data_invalidator_test.go +++ b/fvm/environment/derived_data_invalidator_test.go @@ -3,8 +3,6 @@ package environment_test import ( "testing" - "github.com/onflow/flow-go/fvm/systemcontracts" - "github.com/onflow/cadence/common" "github.com/stretchr/testify/require" @@ -214,7 +212,7 @@ func TestMeterParamOverridesInvalidator(t *testing.T) { require.False(t, invalidator.ShouldInvalidateEntries()) require.False(t, invalidator.ShouldInvalidateEntry( struct{}{}, - derived.MeterParamOverrides{}, + derived.StateExecutionParameters{}, nil)) invalidator = environment.DerivedDataInvalidator{ @@ -225,7 +223,7 @@ func TestMeterParamOverridesInvalidator(t *testing.T) { require.True(t, invalidator.ShouldInvalidateEntries()) require.True(t, invalidator.ShouldInvalidateEntry( struct{}{}, - derived.MeterParamOverrides{}, + derived.StateExecutionParameters{}, nil)) } @@ -268,6 +266,10 @@ func TestMeterParamOverridesUpdated(t *testing.T) { require.NoError(t, err) computer := fvm.NewExecutionParametersComputer( + ctx.Logger, + ctx, + txnState, + ) overrides, err := computer.Compute(txnState, struct{}{}) require.NoError(t, err) diff --git a/fvm/environment/mock/environment.go b/fvm/environment/mock/environment.go index 65ef356b4a3..445dcbd49d8 100644 --- a/fvm/environment/mock/environment.go +++ b/fvm/environment/mock/environment.go @@ -912,7 +912,7 @@ func (_m *Environment) GetCurrentVersionBoundary() (cadence.Value, error) { ret := _m.Called() if len(ret) == 0 { - panic("no return value specified for getMinimuRequiredVersion") + panic("no return value specified for GetCurrentVersionBoundary") } var r0 cadence.Value diff --git a/fvm/environment/programs_test.go b/fvm/environment/programs_test.go index e39d224c9d4..b0368db49e0 100644 --- a/fvm/environment/programs_test.go +++ b/fvm/environment/programs_test.go @@ -156,7 +156,9 @@ func Test_Programs(t *testing.T) { fvm.WithAuthorizationChecksEnabled(false), fvm.WithSequenceNumberCheckAndIncrementEnabled(false), fvm.WithCadenceLogging(true), - fvm.WithDerivedBlockData(derivedBlockData)) + fvm.WithDerivedBlockData(derivedBlockData), + // disable reading version from node version beacon otherwise it loads an extra contract + fvm.WithReadVersionFromNodeVersionBeacon(false)) var contractASnapshot *snapshot.ExecutionSnapshot var contractBSnapshot *snapshot.ExecutionSnapshot @@ -613,7 +615,9 @@ func Test_ProgramsDoubleCounting(t *testing.T) { fvm.WithSequenceNumberCheckAndIncrementEnabled(false), fvm.WithCadenceLogging(true), fvm.WithDerivedBlockData(derivedBlockData), - fvm.WithMetricsReporter(metrics)) + fvm.WithMetricsReporter(metrics), + // disable reading version from node version beacon otherwise it loads an extra contract + fvm.WithReadVersionFromNodeVersionBeacon(false)) t.Run("deploy contracts and ensure cache is empty", func(t *testing.T) { // deploy contract A diff --git a/fvm/executionParameters.go b/fvm/executionParameters.go index 8ec72ba9e8e..db769180db7 100644 --- a/fvm/executionParameters.go +++ b/fvm/executionParameters.go @@ -214,7 +214,7 @@ func (computer ExecutionParametersComputer) getExecutionParameters() ( return overrides, err } - executionVersion, err := GetMinimumExecutionVersion(computer.log, env) + executionVersion, err := GetMinimumExecutionVersion(computer.log, computer.ctx, env) err = setIfOk( "execution version", err, @@ -358,8 +358,12 @@ func GetExecutionMemoryLimit( func GetMinimumExecutionVersion( log zerolog.Logger, + ctx Context, env environment.Environment, ) (semver.Version, error) { + if !ctx.ReadVersionFromNodeVersionBeacon { + return semver.Version{}, nil + } value, err := env.GetCurrentVersionBoundary() diff --git a/fvm/storage/state/transaction_state_test.go b/fvm/storage/state/transaction_state_test.go index e41ff7951f9..c75d5a6cb52 100644 --- a/fvm/storage/state/transaction_state_test.go +++ b/fvm/storage/state/transaction_state_test.go @@ -104,7 +104,9 @@ func TestUnrestrictedNestedTransactionDifferentMeterParams(t *testing.T) { require.Equal(t, uint(math.MaxUint), mainState.TotalMemoryLimit()) id1, err := txn.BeginNestedTransactionWithMeterParams( - meter.DefaultParameters().WithMemoryLimit(1)) + meter.ExecutionParameters{ + MeterParameters: meter.DefaultParameters().WithMemoryLimit(1), + }) require.NoError(t, err) nestedState1 := id1.StateForTestingOnly() @@ -112,7 +114,9 @@ func TestUnrestrictedNestedTransactionDifferentMeterParams(t *testing.T) { require.Equal(t, uint(1), nestedState1.TotalMemoryLimit()) id2, err := txn.BeginNestedTransactionWithMeterParams( - meter.DefaultParameters().WithMemoryLimit(2)) + meter.ExecutionParameters{ + MeterParameters: meter.DefaultParameters().WithMemoryLimit(2), + }) require.NoError(t, err) nestedState2 := id2.StateForTestingOnly() diff --git a/fvm/transactionInvoker.go b/fvm/transactionInvoker.go index b7ab79c19be..6edf2ec5405 100644 --- a/fvm/transactionInvoker.go +++ b/fvm/transactionInvoker.go @@ -34,6 +34,8 @@ type TransactionExecutorParams struct { // Note: This is disabled only by tests TransactionBodyExecutionEnabled bool + + ReadVersionFromNodeVersionBeacon bool } func DefaultTransactionExecutorParams() TransactionExecutorParams { @@ -42,6 +44,7 @@ func DefaultTransactionExecutorParams() TransactionExecutorParams { SequenceNumberCheckAndIncrementEnabled: true, AccountKeyWeightThreshold: AccountKeyWeightThreshold, TransactionBodyExecutionEnabled: true, + ReadVersionFromNodeVersionBeacon: true, } } diff --git a/module/state_synchronization/indexer/indexer_core.go b/module/state_synchronization/indexer/indexer_core.go index 8e5bed0780e..95ddc4fb3b0 100644 --- a/module/state_synchronization/indexer/indexer_core.go +++ b/module/state_synchronization/indexer/indexer_core.go @@ -290,14 +290,9 @@ func (c *IndexerCore) updateProgramCache(header *flow.Header, events []flow.Even invalidated: updatedContracts, invalidateAll: hasAuthorizedTransaction(collections, c.serviceAddress), }, - meterParamOverrides: &meterParamOverridesInvalidator{ + executionParameters: &executionParametersInvalidator{ invalidateAll: hasAuthorizedTransaction(collections, c.serviceAddress), }, - currentVersionBoundary: ¤tVersionBoundaryInvalidator{ - invalidateAll: hasAuthorizedTransaction(collections, c.serviceAddress), - // this will eventually be different from invalidateAll - invalidate: hasAuthorizedTransaction(collections, c.serviceAddress), - }, }) err = tx.Commit() diff --git a/module/state_synchronization/indexer/util.go b/module/state_synchronization/indexer/util.go index 5c52dba0c5c..5526776716b 100644 --- a/module/state_synchronization/indexer/util.go +++ b/module/state_synchronization/indexer/util.go @@ -91,9 +91,8 @@ var _ derived.TransactionInvalidator = (*accessInvalidator)(nil) // accessInvalidator is a derived.TransactionInvalidator that invalidates programs and meter param overrides. type accessInvalidator struct { - programs *programInvalidator - meterParamOverrides *meterParamOverridesInvalidator - currentVersionBoundary *currentVersionBoundaryInvalidator + programs *programInvalidator + executionParameters *executionParametersInvalidator } func (inv *accessInvalidator) ProgramInvalidator() derived.ProgramInvalidator { @@ -101,11 +100,7 @@ func (inv *accessInvalidator) ProgramInvalidator() derived.ProgramInvalidator { } func (inv *accessInvalidator) ExecutionParametersInvalidator() derived.ExecutionParametersInvalidator { - return inv.meterParamOverrides -} - -func (inv *accessInvalidator) CurrentVersionBoundaryInvalidator() derived.CurrentVersionBoundaryInvalidator { - return inv.currentVersionBoundary + return inv.executionParameters } var _ derived.ProgramInvalidator = (*programInvalidator)(nil) @@ -126,31 +121,17 @@ func (inv *programInvalidator) ShouldInvalidateEntry(location common.AddressLoca return inv.invalidateAll || ok } -var _ derived.ExecutionParametersInvalidator = (*meterParamOverridesInvalidator)(nil) +var _ derived.ExecutionParametersInvalidator = (*executionParametersInvalidator)(nil) -// meterParamOverridesInvalidator is a derived.ExecutionParametersInvalidator that invalidates meter param overrides. -type meterParamOverridesInvalidator struct { +// executionParametersInvalidator is a derived.ExecutionParametersInvalidator that invalidates meter param overrides and execution version. +type executionParametersInvalidator struct { invalidateAll bool } -func (inv *meterParamOverridesInvalidator) ShouldInvalidateEntries() bool { +func (inv *executionParametersInvalidator) ShouldInvalidateEntries() bool { return inv.invalidateAll } -func (inv *meterParamOverridesInvalidator) ShouldInvalidateEntry(_ struct{}, _ derived.MeterParamOverrides, _ *snapshot.ExecutionSnapshot) bool { +func (inv *executionParametersInvalidator) ShouldInvalidateEntry(_ struct{}, _ derived.StateExecutionParameters, _ *snapshot.ExecutionSnapshot) bool { return inv.invalidateAll } - -// currentVersionBoundaryInvalidator is a derived.CurrentVersionBoundaryInvalidator that invalidates current version boundary. -type currentVersionBoundaryInvalidator struct { - invalidateAll bool - invalidate bool -} - -func (inv *currentVersionBoundaryInvalidator) ShouldInvalidateEntries() bool { - return inv.invalidate || inv.invalidateAll -} - -func (inv *currentVersionBoundaryInvalidator) ShouldInvalidateEntry(_ struct{}, _ flow.VersionBoundary, _ *snapshot.ExecutionSnapshot) bool { - return inv.invalidate || inv.invalidateAll -} From b2fa1c3afd35b41cf322c24f58cdd5edfbb7d2cd Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Wed, 23 Oct 2024 21:41:15 +0200 Subject: [PATCH 06/13] fix lint --- fvm/executionParameters.go | 6 ++++-- fvm/fvm_test.go | 5 ++++- fvm/storage/derived/derived_block_data.go | 1 + fvm/storage/state/execution_state.go | 1 + 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/fvm/executionParameters.go b/fvm/executionParameters.go index db769180db7..ecda89aaff1 100644 --- a/fvm/executionParameters.go +++ b/fvm/executionParameters.go @@ -3,10 +3,12 @@ package fvm import ( "context" "fmt" + "math" + "github.com/coreos/go-semver/semver" - "github.com/onflow/flow-go/model/convert" "github.com/rs/zerolog" - "math" + + "github.com/onflow/flow-go/model/convert" "github.com/onflow/flow-go/fvm/storage/snapshot" diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 65832fd48c5..c100ceb77de 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -5,12 +5,13 @@ import ( "crypto/rand" "encoding/hex" "fmt" - "github.com/rs/zerolog" "math" "strings" "testing" "time" + "github.com/rs/zerolog" + "github.com/coreos/go-semver/semver" "github.com/onflow/flow-core-contracts/lib/go/templates" @@ -3256,6 +3257,7 @@ func Test_MinimumRequiredVersion(t *testing.T) { executionParams, _, err := txnState.GetStateExecutionParameters( txnState, fvm.NewExecutionParametersComputer(log, ctx, txnState)) + require.NoError(t, err) // this will set the parameters to the txnState. // this is done at the beginning of a transaction/script @@ -3263,6 +3265,7 @@ func Test_MinimumRequiredVersion(t *testing.T) { meter.ExecutionParameters{ ExecutionVersion: executionParams.ExecutionVersion, }) + require.NoError(t, err) mrv := environment.NewMinimumRequiredVersion(txnState) diff --git a/fvm/storage/derived/derived_block_data.go b/fvm/storage/derived/derived_block_data.go index bc0c44bfa7b..14deca315b7 100644 --- a/fvm/storage/derived/derived_block_data.go +++ b/fvm/storage/derived/derived_block_data.go @@ -2,6 +2,7 @@ package derived import ( "fmt" + "github.com/onflow/cadence/common" "github.com/onflow/cadence/interpreter" diff --git a/fvm/storage/state/execution_state.go b/fvm/storage/state/execution_state.go index 56503405135..a3ac6f416e9 100644 --- a/fvm/storage/state/execution_state.go +++ b/fvm/storage/state/execution_state.go @@ -2,6 +2,7 @@ package state import ( "fmt" + "github.com/coreos/go-semver/semver" "github.com/onflow/cadence/common" From 2109f645c46f0444eba5596e19bc7a99e46a134a Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Wed, 23 Oct 2024 22:04:30 +0200 Subject: [PATCH 07/13] cleanup --- fvm/environment/meter.go | 1 - fvm/executionParameters.go | 6 +++--- fvm/fvm_test.go | 2 +- fvm/meter/meter.go | 7 ------- fvm/storage/state/execution_state.go | 11 ++++++++--- fvm/storage/state/transaction_state.go | 8 ++++---- fvm/storage/state/transaction_state_test.go | 4 ++-- module/trace/constants.go | 1 - 8 files changed, 18 insertions(+), 22 deletions(-) diff --git a/fvm/environment/meter.go b/fvm/environment/meter.go index 3576c98263e..00a70df2d30 100644 --- a/fvm/environment/meter.go +++ b/fvm/environment/meter.go @@ -55,7 +55,6 @@ const ( _ ComputationKindEVMEncodeABI ComputationKindEVMDecodeABI - ComputationKindMinimumRequiredVersion ) // MainnetExecutionEffortWeights are the execution effort weights as they are diff --git a/fvm/executionParameters.go b/fvm/executionParameters.go index ecda89aaff1..9155e96e2bb 100644 --- a/fvm/executionParameters.go +++ b/fvm/executionParameters.go @@ -65,14 +65,14 @@ func getExecutionParameters( ctx Context, proc Procedure, txnState storage.TransactionPreparer, -) (meter.ExecutionParameters, *snapshot.ExecutionSnapshot, error) { +) (state.ExecutionParameters, *snapshot.ExecutionSnapshot, error) { meterParams := getBasicMeterParameters(ctx, proc) executionParams, executionParamsStateRead, err := txnState.GetStateExecutionParameters( txnState, NewExecutionParametersComputer(log, ctx, txnState)) if err != nil { - return meter.ExecutionParameters{ + return state.ExecutionParameters{ MeterParameters: meterParams, ExecutionVersion: semver.Version{}, }, nil, err @@ -98,7 +98,7 @@ func getExecutionParameters( WithStorageInteractionLimit(math.MaxUint64) } - return meter.ExecutionParameters{ + return state.ExecutionParameters{ MeterParameters: meterParams, ExecutionVersion: executionParams.ExecutionVersion, }, executionParamsStateRead, nil diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index c100ceb77de..55d4d44bb92 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -3262,7 +3262,7 @@ func Test_MinimumRequiredVersion(t *testing.T) { // this will set the parameters to the txnState. // this is done at the beginning of a transaction/script txnId, err := txnState.BeginNestedTransactionWithMeterParams( - meter.ExecutionParameters{ + state.ExecutionParameters{ ExecutionVersion: executionParams.ExecutionVersion, }) require.NoError(t, err) diff --git a/fvm/meter/meter.go b/fvm/meter/meter.go index 6d6be4ffae6..2654a03aaa7 100644 --- a/fvm/meter/meter.go +++ b/fvm/meter/meter.go @@ -1,7 +1,5 @@ package meter -import "github.com/coreos/go-semver/semver" - type MeterParameters struct { ComputationMeterParameters MemoryMeterParameters @@ -9,11 +7,6 @@ type MeterParameters struct { InteractionMeterParameters } -type ExecutionParameters struct { - MeterParameters - ExecutionVersion semver.Version -} - func DefaultParameters() MeterParameters { return MeterParameters{ ComputationMeterParameters: DefaultComputationMeterParameters(), diff --git a/fvm/storage/state/execution_state.go b/fvm/storage/state/execution_state.go index a3ac6f416e9..c9bde5d4190 100644 --- a/fvm/storage/state/execution_state.go +++ b/fvm/storage/state/execution_state.go @@ -43,6 +43,11 @@ type StateParameters struct { maxValueSizeAllowed uint64 } +type ExecutionParameters struct { + meter.MeterParameters + ExecutionVersion semver.Version +} + func DefaultParameters() StateParameters { return StateParameters{ MeterParameters: meter.DefaultParameters(), @@ -132,7 +137,7 @@ func NewExecutionStateWithSpockStateHasher( // NewChildWithMeterParams generates a new child state using the provide meter // parameters. func (state *ExecutionState) NewChildWithMeterParams( - params meter.ExecutionParameters, + params ExecutionParameters, ) *ExecutionState { return &ExecutionState{ finalized: false, @@ -341,8 +346,8 @@ func (state *ExecutionState) checkSize( return nil } -func (state *ExecutionState) ExecutionParameters() meter.ExecutionParameters { - return meter.ExecutionParameters{ +func (state *ExecutionState) ExecutionParameters() ExecutionParameters { + return ExecutionParameters{ MeterParameters: state.meter.MeterParameters, ExecutionVersion: state.executionVersion, } diff --git a/fvm/storage/state/transaction_state.go b/fvm/storage/state/transaction_state.go index 21b210db0b4..00f2ecce448 100644 --- a/fvm/storage/state/transaction_state.go +++ b/fvm/storage/state/transaction_state.go @@ -45,7 +45,7 @@ type NestedTransactionPreparer interface { Meter // ExecutionParameters returns the execution parameters - ExecutionParameters() meter.ExecutionParameters + ExecutionParameters() ExecutionParameters // NumNestedTransactions returns the number of uncommitted nested // transactions. Note that the main transaction is not considered a @@ -86,7 +86,7 @@ type NestedTransactionPreparer interface { // the provided meter parameters. This returns error if the current nested // transaction is program restricted. BeginNestedTransactionWithMeterParams( - params meter.ExecutionParameters, + params ExecutionParameters, ) ( NestedTransactionId, error, @@ -202,7 +202,7 @@ func (txnState *transactionState) current() nestedTransactionStackFrame { return txnState.nestedTransactions[txnState.NumNestedTransactions()] } -func (txnState *transactionState) ExecutionParameters() meter.ExecutionParameters { +func (txnState *transactionState) ExecutionParameters() ExecutionParameters { return txnState.current().ExecutionParameters() } @@ -273,7 +273,7 @@ func (txnState *transactionState) BeginNestedTransaction() ( } func (txnState *transactionState) BeginNestedTransactionWithMeterParams( - params meter.ExecutionParameters, + params ExecutionParameters, ) ( NestedTransactionId, error, diff --git a/fvm/storage/state/transaction_state_test.go b/fvm/storage/state/transaction_state_test.go index c75d5a6cb52..f8b18b5d186 100644 --- a/fvm/storage/state/transaction_state_test.go +++ b/fvm/storage/state/transaction_state_test.go @@ -104,7 +104,7 @@ func TestUnrestrictedNestedTransactionDifferentMeterParams(t *testing.T) { require.Equal(t, uint(math.MaxUint), mainState.TotalMemoryLimit()) id1, err := txn.BeginNestedTransactionWithMeterParams( - meter.ExecutionParameters{ + state.ExecutionParameters{ MeterParameters: meter.DefaultParameters().WithMemoryLimit(1), }) require.NoError(t, err) @@ -114,7 +114,7 @@ func TestUnrestrictedNestedTransactionDifferentMeterParams(t *testing.T) { require.Equal(t, uint(1), nestedState1.TotalMemoryLimit()) id2, err := txn.BeginNestedTransactionWithMeterParams( - meter.ExecutionParameters{ + state.ExecutionParameters{ MeterParameters: meter.DefaultParameters().WithMemoryLimit(2), }) require.NoError(t, err) diff --git a/module/trace/constants.go b/module/trace/constants.go index f011c1064b0..1241ce765a0 100644 --- a/module/trace/constants.go +++ b/module/trace/constants.go @@ -183,7 +183,6 @@ const ( FVMEnvGetBlockAtHeight SpanName = "fvm.env.getBlockAtHeight" FVMEnvRandom SpanName = "fvm.env.unsafeRandom" FVMEnvRandomSourceHistoryProvider SpanName = "fvm.env.randomSourceHistoryProvider" - FVMEnvMinimumRequiredVersion SpanName = "fvm.env.minimumRequiredVersion" FVMEnvCreateAccount SpanName = "fvm.env.createAccount" FVMEnvAddAccountKey SpanName = "fvm.env.addAccountKey" FVMEnvAccountKeysCount SpanName = "fvm.env.accountKeysCount" From a72cd5290e385656c8a1de39248a140bbb064ed9 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Thu, 24 Oct 2024 15:58:11 +0200 Subject: [PATCH 08/13] address review comments --- fvm/environment/env.go | 4 + fvm/environment/minimum_required_version.go | 11 +- .../minimum_required_version_test.go | 141 ++++++++++++++++++ fvm/environment/system_contracts.go | 4 + fvm/executionParameters.go | 5 +- model/convert/service_event.go | 1 + 6 files changed, 158 insertions(+), 8 deletions(-) create mode 100644 fvm/environment/minimum_required_version_test.go diff --git a/fvm/environment/env.go b/fvm/environment/env.go index 06c2d9f01ea..e23e9c64deb 100644 --- a/fvm/environment/env.go +++ b/fvm/environment/env.go @@ -62,6 +62,10 @@ type Environment interface { error, ) + // GetCurrentVersionBoundary executes the getCurrentVersionBoundary function on the NodeVersionBeacon contract. + // the function will return the version boundary (version, block height) that is currently in effect. + // the version boundary currently in effect is the highest one not above the current block height. + // if there is no existing version boundary lower than the current block height, the function will return version 0 and block height 0. GetCurrentVersionBoundary() (cadence.Value, error) // AccountInfo diff --git a/fvm/environment/minimum_required_version.go b/fvm/environment/minimum_required_version.go index f4b269c39b8..eac3a299cea 100644 --- a/fvm/environment/minimum_required_version.go +++ b/fvm/environment/minimum_required_version.go @@ -27,12 +27,12 @@ func NewMinimumRequiredVersion( func (c minimumRequiredVersion) MinimumRequiredVersion() (string, error) { executionParameters := c.txnPreparer.ExecutionParameters() - cadenceVersion := mapToCadenceVersion(executionParameters.ExecutionVersion) + cadenceVersion := mapToCadenceVersion(executionParameters.ExecutionVersion, fvmToCadenceVersionMapping) return cadenceVersion.String(), nil } -func mapToCadenceVersion(version semver.Version) semver.Version { +func mapToCadenceVersion(flowGoVersion semver.Version, versionMapping []VersionMapEntry) semver.Version { // return 0.0.0 if there is no mapping for the version var cadenceVersion = semver.Version{} @@ -40,8 +40,8 @@ func mapToCadenceVersion(version semver.Version) semver.Version { return version.Compare(versionToCompare) >= 0 } - for _, entry := range fvmToCadenceVersionMapping { - if greaterThanOrEqualTo(version, entry.FlowGoVersion) { + for _, entry := range versionMapping { + if greaterThanOrEqualTo(flowGoVersion, entry.FlowGoVersion) { cadenceVersion = entry.CadenceVersion } else { break @@ -59,8 +59,7 @@ type VersionMapEntry struct { // FVMToCadenceVersionMapping is a hardcoded mapping between FVM versions and Cadence versions. // Entries are only needed for cadence versions where cadence intends to switch behaviour // based on the version. -// This should be ordered. - +// This should be ordered in ascending order by FlowGoVersion. var fvmToCadenceVersionMapping = []VersionMapEntry{ // Leaving this example in, so it's easier to understand //{ diff --git a/fvm/environment/minimum_required_version_test.go b/fvm/environment/minimum_required_version_test.go new file mode 100644 index 00000000000..e0b0c0950b1 --- /dev/null +++ b/fvm/environment/minimum_required_version_test.go @@ -0,0 +1,141 @@ +package environment + +import ( + "testing" + + "github.com/coreos/go-semver/semver" + "github.com/stretchr/testify/require" +) + +func Test_MapToCadenceVersion(t *testing.T) { + v0 := semver.Version{} + flowV1 := semver.Version{ + Major: 1, + Minor: 2, + Patch: 3, + PreRelease: "rc.1", + } + flowV2 := semver.Version{ + Major: 2, + Minor: 2, + Patch: 3, + PreRelease: "rc.1", + } + + cadenceV1 := semver.Version{ + Major: 2, + Minor: 1, + Patch: 3, + PreRelease: "rc.2", + } + cadenceV2 := semver.Version{ + Major: 12, + Minor: 0, + Patch: 0, + PreRelease: "", + } + + mapping := []VersionMapEntry{ + { + FlowGoVersion: flowV1, + CadenceVersion: cadenceV1, + }, + { + FlowGoVersion: flowV2, + CadenceVersion: cadenceV2, + }, + } + + mappingWith2Versions := []VersionMapEntry{ + { + FlowGoVersion: flowV1, + CadenceVersion: cadenceV1, + }, + { + FlowGoVersion: flowV2, + CadenceVersion: cadenceV2, + }, + } + + t.Run("no mapping, v0", func(t *testing.T) { + version := mapToCadenceVersion(v0, nil) + + require.Equal(t, v0, version) + }) + + t.Run("v0", func(t *testing.T) { + version := mapToCadenceVersion(v0, mappingWith2Versions) + + require.Equal(t, semver.Version{}, version) + }) + t.Run("v1 - delta", func(t *testing.T) { + + v := flowV1 + v.Patch -= 1 + + version := mapToCadenceVersion(v, mappingWith2Versions) + + require.Equal(t, v0, version) + }) + t.Run("v1", func(t *testing.T) { + version := mapToCadenceVersion(flowV1, mappingWith2Versions) + + require.Equal(t, cadenceV1, version) + }) + t.Run("v1 + delta", func(t *testing.T) { + + v := flowV1 + v.BumpPatch() + + version := mapToCadenceVersion(v, mappingWith2Versions) + + require.Equal(t, cadenceV1, version) + }) + t.Run("v2 - delta", func(t *testing.T) { + + v := flowV2 + v.Patch -= 1 + + version := mapToCadenceVersion(v, mappingWith2Versions) + + require.Equal(t, cadenceV1, version) + }) + t.Run("v2", func(t *testing.T) { + version := mapToCadenceVersion(flowV2, mappingWith2Versions) + + require.Equal(t, cadenceV2, version) + }) + t.Run("v2 + delta", func(t *testing.T) { + + v := flowV2 + v.BumpPatch() + + version := mapToCadenceVersion(v, mappingWith2Versions) + + require.Equal(t, cadenceV2, version) + }) + + t.Run("v1 - delta, single version in mapping", func(t *testing.T) { + + v := flowV1 + v.Patch -= 1 + + version := mapToCadenceVersion(v, mapping) + + require.Equal(t, v0, version) + }) + t.Run("v1, single version in mapping", func(t *testing.T) { + version := mapToCadenceVersion(flowV1, mapping) + + require.Equal(t, cadenceV1, version) + }) + t.Run("v1 + delta, single version in mapping", func(t *testing.T) { + + v := flowV1 + v.BumpPatch() + + version := mapToCadenceVersion(v, mapping) + + require.Equal(t, cadenceV1, version) + }) +} diff --git a/fvm/environment/system_contracts.go b/fvm/environment/system_contracts.go index d17450fee20..e8e9598aa3c 100644 --- a/fvm/environment/system_contracts.go +++ b/fvm/environment/system_contracts.go @@ -320,6 +320,10 @@ var getCurrentVersionBoundarySpec = ContractFunctionSpec{ ArgumentTypes: []sema.Type{}, } +// GetCurrentVersionBoundary executes the getCurrentVersionBoundary function on the NodeVersionBeacon contract. +// the function will return the version boundary (version, block height) that is currently in effect. +// the version boundary currently in effect is the highest one not above the current block height. +// if there is no existing version boundary lower than the current block height, the function will return version 0 and block height 0. func (sys *SystemContracts) GetCurrentVersionBoundary() (cadence.Value, error) { return sys.Invoke( getCurrentVersionBoundarySpec, diff --git a/fvm/executionParameters.go b/fvm/executionParameters.go index 9155e96e2bb..db88f42ef5f 100644 --- a/fvm/executionParameters.go +++ b/fvm/executionParameters.go @@ -216,7 +216,7 @@ func (computer ExecutionParametersComputer) getExecutionParameters() ( return overrides, err } - executionVersion, err := GetMinimumExecutionVersion(computer.log, computer.ctx, env) + executionVersion, err := GetMinimumRequiredExecutionVersion(computer.log, computer.ctx, env) err = setIfOk( "execution version", err, @@ -358,7 +358,7 @@ func GetExecutionMemoryLimit( return uint64(memoryLimitRaw), nil } -func GetMinimumExecutionVersion( +func GetMinimumRequiredExecutionVersion( log zerolog.Logger, ctx Context, env environment.Environment, @@ -367,6 +367,7 @@ func GetMinimumExecutionVersion( return semver.Version{}, nil } + // the current version boundary defines a block height and a minimum required version that is required past that block height. value, err := env.GetCurrentVersionBoundary() if err != nil { diff --git a/model/convert/service_event.go b/model/convert/service_event.go index dfe0a4d1e83..7482f2ecc6f 100644 --- a/model/convert/service_event.go +++ b/model/convert/service_event.go @@ -1183,6 +1183,7 @@ func convertVersionBoundaries(array cadence.Array) ( return boundaries, nil } +// VersionBoundary decodes a single version boundary from the given Cadence value. func VersionBoundary(value cadence.Value) ( flow.VersionBoundary, error, From 2ccf37086492e1922a99c1813d56335e7ef918fa Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Fri, 25 Oct 2024 14:34:05 +0200 Subject: [PATCH 09/13] add more comments --- fvm/environment/minimum_required_version.go | 41 ++++++++++++------ .../minimum_required_version_test.go | 42 +++++++++---------- 2 files changed, 48 insertions(+), 35 deletions(-) diff --git a/fvm/environment/minimum_required_version.go b/fvm/environment/minimum_required_version.go index eac3a299cea..f6cd86ddf11 100644 --- a/fvm/environment/minimum_required_version.go +++ b/fvm/environment/minimum_required_version.go @@ -27,28 +27,45 @@ func NewMinimumRequiredVersion( func (c minimumRequiredVersion) MinimumRequiredVersion() (string, error) { executionParameters := c.txnPreparer.ExecutionParameters() - cadenceVersion := mapToCadenceVersion(executionParameters.ExecutionVersion, fvmToCadenceVersionMapping) + // map the minimum required flow-go version to a minimum required cadence version + cadenceVersion := mapToCadenceVersion(executionParameters.ExecutionVersion, minimumFvmToMinimumCadenceVersionMapping) return cadenceVersion.String(), nil } +// mapToCadenceVersion finds the entry in the versionMapping with the flow version that is closest to, +// but not higher the given flowGoVersion than returns the cadence version for that entry. +// the versionMapping is assumed to be sorted by flowGoVersion. func mapToCadenceVersion(flowGoVersion semver.Version, versionMapping []VersionMapEntry) semver.Version { // return 0.0.0 if there is no mapping for the version - var cadenceVersion = semver.Version{} - - greaterThanOrEqualTo := func(version semver.Version, versionToCompare semver.Version) bool { - return version.Compare(versionToCompare) >= 0 - } - + var closestEntry = VersionMapEntry{} + + // example setup: flowGoVersion = 2.1 + // versionMapping = [ + // {FlowGoVersion: 1.0, CadenceVersion: 1.1}, + // {FlowGoVersion: 2.0, CadenceVersion: 2.1}, + // {FlowGoVersion: 2.1, CadenceVersion: 2.2}, + // {FlowGoVersion: 3.0, CadenceVersion: 3.1}, + // {FlowGoVersion: 4.0, CadenceVersion: 4.1}, + // ] for _, entry := range versionMapping { - if greaterThanOrEqualTo(flowGoVersion, entry.FlowGoVersion) { - cadenceVersion = entry.CadenceVersion + // loop 1: 2.1 >= 1.0 closest entry is 0 + // loop 2: 2.1 >= 2.0 closest entry is 1 + // loop 3: 2.1 >= 2.1 closest entry is 2 + // loop 4: 2.1 < 3.0 we went too high: closest entry is 2 break + if versionGreaterThanOrEqualTo(flowGoVersion, entry.FlowGoVersion) { + closestEntry = entry } else { break } } - return cadenceVersion + // return the cadence version for the closest entry (2): 2.2 + return closestEntry.CadenceVersion +} + +func versionGreaterThanOrEqualTo(version semver.Version, other semver.Version) bool { + return version.Compare(other) >= 0 } type VersionMapEntry struct { @@ -60,7 +77,7 @@ type VersionMapEntry struct { // Entries are only needed for cadence versions where cadence intends to switch behaviour // based on the version. // This should be ordered in ascending order by FlowGoVersion. -var fvmToCadenceVersionMapping = []VersionMapEntry{ +var minimumFvmToMinimumCadenceVersionMapping = []VersionMapEntry{ // Leaving this example in, so it's easier to understand //{ // FlowGoVersion: *semver.New("0.37.0"), @@ -69,7 +86,7 @@ var fvmToCadenceVersionMapping = []VersionMapEntry{ } func SetFVMToCadenceVersionMappingForTestingOnly(mapping []VersionMapEntry) { - fvmToCadenceVersionMapping = mapping + minimumFvmToMinimumCadenceVersionMapping = mapping } var _ MinimumRequiredVersion = (*minimumRequiredVersion)(nil) diff --git a/fvm/environment/minimum_required_version_test.go b/fvm/environment/minimum_required_version_test.go index e0b0c0950b1..7860da46ea8 100644 --- a/fvm/environment/minimum_required_version_test.go +++ b/fvm/environment/minimum_required_version_test.go @@ -8,31 +8,27 @@ import ( ) func Test_MapToCadenceVersion(t *testing.T) { - v0 := semver.Version{} + flowV0 := semver.Version{} + cadenceV0 := semver.Version{} flowV1 := semver.Version{ - Major: 1, - Minor: 2, - Patch: 3, - PreRelease: "rc.1", + Major: 0, + Minor: 37, + Patch: 0, } flowV2 := semver.Version{ - Major: 2, - Minor: 2, - Patch: 3, - PreRelease: "rc.1", + Major: 0, + Minor: 37, + Patch: 30, } - cadenceV1 := semver.Version{ - Major: 2, - Minor: 1, - Patch: 3, - PreRelease: "rc.2", + Major: 1, + Minor: 0, + Patch: 0, } cadenceV2 := semver.Version{ - Major: 12, - Minor: 0, - Patch: 0, - PreRelease: "", + Major: 1, + Minor: 1, + Patch: 0, } mapping := []VersionMapEntry{ @@ -58,13 +54,13 @@ func Test_MapToCadenceVersion(t *testing.T) { } t.Run("no mapping, v0", func(t *testing.T) { - version := mapToCadenceVersion(v0, nil) + version := mapToCadenceVersion(flowV0, nil) - require.Equal(t, v0, version) + require.Equal(t, cadenceV0, version) }) t.Run("v0", func(t *testing.T) { - version := mapToCadenceVersion(v0, mappingWith2Versions) + version := mapToCadenceVersion(flowV0, mappingWith2Versions) require.Equal(t, semver.Version{}, version) }) @@ -75,7 +71,7 @@ func Test_MapToCadenceVersion(t *testing.T) { version := mapToCadenceVersion(v, mappingWith2Versions) - require.Equal(t, v0, version) + require.Equal(t, cadenceV0, version) }) t.Run("v1", func(t *testing.T) { version := mapToCadenceVersion(flowV1, mappingWith2Versions) @@ -122,7 +118,7 @@ func Test_MapToCadenceVersion(t *testing.T) { version := mapToCadenceVersion(v, mapping) - require.Equal(t, v0, version) + require.Equal(t, cadenceV0, version) }) t.Run("v1, single version in mapping", func(t *testing.T) { version := mapToCadenceVersion(flowV1, mapping) From 9c476677de5a1ad16e030d89aa40f6c4a6aa3299 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Fri, 25 Oct 2024 19:28:45 +0200 Subject: [PATCH 10/13] swith to single mapped version --- fvm/environment/minimum_required_version.go | 49 ++++----------- .../minimum_required_version_test.go | 63 ++++--------------- fvm/fvm_test.go | 36 +---------- 3 files changed, 26 insertions(+), 122 deletions(-) diff --git a/fvm/environment/minimum_required_version.go b/fvm/environment/minimum_required_version.go index f6cd86ddf11..2085072562a 100644 --- a/fvm/environment/minimum_required_version.go +++ b/fvm/environment/minimum_required_version.go @@ -33,59 +33,34 @@ func (c minimumRequiredVersion) MinimumRequiredVersion() (string, error) { return cadenceVersion.String(), nil } -// mapToCadenceVersion finds the entry in the versionMapping with the flow version that is closest to, -// but not higher the given flowGoVersion than returns the cadence version for that entry. -// the versionMapping is assumed to be sorted by flowGoVersion. -func mapToCadenceVersion(flowGoVersion semver.Version, versionMapping []VersionMapEntry) semver.Version { - // return 0.0.0 if there is no mapping for the version - var closestEntry = VersionMapEntry{} - - // example setup: flowGoVersion = 2.1 - // versionMapping = [ - // {FlowGoVersion: 1.0, CadenceVersion: 1.1}, - // {FlowGoVersion: 2.0, CadenceVersion: 2.1}, - // {FlowGoVersion: 2.1, CadenceVersion: 2.2}, - // {FlowGoVersion: 3.0, CadenceVersion: 3.1}, - // {FlowGoVersion: 4.0, CadenceVersion: 4.1}, - // ] - for _, entry := range versionMapping { - // loop 1: 2.1 >= 1.0 closest entry is 0 - // loop 2: 2.1 >= 2.0 closest entry is 1 - // loop 3: 2.1 >= 2.1 closest entry is 2 - // loop 4: 2.1 < 3.0 we went too high: closest entry is 2 break - if versionGreaterThanOrEqualTo(flowGoVersion, entry.FlowGoVersion) { - closestEntry = entry - } else { - break - } +func mapToCadenceVersion(flowGoVersion semver.Version, versionMapping FlowGoToCadenceVersionMapping) semver.Version { + if versionGreaterThanOrEqualTo(flowGoVersion, versionMapping.FlowGoVersion) { + return versionMapping.CadenceVersion + } else { + return semver.Version{} } - - // return the cadence version for the closest entry (2): 2.2 - return closestEntry.CadenceVersion } func versionGreaterThanOrEqualTo(version semver.Version, other semver.Version) bool { return version.Compare(other) >= 0 } -type VersionMapEntry struct { +type FlowGoToCadenceVersionMapping struct { FlowGoVersion semver.Version CadenceVersion semver.Version } -// FVMToCadenceVersionMapping is a hardcoded mapping between FVM versions and Cadence versions. -// Entries are only needed for cadence versions where cadence intends to switch behaviour -// based on the version. -// This should be ordered in ascending order by FlowGoVersion. -var minimumFvmToMinimumCadenceVersionMapping = []VersionMapEntry{ +// This could also be a map, but ist not needed because we only expect one entry at a give time +// we won't be fixing 2 separate issues at 2 separate version with one deploy. +var minimumFvmToMinimumCadenceVersionMapping = FlowGoToCadenceVersionMapping{ // Leaving this example in, so it's easier to understand - //{ + // // FlowGoVersion: *semver.New("0.37.0"), // CadenceVersion: *semver.New("1.0.0"), - //}, + // } -func SetFVMToCadenceVersionMappingForTestingOnly(mapping []VersionMapEntry) { +func SetFVMToCadenceVersionMappingForTestingOnly(mapping FlowGoToCadenceVersionMapping) { minimumFvmToMinimumCadenceVersionMapping = mapping } diff --git a/fvm/environment/minimum_required_version_test.go b/fvm/environment/minimum_required_version_test.go index 7860da46ea8..a3da90beb22 100644 --- a/fvm/environment/minimum_required_version_test.go +++ b/fvm/environment/minimum_required_version_test.go @@ -31,36 +31,19 @@ func Test_MapToCadenceVersion(t *testing.T) { Patch: 0, } - mapping := []VersionMapEntry{ - { - FlowGoVersion: flowV1, - CadenceVersion: cadenceV1, - }, - { - FlowGoVersion: flowV2, - CadenceVersion: cadenceV2, - }, - } - - mappingWith2Versions := []VersionMapEntry{ - { - FlowGoVersion: flowV1, - CadenceVersion: cadenceV1, - }, - { - FlowGoVersion: flowV2, - CadenceVersion: cadenceV2, - }, + mapping := FlowGoToCadenceVersionMapping{ + FlowGoVersion: flowV1, + CadenceVersion: cadenceV1, } t.Run("no mapping, v0", func(t *testing.T) { - version := mapToCadenceVersion(flowV0, nil) + version := mapToCadenceVersion(flowV0, FlowGoToCadenceVersionMapping{}) require.Equal(t, cadenceV0, version) }) t.Run("v0", func(t *testing.T) { - version := mapToCadenceVersion(flowV0, mappingWith2Versions) + version := mapToCadenceVersion(flowV0, mapping) require.Equal(t, semver.Version{}, version) }) @@ -69,12 +52,12 @@ func Test_MapToCadenceVersion(t *testing.T) { v := flowV1 v.Patch -= 1 - version := mapToCadenceVersion(v, mappingWith2Versions) + version := mapToCadenceVersion(v, mapping) require.Equal(t, cadenceV0, version) }) t.Run("v1", func(t *testing.T) { - version := mapToCadenceVersion(flowV1, mappingWith2Versions) + version := mapToCadenceVersion(flowV1, mapping) require.Equal(t, cadenceV1, version) }) @@ -83,7 +66,7 @@ func Test_MapToCadenceVersion(t *testing.T) { v := flowV1 v.BumpPatch() - version := mapToCadenceVersion(v, mappingWith2Versions) + version := mapToCadenceVersion(v, mapping) require.Equal(t, cadenceV1, version) }) @@ -92,12 +75,12 @@ func Test_MapToCadenceVersion(t *testing.T) { v := flowV2 v.Patch -= 1 - version := mapToCadenceVersion(v, mappingWith2Versions) + version := mapToCadenceVersion(v, mapping) require.Equal(t, cadenceV1, version) }) t.Run("v2", func(t *testing.T) { - version := mapToCadenceVersion(flowV2, mappingWith2Versions) + version := mapToCadenceVersion(flowV2, mapping) require.Equal(t, cadenceV2, version) }) @@ -106,32 +89,8 @@ func Test_MapToCadenceVersion(t *testing.T) { v := flowV2 v.BumpPatch() - version := mapToCadenceVersion(v, mappingWith2Versions) - - require.Equal(t, cadenceV2, version) - }) - - t.Run("v1 - delta, single version in mapping", func(t *testing.T) { - - v := flowV1 - v.Patch -= 1 - - version := mapToCadenceVersion(v, mapping) - - require.Equal(t, cadenceV0, version) - }) - t.Run("v1, single version in mapping", func(t *testing.T) { - version := mapToCadenceVersion(flowV1, mapping) - - require.Equal(t, cadenceV1, version) - }) - t.Run("v1 + delta, single version in mapping", func(t *testing.T) { - - v := flowV1 - v.BumpPatch() - version := mapToCadenceVersion(v, mapping) - require.Equal(t, cadenceV1, version) + require.Equal(t, cadenceV2, version) }) } diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 0caabd49cb2..64b9aa2aa67 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -3564,33 +3564,14 @@ func Test_MinimumRequiredVersion(t *testing.T) { Patch: 3, PreRelease: "rc.2", } - flowVersion2 := semver.Version{ - Major: 2, - Minor: 2, - Patch: 3, - PreRelease: "rc.1", - } - cadenceVersion2 := semver.Version{ - Major: 12, - Minor: 0, - Patch: 0, - PreRelease: "", - } environment.SetFVMToCadenceVersionMappingForTestingOnly( - []environment.VersionMapEntry{ - { - FlowGoVersion: flowVersion1, - CadenceVersion: cadenceVersion1, - }, - { - FlowGoVersion: flowVersion2, - CadenceVersion: cadenceVersion2, - }, + environment.FlowGoToCadenceVersionMapping{ + FlowGoVersion: flowVersion1, + CadenceVersion: cadenceVersion1, }) h0 := uint64(100) // starting height hv1 := uint64(2000) // version boundary height - hv2 := uint64(4000) // version boundary height txIndex := uint32(0) @@ -3617,19 +3598,8 @@ func Test_MinimumRequiredVersion(t *testing.T) { // system transaction needs to run to update the flowVersion on chain snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv1+1, ctx, snapshotTree, vm, txIndex) - txIndex += 1 // still cadence version 1 require.Equal(t, cadenceVersion1.String(), getVersion(ctx, snapshotTree)) - - // insert version boundary 2 - snapshotTree = insertVersionBoundary(flowVersion2, hv1+1, hv2, ctx, snapshotTree, vm, txIndex) - txIndex += 1 - - // system transaction needs to run to update the flowVersion on chain - snapshotTree = runSystemTxToUpdateNodeVersionBeaconContract(hv2, ctx, snapshotTree, vm, txIndex) - - // switch cadence version 2 - require.Equal(t, cadenceVersion2.String(), getVersion(ctx, snapshotTree)) })) } From 456e8a6b23a00f17aa590ffbf6494248182d2582 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Fri, 25 Oct 2024 20:04:52 +0200 Subject: [PATCH 11/13] fix test --- .../minimum_required_version_test.go | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/fvm/environment/minimum_required_version_test.go b/fvm/environment/minimum_required_version_test.go index a3da90beb22..a72e10567df 100644 --- a/fvm/environment/minimum_required_version_test.go +++ b/fvm/environment/minimum_required_version_test.go @@ -15,21 +15,11 @@ func Test_MapToCadenceVersion(t *testing.T) { Minor: 37, Patch: 0, } - flowV2 := semver.Version{ - Major: 0, - Minor: 37, - Patch: 30, - } cadenceV1 := semver.Version{ Major: 1, Minor: 0, Patch: 0, } - cadenceV2 := semver.Version{ - Major: 1, - Minor: 1, - Patch: 0, - } mapping := FlowGoToCadenceVersionMapping{ FlowGoVersion: flowV1, @@ -70,27 +60,4 @@ func Test_MapToCadenceVersion(t *testing.T) { require.Equal(t, cadenceV1, version) }) - t.Run("v2 - delta", func(t *testing.T) { - - v := flowV2 - v.Patch -= 1 - - version := mapToCadenceVersion(v, mapping) - - require.Equal(t, cadenceV1, version) - }) - t.Run("v2", func(t *testing.T) { - version := mapToCadenceVersion(flowV2, mapping) - - require.Equal(t, cadenceV2, version) - }) - t.Run("v2 + delta", func(t *testing.T) { - - v := flowV2 - v.BumpPatch() - - version := mapToCadenceVersion(v, mapping) - - require.Equal(t, cadenceV2, version) - }) } From 58e84cda6cb33c024398a073a11e6a478d173cc6 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Fri, 25 Oct 2024 20:40:14 +0200 Subject: [PATCH 12/13] allpy review comments --- fvm/environment/facade_env.go | 4 +- fvm/environment/minimum_required_version.go | 48 +++++++++++++++---- .../mock/minimum_required_version.go | 2 +- fvm/fvm_test.go | 2 +- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/fvm/environment/facade_env.go b/fvm/environment/facade_env.go index ab31db4e1d3..57b01d1a853 100644 --- a/fvm/environment/facade_env.go +++ b/fvm/environment/facade_env.go @@ -38,7 +38,7 @@ type facadeEnvironment struct { ValueStore *SystemContracts - MinimumRequiredVersion + MinimumCadenceRequiredVersion UUIDGenerator AccountLocalIDGenerator @@ -108,7 +108,7 @@ func newFacadeEnvironment( ), SystemContracts: systemContracts, - MinimumRequiredVersion: NewMinimumRequiredVersion( + MinimumCadenceRequiredVersion: NewMinimumCadenceRequiredVersion( txnState, ), diff --git a/fvm/environment/minimum_required_version.go b/fvm/environment/minimum_required_version.go index 2085072562a..3095c33cda9 100644 --- a/fvm/environment/minimum_required_version.go +++ b/fvm/environment/minimum_required_version.go @@ -6,25 +6,57 @@ import ( "github.com/onflow/flow-go/fvm/storage/state" ) -// MinimumRequiredVersion returns the minimum required cadence version for the current environment +// MinimumCadenceRequiredVersion returns the minimum required cadence version for the current environment // in semver format. -type MinimumRequiredVersion interface { +type MinimumCadenceRequiredVersion interface { MinimumRequiredVersion() (string, error) } -type minimumRequiredVersion struct { +type minimumCadenceRequiredVersion struct { txnPreparer state.NestedTransactionPreparer } -func NewMinimumRequiredVersion( +func NewMinimumCadenceRequiredVersion( txnPreparer state.NestedTransactionPreparer, -) MinimumRequiredVersion { - return minimumRequiredVersion{ +) MinimumCadenceRequiredVersion { + return minimumCadenceRequiredVersion{ txnPreparer: txnPreparer, } } -func (c minimumRequiredVersion) MinimumRequiredVersion() (string, error) { +// MinimumRequiredVersion The returned cadence version can be used by cadence runtime for supporting feature flag. +// The feature flag in cadence allows ENs to produce consistent results even if running with +// different cadence versions at the same height, which is useful for rolling out cadence +// upgrade without all ENs restarting all together. +// For instance, we would like to grade cadence from v1 to v3, where v3 has a new cadence feature. +// We first make a cadence v2 that has feature flag only turned on when the MinimumRequiredVersion() +// method returns v2 or above. +// So cadence v2 with the feature flag turned off will produce the same result as v1 which doesn't have the feature. +// And cadence v2 with the feature flag turned on will also produce the same result as v3 which has the feature. +// The feature flag allows us to roll out cadence v2 to all ENs which was running v1. +// And we use the MinimumRequiredVersion to control when the feature flag should be switched from off to on. +// And the switching should happen at the same height for all ENs. +// +// The height-based switch over can be done by using VersionBeacon, however, the VersionBeacon only +// defines the flow-go version, not cadence version. +// So we first read the current minimum required flow-go version from the VersionBeacon control, +// and map it to the cadence version to be used by cadence to decide feature flag status. +// +// For instance, let’s say all ENs are running flow-go v0.37.0 with cadence v1. +// We first create a version mapping entry for flow-go v0.37.1 to cadence v2, and roll out v0.37.1 to all ENs. +// v0.37.1 ENs will produce the same result as v0.37.0 ENs, because the current version beacon still returns v0.37.0, +// which maps zero cadence version, and cadence will keep the feature flag off. +// +// After all ENs have upgraded to v0.37.1, we send out a version beacon to switch to v0.37.1 at a future height, +// let’s say height 1000. +// Then what happens is that: +// 1. ENs running v0.37.0 will crash after height 999, until upgrade to higher version +// 2. ENs running v0.37.1 will execute with cadence v2 with feature flag off up until height 999, and from height 1000, +// the feature flag will be on, which means all v0.37.1 ENs will again produce consistent results for blocks above 1000. +// +// After height 1000 have been sealed, we can roll out v0.37.2 to all ENs with cadence v3, and it will produce the consistent +// result as v0.37.1. +func (c minimumCadenceRequiredVersion) MinimumRequiredVersion() (string, error) { executionParameters := c.txnPreparer.ExecutionParameters() // map the minimum required flow-go version to a minimum required cadence version @@ -64,4 +96,4 @@ func SetFVMToCadenceVersionMappingForTestingOnly(mapping FlowGoToCadenceVersionM minimumFvmToMinimumCadenceVersionMapping = mapping } -var _ MinimumRequiredVersion = (*minimumRequiredVersion)(nil) +var _ MinimumCadenceRequiredVersion = (*minimumCadenceRequiredVersion)(nil) diff --git a/fvm/environment/mock/minimum_required_version.go b/fvm/environment/mock/minimum_required_version.go index 3894d135148..f758a4b8fb1 100644 --- a/fvm/environment/mock/minimum_required_version.go +++ b/fvm/environment/mock/minimum_required_version.go @@ -14,7 +14,7 @@ func (_m *MinimumRequiredVersion) MinimumRequiredVersion() (string, error) { ret := _m.Called() if len(ret) == 0 { - panic("no return value specified for MinimumRequiredVersion") + panic("no return value specified for MinimumCadenceRequiredVersion") } var r0 string diff --git a/fvm/fvm_test.go b/fvm/fvm_test.go index 64b9aa2aa67..fb88f78f287 100644 --- a/fvm/fvm_test.go +++ b/fvm/fvm_test.go @@ -3442,7 +3442,7 @@ func Test_MinimumRequiredVersion(t *testing.T) { }) require.NoError(t, err) - mrv := environment.NewMinimumRequiredVersion(txnState) + mrv := environment.NewMinimumCadenceRequiredVersion(txnState) v, err := mrv.MinimumRequiredVersion() From b2ee0e99aefb48bd76e7a46dddd00d10a60881f7 Mon Sep 17 00:00:00 2001 From: Janez Podhostnik Date: Fri, 25 Oct 2024 20:45:29 +0200 Subject: [PATCH 13/13] fix mocks --- ...on.go => minimum_cadence_required_version.go} | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) rename fvm/environment/mock/{minimum_required_version.go => minimum_cadence_required_version.go} (53%) diff --git a/fvm/environment/mock/minimum_required_version.go b/fvm/environment/mock/minimum_cadence_required_version.go similarity index 53% rename from fvm/environment/mock/minimum_required_version.go rename to fvm/environment/mock/minimum_cadence_required_version.go index f758a4b8fb1..d3bff30e08b 100644 --- a/fvm/environment/mock/minimum_required_version.go +++ b/fvm/environment/mock/minimum_cadence_required_version.go @@ -4,17 +4,17 @@ package mock import mock "github.com/stretchr/testify/mock" -// MinimumRequiredVersion is an autogenerated mock type for the MinimumRequiredVersion type -type MinimumRequiredVersion struct { +// MinimumCadenceRequiredVersion is an autogenerated mock type for the MinimumCadenceRequiredVersion type +type MinimumCadenceRequiredVersion struct { mock.Mock } // MinimumRequiredVersion provides a mock function with given fields: -func (_m *MinimumRequiredVersion) MinimumRequiredVersion() (string, error) { +func (_m *MinimumCadenceRequiredVersion) MinimumRequiredVersion() (string, error) { ret := _m.Called() if len(ret) == 0 { - panic("no return value specified for MinimumCadenceRequiredVersion") + panic("no return value specified for MinimumRequiredVersion") } var r0 string @@ -37,13 +37,13 @@ func (_m *MinimumRequiredVersion) MinimumRequiredVersion() (string, error) { return r0, r1 } -// NewMinimumRequiredVersion creates a new instance of MinimumRequiredVersion. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. +// NewMinimumCadenceRequiredVersion creates a new instance of MinimumCadenceRequiredVersion. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. -func NewMinimumRequiredVersion(t interface { +func NewMinimumCadenceRequiredVersion(t interface { mock.TestingT Cleanup(func()) -}) *MinimumRequiredVersion { - mock := &MinimumRequiredVersion{} +}) *MinimumCadenceRequiredVersion { + mock := &MinimumCadenceRequiredVersion{} mock.Mock.Test(t) t.Cleanup(func() { mock.AssertExpectations(t) })