From ecb3e61ee8bfb5ae90f8e0399afd753f56b1edbe Mon Sep 17 00:00:00 2001
From: Yawning Angel <yawning@schwanenlied.me>
Date: Tue, 3 Mar 2020 12:10:13 +0000
Subject: [PATCH] go/scheduler: Validators now returns validators by node ID

The consensus ID isn't all that useful for most external callers, so
querying it should just return the validators by node ID instead.
---
 .changelog/2739.feature.md                    |  4 +++
 .../tendermint/apps/registry/query.go         | 28 ++++++++++++++-----
 .../tendermint/apps/scheduler/query.go        | 28 +++++++++++++++++--
 go/scheduler/api/api.go                       |  2 +-
 4 files changed, 51 insertions(+), 11 deletions(-)
 create mode 100644 .changelog/2739.feature.md

diff --git a/.changelog/2739.feature.md b/.changelog/2739.feature.md
new file mode 100644
index 00000000000..52ebd8dbe5d
--- /dev/null
+++ b/.changelog/2739.feature.md
@@ -0,0 +1,4 @@
+go/scheduler: Validators now returns validators by node ID
+
+The consensus ID isn't all that useful for most external callers, so
+querying it should just return the validators by node ID instead.
diff --git a/go/consensus/tendermint/apps/registry/query.go b/go/consensus/tendermint/apps/registry/query.go
index c763c984cec..30eebd0c3ae 100644
--- a/go/consensus/tendermint/apps/registry/query.go
+++ b/go/consensus/tendermint/apps/registry/query.go
@@ -30,18 +30,22 @@ type QueryFactory struct {
 	app *registryApplication
 }
 
-// QueryAt returns the registry query interface for a specific height.
-func (sf *QueryFactory) QueryAt(ctx context.Context, height int64) (Query, error) {
-	var state *registryState.ImmutableState
-	var err error
+// ImmutableStateAt creates a registry ImmutableState suitable for use
+// from external query instances.
+func ImmutableStateAt(ctx context.Context, appState abci.ApplicationState, height int64) (*registryState.ImmutableState, error) {
+	var (
+		state *registryState.ImmutableState
+		err   error
+	)
 	abciCtx := abci.FromCtx(ctx)
+	abciState := abciCtx.State()
 
 	// If this request was made from InitChain, no blocks and states have been
 	// submitted yet, so we use the existing state instead.
 	if abciCtx != nil && abciCtx.IsInitChain() {
-		state = registryState.NewMutableState(abciCtx.State()).ImmutableState
+		state = registryState.NewMutableState(abciState).ImmutableState
 	} else {
-		state, err = registryState.NewImmutableState(sf.app.state, height)
+		state, err = registryState.NewImmutableState(appState, height)
 		if err != nil {
 			return nil, err
 		}
@@ -50,7 +54,17 @@ func (sf *QueryFactory) QueryAt(ctx context.Context, height int64) (Query, error
 	// If this request was made from an ABCI app, make sure to use the associated
 	// context for querying state instead of the default one.
 	if abciCtx != nil && height == abciCtx.BlockHeight()+1 {
-		state.Snapshot = abciCtx.State().ImmutableTree
+		state.Snapshot = abciState.ImmutableTree
+	}
+
+	return state, nil
+}
+
+// QueryAt returns the registry query interface for a specific height.
+func (sf *QueryFactory) QueryAt(ctx context.Context, height int64) (Query, error) {
+	state, err := ImmutableStateAt(ctx, sf.app.state, height)
+	if err != nil {
+		return nil, err
 	}
 
 	return &registryQuerier{sf.app, state, height}, nil
diff --git a/go/consensus/tendermint/apps/scheduler/query.go b/go/consensus/tendermint/apps/scheduler/query.go
index e59bb195571..e0628cc0b6b 100644
--- a/go/consensus/tendermint/apps/scheduler/query.go
+++ b/go/consensus/tendermint/apps/scheduler/query.go
@@ -5,6 +5,8 @@ import (
 
 	consensus "github.com/oasislabs/oasis-core/go/consensus/api"
 	"github.com/oasislabs/oasis-core/go/consensus/tendermint/abci"
+	registryapp "github.com/oasislabs/oasis-core/go/consensus/tendermint/apps/registry"
+	registryState "github.com/oasislabs/oasis-core/go/consensus/tendermint/apps/registry/state"
 	schedulerState "github.com/oasislabs/oasis-core/go/consensus/tendermint/apps/scheduler/state"
 	scheduler "github.com/oasislabs/oasis-core/go/scheduler/api"
 )
@@ -45,11 +47,18 @@ func (sf *QueryFactory) QueryAt(ctx context.Context, height int64) (Query, error
 		state.Snapshot = abciCtx.State().ImmutableTree
 	}
 
-	return &schedulerQuerier{state}, nil
+	// Some queries need access to the registry to give useful responses.
+	regState, err := registryapp.ImmutableStateAt(ctx, sf.app.state, height)
+	if err != nil {
+		return nil, err
+	}
+
+	return &schedulerQuerier{state, regState}, nil
 }
 
 type schedulerQuerier struct {
-	state *schedulerState.ImmutableState
+	state    *schedulerState.ImmutableState
+	regState *registryState.ImmutableState
 }
 
 func (sq *schedulerQuerier) Validators(ctx context.Context) ([]*scheduler.Validator, error) {
@@ -63,8 +72,21 @@ func (sq *schedulerQuerier) Validators(ctx context.Context) ([]*scheduler.Valida
 	// ABCI state.
 	ret := make([]*scheduler.Validator, 0, len(valPks))
 	for _, v := range valPks {
+		// The validator list uses consensus addresses, so convert them
+		// to node identifiers.
+		//
+		// This is probably better than switching the scheduler to use
+		// node identifiers for validators, because user queries are
+		// likely more infrequent than all the business of actually
+		// scheduling...
+		node, err := sq.regState.NodeByConsensusOrP2PKey(v)
+		if err != nil {
+			// Should NEVER happen.
+			return nil, err
+		}
+
 		ret = append(ret, &scheduler.Validator{
-			ID:          v,
+			ID:          node.ID,
 			VotingPower: consensus.VotingPower,
 		})
 	}
diff --git a/go/scheduler/api/api.go b/go/scheduler/api/api.go
index 52a14a8cc4f..922de6f7ab5 100644
--- a/go/scheduler/api/api.go
+++ b/go/scheduler/api/api.go
@@ -150,7 +150,7 @@ func (c *Committee) EncodedMembersHash() hash.Hash {
 
 // Validator is a consensus validator.
 type Validator struct {
-	// ID is the validator consensus (NOT oasis) identifier.
+	// ID is the validator Oasis node identifier.
 	ID signature.PublicKey `json:"id"`
 
 	// VotingPower is the validator's consensus voting power.