review fixes

.changelog/2687.internal.3.md

@@ -1,4 +1,4 @@
-buildkite: New benchmarks pipeline has been added
+ci: New benchmarks pipeline has been added
 
 `benchmarks.pipeline.yml` runs all E2E tests and compares the benchmark
 results from the previous batch using the new `oasis-test-runner cmp` command.

go/oasis-node/cmd/common/grpc/grpc.go

@@ -2,7 +2,6 @@
 package grpc
 
 import (
-	"crypto/sha256"
 	"crypto/tls"
 	"errors"
 	"fmt"
@@ -31,8 +30,10 @@ const (
 	// CfgDebugGrpcInternalSocketPath sets custom internal socket path.
 	CfgDebugGrpcInternalSocketPath = "debug.grpc.internal.socket_path"
 
-	defaultAddress      = "unix:" + LocalSocketFilename
+	// LocalSocketFilename is the filename of the unix socket in node datadir.
 	LocalSocketFilename = "internal.sock"
+
+	defaultAddress = "unix:" + LocalSocketFilename
 )
 
 var (
@@ -74,6 +75,7 @@ func NewServerLocal(installWrapper bool) (*cmnGrpc.Server, error) {
 	}
 	path := filepath.Join(dataDir, LocalSocketFilename)
 	if viper.IsSet(CfgDebugGrpcInternalSocketPath) && flags.DebugDontBlameOasis() {
+		logger.Info("overriding internal socket path", "path", viper.GetString(CfgDebugGrpcInternalSocketPath))
 		path = viper.GetString(CfgDebugGrpcInternalSocketPath)
 	}
 
@@ -110,12 +112,6 @@ func NewClient(cmd *cobra.Command) (*grpc.ClientConn, error) {
 	return conn, nil
 }
 
-// GetAbstractSocketAddress returns abstract socket address for internal gRPC based on hashed datadir.
-func GetAbstractSocketAddress(datadir string) string {
-	p := sha256.Sum256([]byte(datadir))
-	return fmt.Sprintf("@oasis-%x", p[0:4])
-}
-
 func init() {
 	ServerTCPFlags.Uint16(CfgServerPort, 9001, "gRPC server port")
 	_ = viper.BindPFlags(ServerTCPFlags)

go/oasis-test-runner/cmd/root.go

@@ -432,10 +432,10 @@ func doScenario(childEnv *env.Env, sc scenario.Scenario) (err error) {
 		}
 	}
 
-	// If network is used, enable shorter per-node socket paths, because some e2e test datadir exceed maximum unix
-	// socket path length.
+	// If network is used, enable shorter per-node socket paths, because some e2e test datadir
+	// exceed maximum unix socket path length.
 	if net != nil {
-		net.Config().UseCustomGrpcSocketPath = true
+		net.Config().UseCustomGrpcSocketPaths = true
 	}
 
 	if err = sc.Init(childEnv, net); err != nil {

go/oasis-test-runner/oasis/args.go

@@ -81,7 +81,7 @@ func (args *argBuilder) grpcLogDebug() *argBuilder {
 	return args
 }
 
-func (args *argBuilder) grpcDebugGrpcSocketPath(path string) *argBuilder {
+func (args *argBuilder) grpcDebugGrpcInternalSocketPath(path string) *argBuilder {
 	args.vec = append(args.vec, "--"+grpc.CfgDebugGrpcInternalSocketPath, path)
 	return args
 }

go/oasis-test-runner/oasis/ias.go

@@ -35,8 +35,8 @@ func (ias *iasProxy) startNode() error {
 		iasSPID(mockSPID)
 	if ias.useRegistry {
 		// XXX: IAS proxy is started before the validators. Pregenerate temp validator internal socket path, if needed.
-		if ias.net.cfg.UseCustomGrpcSocketPath && ias.net.validators[0].customGrpcSocketPath == "" {
-			ias.net.validators[0].customGrpcSocketPath = ias.net.genTempSocketPath()
+		if ias.net.cfg.UseCustomGrpcSocketPaths && ias.net.validators[0].customGrpcSocketPath == "" {
+			ias.net.validators[0].customGrpcSocketPath = ias.net.generateTempSocketPath()
 		}
 
 		args = args.internalSocketAddress(ias.net.validators[0].SocketPath())

go/oasis-test-runner/oasis/oasis.go

@@ -254,11 +254,9 @@ type NetworkCfg struct { // nolint: maligned
 	// created in this test network.
 	DefaultLogWatcherHandlerFactories []log.WatcherHandlerFactory `json:"-"`
 
-	// UseCustomGrpcSocketPath specifies whether nodes should use internal.sock in datadir or externally-provided.
-	//
-	// WARNING: Using arbitrary unix socket paths poses security risk! This flag should only be used for debugging and
-	// end-to-end tests!
-	UseCustomGrpcSocketPath bool `json:"-"`
+	// UseCustomGrpcSocketPaths specifies whether nodes should use internal.sock in datadir or
+	// externally-provided.
+	UseCustomGrpcSocketPaths bool `json:"-"`
 }
 
 // Config returns the network configuration.
@@ -611,11 +609,11 @@ func (net *Network) generateDeterministicNodeIdentity(dir *env.Dir, rawSeed stri
 	return nil
 }
 
-// genTempSocketPath returns unique filename for internal socket in test base dir.
+// generateTempSocketPath returns a unique filename for a node's internal socket in the test base dir
 //
-// This function is used to obtain shorter socket path than the one in datadir since that one might be too long for unix
-// socket path.
-func (net *Network) genTempSocketPath() string {
+// This function is used to obtain shorter socket path than the one in datadir since that one might
+// be too long for unix socket path.
+func (net *Network) generateTempSocketPath() string {
 	f, err := ioutil.TempFile(env.GetRootDir().String(), "internal-*.sock")
 	if err != nil {
 		return ""
@@ -645,13 +643,13 @@ func (net *Network) startOasisNode(
 			tendermintDebugAddrBookLenient().
 			tendermintDebugAllowDuplicateIP()
 	}
-	if net.cfg.UseCustomGrpcSocketPath {
+	if net.cfg.UseCustomGrpcSocketPaths {
 		// Keep the socket, if it was already generated!
 		if node.customGrpcSocketPath == "" {
-			node.customGrpcSocketPath = net.genTempSocketPath()
+			node.customGrpcSocketPath = net.generateTempSocketPath()
 		}
 		extraArgs = extraArgs.debugDontBlameOasis()
-		extraArgs = extraArgs.grpcDebugGrpcSocketPath(node.customGrpcSocketPath)
+		extraArgs = extraArgs.grpcDebugGrpcInternalSocketPath(node.customGrpcSocketPath)
 	}
 	if viper.IsSet(metrics.CfgMetricsAddr) {
 		extraArgs = extraArgs.appendNodeMetrics(node)

go/oasis-test-runner/scenario/e2e/halt_restore.go

@@ -213,9 +213,7 @@ func (sc *haltRestoreImpl) Run(childEnv *env.Env) error {
 
 	// If network is used, enable shorter per-node socket paths, because some e2e test datadir exceed maximum unix
 	// socket path length.
-	if sc.runtimeImpl.net != nil {
-		sc.runtimeImpl.net.Config().UseCustomGrpcSocketPath = true
-	}
+	sc.runtimeImpl.net.Config().UseCustomGrpcSocketPaths = true
 
 	sc.runtimeImpl.clientArgs = []string{"--mode", "part2"}
 	return sc.runtimeImpl.Run(childEnv)

go/oasis-test-runner/scenario/e2e/runtime.go

@@ -407,11 +407,10 @@ func (sc *runtimeImpl) dumpRestoreNetwork(childEnv *env.Env, fixture *oasis.Netw
 		return err
 	}
 
-	// If network is used, enable shorter per-node socket paths, because some e2e test datadir exceed maximum unix
-	// socket path length.
-	if sc.net != nil {
-		sc.net.Config().UseCustomGrpcSocketPath = true
-	}
+	// If network is used, enable shorter per-node socket paths, because some e2e test datadir
+	// exceed maximum unix socket path length.
+	sc.net.Config().UseCustomGrpcSocketPaths = true
+
 	return nil
 }
 

go/oasis-test-runner/scenario/e2e/txsource.go

@@ -100,17 +100,19 @@ type txSourceImpl struct { // nolint: maligned
 
 func (sc *txSourceImpl) PreInit(childEnv *env.Env) error {
 	// Generate a new random seed and log it so we can reproduce the run.
-	// TODO: Allow the seed to be overriden in order to reproduce randomness (minus timing).
-	rawSeed := make([]byte, 16)
-	_, err := cryptoRand.Read(rawSeed)
-	if err != nil {
-		return fmt.Errorf("failed to generate random seed: %w", err)
-	}
-	sc.seed = hex.EncodeToString(rawSeed)
+	// Use existing seed, if it already exists.
+	if sc.seed == "" {
+		rawSeed := make([]byte, 16)
+		_, err := cryptoRand.Read(rawSeed)
+		if err != nil {
+			return fmt.Errorf("failed to generate random seed: %w", err)
+		}
+		sc.seed = hex.EncodeToString(rawSeed)
 
-	sc.logger.Info("using random seed",
-		"seed", sc.seed,
-	)
+		sc.logger.Info("using random seed",
+			"seed", sc.seed,
+		)
+	}
 
 	// Set up the deterministic random source.
 	hash := crypto.SHA512
@@ -193,11 +195,6 @@ func (sc *txSourceImpl) Fixture() (*oasis.NetworkFixture, error) {
 	return f, nil
 }
 
-func (sc *txSourceImpl) Init(childEnv *env.Env, net *oasis.Network) error {
-	sc.net = net
-	return nil
-}
-
 func (sc *txSourceImpl) manager(env *env.Env, errCh chan error) {
 	// Make sure we exit when the environment gets torn down.
 	stopCh := make(chan struct{})
@@ -344,12 +341,17 @@ func (sc *txSourceImpl) startWorkload(childEnv *env.Env, errCh chan error, name
 
 func (sc *txSourceImpl) Clone() scenario.Scenario {
 	return &txSourceImpl{
-		runtimeImpl:           *sc.runtimeImpl.Clone().(*runtimeImpl),
-		workloads:             sc.workloads,
-		timeLimit:             sc.timeLimit,
-		nodeRestartInterval:   sc.nodeRestartInterval,
-		livenessCheckInterval: sc.livenessCheckInterval,
-		rng:                   sc.rng,
+		runtimeImpl:                       *sc.runtimeImpl.Clone().(*runtimeImpl),
+		workloads:                         sc.workloads,
+		timeLimit:                         sc.timeLimit,
+		nodeRestartInterval:               sc.nodeRestartInterval,
+		livenessCheckInterval:             sc.livenessCheckInterval,
+		consensusPruneDisabledProbability: sc.consensusPruneDisabledProbability,
+		consensusPruneMinKept:             sc.consensusPruneMinKept,
+		consensusPruneMaxKept:             sc.consensusPruneMaxKept,
+		tendermintRecoverCorruptedWAL:     sc.tendermintRecoverCorruptedWAL,
+		seed:                              sc.seed,
+		// rng must always be reinitialized from seed by calling PreInit().
 	}
 }
 

go/oasis-test-runner/scenario/scenario.go

@@ -22,16 +22,16 @@ type Scenario interface {
 	// Parameters returns the settable test parameters.
 	Parameters() *flag.FlagSet
 
+	// PreInit performs initial scenario initialization. It is guaranteed to be called before
+	// a new fixture is initialized in Fixture.
+	PreInit(childEnv *env.Env) error
+
 	// Fixture returns a network fixture to use for this scenario.
 	//
 	// It may return nil in case the scenario doesn't use a fixture and
 	// performs all setup in Init.
 	Fixture() (*oasis.NetworkFixture, error)
 
-	// PreInit performs initial scenario initialization. It is guaranteed to be called before
-	// a new fixture is initialized in Fixture.
-	PreInit(childEnv *env.Env) error
-
 	// Init initializes the scenario.
 	//
 	// Network will be provided in case Fixture returned a non-nil value,