chore(cleanup): Finish removing the AppConfig singleton and move Opti…

…ons processing to the config package (#630)

* Remove the AppConfig singleton

* Move Options handling to the config package

* Add NodeOptions test

cmd/masa-node/main.go

@@ -16,7 +16,6 @@ import (
 	"github.com/masa-finance/masa-oracle/pkg/api"
 	"github.com/masa-finance/masa-oracle/pkg/config"
 	"github.com/masa-finance/masa-oracle/pkg/db"
-	"github.com/masa-finance/masa-oracle/pkg/masacrypto"
 	"github.com/masa-finance/masa-oracle/pkg/staking"
 )
 
@@ -30,20 +29,19 @@ func main() {
 		os.Exit(0)
 	}
 
-	cfg := config.GetInstance()
-	cfg.LogConfig()
-	cfg.SetupLogging()
-
-	keyManager, err := masacrypto.NewKeyManager(cfg.PrivateKey, cfg.PrivateKeyFile)
+	cfg, err := config.GetConfig()
 	if err != nil {
-		logrus.Fatal("[-] Failed to initialize keys:", err)
+		logrus.Fatalf("[-] %v", err)
 	}
 
+	cfg.SetupLogging()
+	cfg.LogConfig()
+
 	// Create a cancellable context
 	ctx, cancel := context.WithCancel(context.Background())
 
 	if cfg.Faucet {
-		err := handleFaucet(cfg.RpcUrl, keyManager.EcdsaPrivKey)
+		err := handleFaucet(cfg.RpcUrl, cfg.KeyManager.EcdsaPrivKey)
 		if err != nil {
 			logrus.Errorf("[-] %v", err)
 			os.Exit(1)
@@ -54,7 +52,7 @@ func main() {
 	}
 
 	if cfg.StakeAmount != "" {
-		err := handleStaking(cfg.RpcUrl, keyManager.EcdsaPrivKey, cfg.StakeAmount)
+		err := handleStaking(cfg.RpcUrl, cfg.KeyManager.EcdsaPrivKey, cfg.StakeAmount)
 		if err != nil {
 			logrus.Warningf("%v", err)
 		} else {
@@ -64,7 +62,7 @@ func main() {
 	}
 
 	// Verify the staking event
-	isStaked, err := staking.VerifyStakingEvent(cfg.RpcUrl, keyManager.EthAddress)
+	isStaked, err := staking.VerifyStakingEvent(cfg.RpcUrl, cfg.KeyManager.EthAddress)
 	if err != nil {
 		logrus.Error(err)
 	}
@@ -73,7 +71,7 @@ func main() {
 		logrus.Warn("No staking event found for this address")
 	}
 
-	masaNodeOptions, workHandlerManager, pubKeySub := initOptions(cfg, keyManager)
+	masaNodeOptions, workHandlerManager, pubKeySub := config.InitOptions(cfg)
 	// Create a new OracleNode
 	masaNode, err := node.NewOracleNode(ctx, masaNodeOptions...)
 
@@ -91,14 +89,14 @@ func main() {
 
 	if cfg.AllowedPeer {
 		cfg.AllowedPeerId = masaNode.Host.ID().String()
-		cfg.AllowedPeerPublicKey = keyManager.HexPubKey
+		cfg.AllowedPeerPublicKey = cfg.KeyManager.HexPubKey
 		logrus.Infof("[+] Allowed peer with ID: %s and PubKey: %s", cfg.AllowedPeerId, cfg.AllowedPeerPublicKey)
 	} else {
 		logrus.Warn("[-] This node is not set as the allowed peer")
 	}
 
 	// Init cache resolver
-	db.InitResolverCache(masaNode, keyManager, cfg.AllowedPeerId, cfg.AllowedPeerPublicKey, cfg.Validator)
+	db.InitResolverCache(masaNode, cfg.KeyManager, cfg.AllowedPeerId, cfg.AllowedPeerPublicKey, cfg.Validator)
 
 	// Cancel the context when SIGINT is received
 	go handleSignals(cancel, masaNode, cfg)
@@ -122,7 +120,7 @@ func main() {
 		logrus.Errorf("[-] Error while getting node IP address from %v: %v", multiAddr, err)
 	}
 	// Display the welcome message with the multiaddress and IP address
-	config.DisplayWelcomeMessage(multiAddr.String(), ipAddr, keyManager.EthAddress, isStaked, cfg.Validator, cfg.TwitterScraper, cfg.TelegramScraper, cfg.DiscordScraper, cfg.WebScraper, versioning.ApplicationVersion, versioning.ProtocolVersion)
+	config.DisplayWelcomeMessage(multiAddr.String(), ipAddr, cfg.KeyManager.EthAddress, isStaked, cfg.Validator, cfg.TwitterScraper, cfg.TelegramScraper, cfg.DiscordScraper, cfg.WebScraper, versioning.ApplicationVersion, versioning.ProtocolVersion)
 
 	<-ctx.Done()
 }

node/options.go

@@ -32,7 +32,15 @@ type NodeOption struct {
 	Version              string
 	MasaDir              string
 	CachePath            string
-	KeyManager           *masacrypto.KeyManager
+
+	OracleProtocol       string
+	NodeDataSyncProtocol string
+	NodeGossipTopic      string
+	Rendezvous           string
+	WorkerProtocol       string
+	PageSize             int
+
+	KeyManager *masacrypto.KeyManager
 }
 
 type PubSubHandlers struct {
@@ -167,3 +175,39 @@ func WithKeyManager(km *masacrypto.KeyManager) Option {
 		o.KeyManager = km
 	}
 }
+
+func WithOracleProtocol(s string) Option {
+	return func(o *NodeOption) {
+		o.OracleProtocol = s
+	}
+}
+
+func WithNodeDataSyncProtocol(s string) Option {
+	return func(o *NodeOption) {
+		o.NodeDataSyncProtocol = s
+	}
+}
+
+func WithNodeGossipTopic(s string) Option {
+	return func(o *NodeOption) {
+		o.NodeGossipTopic = s
+	}
+}
+
+func WithRendezvous(s string) Option {
+	return func(o *NodeOption) {
+		o.Rendezvous = s
+	}
+}
+
+func WithWorkerProtocol(s string) Option {
+	return func(o *NodeOption) {
+		o.WorkerProtocol = s
+	}
+}
+
+func WithPageSize(size int) Option {
+	return func(o *NodeOption) {
+		o.PageSize = size
+	}
+}

node/oracle_node.go

@@ -26,7 +26,6 @@ import (
 
 	"github.com/masa-finance/masa-oracle/internal/versioning"
 	"github.com/masa-finance/masa-oracle/pkg/chain"
-	"github.com/masa-finance/masa-oracle/pkg/config"
 	myNetwork "github.com/masa-finance/masa-oracle/pkg/network"
 	"github.com/masa-finance/masa-oracle/pkg/pubsub"
 )
@@ -47,7 +46,6 @@ type OracleNode struct {
 	Blockchain    *chain.Chain
 	Options       NodeOption
 	Context       context.Context
-	Config        *config.AppConfig
 }
 
 // GetMultiAddrs returns the priority multiaddr for this node.
@@ -150,7 +148,7 @@ func NewOracleNode(ctx context.Context, opts ...Option) (*OracleNode, error) {
 		Options:       *o,
 	}
 
-	n.Protocol = n.protocolWithVersion(config.OracleProtocol)
+	n.Protocol = n.protocolWithVersion(n.Options.OracleProtocol)
 	return n, nil
 }
 
@@ -204,7 +202,7 @@ func (node *OracleNode) Start() (err error) {
 	logrus.Infof("[+] Starting node with ID: %s", node.GetMultiAddrs().String())
 
 	node.Host.SetStreamHandler(node.Protocol, node.handleStream)
-	node.Host.SetStreamHandler(node.protocolWithVersion(config.NodeDataSyncProtocol), node.ReceiveNodeData)
+	node.Host.SetStreamHandler(node.protocolWithVersion(node.Options.NodeDataSyncProtocol), node.ReceiveNodeData)
 
 	for pid, n := range node.Options.ProtocolHandlers {
 		node.Host.SetStreamHandler(pid, n)
@@ -215,7 +213,7 @@ func (node *OracleNode) Start() (err error) {
 	}
 
 	if node.Options.IsStaked {
-		node.Host.SetStreamHandler(node.protocolWithVersion(config.NodeGossipTopic), node.GossipNodeData)
+		node.Host.SetStreamHandler(node.protocolWithVersion(node.Options.NodeGossipTopic), node.GossipNodeData)
 	}
 
 	node.Host.Network().Notify(node.NodeTracker)
@@ -236,7 +234,7 @@ func (node *OracleNode) Start() (err error) {
 		return err
 	}
 
-	err = myNetwork.EnableMDNS(node.Host, config.Rendezvous, node.PeerChan)
+	err = myNetwork.EnableMDNS(node.Host, node.Options.Rendezvous, node.PeerChan)
 	if err != nil {
 		return err
 	}

node/oracle_node_listener.go

@@ -14,7 +14,6 @@ import (
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/sirupsen/logrus"
 
-	"github.com/masa-finance/masa-oracle/pkg/config"
 	pubsub2 "github.com/masa-finance/masa-oracle/pkg/pubsub"
 )
 
@@ -42,7 +41,7 @@ func (node *OracleNode) ListenToNodeTracker() {
 				continue
 			}
 			// Publish the JSON data on the node.topic
-			err = node.PublishTopic(config.NodeGossipTopic, jsonData)
+			err = node.PublishTopic(node.Options.NodeGossipTopic, jsonData)
 			if err != nil {
 				logrus.Errorf("[-] Error publishing node data: %v", err)
 			}
@@ -88,10 +87,10 @@ type NodeDataPage struct {
 func (node *OracleNode) SendNodeDataPage(allNodeData []pubsub2.NodeData, stream network.Stream, pageNumber int) {
 	logrus.Debugf("[+] SendNodeDataPage --> %s: Page: %d", stream.Conn().RemotePeer(), pageNumber)
 	totalRecords := len(allNodeData)
-	totalPages := int(math.Ceil(float64(totalRecords) / config.PageSize))
+	totalPages := int(math.Ceil(float64(totalRecords) / float64(node.Options.PageSize)))
 
-	startIndex := pageNumber * config.PageSize
-	endIndex := startIndex + config.PageSize
+	startIndex := pageNumber * node.Options.PageSize
+	endIndex := startIndex + node.Options.PageSize
 	if endIndex > totalRecords {
 		endIndex = totalRecords
 	}
@@ -133,9 +132,9 @@ func (node *OracleNode) SendNodeData(peerID peer.ID) {
 		nodeData = node.NodeTracker.GetUpdatedNodes(sinceTime)
 	}
 	totalRecords := len(nodeData)
-	totalPages := int(math.Ceil(float64(totalRecords) / float64(config.PageSize)))
+	totalPages := int(math.Ceil(float64(totalRecords) / float64(node.Options.PageSize)))
 
-	stream, err := node.Host.NewStream(node.Context, peerID, node.protocolWithVersion(config.NodeDataSyncProtocol))
+	stream, err := node.Host.NewStream(node.Context, peerID, node.protocolWithVersion(node.Options.NodeDataSyncProtocol))
 	if err != nil {
 		// node.NodeTracker.RemoveNodeData(peerID.String())
 		return

node/protocol.go

@@ -9,7 +9,6 @@ import (
 	"github.com/libp2p/go-libp2p/core/network"
 	"github.com/libp2p/go-libp2p/core/peer"
 	"github.com/libp2p/go-libp2p/core/protocol"
-	"github.com/masa-finance/masa-oracle/pkg/config"
 )
 
 const (
@@ -50,7 +49,7 @@ func (node *OracleNode) subscribeToTopics() error {
 	}
 
 	// Subscribe to NodeGossipTopic to participate in the network's gossip protocol.
-	if err := node.SubscribeTopic(config.NodeGossipTopic, node.NodeTracker, false); err != nil {
+	if err := node.SubscribeTopic(node.Options.NodeGossipTopic, node.NodeTracker, false); err != nil {
 		return err
 	}
 

pkg/config/app.go

@@ -1,14 +1,15 @@
 package config
 
 import (
+	"fmt"
 	"log"
 	"os/user"
 	"path/filepath"
 	"reflect"
 	"strings"
-	"sync"
 
 	"github.com/masa-finance/masa-oracle/internal/versioning"
+	"github.com/masa-finance/masa-oracle/pkg/masacrypto"
 
 	"github.com/gotd/contrib/bg"
 	"github.com/joho/godotenv"
@@ -17,11 +18,6 @@ import (
 	"github.com/spf13/viper"
 )
 
-var (
-	instance *AppConfig
-	once     sync.Once
-)
-
 // AppConfig represents the configuration settings for the application.
 // It holds various parameters and settings that control the behavior and runtime environment of the application.
 // The fields in this struct are tagged with `mapstructure` to facilitate configuration loading from various sources
@@ -63,44 +59,43 @@ type AppConfig struct {
 	WebScraper         bool   `mapstructure:"webScraper"`
 	APIEnabled         bool   `mapstructure:"api_enabled"`
 
+	KeyManager   *masacrypto.KeyManager
 	TelegramStop bg.StopFunc
 }
 
-// GetInstance returns the singleton instance of AppConfig.
-//
-// If the instance has not been initialized yet, GetInstance will initialize it by:
-// 1. Creating a new AppConfig instance.
-// 2. Setting default configuration values.
-// 3. Overriding defaults with values from environment variables.
-// 4. Overriding defaults and environment variables with values from the configuration file.
-// 5. Overriding all previous values with command-line flags.
-// 6. Unmarshalling the configuration into the AppConfig instance.
+// GetConfig parses and fills in the AppConfig. The configuration values are generated
+// by the following steps in order:
 //
-// If the unmarshalling fails, the instance is set to nil.
+// 1. Set default configuration values.
+// 2. Override with values from environment variables.
+// 3. Override with values from the configuration file.
+// 4. Override with command-line flags.
 //
-// Subsequent calls to GetInstance will return the same initialized instance.
-func GetInstance() *AppConfig {
-	once.Do(func() {
-		instance = &AppConfig{}
-
-		instance.setDefaultConfig()
-		// TODO Shouldn't the env vars override the file config, instead of the other way around?
-		instance.setEnvVariableConfig()
+// In case of any errors it returns nill with an error object
+func GetConfig() (*AppConfig, error) {
+	instance := &AppConfig{}
+	instance.setDefaultConfig()
+	// TODO Shouldn't the env vars override the file config, instead of the other way around?
+	instance.setEnvVariableConfig()
+	instance.setFileConfig(viper.GetString("FILE_PATH"))
+
+	if err := instance.setCommandLineConfig(); err != nil {
+		return nil, err
+	}
 
-		instance.setFileConfig(viper.GetString("FILE_PATH"))
+	if err := viper.Unmarshal(instance); err != nil {
+		return nil, fmt.Errorf("Unable to unmarshal config into struct, %v", err)
+	}
 
-		if err := instance.setCommandLineConfig(); err != nil {
-			logrus.Fatal(err)
-		}
+	instance.APIEnabled = viper.GetBool("api_enabled")
 
-		if err := viper.Unmarshal(instance); err != nil {
-			logrus.Errorf("[-] Unable to unmarshal config into struct, %v", err)
-			instance = nil
-		}
+	keyManager, err := masacrypto.NewKeyManager(instance.PrivateKey, instance.PrivateKeyFile)
+	if err != nil {
+		return nil, fmt.Errorf("Failed to initialize keys: %v", err)
+	}
+	instance.KeyManager = keyManager
 
-		instance.APIEnabled = viper.GetBool("api_enabled")
-	})
-	return instance
+	return instance, nil
 }
 
 // setDefaultConfig sets the default configuration values for the AppConfig instance.

pkg/config/config_suite_test.go

@@ -0,0 +1,13 @@
+package config_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestConfig(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Config Suite")
+}