Merge pull request #257 from ava-labs/improve-apis

add isBootstrapped method; handle errors when creating APIs

api/health/checks.go

@@ -20,36 +20,66 @@ type CheckFn func() (interface{}, error)
 
 // Check defines a single health check that we want to monitor and consider as
 // part of our wider healthiness
-type Check struct {
+type Check interface {
 	// Name is the identifier for this check and must be unique among all Checks
-	Name string
+	Name() string
 
-	// CheckFn is the function to call to perform the the health check
-	CheckFn CheckFn
+	// Execute performs the health check. It returns nil if the check passes.
+	// It can also return additional information to marshal and display to the caller
+	Execute() (interface{}, error)
 
 	// ExecutionPeriod is the duration to wait between executions of this Check
-	ExecutionPeriod time.Duration
+	ExecutionPeriod() time.Duration
 
 	// InitialDelay is the duration to wait before executing the first time
-	InitialDelay time.Duration
+	InitialDelay() time.Duration
 
 	// InitiallyPassing is whether or not to consider the Check healthy before the
 	// initial execution
-	InitiallyPassing bool
+	InitiallyPassing() bool
 }
 
-// gosundheitCheck implements the health.Check interface backed by a CheckFn
-type gosundheitCheck struct {
-	name    string
-	checkFn CheckFn
+// check implements the Check interface
+type check struct {
+	name                          string
+	checkFn                       CheckFn
+	executionPeriod, initialDelay time.Duration
+	initiallyPassing              bool
 }
 
-// Name implements the health.Check interface by returning a unique name
-func (c gosundheitCheck) Name() string { return c.name }
+// Name is the identifier for this check and must be unique among all Checks
+func (c check) Name() string { return c.name }
 
-// Execute implements the health.Check interface by executing the checkFn and
-// returning the results
-func (c gosundheitCheck) Execute() (interface{}, error) { return c.checkFn() }
+// Execute performs the health check. It returns nil if the check passes.
+// It can also return additional information to marshal and display to the caller
+func (c check) Execute() (interface{}, error) { return c.checkFn() }
+
+// ExecutionPeriod is the duration to wait between executions of this Check
+func (c check) ExecutionPeriod() time.Duration { return c.executionPeriod }
+
+// InitialDelay is the duration to wait before executing the first time
+func (c check) InitialDelay() time.Duration { return c.initialDelay }
+
+// InitiallyPassing is whether or not to consider the Check healthy before the initial execution
+func (c check) InitiallyPassing() bool { return c.initiallyPassing }
+
+// monotonicCheck is a check that will run until it passes once, and after that it will
+// always pass without performing any logic. Used for bootstrapping, for example.
+type monotonicCheck struct {
+	passed bool
+	check
+}
+
+func (mc monotonicCheck) Execute() (interface{}, error) {
+	if mc.passed {
+		return nil, nil
+	}
+	details, pass := mc.check.Execute()
+	if pass == nil {
+		mc.passed = true
+	}
+	return details, pass
+}
 
 // Heartbeater provides a getter to the most recently observed heartbeat
 type Heartbeater interface {

api/health/service.go

@@ -17,7 +17,7 @@ import (
 )
 
 // defaultCheckOpts is a Check whose properties represent a default Check
-var defaultCheckOpts = Check{ExecutionPeriod: time.Minute}
+var defaultCheckOpts = check{executionPeriod: time.Minute}
 
 // Health observes a set of vital signs and makes them available through an HTTP
 // API.
@@ -39,11 +39,11 @@ func (h *Health) Handler() *common.HTTPHandler {
 	newServer.RegisterCodec(codec, "application/json;charset=UTF-8")
 	newServer.RegisterService(h, "health")
 	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.Method == http.MethodGet { // GET request --> return 200 if getLiveness returns true, else 500
+		if r.Method == http.MethodGet { // GET request --> return 200 if getLiveness returns true, else 503
 			if _, healthy := h.health.Results(); healthy {
 				w.WriteHeader(http.StatusOK)
 			} else {
-				w.WriteHeader(http.StatusInternalServerError)
+				w.WriteHeader(http.StatusServiceUnavailable)
 			}
 		} else {
 			newServer.ServeHTTP(w, r) // Other request --> use JSON RPC
@@ -61,18 +61,27 @@ func (h *Health) RegisterHeartbeat(name string, hb Heartbeater, max time.Duratio
 // RegisterCheckFunc adds a Check with default options and the given CheckFn
 func (h *Health) RegisterCheckFunc(name string, checkFn CheckFn) error {
 	check := defaultCheckOpts
-	check.Name = name
-	check.CheckFn = checkFn
+	check.name = name
+	check.checkFn = checkFn
+	return h.RegisterCheck(check)
+}
+
+// RegisterMonotonicCheckFunc adds a Check with default options and the given CheckFn
+// After it passes once, its logic (checkFunc) is never run again; it just passes
+func (h *Health) RegisterMonotonicCheckFunc(name string, checkFn CheckFn) error {
+	check := monotonicCheck{check: defaultCheckOpts}
+	check.name = name
+	check.checkFn = checkFn
 	return h.RegisterCheck(check)
 }
 
 // RegisterCheck adds the given Check
 func (h *Health) RegisterCheck(c Check) error {
 	return h.health.RegisterCheck(&health.Config{
-		InitialDelay:     c.InitialDelay,
-		ExecutionPeriod:  c.ExecutionPeriod,
-		InitiallyPassing: c.InitiallyPassing,
-		Check:            gosundheitCheck{c.Name, c.CheckFn},
+		InitialDelay:     c.InitialDelay(),
+		ExecutionPeriod:  c.ExecutionPeriod(),
+		InitiallyPassing: c.InitiallyPassing(),
+		Check:            c,
 	})
 }
 

api/info/service.go

@@ -4,6 +4,7 @@
 package info
 
 import (
+	"fmt"
 	"net/http"
 
 	"github.com/gorilla/rpc/v2"
@@ -129,3 +130,31 @@ func (service *Info) Peers(_ *http.Request, _ *struct{}, reply *PeersReply) erro
 	reply.Peers = service.networking.Peers()
 	return nil
 }
+
+// IsBootstrappedArgs are the arguments for calling IsBootstrapped
+type IsBootstrappedArgs struct {
+	// Alias of the chain
+	// Can also be the string representation of the chain's ID
+	Chain string `json:"chain"`
+}
+
+// IsBootstrappedResponse are the results from calling IsBootstrapped
+type IsBootstrappedResponse struct {
+	// True iff the chain exists and is done bootstrapping
+	IsBootstrapped bool `json:"isBootstrapped"`
+}
+
+// IsBootstrapped returns nil and sets [reply.IsBootstrapped] == true iff [args.Chain] exists and is done bootstrapping
+// Returns an error if the chain doesn't exist
+func (service *Info) IsBootstrapped(_ *http.Request, args *IsBootstrappedArgs, reply *IsBootstrappedResponse) error {
+	service.log.Info("Info: IsBootstrapped called")
+	if args.Chain == "" {
+		return fmt.Errorf("argument 'chain' not given")
+	}
+	chainID, err := service.chainManager.Lookup(args.Chain)
+	if err != nil {
+		return fmt.Errorf("there is no chain with alias/ID '%s'", args.Chain)
+	}
+	reply.IsBootstrapped = service.chainManager.IsBootstrapped(chainID)
+	return nil
+}

chains/manager.go

@@ -76,6 +76,9 @@ type Manager interface {
 	// Add an alias to a chain
 	Alias(ids.ID, string) error
 
+	// Returns true iff the chain with the given ID exists and is finished bootstrapping
+	IsBootstrapped(ids.ID) bool
+
 	Shutdown()
 }
 
@@ -114,6 +117,10 @@ type manager struct {
 	keystore        *keystore.Keystore
 	sharedMemory    *atomic.SharedMemory
 
+	// Key: Chain's ID
+	// Value: The chain
+	chains map[[32]byte]*router.Handler
+
 	unblocked     bool
 	blockedChains []ChainParameters
 }
@@ -131,7 +138,7 @@ func New(
 	decisionEvents *triggers.EventDispatcher,
 	consensusEvents *triggers.EventDispatcher,
 	db database.Database,
-	router router.Router,
+	rtr router.Router,
 	net network.Network,
 	consensusParams avacon.Parameters,
 	validators validators.Manager,
@@ -145,7 +152,7 @@ func New(
 	timeoutManager.Initialize(requestTimeout)
 	go log.RecoverAndPanic(timeoutManager.Dispatch)
 
-	router.Initialize(log, &timeoutManager, gossipFrequency, shutdownTimeout)
+	rtr.Initialize(log, &timeoutManager, gossipFrequency, shutdownTimeout)
 
 	m := &manager{
 		stakingEnabled:  stakingEnabled,
@@ -155,7 +162,7 @@ func New(
 		decisionEvents:  decisionEvents,
 		consensusEvents: consensusEvents,
 		db:              db,
-		chainRouter:     router,
+		chainRouter:     rtr,
 		net:             net,
 		timeoutManager:  &timeoutManager,
 		consensusParams: consensusParams,
@@ -165,6 +172,7 @@ func New(
 		server:          server,
 		keystore:        keystore,
 		sharedMemory:    sharedMemory,
+		chains:          make(map[[32]byte]*router.Handler),
 	}
 	m.Initialize()
 	return m
@@ -454,7 +462,7 @@ func (m *manager) createAvalancheChain(
 			eng:       &engine,
 		})
 	}
-
+	m.chains[ctx.ChainID.Key()] = handler
 	return nil
 }
 
@@ -546,9 +554,20 @@ func (m *manager) createSnowmanChain(
 			eng:       &engine,
 		})
 	}
+	m.chains[ctx.ChainID.Key()] = handler
 	return nil
 }
 
+func (m *manager) IsBootstrapped(id ids.ID) bool {
+	chain, exists := m.chains[id.Key()]
+	if !exists {
+		return false
+	}
+	chain.Context().Lock.Lock()
+	defer chain.Context().Lock.Unlock()
+	return chain.Engine().IsBootstrapped()
+}
+
 // Shutdown stops all the chains
 func (m *manager) Shutdown() { m.chainRouter.Shutdown() }
 

chains/mock_manager.go

@@ -35,3 +35,6 @@ func (mm MockManager) Alias(ids.ID, string) error { return nil }
 
 // Shutdown ...
 func (mm MockManager) Shutdown() {}
+
+// IsBootstrapped ...
+func (mm MockManager) IsBootstrapped(ids.ID) bool { return false }

node/node.go

@@ -7,6 +7,7 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"io/ioutil"
 	"net"
@@ -443,66 +444,105 @@ func (n *Node) initSharedMemory() {
 
 // initKeystoreAPI initializes the keystore service
 // Assumes n.APIServer is already set
-func (n *Node) initKeystoreAPI() {
-	n.Log.Info("initializing Keystore API")
+func (n *Node) initKeystoreAPI() error {
+	n.Log.Info("initializing keystore")
 	keystoreDB := prefixdb.New([]byte("keystore"), n.DB)
 	n.keystoreServer.Initialize(n.Log, keystoreDB)
 	keystoreHandler := n.keystoreServer.CreateHandler()
-	if n.Config.KeystoreAPIEnabled {
-		n.APIServer.AddRoute(keystoreHandler, &sync.RWMutex{}, "keystore", "", n.HTTPLog)
+	if !n.Config.KeystoreAPIEnabled {
+		n.Log.Info("skipping keystore API initializaion because it has been disabled")
+		return nil
 	}
+	n.Log.Info("initializing keystore API")
+	return n.APIServer.AddRoute(keystoreHandler, &sync.RWMutex{}, "keystore", "", n.HTTPLog)
+
 }
 
 // initMetricsAPI initializes the Metrics API
 // Assumes n.APIServer is already set
-func (n *Node) initMetricsAPI() {
-	n.Log.Info("initializing Metrics API")
+func (n *Node) initMetricsAPI() error {
 	registry, handler := metrics.NewService()
-	if n.Config.MetricsAPIEnabled {
-		n.APIServer.AddRoute(handler, &sync.RWMutex{}, "metrics", "", n.HTTPLog)
-	}
+	// It is assumed by components of the system that the Metrics interface is
+	// non-nil. So, it is set regardless of if the metrics API is available or not.
 	n.Config.ConsensusParams.Metrics = registry
+	if !n.Config.MetricsAPIEnabled {
+		n.Log.Info("skipping metrics API initialization because it has been disabled")
+		return nil
+	}
+	n.Log.Info("initializing metrics API")
+	return n.APIServer.AddRoute(handler, &sync.RWMutex{}, "metrics", "", n.HTTPLog)
 }
 
 // initAdminAPI initializes the Admin API service
 // Assumes n.log, n.chainManager, and n.ValidatorAPI already initialized
-func (n *Node) initAdminAPI() {
-	if n.Config.AdminAPIEnabled {
-		n.Log.Info("initializing Admin API")
-		service := admin.NewService(Version, n.ID, n.Config.NetworkID, n.Log, n.chainManager, n.Net, &n.APIServer)
-		n.APIServer.AddRoute(service, &sync.RWMutex{}, "admin", "", n.HTTPLog)
+func (n *Node) initAdminAPI() error {
+	if !n.Config.AdminAPIEnabled {
+		n.Log.Info("skipping admin API initializaion because it has been disabled")
+		return nil
 	}
+	n.Log.Info("initializing admin API")
+	service := admin.NewService(Version, n.ID, n.Config.NetworkID, n.Log, n.chainManager, n.Net, &n.APIServer)
+	return n.APIServer.AddRoute(service, &sync.RWMutex{}, "admin", "", n.HTTPLog)
 }
 
-func (n *Node) initInfoAPI() {
-	if n.Config.InfoAPIEnabled {
-		n.Log.Info("initializing Info API")
-		service := info.NewService(n.Log, Version, n.ID, n.Config.NetworkID, n.chainManager, n.Net)
-		n.APIServer.AddRoute(service, &sync.RWMutex{}, "info", "", n.HTTPLog)
+func (n *Node) initInfoAPI() error {
+	if !n.Config.InfoAPIEnabled {
+		n.Log.Info("skipping info API initializaion because it has been disabled")
+		return nil
 	}
+	n.Log.Info("initializing info API")
+	service := info.NewService(n.Log, Version, n.ID, n.Config.NetworkID, n.chainManager, n.Net)
+	return n.APIServer.AddRoute(service, &sync.RWMutex{}, "info", "", n.HTTPLog)
+
 }
 
 // initHealthAPI initializes the Health API service
-// Assumes n.Log, n.ConsensusAPI, and n.ValidatorAPI already initialized
-func (n *Node) initHealthAPI() {
+// Assumes n.Log, n.Net, n.APIServer, n.HTTPLog already initialized
+func (n *Node) initHealthAPI() error {
 	if !n.Config.HealthAPIEnabled {
-		return
+		n.Log.Info("skipping health API initializaion because it has been disabled")
+		return nil
 	}
-
 	n.Log.Info("initializing Health API")
 	service := health.NewService(n.Log)
-	service.RegisterHeartbeat("network.validators.heartbeat", n.Net, 5*time.Minute)
-	n.APIServer.AddRoute(service.Handler(), &sync.RWMutex{}, "health", "", n.HTTPLog)
+	if err := service.RegisterHeartbeat("network.validators.heartbeat", n.Net, 5*time.Minute); err != nil {
+		return fmt.Errorf("couldn't register heartbeat health check: %w", err)
+	}
+	isBootstrappedFunc := func() (interface{}, error) {
+		if pChainID, err := n.chainManager.Lookup("P"); err != nil {
+			return nil, errors.New("P-Chain not created")
+		} else if !n.chainManager.IsBootstrapped(pChainID) {
+			return nil, errors.New("P-Chain not bootstrapped")
+		}
+		if xChainID, err := n.chainManager.Lookup("X"); err != nil {
+			return nil, errors.New("X-Chain not created")
+		} else if !n.chainManager.IsBootstrapped(xChainID) {
+			return nil, errors.New("X-Chain not bootstrapped")
+		}
+		if cChainID, err := n.chainManager.Lookup("C"); err != nil {
+			return nil, errors.New("C-Chain not created")
+		} else if !n.chainManager.IsBootstrapped(cChainID) {
+			return nil, errors.New("C-Chain not bootstrapped")
+		}
+		return nil, nil
+	}
+	// Passes if the P, X and C chains are finished bootstrapping
+	if err := service.RegisterMonotonicCheckFunc("chains.default.bootstrapped", isBootstrappedFunc); err != nil {
+		return err
+	}
+	return n.APIServer.AddRoute(service.Handler(), &sync.RWMutex{}, "health", "", n.HTTPLog)
 }
 
 // initIPCAPI initializes the IPC API service
 // Assumes n.log and n.chainManager already initialized
-func (n *Node) initIPCAPI() {
-	if n.Config.IPCEnabled {
-		n.Log.Info("initializing IPC API")
-		service := ipcs.NewService(n.Log, n.chainManager, n.DecisionDispatcher, &n.APIServer)
-		n.APIServer.AddRoute(service, &sync.RWMutex{}, "ipcs", "", n.HTTPLog)
+func (n *Node) initIPCAPI() error {
+	if !n.Config.IPCEnabled {
+		n.Log.Info("skipping ipc API initializaion because it has been disabled")
+		return nil
 	}
+	n.Log.Info("initializing ipc API")
+	service := ipcs.NewService(n.Log, n.chainManager, n.DecisionDispatcher, &n.APIServer)
+	return n.APIServer.AddRoute(service, &sync.RWMutex{}, "ipcs", "", n.HTTPLog)
 }
 
 // Give chains and VMs aliases as specified by the genesis information
@@ -561,9 +601,13 @@ func (n *Node) Initialize(Config *Config, logger logging.Logger, logFactory logg
 	n.initBeacons()
 
 	// Start HTTP APIs
-	n.initAPIServer()   // Start the API Server
-	n.initKeystoreAPI() // Start the Keystore API
-	n.initMetricsAPI()  // Start the Metrics API
+	n.initAPIServer()                           // Start the API Server
+	if err := n.initKeystoreAPI(); err != nil { // Start the Keystore API
+		return fmt.Errorf("couldn't initialize keystore API: %w", err)
+	}
+	if err := n.initMetricsAPI(); err != nil { // Start the Metrics API
+		return fmt.Errorf("couldn't initialize metrics API: %w", err)
+	}
 
 	// initialize shared memory
 	n.initSharedMemory()
@@ -579,15 +623,25 @@ func (n *Node) Initialize(Config *Config, logger logging.Logger, logFactory logg
 	n.initEventDispatcher() // Set up the event dipatcher
 	n.initChainManager()    // Set up the chain manager
 
-	n.initAdminAPI()  // Start the Admin API
-	n.initInfoAPI()   // Start the Info API
-	n.initHealthAPI() // Start the Health API
-	n.initIPCAPI()    // Start the IPC API
-
-	if err := n.initAliases(); err != nil { // Set up aliases
-		return err
+	if err := n.initAdminAPI(); err != nil { // Start the Admin API
+		return fmt.Errorf("couldn't initialize admin API: %w", err)
 	}
-	return n.initChains() // Start the Platform chain
+	if err := n.initInfoAPI(); err != nil { // Start the Info API
+		return fmt.Errorf("couldn't initialize info API: %w", err)
+	}
+	if err := n.initHealthAPI(); err != nil { // Start the Health API
+		return fmt.Errorf("couldn't initialize health API: %w", err)
+	}
+	if err := n.initIPCAPI(); err != nil { // Start the IPC API
+		return fmt.Errorf("couldn't initialize ipc API: %w", err)
+	}
+	if err := n.initAliases(); err != nil { // Set up aliases
+		return fmt.Errorf("couldn't initialize aliases: %w", err)
+	}
+	if err := n.initChains(); err != nil { // Start the Platform chain
+		return fmt.Errorf("couldn't initialize chains: %w", err)
+	}
+	return nil
 }
 
 // Shutdown this node

snow/engine/avalanche/transitive.go

@@ -521,3 +521,8 @@ func (t *Transitive) sendRequest(vdr ids.ShortID, vtxID ids.ID) {
 
 	t.numVtxRequests.Set(float64(t.vtxReqs.Len())) // Tracks performance statistics
 }
+
+// IsBootstrapped returns true iff this chain is done bootstrapping
+func (t *Transitive) IsBootstrapped() bool {
+	return t.bootstrapped
+}

snow/engine/common/engine.go

@@ -14,6 +14,9 @@ type Engine interface {
 
 	// Return the context of the chain this engine is working on
 	Context() *snow.Context
+
+	// Returns true iff the chain is done bootstrapping
+	IsBootstrapped() bool
 }
 
 // Handler defines the functions that are acted on the node

snow/engine/common/test_engine.go

@@ -15,6 +15,7 @@ import (
 type EngineTest struct {
 	T *testing.T
 
+	CantIsBootstrapped,
 	CantStartup,
 	CantGossip,
 	CantShutdown,
@@ -43,6 +44,7 @@ type EngineTest struct {
 	CantQueryFailed,
 	CantChits bool
 
+	IsBootstrappedF                                    func() bool
 	ContextF                                           func() *snow.Context
 	StartupF, GossipF, ShutdownF                       func() error
 	NotifyF                                            func(Message) error
@@ -58,6 +60,8 @@ var _ Engine = &EngineTest{}
 
 // Default ...
 func (e *EngineTest) Default(cant bool) {
+	e.CantIsBootstrapped = cant
+
 	e.CantStartup = cant
 	e.CantGossip = cant
 	e.CantShutdown = cant
@@ -354,3 +358,14 @@ func (e *EngineTest) Chits(validatorID ids.ShortID, requestID uint32, containerI
 	}
 	return nil
 }
+
+// IsBootstrapped ...
+func (e *EngineTest) IsBootstrapped() bool {
+	if e.IsBootstrappedF != nil {
+		return e.IsBootstrappedF()
+	}
+	if e.CantIsBootstrapped && e.T != nil {
+		e.T.Fatalf("Unexpectedly called IsBootstrapped")
+	}
+	return false
+}

snow/engine/snowman/transitive.go

@@ -647,3 +647,8 @@ func (t *Transitive) deliver(blk snowman.Block) error {
 	t.numBlockedBlk.Set(float64(t.pending.Len()))
 	return t.errs.Err
 }
+
+// IsBootstrapped returns true iff this chain is done bootstrapping
+func (t *Transitive) IsBootstrapped() bool {
+	return t.bootstrapped
+}