Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose minimum required version to cadence interface #6560

Merged
merged 15 commits into from
Oct 28, 2024
49 changes: 49 additions & 0 deletions fvm/environment/derived_data_invalidator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -26,6 +27,8 @@ type DerivedDataInvalidator struct {
ContractUpdates

MeterParamOverridesUpdated bool

CurrentVersionBoundaryUpdated bool
}

var _ derived.TransactionInvalidator = DerivedDataInvalidator{}
Expand All @@ -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),
}
}

Expand Down Expand Up @@ -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}
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
7 changes: 6 additions & 1 deletion fvm/environment/derived_data_invalidator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package environment_test
import (
"testing"

"github.com/onflow/flow-go/fvm/systemcontracts"

"github.com/onflow/cadence/common"
"github.com/stretchr/testify/require"

Expand Down Expand Up @@ -296,10 +298,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)
}

Expand Down
2 changes: 2 additions & 0 deletions fvm/environment/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ type Environment interface {
error,
)

GetCurrentVersionBoundary() (cadence.Value, error)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
GetCurrentVersionBoundary() (cadence.Value, error)
// GetCurrentVersionBoundary returns the current version boundary stored in the VersionBeacon
// system contract.
// There are three cases:
// 1. If it's never set in the smart contract, the current value would be a zero version boundary value.
// 2. The current boundary value is set with a higher version than the node, and
// a higher height than the current block to be executed.
// 3. The current boundary value is set with a lower or same version than the node, and
//. a lower height than the current block to be executed.
GetCurrentVersionBoundary() (cadence.Value, error)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a comment, but this is a different function. This one just exposes the one on SystemContracts to the Environment interface


// AccountInfo
GetAccount(address flow.Address) (*flow.Account, error)
GetAccountKeys(address flow.Address) ([]flow.AccountPublicKey, error)
Expand Down
10 changes: 10 additions & 0 deletions fvm/environment/facade_env.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ type facadeEnvironment struct {
ValueStore

*SystemContracts
MinimumRequiredVersion

UUIDGenerator
AccountLocalIDGenerator
Expand Down Expand Up @@ -103,6 +104,12 @@ func newFacadeEnvironment(
),

SystemContracts: systemContracts,
MinimumRequiredVersion: NewMinimumRequiredVersion(
tracer,
meter,
txnState,
params,
),

UUIDGenerator: NewUUIDGenerator(
tracer,
Expand Down Expand Up @@ -293,6 +300,9 @@ 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)
Expand Down
1 change: 1 addition & 0 deletions fvm/environment/meter.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const (
_
ComputationKindEVMEncodeABI
ComputationKindEVMDecodeABI
ComputationKindMinimumRequiredVersion
)

// MainnetExecutionEffortWeights are the execution effort weights as they are
Expand Down
190 changes: 190 additions & 0 deletions fvm/environment/minimum_required_version.go
Original file line number Diff line number Diff line change
@@ -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)
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be more specific?

Suggested change
type MinimumRequiredVersion interface {
MinimumRequiredVersion() (string, error)
}
type MinimumRequiredCadenceVersion interface {
MinimumRequiredCadenceVersion() (string, error)
}

Copy link
Contributor Author

@janezpodhostnik janezpodhostnik Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is a bit tricky. This method satisfies the cadence runtime interface (defined by cadence) and from a cadence perspective it does not make sense to name it a MinimumRequiredCadenceVersion so its just named MinimumRequiredVersion and in the FVM ve just have to implement that interface method.

I'll add a comment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Maybe we keep the method name as MinimumRequiredVersion, but rename the interface as MinimumCadenceRequiredVersion ?


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) {
Copy link
Member

@zhangchiqing zhangchiqing Oct 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
func (c minimumRequiredVersion) MinimumRequiredVersion() (string, error) {
// The returned cadence version can be used by cadnece 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 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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can move this pure function outside

return version.Compare(versionToCompare) >= 0
}

for _, entry := range fvmToCadenceVersionMapping {
if greaterThanOrEqualTo(*semVer, entry.FlowGoVersion) {
cadenceVersion = entry.CadenceVersion
} else {
break
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why break here?

It seems we always just look at the very first version entry only, in that case, why not just having one version entry instead of a mapping?

If we are going to hardcode a version map entry, I would say we just hard code the default one:

var DefaultVersionMapEntry = VersionMapEntry{ 
  FlowGoVersion: semver.Version{"0.0.0"}, CadenceVersion: semver.Version{"0.0.0"} }

// change this with versions FlowGoVersion: F, and CadenceVersion: C, where
// CadenceVersion C had a feature toggle which won't be enabled if FlowGoVersion is below F.
var RequiredVersionMapEntry = VersionMapEntry{ 
  FlowGoVersion: semver.Version{"0.0.0"}, CadenceVersion: semver.Version{"0.0.0"} }

then we can simplify this function into:

func mapToCadenceVersion(flowGoVersion semver.Version, versionMapping VersionMapEntry) semver.Version {
  if greateThanOrEqualTo(flowGoVersion, versionMapping.FlowGoVersion) {
    return versionMapping.CadenceVersion
  }
  return DefaultVersionEntry.CadenceVersion
}

}
}

return cadenceVersion.String(), nil
}

type VersionMapEntry struct {
FlowGoVersion semver.Version
CadenceVersion semver.Version
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type VersionMapEntry struct {
FlowGoVersion semver.Version
CadenceVersion semver.Version
}
// MinVersionMapEntry defines a version map between flow-go and cadence version,
// such that cadence at that version must be run with a flow-go version that is greater
// or equal to the given min version.
type MinVersionMapEntry struct {
MinFlowGoVersion semver.Version
CadenceVersion semver.Version
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the version map doesn't need to know that this is a mapping between minimums.


// 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.
janezpodhostnik marked this conversation as resolved.
Show resolved Hide resolved
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
}
30 changes: 30 additions & 0 deletions fvm/environment/mock/environment.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading