op-node: protocol versions review suggestions

go.mod

@@ -208,6 +208,6 @@ require (
 	rsc.io/tmplfunc v0.0.3 // indirect
 )
 
-replace github.com/ethereum/go-ethereum v1.12.0 => github.com/ethereum-optimism/op-geth v1.101200.2-rc.1.0.20230913104329-60b827bfc2de
+replace github.com/ethereum/go-ethereum v1.12.0 => github.com/ethereum-optimism/op-geth v1.101200.2-rc.1.0.20230914220922-92eb9d4805dd
 
 //replace github.com/ethereum/go-ethereum v1.12.0 => ../go-ethereum

go.sum

@@ -162,8 +162,8 @@ github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7
 github.com/etcd-io/bbolt v1.3.3/go.mod h1:ZF2nL25h33cCyBtcyWeZ2/I3HQOfTP+0PIEvHjkjCrw=
 github.com/ethereum-optimism/go-ethereum-hdwallet v0.1.3 h1:RWHKLhCrQThMfch+QJ1Z8veEq5ZO3DfIhZ7xgRP9WTc=
 github.com/ethereum-optimism/go-ethereum-hdwallet v0.1.3/go.mod h1:QziizLAiF0KqyLdNJYD7O5cpDlaFMNZzlxYNcWsJUxs=
-github.com/ethereum-optimism/op-geth v1.101200.2-rc.1.0.20230913104329-60b827bfc2de h1:nxgmsH13qzkWlX5dXrVzsTHqtu/XHfAWkyfpH+9ljqw=
-github.com/ethereum-optimism/op-geth v1.101200.2-rc.1.0.20230913104329-60b827bfc2de/go.mod h1:gRnPb21PoKcHm3kHqj9BQlQkwmhOGUvQoGEbC7z852Q=
+github.com/ethereum-optimism/op-geth v1.101200.2-rc.1.0.20230914220922-92eb9d4805dd h1:kBfKlCNXXfEupdPk3vPKhTJbyXlQaju31L/row1DaCs=
+github.com/ethereum-optimism/op-geth v1.101200.2-rc.1.0.20230914220922-92eb9d4805dd/go.mod h1:gRnPb21PoKcHm3kHqj9BQlQkwmhOGUvQoGEbC7z852Q=
 github.com/ethereum-optimism/superchain-registry/superchain v0.0.0-20230817174831-5d3ca1966435 h1:2CzkJkkTLuVyoVFkoW5w6vDB2Q7eJzxXw/ybA17xjqM=
 github.com/ethereum-optimism/superchain-registry/superchain v0.0.0-20230817174831-5d3ca1966435/go.mod h1:v2YpePbdGBF0Gr6VWq49MFFmcTW0kRYZ2ingBJYWEwg=
 github.com/ethereum/c-kzg-4844 v0.2.0 h1:+cUvymlnoDDQgMInp25Bo3OmLajmmY8mLJ/tLjqd77Q=

op-e2e/system_test.go

@@ -1454,7 +1454,7 @@ func TestRecommendedProtocolVersionChange(t *testing.T) {
 	l1 := sys.Clients["l1"]
 
 	_, build, major, minor, patch, preRelease := params.OPStackSupport.Parse()
-	newRecommendedProtocolVersion := params.ToProtocolVersion(build, major+1, minor, patch, preRelease)
+	newRecommendedProtocolVersion := params.ProtocolVersionV0{Build: build, Major: major + 1, Minor: minor, Patch: patch, PreRelease: preRelease}.Encode()
 	require.NotEqual(t, runtimeConfig.RecommendedProtocolVersion(), newRecommendedProtocolVersion, "changing to a different protocol version")
 
 	protVersions, err := bindings.NewProtocolVersions(cfg.L1Deployments.ProtocolVersionsProxy, l1)
@@ -1509,7 +1509,7 @@ func TestRequiredProtocolVersionChangeAndHalt(t *testing.T) {
 	l1 := sys.Clients["l1"]
 
 	_, build, major, minor, patch, preRelease := params.OPStackSupport.Parse()
-	newRequiredProtocolVersion := params.ToProtocolVersion(build, major+1, minor, patch, preRelease)
+	newRequiredProtocolVersion := params.ProtocolVersionV0{Build: build, Major: major + 1, Minor: minor, Patch: patch, PreRelease: preRelease}.Encode()
 	require.NotEqual(t, runtimeConfig.RequiredProtocolVersion(), newRequiredProtocolVersion, "changing to a different protocol version")
 
 	protVersions, err := bindings.NewProtocolVersions(cfg.L1Deployments.ProtocolVersionsProxy, l1)

op-node/node/superchain.go

@@ -36,8 +36,20 @@ func (n *OpNode) handleProtocolVersionsUpdate(ctx context.Context) {
 // HaltMaybe halts the rollup node if the runtime config indicates an incompatible required protocol change
 // and the node is configured to opt-in to halting at this protocol-change level.
 func (n *OpNode) HaltMaybe() {
+	local := rollup.OPStackSupport
+	required := n.runCfg.RequiredProtocolVersion()
+	if haltMaybe(n.rollupHalt, local.Compare(required)) { // halt if we opted in to do so at this granularity
+		n.log.Error("Opted to halt, unprepared for protocol change", "required", required, "local", local)
+		if err := n.Close(); err != nil {
+			n.log.Error("Failed to halt rollup", "err", err)
+		}
+	}
+}
+
+// haltMaybe returns true when we should halt, given the halt-option and required-version comparison
+func haltMaybe(haltOption string, reqCmp params.ProtocolVersionComparison) bool {
 	var needLevel int
-	switch n.rollupHalt {
+	switch haltOption {
 	case "major":
 		needLevel = 3
 	case "minor":
@@ -45,12 +57,10 @@ func (n *OpNode) HaltMaybe() {
 	case "patch":
 		needLevel = 1
 	default:
-		return // do not consider halting if not configured to
+		return false // do not consider halting if not configured to
 	}
 	haveLevel := 0
-	local := rollup.OPStackSupport
-	required := n.runCfg.RequiredProtocolVersion()
-	switch local.Compare(required) {
+	switch reqCmp {
 	case params.OutdatedMajor:
 		haveLevel = 3
 	case params.OutdatedMinor:
@@ -58,10 +68,5 @@ func (n *OpNode) HaltMaybe() {
 	case params.OutdatedPatch:
 		haveLevel = 1
 	}
-	if haveLevel >= needLevel { // halt if we opted in to do so at this granularity
-		n.log.Error("opted to halt, unprepared for protocol change", "required", required, "local", local)
-		if err := n.Close(); err != nil {
-			n.log.Error("failed to halt rollup", "err", err)
-		}
-	}
+	return haveLevel >= needLevel
 }

op-node/node/superchain_test.go

+package node
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"golang.org/x/exp/slices"
+
+	"github.com/ethereum/go-ethereum/params"
+)
+
+func TestHaltMaybe(t *testing.T) {
+	haltTest := func(opt string, halts ...params.ProtocolVersionComparison) {
+		t.Run(opt, func(t *testing.T) {
+			for _, h := range []params.ProtocolVersionComparison{
+				params.AheadMajor,
+				params.OutdatedMajor,
+				params.AheadMinor,
+				params.OutdatedMinor,
+				params.AheadPatch,
+				params.OutdatedPatch,
+				params.AheadPrerelease,
+				params.OutdatedPrerelease,
+				params.Matching,
+				params.DiffVersionType,
+				params.DiffBuild,
+				params.EmptyVersion,
+			} {
+				expectedHalt := slices.Contains(halts, h)
+				gotHalt := haltMaybe(opt, h)
+				require.Equal(t, expectedHalt, gotHalt, "%s %d", opt, h)
+			}
+		})
+	}
+	haltTest("")
+	haltTest("major", params.OutdatedMajor)
+	haltTest("minor", params.OutdatedMajor, params.OutdatedMinor)
+	haltTest("patch", params.OutdatedMajor, params.OutdatedMinor, params.OutdatedPatch)
+}

op-node/rollup/superchain.go

@@ -12,7 +12,7 @@ import (
 	"github.com/ethereum-optimism/superchain-registry/superchain"
 )
 
-var OPStackSupport = params.ToProtocolVersion([8]byte{}, 3, 1, 0, 1)
+var OPStackSupport = params.ProtocolVersionV0{Build: [8]byte{}, Major: 3, Minor: 1, Patch: 0, PreRelease: 1}.Encode()
 
 const (
 	opMainnet   = 10

op-node/service.go

@@ -37,6 +37,7 @@ func NewConfig(ctx *cli.Context, log log.Logger) (*node.Config, error) {
 	}
 
 	if !ctx.Bool(flags.BetaRollupLoadProtocolVersions.Name) {
+		log.Info("Not opted in to ProtocolVersions signal loading, disabling ProtocolVersions contract now.")
 		rollupConfig.ProtocolVersionsAddress = common.Address{}
 	}