From 23ff95dd48f4c8909425edfd9fd8a8a545d771af Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Sun, 6 Nov 2022 18:02:59 +0530 Subject: [PATCH 01/21] Bug fix and improves to Node.Start() and Node.Stop() - Create Meaningful error macros - Make IsRunning flag atomic for safety purpose. - Fix https://github.com/deso-protocol/core/issues/413 - Add utility functions for change the status of the node. --- cmd/node.go | 117 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 104 insertions(+), 13 deletions(-) diff --git a/cmd/node.go b/cmd/node.go index 06f3cbedc..7659a8c2b 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -2,12 +2,14 @@ package cmd import ( "encoding/hex" + "errors" "flag" "fmt" "net" "os" "os/signal" "sync" + "sync/atomic" "syscall" "time" @@ -26,6 +28,20 @@ import ( "gopkg.in/DataDog/dd-trace-go.v1/profiler" ) +var ( + // ErrAlreadyStarted is returned when somebody tries to start an already + // running service. + ErrAlreadyStarted = errors.New("already started") + // ErrAlreadyStopped is returned when somebody tries to stop an already + // stopped service (without resetting it). + ErrAlreadyStopped = errors.New("already stopped") + // ErrNodeNeverStarted is returned when somebody tries to find or changge the + // running status of the server without never starting it once. + ErrNodeNeverStarted = errors.New("never started the node instance") + // Error if the atomic compare of swap of the node status fails + ErrFailedtoChangeNodeStatus = errors.New("Failed to change the runnnin status of the Node") +) + type Node struct { Server *lib.Server ChainDB *badger.DB @@ -34,9 +50,11 @@ type Node struct { Config *Config Postgres *lib.Postgres - // IsRunning is false when a NewNode is created, set to true on Start(), set to false - // after Stop() is called. Mainly used in testing. - IsRunning bool + // running is false when a NewNode is created, it is set to true on node.Start(), + // set to false after Stop() is called. Mainly used in testing. + // Use the convinience methods changeRunningStatus/StatusStartToStop/StatusStopToStart to change node status + // Use *Node.IsRunning() to retreive the node running status. + running *atomic.Bool // runningMutex is held whenever we call Start() or Stop() on the node. runningMutex sync.Mutex @@ -50,6 +68,7 @@ type Node struct { func NewNode(config *Config) *Node { result := Node{} + result.Config = config result.Params = config.Params result.internalExitChan = make(chan struct{}) @@ -73,6 +92,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { node.internalExitChan = make(chan struct{}) node.nodeMessageChan = make(chan lib.NodeMessage) + node.running = &atomic.Bool{} // listenToNodeMessages handles the messages received from the engine through the nodeMessageChan. go node.listenToNodeMessages() @@ -251,7 +271,10 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { } } } - node.IsRunning = true + // Change nodes running status to start + if err := node.StatusStopToStart(); err != nil { + panic(err) + } if shouldRestart { if node.nodeMessageChan != nil { @@ -284,14 +307,74 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { }() } +// Changes node running status from true to false +func (node *Node) StatusStartToStop() error { + if err := changeRunningStatus(node, true, false); err != nil { + glog.Errorf("Error stopping Node -- %v", err) + return err + } + return nil +} + +// Changes node running status from false to true +func (node *Node) StatusStopToStart() error { + if err := changeRunningStatus(node, false, true); err != nil { + glog.Errorf("Error stopping Node -- %v", err) + return err + } + return nil +} + +// changes the running status of the node +func changeRunningStatus(node *Node, currentStatus, newStatus bool) error { + // Cannot change status of the node before initializing it + // Node is initialized only after calling node.Start() + if node.running == nil { + return ErrNodeNeverStarted + } + + // Cannot change the status of the current and the new status are the same! + if currentStatus == newStatus { + return errors.New("error invalid input. current and new status values should be different") + } + + // No need to change the status if the new status is already the running status + if newStatus == node.running.Load() { + // cannot change running status to true if the node is already running + if newStatus { + return ErrAlreadyStarted + } + // cannot change running status to false if the node is already stopped + return ErrAlreadyStopped + } + + success := node.running.CompareAndSwap(currentStatus, newStatus) + if !success { + return ErrFailedtoChangeNodeStatus + } + + return nil +} + +// Helper function to check if the node is running. +// returns false if node.running is not initalized. +func (node *Node) IsRunning() bool { + if node.running == nil { + return false + } + return node.running.Load() +} + func (node *Node) Stop() { node.runningMutex.Lock() defer node.runningMutex.Unlock() - if !node.IsRunning { + // Change nodes running status to stop + if err := node.StatusStartToStop(); err != nil { + glog.Errorf("Error stopping Node -- %v", err) return } - node.IsRunning = false + glog.Infof(lib.CLog(lib.Yellow, "Node is shutting down. This might take a minute. Please don't "+ "close the node now or else you might corrupt the state.")) @@ -353,19 +436,27 @@ func (node *Node) listenToNodeMessages(exitChannels ...*chan struct{}) { case <-node.internalExitChan: break case operation := <-node.nodeMessageChan: - if !node.IsRunning { - panic("Node.listenToNodeMessages: Node is currently not running, nodeMessageChan should've not been called!") - } - glog.Infof("Node.listenToNodeMessages: Stopping node") - node.Stop() - glog.Infof("Node.listenToNodeMessages: Finished stopping node") switch operation { + case lib.NodeRestart: + if !node.IsRunning() { + panic("Node.listenToNodeMessages: Node is currently not running, nodeMessageChan should've not been called!") + } + + glog.Infof("Node.listenToNodeMessages: Restarting node.") + glog.Infof("Node.listenToNodeMessages: Stopping node") + node.Stop() + glog.Infof("Node.listenToNodeMessages: Finished stopping node") + case lib.NodeErase: + glog.Infof("Node.listenToNodeMessages: Restarting node with a Database erase.") + glog.Infof("Node.listenToNodeMessages: Stopping node") + node.Stop() + glog.Infof("Node.listenToNodeMessages: Finished stopping node") + if err := os.RemoveAll(node.Config.DataDirectory); err != nil { glog.Fatal(lib.CLog(lib.Red, fmt.Sprintf("IMPORTANT: Problem removing the directory (%v), you "+ "should run `rm -rf %v` to delete it manually. Error: (%v)", node.Config.DataDirectory, node.Config.DataDirectory, err))) - return } } From 75d34607fa946d726daca07d97d97f3cec1f9974 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Sun, 6 Nov 2022 18:06:15 +0530 Subject: [PATCH 02/21] Unit tests for the utility functions Unit tests for the following utility functions to change the status of the node. - *Node.IsRunning() - *Node.StatusStartToStop() - *Node.StatusStopToStart() - changeRunningStatus --- cmd/node_test.go | 155 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 cmd/node_test.go diff --git a/cmd/node_test.go b/cmd/node_test.go new file mode 100644 index 000000000..6e63d886f --- /dev/null +++ b/cmd/node_test.go @@ -0,0 +1,155 @@ +package cmd + +import ( + "os" + "testing" + + //"github.com/deso-protocol/core/lib" + //"github.com/deso-protocol/core/lib" + "github.com/stretchr/testify/require" +) + +func TestNodeIsRunning(t *testing.T) { + testDir := getTestDirectory(t, "test_node_is_running") + defer os.RemoveAll(testDir) + + testConfig := GenerateTestConfig(t, 18000, testDir, 10) + + testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} + + testNode := NewNode(&testConfig) + + // expected running status should be false before the node is started + expectedRunningStatus := false + actualRunningstatus := testNode.IsRunning() + require.Equal(t, expectedRunningStatus, actualRunningstatus) + + // Start the node + testNode.Start() + // expected running status should be true after the server is started + expectedRunningStatus = true + actualRunningstatus = testNode.IsRunning() + require.Equal(t, expectedRunningStatus, actualRunningstatus) + + // stop the node + testNode.Stop() + expectedRunningStatus = false + actualRunningstatus = testNode.IsRunning() + require.Equal(t, expectedRunningStatus, actualRunningstatus) + +} + +func TestNodeStatusStopToStart(t *testing.T) { + testDir := getTestDirectory(t, "test_node_change_running_status") + defer os.RemoveAll(testDir) + + testConfig := GenerateTestConfig(t, 18000, testDir, 10) + + testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} + + testNode := NewNode(&testConfig) + + // Need to call node.start() to atleast once to be able to change the running status of a node + // Cannot change status of node which never got initialized in the first place + expErr := ErrNodeNeverStarted + actualErr := testNode.StatusStopToStart() + require.ErrorIs(t, expErr, actualErr) + + // start the server + // Cannot change the running status of a node from stop to start while + // the node is still running + testNode.Start() + expErr = ErrAlreadyStarted + actualErr = testNode.StatusStopToStart() + require.ErrorIs(t, expErr, actualErr) + + // Stop the node + // Should successfully change the status from stop to start after the node is stopped + // expect no error + testNode.Stop() + actualErr = testNode.StatusStopToStart() + require.NoError(t, actualErr) + // Once the running flag is changed, the isRunning function should return true + require.Equal(t, true, testNode.IsRunning()) + +} + +func TestNodeStatusStartToStop(t *testing.T) { + testDir := getTestDirectory(t, "test_node_change_running_status") + defer os.RemoveAll(testDir) + + testConfig := GenerateTestConfig(t, 18000, testDir, 10) + + testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} + + testNode := NewNode(&testConfig) + + // Need to call node.start() to atleast once to be able to change node status + // Cannot change status of node which never got initialized in the first place + expErr := ErrNodeNeverStarted + actualErr := testNode.StatusStartToStop() + require.ErrorIs(t, expErr, actualErr) + + // start the node + // Should be able to successfully change the status of the node + // Once the server is started + testNode.Start() + + actualErr = testNode.StatusStartToStop() + require.NoError(t, actualErr) + + // stop the node + testNode.Stop() + + expErr = ErrAlreadyStopped + actualErr = testNode.StatusStartToStop() + require.ErrorIs(t, expErr, actualErr) +} + +func TestNodeChangeRunningStatus(t *testing.T) { + testDir := getTestDirectory(t, "test_node_change_running_status") + defer os.RemoveAll(testDir) + + testConfig := GenerateTestConfig(t, 18000, testDir, 10) + + testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} + + testNode := NewNode(&testConfig) + + // Need to call node.start() to atleast once to be able to change the running status of a node + // Cannot change status of node which never got initialized in the first place + expError := ErrNodeNeverStarted + // Changing status from false to true + actualError := changeRunningStatus(testNode, false, true) + require.ErrorIs(t, expError, actualError) + // Changing status from true to false + actualError = changeRunningStatus(testNode, true, false) + require.ErrorIs(t, expError, actualError) + + // start the node + testNode.Start() + + // Cannot change node running status to true while it's running + // its already set to true + expError = ErrAlreadyStarted + actualError = changeRunningStatus(testNode, false, true) + require.ErrorIs(t, expError, actualError) + + // Should be able to change the node running status to false once started + actualError = changeRunningStatus(testNode, true, false) + require.NoError(t, actualError) + + // stop the node + testNode.Stop() + + // Cannot change the running status of the node to false + // when its not running + expError = ErrAlreadyStopped + actualError = changeRunningStatus(testNode, true, false) + require.ErrorIs(t, expError, actualError) + + // Should be able to change the running status of the node to true + // after the node is stopped + actualError = changeRunningStatus(testNode, false, true) + require.NoError(t, actualError) +} From 85480f515c16bd37867235ea30e491a142d07178 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Sun, 6 Nov 2022 18:08:49 +0530 Subject: [PATCH 03/21] Duplicating test utility functions Duplicating two test utility functions to avoid the cyclic issues. Cannot import integration_testing in cmd/ packager, this creates a cycle. --- cmd/config.go | 61 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 64b2e8c00..a38af694c 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -1,13 +1,23 @@ package cmd import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + "github.com/deso-protocol/core/lib" "github.com/golang/glog" "github.com/spf13/viper" - "os" - "path/filepath" + "github.com/stretchr/testify/require" ) +// Global variable that determines the max tip blockheight of syncing nodes throughout test cases. +const MaxSyncBlockHeight = 1500 + +// Global variable that allows setting node configuration hypersync snapshot period. +const HyperSyncSnapshotPeriod = 1000 + type Config struct { // Core Params *lib.DeSoParams @@ -220,3 +230,50 @@ func (config *Config) Print() { glog.Infof("Rate Limit Feerate: %d", config.RateLimitFeerate) glog.Infof("Min Feerate: %d", config.MinFeerate) } + +// GenerateTestConfig creates a default config for a node, with provided port, db directory, and number of max peers. +// It's usually the first step to starting a node.\ +// FIXME: Duplicating the function to avoid cyclic error in node_test.go. Need to centralizer it. +func GenerateTestConfig(t *testing.T, port uint32, dataDir string, maxPeers uint32) Config { + config := Config{} + params := lib.DeSoMainnetParams + + params.DNSSeeds = []string{} + config.Params = ¶ms + config.ProtocolPort = uint16(port) + // "/Users/piotr/data_dirs/n98_1" + config.DataDirectory = dataDir + if err := os.MkdirAll(config.DataDirectory, os.ModePerm); err != nil { + t.Fatalf("Could not create data directories (%s): %v", config.DataDirectory, err) + } + config.TXIndex = false + config.HyperSync = false + config.MaxSyncBlockHeight = 0 + config.ConnectIPs = []string{} + config.PrivateMode = true + config.GlogV = 0 + config.GlogVmodule = "*bitcoin_manager*=0,*balance*=0,*view*=0,*frontend*=0,*peer*=0,*addr*=0,*network*=0,*utils*=0,*connection*=0,*main*=0,*server*=0,*mempool*=0,*miner*=0,*blockchain*=0" + config.MaxInboundPeers = maxPeers + config.TargetOutboundPeers = maxPeers + config.StallTimeoutSeconds = 900 + config.MinFeerate = 1000 + config.OneInboundPerIp = false + config.MaxBlockTemplatesCache = 100 + config.MaxSyncBlockHeight = 100 + config.MinBlockUpdateInterval = 10 + config.SnapshotBlockHeightPeriod = HyperSyncSnapshotPeriod + config.MaxSyncBlockHeight = MaxSyncBlockHeight + config.SyncType = lib.NodeSyncTypeBlockSync + //config.ArchivalMode = true + + return config +} + +func getTestDirectory(t *testing.T, testName string) string { + require := require.New(t) + dbDir, err := ioutil.TempDir("", testName) + if err != nil { + require.NoError(err) + } + return dbDir +} From fd9bcfb434c231a31a9553c4550c15f6d4b1df22 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Sun, 6 Nov 2022 18:10:53 +0530 Subject: [PATCH 04/21] Change test util names Changing the names of the test utility functions to improve the readability. 1. Changing getDirectory to getTestDirectory. 2. Changing generateConfig to GenerateTestConfig() --- integration_testing/tools.go | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/integration_testing/tools.go b/integration_testing/tools.go index b393c0e44..8a2f83802 100644 --- a/integration_testing/tools.go +++ b/integration_testing/tools.go @@ -3,6 +3,13 @@ package integration_testing import ( "encoding/hex" "fmt" + "io/ioutil" + "os" + "reflect" + "sort" + "testing" + "time" + "github.com/btcsuite/btcd/wire" "github.com/deso-protocol/core/cmd" "github.com/deso-protocol/core/lib" @@ -10,12 +17,6 @@ import ( "github.com/golang/glog" "github.com/pkg/errors" "github.com/stretchr/testify/require" - "io/ioutil" - "os" - "reflect" - "sort" - "testing" - "time" ) // This testing suite is the first serious attempt at making a comprehensive functional testing framework for DeSo nodes. @@ -41,18 +42,18 @@ const MaxSyncBlockHeight = 1500 const HyperSyncSnapshotPeriod = 1000 // get a random temporary directory. -func getDirectory(t *testing.T) string { +func getTestDirectory(t *testing.T, testName string) string { require := require.New(t) - dbDir, err := ioutil.TempDir("", "badgerdb") + dbDir, err := ioutil.TempDir("", testName) if err != nil { require.NoError(err) } return dbDir } -// generateConfig creates a default config for a node, with provided port, db directory, and number of max peers. +// GenerateTestConfig creates a default config for a node, with provided port, db directory, and number of max peers. // It's usually the first step to starting a node. -func generateConfig(t *testing.T, port uint32, dataDir string, maxPeers uint32) *cmd.Config { +func GenerateTestConfig(t *testing.T, port uint32, dataDir string, maxPeers uint32) *cmd.Config { config := &cmd.Config{} params := lib.DeSoMainnetParams @@ -353,7 +354,7 @@ func computeNodeStateChecksum(t *testing.T, node *cmd.Node, blockHeight uint64) // Stop the provided node. func shutdownNode(t *testing.T, node *cmd.Node) *cmd.Node { - if !node.IsRunning { + if !node.IsRunning() { t.Fatalf("shutdownNode: can't shutdown, node is already down") } @@ -364,7 +365,7 @@ func shutdownNode(t *testing.T, node *cmd.Node) *cmd.Node { // Start the provided node. func startNode(t *testing.T, node *cmd.Node) *cmd.Node { - if node.IsRunning { + if node.IsRunning() { t.Fatalf("startNode: node is already running") } // Start the node. @@ -372,9 +373,9 @@ func startNode(t *testing.T, node *cmd.Node) *cmd.Node { return node } -// Restart the provided node.A +// Restart the provided node. func restartNode(t *testing.T, node *cmd.Node) *cmd.Node { - if !node.IsRunning { + if !node.IsRunning() { t.Fatalf("shutdownNode: can't restart, node already down") } From 20e57e8fcc08bb58d9747300961c959da481b58d Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Sun, 6 Nov 2022 18:12:01 +0530 Subject: [PATCH 05/21] Updating test utility function names Updating test utility function names to match the latest updates. --- integration_testing/blocksync_test.go | 65 +++++++------- integration_testing/hypersync_test.go | 119 +++++++++++++------------ integration_testing/migrations_test.go | 13 +-- integration_testing/mining_test.go | 9 +- integration_testing/rollback_test.go | 15 ++-- integration_testing/txindex_test.go | 23 ++--- 6 files changed, 125 insertions(+), 119 deletions(-) diff --git a/integration_testing/blocksync_test.go b/integration_testing/blocksync_test.go index af1bc3637..5b4e07c9f 100644 --- a/integration_testing/blocksync_test.go +++ b/integration_testing/blocksync_test.go @@ -2,31 +2,32 @@ package integration_testing import ( "fmt" + "os" + "testing" + "github.com/deso-protocol/core/cmd" "github.com/deso-protocol/core/lib" "github.com/stretchr/testify/require" - "os" - "testing" ) // TestSimpleBlockSync test if a node can successfully sync from another node: -// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks. -// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator. -// 3. bridge node1 and node2 -// 4. node2 syncs MaxSyncBlockHeight blocks from node1. -// 5. compare node1 db matches node2 db. +// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks. +// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator. +// 3. bridge node1 and node2 +// 4. node2 syncs MaxSyncBlockHeight blocks from node1. +// 5. compare node1 db matches node2 db. func TestSimpleBlockSync(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) + dbDir1 := getTestDirectory(t, "block_sync_test") + dbDir2 := getTestDirectory(t, "block_sync_test_dir2") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := generateConfig(t, 18001, dbDir2, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} @@ -54,25 +55,25 @@ func TestSimpleBlockSync(t *testing.T) { } // TestSimpleSyncRestart tests if a node can successfully restart while syncing blocks. -// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks. -// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator. -// 3. bridge node1 and node2 -// 4. node2 syncs between 10 and MaxSyncBlockHeight blocks from node1. +// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks. +// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator. +// 3. bridge node1 and node2 +// 4. node2 syncs between 10 and MaxSyncBlockHeight blocks from node1. // 5. node2 disconnects from node1 and reboots. // 6. node2 reconnects with node1 and syncs remaining blocks. -// 7. compare node1 db matches node2 db. +// 7. compare node1 db matches node2 db. func TestSimpleSyncRestart(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_simple_sync_restart") + dbDir2 := getTestDirectory(t, "test_simple_sync_restart_2") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := generateConfig(t, 18001, dbDir2, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} @@ -105,30 +106,30 @@ func TestSimpleSyncRestart(t *testing.T) { // TestSimpleSyncDisconnectWithSwitchingToNewPeer tests if a node can successfully restart while syncing blocks, and // then connect to a different node and sync the remaining blocks. -// 1. Spawn three nodes node1, node2, node3 with max block height of MaxSyncBlockHeight blocks. -// 2. node1 and node3 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator. -// 3. bridge node1 and node2 -// 4. node2 syncs between 10 and MaxSyncBlockHeight blocks from node1. +// 1. Spawn three nodes node1, node2, node3 with max block height of MaxSyncBlockHeight blocks. +// 2. node1 and node3 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator. +// 3. bridge node1 and node2 +// 4. node2 syncs between 10 and MaxSyncBlockHeight blocks from node1. // 5. node2 disconnects from node1 and reboots. // 6. node2 reconnects with node3 and syncs remaining blocks. -// 7. compare node1 state matches node2 state. -// 8. compare node3 state matches node2 state. +// 7. compare node1 state matches node2 state. +// 8. compare node3 state matches node2 state. func TestSimpleSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) - dbDir3 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_simple_sync_disconnect_switch_new_peer") + dbDir2 := getTestDirectory(t, "test_simple_sync_disconnect_switch_new_peer_2") + dbDir3 := getTestDirectory(t, "test_simple_sync_disconnect_switch_new_peer_3") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := generateConfig(t, 18001, dbDir2, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync - config3 := generateConfig(t, 18002, dbDir3, 10) + config3 := GenerateTestConfig(t, 18002, dbDir3, 10) config3.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} diff --git a/integration_testing/hypersync_test.go b/integration_testing/hypersync_test.go index fc0b9bd87..3fde8a4d8 100644 --- a/integration_testing/hypersync_test.go +++ b/integration_testing/hypersync_test.go @@ -2,31 +2,32 @@ package integration_testing import ( "fmt" + "os" + "testing" + "github.com/deso-protocol/core/cmd" "github.com/deso-protocol/core/lib" "github.com/stretchr/testify/require" - "os" - "testing" ) // TestSimpleHyperSync test if a node can successfully hyper sync from another node: -// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks, and snapshot period of HyperSyncSnapshotPeriod. -// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator and builds ancestral records. -// 3. bridge node1 and node2. -// 4. node2 hypersyncs from node1 -// 5. once done, compare node1 state, db, and checksum matches node2. +// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks, and snapshot period of HyperSyncSnapshotPeriod. +// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator and builds ancestral records. +// 3. bridge node1 and node2. +// 4. node2 hypersyncs from node1 +// 5. once done, compare node1 state, db, and checksum matches node2. func TestSimpleHyperSync(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) + dbDir1 := getTestDirectory(t, "simple_hyper_sync") + dbDir2 := getTestDirectory(t, "simple_hyper_sync_2") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := generateConfig(t, 18001, dbDir2, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSync config1.HyperSync = true @@ -58,28 +59,28 @@ func TestSimpleHyperSync(t *testing.T) { } // TestHyperSyncFromHyperSyncedNode test if a node can successfully hypersync from another hypersynced node: -// 1. Spawn three nodes node1, node2, node3 with max block height of MaxSyncBlockHeight blocks, and snapshot period of HyperSyncSnapshotPeriod -// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator and builds ancestral records. -// 3. bridge node1 and node2. -// 4. node2 hypersyncs state. -// 5. once done, bridge node3 and node2 so that node3 hypersyncs from node2. -// 6. compare node1 state, db, and checksum matches node2, and node3. +// 1. Spawn three nodes node1, node2, node3 with max block height of MaxSyncBlockHeight blocks, and snapshot period of HyperSyncSnapshotPeriod +// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator and builds ancestral records. +// 3. bridge node1 and node2. +// 4. node2 hypersyncs state. +// 5. once done, bridge node3 and node2 so that node3 hypersyncs from node2. +// 6. compare node1 state, db, and checksum matches node2, and node3. func TestHyperSyncFromHyperSyncedNode(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) - dbDir3 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_hyper_synced_node") + dbDir2 := getTestDirectory(t, "test_hyper_synced_node_2") + dbDir3 := getTestDirectory(t, "test_hyper_synced_node_3") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := generateConfig(t, 18001, dbDir2, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSyncArchival - config3 := generateConfig(t, 18002, dbDir3, 10) + config3 := GenerateTestConfig(t, 18002, dbDir3, 10) config3.SyncType = lib.NodeSyncTypeHyperSyncArchival config1.HyperSync = true @@ -128,23 +129,23 @@ func TestHyperSyncFromHyperSyncedNode(t *testing.T) { } // TestSimpleHyperSyncRestart test if a node can successfully hyper sync from another node: -// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks, and snapshot period of HyperSyncSnapshotPeriod. -// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator and builds ancestral records. -// 3. bridge node1 and node2. -// 4. node2 hyper syncs a portion of the state from node1 and then restarts. -// 5. node2 reconnects to node1 and hypersyncs again. -// 6. Once node2 finishes sync, compare node1 state, db, and checksum matches node2. +// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks, and snapshot period of HyperSyncSnapshotPeriod. +// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator and builds ancestral records. +// 3. bridge node1 and node2. +// 4. node2 hyper syncs a portion of the state from node1 and then restarts. +// 5. node2 reconnects to node1 and hypersyncs again. +// 6. Once node2 finishes sync, compare node1 state, db, and checksum matches node2. func TestSimpleHyperSyncRestart(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_hyper_sync_restart") + dbDir2 := getTestDirectory(t, "test_hyper_sync_restart") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := generateConfig(t, 18000, dbDir1, 10) - config2 := generateConfig(t, 18001, dbDir2, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config1.HyperSync = true config1.SyncType = lib.NodeSyncTypeBlockSync @@ -183,28 +184,28 @@ func TestSimpleHyperSyncRestart(t *testing.T) { } // TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer tests if a node can successfully restart while hypersyncing. -// 1. Spawn three nodes node1, node2, and node3 with max block height of MaxSyncBlockHeight blocks. -// 2. node1, node3 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator. -// 3. bridge node1 and node2 -// 4. node2 hypersyncs from node1 but we restart node2 midway. -// 5. after restart, bridge node2 with node3 and resume hypersync. -// 6. once node2 finishes, compare node1, node2, node3 state, db, and checksums are identical. +// 1. Spawn three nodes node1, node2, and node3 with max block height of MaxSyncBlockHeight blocks. +// 2. node1, node3 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator. +// 3. bridge node1 and node2 +// 4. node2 hypersyncs from node1 but we restart node2 midway. +// 5. after restart, bridge node2 with node3 and resume hypersync. +// 6. once node2 finishes, compare node1, node2, node3 state, db, and checksums are identical. func TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) - dbDir3 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_hyper_sync_disconnect_switch_peer") + dbDir2 := getTestDirectory(t, "test_hyper_sync_disconnect_switch_peer_2") + dbDir3 := getTestDirectory(t, "test_hyper_sync_disconnect_switch_peer_3") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := generateConfig(t, 18001, dbDir2, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSyncArchival - config3 := generateConfig(t, 18002, dbDir3, 10) + config3 := GenerateTestConfig(t, 18002, dbDir3, 10) config3.SyncType = lib.NodeSyncTypeBlockSync config1.HyperSync = true @@ -265,13 +266,13 @@ func TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { // require := require.New(t) // _ = require // -// dbDir1 := getDirectory(t) -// dbDir2 := getDirectory(t) +// dbDir1 := getTestDirectory(t) +// dbDir2 := getTestDirectory(t) // defer os.RemoveAll(dbDir1) // defer os.RemoveAll(dbDir2) // -// config1 := generateConfig(t, 18000, dbDir1, 10) -// config2 := generateConfig(t, 18001, dbDir2, 10) +// config1 := GenerateTestConfig(t, 18000, dbDir1, 10) +// config2 := GenerateTestConfig(t, 18001, dbDir2, 10) // // config1.HyperSync = true // config2.HyperSync = true @@ -314,13 +315,13 @@ func TestArchivalMode(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_archival_mode") + dbDir2 := getTestDirectory(t, "test_archival_mode_2") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := generateConfig(t, 18000, dbDir1, 10) - config2 := generateConfig(t, 18001, dbDir2, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config1.HyperSync = true config2.HyperSync = true @@ -357,16 +358,16 @@ func TestBlockSyncFromArchivalModeHyperSync(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) - dbDir3 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_block_sync_archival") + dbDir2 := getTestDirectory(t, "test_block_sync_archival_2") + dbDir3 := getTestDirectory(t, "test_block_sync_archival_3") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := generateConfig(t, 18000, dbDir1, 10) - config2 := generateConfig(t, 18001, dbDir2, 10) - config3 := generateConfig(t, 18002, dbDir3, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config3 := GenerateTestConfig(t, 18002, dbDir3, 10) config1.HyperSync = true config1.SyncType = lib.NodeSyncTypeBlockSync diff --git a/integration_testing/migrations_test.go b/integration_testing/migrations_test.go index b0a692b52..c47a766e2 100644 --- a/integration_testing/migrations_test.go +++ b/integration_testing/migrations_test.go @@ -2,11 +2,12 @@ package integration_testing import ( "fmt" + "os" + "testing" + "github.com/deso-protocol/core/cmd" "github.com/deso-protocol/core/lib" "github.com/stretchr/testify/require" - "os" - "testing" ) // TODO: Add an encoder migration height in constants.go then modify some @@ -15,14 +16,14 @@ func TestEncoderMigrations(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_encoder_migration") + dbDir2 := getTestDirectory(t, "test_encoder_migration_2") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := generateConfig(t, 18001, dbDir2, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} diff --git a/integration_testing/mining_test.go b/integration_testing/mining_test.go index 49a23333c..91398468c 100644 --- a/integration_testing/mining_test.go +++ b/integration_testing/mining_test.go @@ -1,11 +1,12 @@ package integration_testing import ( + "os" + "testing" + "github.com/deso-protocol/core/cmd" "github.com/deso-protocol/core/lib" "github.com/stretchr/testify/require" - "os" - "testing" ) // TestSimpleBlockSync test if a node can mine blocks on regtest @@ -13,10 +14,10 @@ func TestRegtestMiner(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_regtest_miner") defer os.RemoveAll(dbDir1) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync config1.Params = &lib.DeSoTestnetParams config1.MaxSyncBlockHeight = 0 diff --git a/integration_testing/rollback_test.go b/integration_testing/rollback_test.go index 154a392c4..065d292b7 100644 --- a/integration_testing/rollback_test.go +++ b/integration_testing/rollback_test.go @@ -1,12 +1,13 @@ package integration_testing import ( - "github.com/deso-protocol/core/cmd" - "github.com/deso-protocol/core/lib" - "github.com/stretchr/testify/require" "os" "reflect" "testing" + + "github.com/deso-protocol/core/cmd" + "github.com/deso-protocol/core/lib" + "github.com/stretchr/testify/require" ) // Start blocks to height 5000 and then disconnect @@ -14,14 +15,14 @@ func TestStateRollback(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) + dbDir1 := getTestDirectory(t, "get_state_rollback") + dbDir2 := getTestDirectory(t, "get_state_rollback_2") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := generateConfig(t, 18001, dbDir2, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync config1.MaxSyncBlockHeight = 5000 diff --git a/integration_testing/txindex_test.go b/integration_testing/txindex_test.go index b01f7d3b2..77ffbe08e 100644 --- a/integration_testing/txindex_test.go +++ b/integration_testing/txindex_test.go @@ -2,32 +2,33 @@ package integration_testing import ( "fmt" + "os" + "testing" + "github.com/deso-protocol/core/cmd" "github.com/deso-protocol/core/lib" "github.com/stretchr/testify/require" - "os" - "testing" ) // TestSimpleTxIndex test if a node can successfully build txindex after block syncing from another node: -// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks. -// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator, and builds txindex afterwards. -// 3. bridge node1 and node2 -// 4. node2 syncs MaxSyncBlockHeight blocks from node1, and builds txindex afterwards. -// 5. compare node1 db and txindex matches node2. +// 1. Spawn two nodes node1, node2 with max block height of MaxSyncBlockHeight blocks. +// 2. node1 syncs MaxSyncBlockHeight blocks from the "deso-seed-2.io" generator, and builds txindex afterwards. +// 3. bridge node1 and node2 +// 4. node2 syncs MaxSyncBlockHeight blocks from node1, and builds txindex afterwards. +// 5. compare node1 db and txindex matches node2. func TestSimpleTxIndex(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getDirectory(t) - dbDir2 := getDirectory(t) + dbDir1 := getTestDirectory(t, "test_simple_tx_index") + dbDir2 := getTestDirectory(t, "test_simple_tx_index_2") defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := generateConfig(t, 18000, dbDir1, 10) + config1 := GenerateTestConfig(t, 18000, dbDir1, 10) config1.HyperSync = true config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := generateConfig(t, 18001, dbDir2, 10) + config2 := GenerateTestConfig(t, 18001, dbDir2, 10) config2.HyperSync = true config2.SyncType = lib.NodeSyncTypeHyperSyncArchival From f0d4765073e4b34660e0ea3bfa03cbcbf8ce9999 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Tue, 8 Nov 2022 06:06:15 +0530 Subject: [PATCH 06/21] Moving GenerateTestConfig to cmd package - Moving GenerateTestConfig to cmd package to avoid issues related to cyclic usage of the test utility inside the cmd package. --- cmd/config.go | 3 +- integration_testing/hypersync_test.go | 60 +++++++++++++------------- integration_testing/migrations_test.go | 8 ++-- integration_testing/mining_test.go | 4 +- integration_testing/rollback_test.go | 8 ++-- integration_testing/tools.go | 38 ---------------- integration_testing/txindex_test.go | 8 ++-- 7 files changed, 45 insertions(+), 84 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index a38af694c..15afbc214 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -232,8 +232,7 @@ func (config *Config) Print() { } // GenerateTestConfig creates a default config for a node, with provided port, db directory, and number of max peers. -// It's usually the first step to starting a node.\ -// FIXME: Duplicating the function to avoid cyclic error in node_test.go. Need to centralizer it. +// It's usually the first step to starting a node. func GenerateTestConfig(t *testing.T, port uint32, dataDir string, maxPeers uint32) Config { config := Config{} params := lib.DeSoMainnetParams diff --git a/integration_testing/hypersync_test.go b/integration_testing/hypersync_test.go index 3fde8a4d8..49e8ed110 100644 --- a/integration_testing/hypersync_test.go +++ b/integration_testing/hypersync_test.go @@ -25,17 +25,17 @@ func TestSimpleHyperSync(t *testing.T) { defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSync config1.HyperSync = true config2.HyperSync = true config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -76,11 +76,11 @@ func TestHyperSyncFromHyperSyncedNode(t *testing.T) { defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSyncArchival - config3 := GenerateTestConfig(t, 18002, dbDir3, 10) + config3 := cmd.GenerateTestConfig(t, 18002, dbDir3, 10) config3.SyncType = lib.NodeSyncTypeHyperSyncArchival config1.HyperSync = true @@ -88,9 +88,9 @@ func TestHyperSyncFromHyperSyncedNode(t *testing.T) { config3.HyperSync = true config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) - node3 := cmd.NewNode(config3) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) + node3 := cmd.NewNode(&config3) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -144,8 +144,8 @@ func TestSimpleHyperSyncRestart(t *testing.T) { defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config1.HyperSync = true config1.SyncType = lib.NodeSyncTypeBlockSync @@ -153,8 +153,8 @@ func TestSimpleHyperSyncRestart(t *testing.T) { config2.SyncType = lib.NodeSyncTypeHyperSyncArchival config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -201,11 +201,11 @@ func TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSyncArchival - config3 := GenerateTestConfig(t, 18002, dbDir3, 10) + config3 := cmd.GenerateTestConfig(t, 18002, dbDir3, 10) config3.SyncType = lib.NodeSyncTypeBlockSync config1.HyperSync = true @@ -214,9 +214,9 @@ func TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { config1.ConnectIPs = []string{"deso-seed-2.io:17000"} config3.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) - node3 := cmd.NewNode(config3) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) + node3 := cmd.NewNode(&config3) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -320,8 +320,8 @@ func TestArchivalMode(t *testing.T) { defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config1.HyperSync = true config2.HyperSync = true @@ -329,8 +329,8 @@ func TestArchivalMode(t *testing.T) { config1.SyncType = lib.NodeSyncTypeBlockSync config2.SyncType = lib.NodeSyncTypeHyperSyncArchival - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -365,9 +365,9 @@ func TestBlockSyncFromArchivalModeHyperSync(t *testing.T) { defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) - config3 := GenerateTestConfig(t, 18002, dbDir3, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config3 := cmd.GenerateTestConfig(t, 18002, dbDir3, 10) config1.HyperSync = true config1.SyncType = lib.NodeSyncTypeBlockSync @@ -377,9 +377,9 @@ func TestBlockSyncFromArchivalModeHyperSync(t *testing.T) { config3.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) - node3 := cmd.NewNode(config3) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) + node3 := cmd.NewNode(&config3) node1 = startNode(t, node1) node2 = startNode(t, node2) diff --git a/integration_testing/migrations_test.go b/integration_testing/migrations_test.go index c47a766e2..9a9e17a6a 100644 --- a/integration_testing/migrations_test.go +++ b/integration_testing/migrations_test.go @@ -21,17 +21,17 @@ func TestEncoderMigrations(t *testing.T) { defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} config1.HyperSync = true config2.HyperSync = true - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) node1 = startNode(t, node1) node2 = startNode(t, node2) diff --git a/integration_testing/mining_test.go b/integration_testing/mining_test.go index 91398468c..737cdb3ee 100644 --- a/integration_testing/mining_test.go +++ b/integration_testing/mining_test.go @@ -17,7 +17,7 @@ func TestRegtestMiner(t *testing.T) { dbDir1 := getTestDirectory(t, "test_regtest_miner") defer os.RemoveAll(dbDir1) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync config1.Params = &lib.DeSoTestnetParams config1.MaxSyncBlockHeight = 0 @@ -25,7 +25,7 @@ func TestRegtestMiner(t *testing.T) { config1.Regtest = true - node1 := cmd.NewNode(config1) + node1 := cmd.NewNode(&config1) node1 = startNode(t, node1) // wait for node1 to sync blocks diff --git a/integration_testing/rollback_test.go b/integration_testing/rollback_test.go index 065d292b7..8da094a6e 100644 --- a/integration_testing/rollback_test.go +++ b/integration_testing/rollback_test.go @@ -20,9 +20,9 @@ func TestStateRollback(t *testing.T) { defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync config1.MaxSyncBlockHeight = 5000 @@ -32,8 +32,8 @@ func TestStateRollback(t *testing.T) { config1.ConnectIPs = []string{"deso-seed-2.io:17000"} config2.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) node1 = startNode(t, node1) node2 = startNode(t, node2) diff --git a/integration_testing/tools.go b/integration_testing/tools.go index 8a2f83802..ebe3cf0de 100644 --- a/integration_testing/tools.go +++ b/integration_testing/tools.go @@ -4,7 +4,6 @@ import ( "encoding/hex" "fmt" "io/ioutil" - "os" "reflect" "sort" "testing" @@ -51,43 +50,6 @@ func getTestDirectory(t *testing.T, testName string) string { return dbDir } -// GenerateTestConfig creates a default config for a node, with provided port, db directory, and number of max peers. -// It's usually the first step to starting a node. -func GenerateTestConfig(t *testing.T, port uint32, dataDir string, maxPeers uint32) *cmd.Config { - config := &cmd.Config{} - params := lib.DeSoMainnetParams - - params.DNSSeeds = []string{} - config.Params = ¶ms - config.ProtocolPort = uint16(port) - // "/Users/piotr/data_dirs/n98_1" - config.DataDirectory = dataDir - if err := os.MkdirAll(config.DataDirectory, os.ModePerm); err != nil { - t.Fatalf("Could not create data directories (%s): %v", config.DataDirectory, err) - } - config.TXIndex = false - config.HyperSync = false - config.MaxSyncBlockHeight = 0 - config.ConnectIPs = []string{} - config.PrivateMode = true - config.GlogV = 0 - config.GlogVmodule = "*bitcoin_manager*=0,*balance*=0,*view*=0,*frontend*=0,*peer*=0,*addr*=0,*network*=0,*utils*=0,*connection*=0,*main*=0,*server*=0,*mempool*=0,*miner*=0,*blockchain*=0" - config.MaxInboundPeers = maxPeers - config.TargetOutboundPeers = maxPeers - config.StallTimeoutSeconds = 900 - config.MinFeerate = 1000 - config.OneInboundPerIp = false - config.MaxBlockTemplatesCache = 100 - config.MaxSyncBlockHeight = 100 - config.MinBlockUpdateInterval = 10 - config.SnapshotBlockHeightPeriod = HyperSyncSnapshotPeriod - config.MaxSyncBlockHeight = MaxSyncBlockHeight - config.SyncType = lib.NodeSyncTypeBlockSync - //config.ArchivalMode = true - - return config -} - // waitForNodeToFullySync will busy-wait until provided node is fully current. func waitForNodeToFullySync(node *cmd.Node) { ticker := time.NewTicker(5 * time.Millisecond) diff --git a/integration_testing/txindex_test.go b/integration_testing/txindex_test.go index 77ffbe08e..44cc4ed2d 100644 --- a/integration_testing/txindex_test.go +++ b/integration_testing/txindex_test.go @@ -25,10 +25,10 @@ func TestSimpleTxIndex(t *testing.T) { defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.HyperSync = true config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config2.HyperSync = true config2.SyncType = lib.NodeSyncTypeHyperSyncArchival @@ -36,8 +36,8 @@ func TestSimpleTxIndex(t *testing.T) { config2.TXIndex = true config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) node1 = startNode(t, node1) node2 = startNode(t, node2) From 3cd1c5f8daa8bf71c6544d372d0be69d5abb2690 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Tue, 8 Nov 2022 06:07:08 +0530 Subject: [PATCH 07/21] Fixing Camel Casing --- cmd/node_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/node_test.go b/cmd/node_test.go index 6e63d886f..80d89dcf4 100644 --- a/cmd/node_test.go +++ b/cmd/node_test.go @@ -21,21 +21,21 @@ func TestNodeIsRunning(t *testing.T) { // expected running status should be false before the node is started expectedRunningStatus := false - actualRunningstatus := testNode.IsRunning() - require.Equal(t, expectedRunningStatus, actualRunningstatus) + actualRunningStatus := testNode.IsRunning() + require.Equal(t, expectedRunningStatus, actualRunningStatus) // Start the node testNode.Start() // expected running status should be true after the server is started expectedRunningStatus = true - actualRunningstatus = testNode.IsRunning() - require.Equal(t, expectedRunningStatus, actualRunningstatus) + actualRunningStatus = testNode.IsRunning() + require.Equal(t, expectedRunningStatus, actualRunningStatus) // stop the node testNode.Stop() expectedRunningStatus = false - actualRunningstatus = testNode.IsRunning() - require.Equal(t, expectedRunningStatus, actualRunningstatus) + actualRunningStatus = testNode.IsRunning() + require.Equal(t, expectedRunningStatus, actualRunningStatus) } From fca24b38818c9f49320cc4d32881500922b303dc Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Tue, 8 Nov 2022 21:10:42 +0530 Subject: [PATCH 08/21] Updating testconfig in blocksync tests - Updating testconfig in blocksync tests. GenerateTestConfig is moved to cmd package. --- integration_testing/blocksync_test.go | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/integration_testing/blocksync_test.go b/integration_testing/blocksync_test.go index 5b4e07c9f..eb20a983b 100644 --- a/integration_testing/blocksync_test.go +++ b/integration_testing/blocksync_test.go @@ -25,15 +25,15 @@ func TestSimpleBlockSync(t *testing.T) { defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -71,15 +71,15 @@ func TestSimpleSyncRestart(t *testing.T) { defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -125,19 +125,19 @@ func TestSimpleSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync - config3 := GenerateTestConfig(t, 18002, dbDir3, 10) + config3 := cmd.GenerateTestConfig(t, 18002, dbDir3, 10) config3.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} config3.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(config1) - node2 := cmd.NewNode(config2) - node3 := cmd.NewNode(config3) + node1 := cmd.NewNode(&config1) + node2 := cmd.NewNode(&config2) + node3 := cmd.NewNode(&config3) node1 = startNode(t, node1) node2 = startNode(t, node2) From 51e2f0b6b80ad90a58ba9c56552496a0b9e8779e Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Wed, 9 Nov 2022 04:09:19 +0530 Subject: [PATCH 09/21] changing node status to custom type - changing node status to custom NodeStatus byte type - Setting valid status codes - Utility functions to validate and change the status codes - Using new status locks for safely reading and writing the node status. --- cmd/node.go | 218 ++++++++++++++++++++++++++++++++++++++++++---------- cmd/run.go | 2 + 2 files changed, 180 insertions(+), 40 deletions(-) diff --git a/cmd/node.go b/cmd/node.go index 7659a8c2b..20d841bab 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -9,7 +9,6 @@ import ( "os" "os/signal" "sync" - "sync/atomic" "syscall" "time" @@ -38,10 +37,32 @@ var ( // ErrNodeNeverStarted is returned when somebody tries to find or changge the // running status of the server without never starting it once. ErrNodeNeverStarted = errors.New("never started the node instance") + // Cannot set node status to NEVERSTARTED. + // NEVERSTARTED is the default status, cannot set deliberately. + ErrCannotSetToNeverStarted = errors.New("cannot set the node status to neverstarted") // Error if the atomic compare of swap of the node status fails - ErrFailedtoChangeNodeStatus = errors.New("Failed to change the runnnin status of the Node") + ErrFailedtoChangeNodeStatus = errors.New("failed to change the status of the node") + // Erorr when invalid status is set for node + ErrInvalidNodeStatus = errors.New("invalid node status. check cmd/node.go for valid states") + // Invalid Status return value for the *Node.LoadStatus() helper function. + // byte(255) is a reserved status for invalid status code for the Node + INVALIDNODESTATUS = NodeStatus(255) ) +// Valid status codes for the Node. +// Status should be retrieved using the helper functions *Node.LoadStatus() +const ( + // Status when the node is not initialized or started for the first time + NEVERSTARTED NodeStatus = iota // byte(0) + // Status when the node is initialized using *Node.Start + RUNNING // byte(1) + // Status when the node is stopped + STOPPED // byte(2) +) + +// custom byte type to indicate the running status of the Node. +type NodeStatus byte + type Node struct { Server *lib.Server ChainDB *badger.DB @@ -50,11 +71,16 @@ type Node struct { Config *Config Postgres *lib.Postgres - // running is false when a NewNode is created, it is set to true on node.Start(), - // set to false after Stop() is called. Mainly used in testing. - // Use the convinience methods changeRunningStatus/StatusStartToStop/StatusStopToStart to change node status - // Use *Node.IsRunning() to retreive the node running status. - running *atomic.Bool + // status is nil when a NewNode is created, it is initialized and set to RUNNING [byte(1)] on node.Start(), + // set to STOPPED [byte(2)] after Stop() is called. + + // Use the convinience methods changeNodeStatus/UpdateStatusRunning/UpdateStatusStopped to change node status. + // Use *Node.IsRunning() to check if the node is running. + // Use *Node.LoadStatus() to retrieve the status of the node. + status *NodeStatus + // Held whenver the status of the node is read or altered. + // Cannot use runningMutex to safe guard Node status due to deadlocks! + statusMutex sync.Mutex // runningMutex is held whenever we call Start() or Stop() on the node. runningMutex sync.Mutex @@ -92,7 +118,6 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { node.internalExitChan = make(chan struct{}) node.nodeMessageChan = make(chan lib.NodeMessage) - node.running = &atomic.Bool{} // listenToNodeMessages handles the messages received from the engine through the nodeMessageChan. go node.listenToNodeMessages() @@ -271,10 +296,43 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { } } } - // Change nodes running status to start - if err := node.StatusStopToStart(); err != nil { - panic(err) + + node.statusMutex.Lock() + // Load the node status. + // This is identify whether the node is initialized for the first time or it's a restart. + status, err := node.LoadStatus() + // FIXME: Replace panics with return value, + // and leave the decision making to the client library of the core whether to crash the app. + if err != nil { + panic("Failed to load node status") + } + // Handling the first time initialization and node restart cases separately. + // This allows us to log the events and even run exclusive logic if any. + switch status { + case NEVERSTARTED: + glog.Info("Changing node status from NEVERSTARTED to RUNNING...") + // The node status is changed from NEVERSTARTED to RUNNING only once + // when the node is started for the first time. + err = node.UpdateStatusRunning() + if err != nil { + panic("Failed to initialize the node status to running") + } + case STOPPED: + // This case is called during a node restart. + // During restart the Node is first STOPPED before setting the + // status again to RUNNING. + glog.Info("Changing node status from STOP to RUNNING...") + err = node.UpdateStatusRunning() + if err != nil { + panic("Failed to initialize the node status to running") + } + default: + // Rare occurance. Happens if you set an invalid node status while restarting a node. + // cannot start the node if the status of the Node is already set to RUNNING. + panic(fmt.Sprintf("Cannot change node status to RUNNING from the current status %v", status)) + } + node.statusMutex.Unlock() if shouldRestart { if node.nodeMessageChan != nil { @@ -288,6 +346,10 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { go func() { // If an internalExitChan is triggered then we won't immediately signal a shutdown to the parent context through // the exitChannels. When internal exit is called, we will just restart the node in the background. + + // Example: First call Node.Stop(), then call Node.Start() to internally restart the service. + // Since Stop() closes the internalExitChain, the exitChannels sent by the users of the + // core library will not closed. Thus ensuring that an internal restart doesn't affect the users of the core library. select { case _, open := <-node.internalExitChan: if !open { @@ -296,7 +358,9 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { case <-syscallChannel: } + node.statusMutex.Lock() node.Stop() + node.statusMutex.Unlock() for _, channel := range exitChannels { if *channel != nil { close(*channel) @@ -307,62 +371,125 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { }() } -// Changes node running status from true to false -func (node *Node) StatusStartToStop() error { - if err := changeRunningStatus(node, true, false); err != nil { +// Changes node status to STOPPED. +// Cannot transition from NEVERSTARTED to STOPPED. +// Valid transition sequence is NEVERSTARTED->RUNNING->STOPPED. +func (node *Node) UpdateStatusStopped() error { + if err := node.changeNodeStatus(STOPPED); err != nil { glog.Errorf("Error stopping Node -- %v", err) return err } return nil } -// Changes node running status from false to true -func (node *Node) StatusStopToStart() error { - if err := changeRunningStatus(node, false, true); err != nil { +// Changes node status to RUNNING. +func (node *Node) UpdateStatusRunning() error { + if err := node.changeNodeStatus(RUNNING); err != nil { glog.Errorf("Error stopping Node -- %v", err) return err } return nil } +// Loads the status of Node. +// Modifying LoadStatus() and the validateStatus() functions and their tests are hard requirements +// to add new status codes for the node. +func (node *Node) LoadStatus() (NodeStatus, error) { + // Never initialized the server using *Node.Start() + if node.status == nil { + return NEVERSTARTED, nil + } + // using switch and case prevents from adding new invalid codes + // without changing the LoadStatus() function and its unit test. + switch *node.status { + // Node is started using *Node.Start() and not stopped yet. + case RUNNING: + return RUNNING, nil + // Node was once initialized, but currently stopped. + // Set to STOPPED on calling *Node.Stop() + case STOPPED: + return STOPPED, nil + // Any other status code apart from the cases above are considered INVALID!!!! + default: + return INVALIDNODESTATUS, ErrInvalidNodeStatus + } +} + +// Verifies whether a status code of the node is a valid status code. +// Used while changing the status of the node +// Modifying LoadStatus() and the validateStatus() functions and their tests are hard requirements +// to add new status codes for the node. +func validateNodeStatus(status NodeStatus) error { + switch status { + // Instance of the *Node is created, but not yet intialized using *Node.Start() + case NEVERSTARTED: + return nil + // Node is started using *Node.Start() and not stopped yet. + case RUNNING: + return nil + // Node was once initialized, but currently stopped. + // Set to STOPPED on calling *Node.Stop() + case STOPPED: + return nil + // Any other status code apart from the cases above are considered INVALID, + default: + return ErrInvalidNodeStatus + } +} + // changes the running status of the node -func changeRunningStatus(node *Node, currentStatus, newStatus bool) error { - // Cannot change status of the node before initializing it - // Node is initialized only after calling node.Start() - if node.running == nil { - return ErrNodeNeverStarted +// Always use this function to change the status of the node. +func (node *Node) changeNodeStatus(newStatus NodeStatus) error { + if err := validateNodeStatus(newStatus); err != nil { + return err + } + + // Cannot deliberately change the status to NEVERSTARTED. + // NEVERSTARTED is the default status of the node before the + // node is started using *Node.Start(). + if newStatus == NEVERSTARTED { + return ErrCannotSetToNeverStarted + } + // Load the current status of the Node. + status, err := node.LoadStatus() + if err != nil { + return err } - // Cannot change the status of the current and the new status are the same! - if currentStatus == newStatus { - return errors.New("error invalid input. current and new status values should be different") + // Cannot change the status of the server to STOPPED while it was never initalized + // in the first place. Can stop the never only after it's started using *Node.Start(). + // Valid status transition is NEVERSTARTED -> RUNNING -> STOPPED -> RUNNING + if status == NEVERSTARTED && newStatus == STOPPED { + return ErrNodeNeverStarted } - // No need to change the status if the new status is already the running status - if newStatus == node.running.Load() { - // cannot change running status to true if the node is already running - if newStatus { + // No need to change the status if the new status is same as current status. + if newStatus == status { + switch newStatus { + case RUNNING: return ErrAlreadyStarted + case STOPPED: + return ErrAlreadyStopped } - // cannot change running status to false if the node is already stopped - return ErrAlreadyStopped } - success := node.running.CompareAndSwap(currentStatus, newStatus) - if !success { - return ErrFailedtoChangeNodeStatus - } + node.status = &newStatus return nil } // Helper function to check if the node is running. -// returns false if node.running is not initalized. +// returns false if node status is not set to RUNNING func (node *Node) IsRunning() bool { - if node.running == nil { + status, err := node.LoadStatus() + + if err != nil { return false } - return node.running.Load() + if status == RUNNING { + return true + } + return false } func (node *Node) Stop() { @@ -370,7 +497,9 @@ func (node *Node) Stop() { defer node.runningMutex.Unlock() // Change nodes running status to stop - if err := node.StatusStartToStop(); err != nil { + // Node has to be in RUNNING status to be able to STOP! + // FIXME: Return error in cases where the the node fails to stop. This makes the function testable. + if err := node.UpdateStatusStopped(); err != nil { glog.Errorf("Error stopping Node -- %v", err) return } @@ -438,6 +567,8 @@ func (node *Node) listenToNodeMessages(exitChannels ...*chan struct{}) { case operation := <-node.nodeMessageChan: switch operation { case lib.NodeRestart: + // Using Mutex while accessing the Node status to avoid any race conditions + node.statusMutex.Lock() if !node.IsRunning() { panic("Node.listenToNodeMessages: Node is currently not running, nodeMessageChan should've not been called!") } @@ -445,12 +576,19 @@ func (node *Node) listenToNodeMessages(exitChannels ...*chan struct{}) { glog.Infof("Node.listenToNodeMessages: Restarting node.") glog.Infof("Node.listenToNodeMessages: Stopping node") node.Stop() + node.statusMutex.Unlock() glog.Infof("Node.listenToNodeMessages: Finished stopping node") case lib.NodeErase: glog.Infof("Node.listenToNodeMessages: Restarting node with a Database erase.") glog.Infof("Node.listenToNodeMessages: Stopping node") - node.Stop() + // cannot stop the node if the node is not already in RUNNING state. + // Stop the node and remove the data directory if the server is in RUNNING state. + node.statusMutex.Lock() + if node.IsRunning() { + node.Stop() + } + node.statusMutex.Unlock() glog.Infof("Node.listenToNodeMessages: Finished stopping node") if err := os.RemoveAll(node.Config.DataDirectory); err != nil { diff --git a/cmd/run.go b/cmd/run.go index 57a03c8a3..6bc91fe59 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -29,7 +29,9 @@ func Run(cmd *cobra.Command, args []string) { node.Start(&shutdownListener) defer func() { + node.statusMutex.Lock() node.Stop() + node.statusMutex.Unlock() glog.Info("Shutdown complete") }() <-shutdownListener From 82896846cd87544b3b58aee6b1e9acf0ae3f8402 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Wed, 9 Nov 2022 04:09:55 +0530 Subject: [PATCH 10/21] Tests for Node Status utility functions --- cmd/node_test.go | 210 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 157 insertions(+), 53 deletions(-) diff --git a/cmd/node_test.go b/cmd/node_test.go index 80d89dcf4..d59a15719 100644 --- a/cmd/node_test.go +++ b/cmd/node_test.go @@ -4,8 +4,6 @@ import ( "os" "testing" - //"github.com/deso-protocol/core/lib" - //"github.com/deso-protocol/core/lib" "github.com/stretchr/testify/require" ) @@ -22,24 +20,24 @@ func TestNodeIsRunning(t *testing.T) { // expected running status should be false before the node is started expectedRunningStatus := false actualRunningStatus := testNode.IsRunning() - require.Equal(t, expectedRunningStatus, actualRunningStatus) + require.Equal(t, actualRunningStatus, expectedRunningStatus) // Start the node testNode.Start() // expected running status should be true after the server is started expectedRunningStatus = true actualRunningStatus = testNode.IsRunning() - require.Equal(t, expectedRunningStatus, actualRunningStatus) + require.Equal(t, actualRunningStatus, expectedRunningStatus) // stop the node testNode.Stop() expectedRunningStatus = false actualRunningStatus = testNode.IsRunning() - require.Equal(t, expectedRunningStatus, actualRunningStatus) + require.Equal(t, actualRunningStatus, expectedRunningStatus) } -func TestNodeStatusStopToStart(t *testing.T) { +func TestNodeStatusRunning(t *testing.T) { testDir := getTestDirectory(t, "test_node_change_running_status") defer os.RemoveAll(testDir) @@ -49,32 +47,32 @@ func TestNodeStatusStopToStart(t *testing.T) { testNode := NewNode(&testConfig) - // Need to call node.start() to atleast once to be able to change the running status of a node - // Cannot change status of node which never got initialized in the first place - expErr := ErrNodeNeverStarted - actualErr := testNode.StatusStopToStart() - require.ErrorIs(t, expErr, actualErr) + // Can change the status to RUNNING from the state NEVERSTARTED + actualErr := testNode.UpdateStatusRunning() + require.NoError(t, actualErr) + + // Change status from RUNNING to STOPPED + actualErr = testNode.UpdateStatusStopped() + require.NoError(t, actualErr) // start the server - // Cannot change the running status of a node from stop to start while - // the node is still running + // Cannot change status to RUNNING while the node is already RUNNING! testNode.Start() - expErr = ErrAlreadyStarted - actualErr = testNode.StatusStopToStart() - require.ErrorIs(t, expErr, actualErr) + expErr := ErrAlreadyStarted + actualErr = testNode.UpdateStatusRunning() + require.ErrorIs(t, actualErr, expErr) // Stop the node - // Should successfully change the status from stop to start after the node is stopped - // expect no error testNode.Stop() - actualErr = testNode.StatusStopToStart() + // Should be able to change status to RUNNING from STOP. + actualErr = testNode.UpdateStatusRunning() require.NoError(t, actualErr) // Once the running flag is changed, the isRunning function should return true require.Equal(t, true, testNode.IsRunning()) } -func TestNodeStatusStartToStop(t *testing.T) { +func TestNodeUpdateStatusStopped(t *testing.T) { testDir := getTestDirectory(t, "test_node_change_running_status") defer os.RemoveAll(testDir) @@ -87,26 +85,29 @@ func TestNodeStatusStartToStop(t *testing.T) { // Need to call node.start() to atleast once to be able to change node status // Cannot change status of node which never got initialized in the first place expErr := ErrNodeNeverStarted - actualErr := testNode.StatusStartToStop() - require.ErrorIs(t, expErr, actualErr) + actualErr := testNode.UpdateStatusStopped() + require.ErrorIs(t, actualErr, expErr) // start the node // Should be able to successfully change the status of the node // Once the server is started testNode.Start() - actualErr = testNode.StatusStartToStop() + actualErr = testNode.UpdateStatusStopped() require.NoError(t, actualErr) // stop the node testNode.Stop() expErr = ErrAlreadyStopped - actualErr = testNode.StatusStartToStop() - require.ErrorIs(t, expErr, actualErr) + actualErr = testNode.UpdateStatusStopped() + require.ErrorIs(t, actualErr, expErr) } -func TestNodeChangeRunningStatus(t *testing.T) { +// Node status is change in the following sequence, +// NEVERSTARTED -> RUNNING -> STOP -> RUNNING +// In each state change it's tested for valid change in status. +func TestNodechangeNodeStatus(t *testing.T) { testDir := getTestDirectory(t, "test_node_change_running_status") defer os.RemoveAll(testDir) @@ -116,40 +117,143 @@ func TestNodeChangeRunningStatus(t *testing.T) { testNode := NewNode(&testConfig) - // Need to call node.start() to atleast once to be able to change the running status of a node - // Cannot change status of node which never got initialized in the first place + // Node is in NEVERSTARTED STATE + + // Changing status from NEVERSTARTED to STOPPED + // This is an invalid status transition. + // Node status cannot needs to transitioned to RUNNING before changing to STOPPED expError := ErrNodeNeverStarted - // Changing status from false to true - actualError := changeRunningStatus(testNode, false, true) - require.ErrorIs(t, expError, actualError) - // Changing status from true to false - actualError = changeRunningStatus(testNode, true, false) - require.ErrorIs(t, expError, actualError) + actualError := testNode.changeNodeStatus(STOPPED) + require.ErrorIs(t, actualError, expError) + + // Cannot set the status to NEVERSTARTED, + // It's the default value before the Node is initialized. + expError = ErrCannotSetToNeverStarted + actualError = testNode.changeNodeStatus(NEVERSTARTED) + require.ErrorIs(t, actualError, expError) + // Starting the node. + // The current status of the node is RUNNING. + testNode.Start() + // The status should be changed to RUNNING. + // This successfully tests the transition of status from NEVERSTARTED to RUNNING + expectedStatus := RUNNING + actualStatus, err := testNode.LoadStatus() + require.NoError(t, err) + require.Equal(t, actualStatus, expectedStatus) + + // Cannot set the status to NEVERSTARTED, + // It's the default value before the Node is initialized. + expError = ErrCannotSetToNeverStarted + actualError = testNode.changeNodeStatus(NEVERSTARTED) + require.ErrorIs(t, actualError, expError) + + // Cannot expect the Node status to changed from STOPPED to RUNNING, + // while it's current state is RUNNING + expError = ErrAlreadyStarted + actualError = testNode.changeNodeStatus(RUNNING) + require.ErrorIs(t, actualError, expError) - // start the node + // Stopping the node. + // This should transition the Node state from RUNNING to STOPPED. + testNode.Stop() + expectedStatus = STOPPED + actualStatus, err = testNode.LoadStatus() + require.NoError(t, err) + require.Equal(t, actualStatus, expectedStatus) + + // Cannot set the status to NEVERSTARTED, + // It's the default value before the Node is initialized. + expError = ErrCannotSetToNeverStarted + actualError = testNode.changeNodeStatus(NEVERSTARTED) + require.ErrorIs(t, actualError, expError) + + // Cannot expect the Node status to changed from NEVERSTARTED to STOPPED, + // while it's current state is STOPPED + expError = ErrAlreadyStopped + actualError = testNode.changeNodeStatus(STOPPED) + require.ErrorIs(t, actualError, expError) + + // Changing status from STOPPED to RUNNING testNode.Start() + // The following tests validates a successful transition of state from STOP to RUNNING + expectedStatus = RUNNING + actualStatus, err = testNode.LoadStatus() + require.NoError(t, err) + require.Equal(t, actualStatus, expectedStatus) + testNode.Stop() - // Cannot change node running status to true while it's running - // its already set to true - expError = ErrAlreadyStarted - actualError = changeRunningStatus(testNode, false, true) - require.ErrorIs(t, expError, actualError) + // Set the Node status to an invalid status code + // protects from the ignorance of adding new status codes to the iota sequence! + expError = ErrInvalidNodeStatus + actualError = testNode.changeNodeStatus(NodeStatus(3)) + require.ErrorIs(t, actualError, expError) - // Should be able to change the node running status to false once started - actualError = changeRunningStatus(testNode, true, false) - require.NoError(t, actualError) + expError = ErrInvalidNodeStatus + actualError = testNode.changeNodeStatus(NodeStatus(4)) + require.ErrorIs(t, actualError, expError) - // stop the node + expError = ErrInvalidNodeStatus + actualError = testNode.changeNodeStatus(NodeStatus(5)) + require.ErrorIs(t, actualError, expError) + +} + +// Tests for *Node.LoadStatus() +// Loads the status of node after node operations and tests its correctness. +func TestLoadStatus(t *testing.T) { + testDir := getTestDirectory(t, "test_load_status") + defer os.RemoveAll(testDir) + + testConfig := GenerateTestConfig(t, 18000, testDir, 10) + + testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} + + testNode := NewNode(&testConfig) + + // Status is set to NEVERSTARTED before the node is started. + expectedStatus := NEVERSTARTED + actualStatus, err := testNode.LoadStatus() + require.NoError(t, err) + require.Equal(t, actualStatus, expectedStatus) + + // Start the node + testNode.Start() + + // The status is expected to be RUNNING once the node is started. + expectedStatus = RUNNING + actualStatus, err = testNode.LoadStatus() + require.NoError(t, err) + require.Equal(t, actualStatus, expectedStatus) + + // Stop the node. testNode.Stop() - // Cannot change the running status of the node to false - // when its not running - expError = ErrAlreadyStopped - actualError = changeRunningStatus(testNode, true, false) - require.ErrorIs(t, expError, actualError) + // The status is expected to be STOPPED once the node is stopped. + expectedStatus = STOPPED + actualStatus, err = testNode.LoadStatus() + require.NoError(t, err) + require.Equal(t, actualStatus, expectedStatus) + + // set an invalid status + wrongStatus := NodeStatus(5) + testNode.status = &wrongStatus + + // expect error and invalid status code + expectedStatus = INVALIDNODESTATUS + actualStatus, err = testNode.LoadStatus() + require.ErrorIs(t, err, ErrInvalidNodeStatus) + require.Equal(t, actualStatus, expectedStatus) + +} + +func TestValidateNodeStatus(t *testing.T) { + + inputs := []NodeStatus{NEVERSTARTED, RUNNING, STOPPED, NodeStatus(3), NodeStatus(4)} + errors := []error{nil, nil, nil, ErrInvalidNodeStatus, ErrInvalidNodeStatus} - // Should be able to change the running status of the node to true - // after the node is stopped - actualError = changeRunningStatus(testNode, false, true) - require.NoError(t, actualError) + var err error + for i := 0; i < len(inputs); i++ { + err = validateNodeStatus(inputs[i]) + require.ErrorIs(t, err, errors[i]) + } } From 522f1e5d2aab85c4109b099218e4c4ddd70b2d5e Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Sun, 13 Nov 2022 06:49:31 +0530 Subject: [PATCH 11/21] Simplifying node test assertions --- .DS_Store | Bin 0 -> 6148 bytes cmd/node_test.go | 14 ++++---------- integration_testing/.DS_Store | Bin 0 -> 6148 bytes 3 files changed, 4 insertions(+), 10 deletions(-) create mode 100644 .DS_Store create mode 100644 integration_testing/.DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..9cbdf7378ee75eb330f890b50ff42acd5ebf06a4 GIT binary patch literal 6148 zcmeHKyGjH>5Ukb<4onVA4Cfd8gXK8C;66YFWf5c!&PTkF-{q%S{Xkgu#K=Udp}S^! zyJpx5wzmP;_U+*s*Z^459r58|Y5v@OVP};wBAsW9IN~0!c*0>(eLCUXOV)R?9%uZH z+sM7dc^%{X{@9-neazcVAE&GokOERb3P=Gd@QVW8dui*7L`5ke1*E{Y0{(qybjP7^ zN{mkjLyQ2#71Lo{$1Fi?o*)i|QzA1oODZv`RwITbo%vREL*bN|bXW}^RySKsC>FQ# z{1)Y~AyH8ZNP)Qm=egZ@|G%OCF#pd<+DQQ^@UIlG)o#Dr@|CK$E?&-iZKL1Oz2=kd q#&u8_q8$^X9dqOD_$rFBuKAkJhr%f_=*$P5sGkAXMJ5IQT7fT_i5Y?b literal 0 HcmV?d00001 diff --git a/cmd/node_test.go b/cmd/node_test.go index d59a15719..aff3768df 100644 --- a/cmd/node_test.go +++ b/cmd/node_test.go @@ -18,22 +18,16 @@ func TestNodeIsRunning(t *testing.T) { testNode := NewNode(&testConfig) // expected running status should be false before the node is started - expectedRunningStatus := false - actualRunningStatus := testNode.IsRunning() - require.Equal(t, actualRunningStatus, expectedRunningStatus) + require.False(t, testNode.IsRunning()) // Start the node testNode.Start() // expected running status should be true after the server is started - expectedRunningStatus = true - actualRunningStatus = testNode.IsRunning() - require.Equal(t, actualRunningStatus, expectedRunningStatus) + require.True(t, testNode.IsRunning()) // stop the node testNode.Stop() - expectedRunningStatus = false - actualRunningStatus = testNode.IsRunning() - require.Equal(t, actualRunningStatus, expectedRunningStatus) + require.False(t, testNode.IsRunning()) } @@ -68,7 +62,7 @@ func TestNodeStatusRunning(t *testing.T) { actualErr = testNode.UpdateStatusRunning() require.NoError(t, actualErr) // Once the running flag is changed, the isRunning function should return true - require.Equal(t, true, testNode.IsRunning()) + require.True(t, testNode.IsRunning()) } diff --git a/integration_testing/.DS_Store b/integration_testing/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..2c6db71aef675844f4e8feda6864ca011a308989 GIT binary patch literal 6148 zcmeHKOHRWu5PdFPR3Jf%Sg@&-6I9{`p(+d39H2C9RS+RYZIQ|@d+xv$I3m0mTUGkY z5+Rt8#?PO5W9Lb>V*sxII=KSc09tgx<}r&sCgb8e)`&ea=rqR@7I;H}F@`(Q)?o^m z0>4cG*}ENN$l}j5G2_>vb%68r9@z%(ilzYL0?= z3TCS1;+ou#iV|(k?5SqXZy%|m^?XsX%5QO{UNbBXDprj@WBf{2I-zQ-`qiphRW>I# z`?m`4&K7H(1hmo=Fa=D3wE}WJBy_VVgFZhg-mN1rHHq(V$1mI2vA PvmXJK!3tC0M-})6`+JZ$ literal 0 HcmV?d00001 From 8158214d0f4025ae6c45321995e4a79f03592473 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Sun, 13 Nov 2022 07:20:50 +0530 Subject: [PATCH 12/21] Fixing the typos in the comments --- cmd/node.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/cmd/node.go b/cmd/node.go index 20d841bab..77748f961 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -74,11 +74,11 @@ type Node struct { // status is nil when a NewNode is created, it is initialized and set to RUNNING [byte(1)] on node.Start(), // set to STOPPED [byte(2)] after Stop() is called. - // Use the convinience methods changeNodeStatus/UpdateStatusRunning/UpdateStatusStopped to change node status. + // Use the convenience methods changeNodeStatus/UpdateStatusRunning/UpdateStatusStopped to change node status. // Use *Node.IsRunning() to check if the node is running. // Use *Node.LoadStatus() to retrieve the status of the node. status *NodeStatus - // Held whenver the status of the node is read or altered. + // Held whenever the status of the node is read or altered. // Cannot use runningMutex to safe guard Node status due to deadlocks! statusMutex sync.Mutex // runningMutex is held whenever we call Start() or Stop() on the node. @@ -299,7 +299,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { node.statusMutex.Lock() // Load the node status. - // This is identify whether the node is initialized for the first time or it's a restart. + // This is to identify whether the node is initialized for the first time or it's a restart. status, err := node.LoadStatus() // FIXME: Replace panics with return value, // and leave the decision making to the client library of the core whether to crash the app. @@ -327,7 +327,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { panic("Failed to initialize the node status to running") } default: - // Rare occurance. Happens if you set an invalid node status while restarting a node. + // Rare occurrence. Happens if you set an invalid node status while restarting a node. // cannot start the node if the status of the Node is already set to RUNNING. panic(fmt.Sprintf("Cannot change node status to RUNNING from the current status %v", status)) @@ -421,7 +421,7 @@ func (node *Node) LoadStatus() (NodeStatus, error) { // to add new status codes for the node. func validateNodeStatus(status NodeStatus) error { switch status { - // Instance of the *Node is created, but not yet intialized using *Node.Start() + // Instance of the *Node is created, but not yet initialized using *Node.Start() case NEVERSTARTED: return nil // Node is started using *Node.Start() and not stopped yet. @@ -456,7 +456,7 @@ func (node *Node) changeNodeStatus(newStatus NodeStatus) error { return err } - // Cannot change the status of the server to STOPPED while it was never initalized + // Cannot change the status of the server to STOPPED while it was never initialized // in the first place. Can stop the never only after it's started using *Node.Start(). // Valid status transition is NEVERSTARTED -> RUNNING -> STOPPED -> RUNNING if status == NEVERSTARTED && newStatus == STOPPED { From 95a477acd144738a2851e7799cf21d3e6345a987 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Sun, 13 Nov 2022 08:04:36 +0530 Subject: [PATCH 13/21] Fixing the glog messages - replacing a panic with glag.Fatalf - simplifying the logic of node.IsRunning() - Deduplicating the glog messages, shifting the responsibility of logging to the caller. --- cmd/node.go | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/cmd/node.go b/cmd/node.go index 77748f961..f322e1825 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -301,10 +301,8 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { // Load the node status. // This is to identify whether the node is initialized for the first time or it's a restart. status, err := node.LoadStatus() - // FIXME: Replace panics with return value, - // and leave the decision making to the client library of the core whether to crash the app. if err != nil { - panic("Failed to load node status") + glog.Fatal("Failed to load node status") } // Handling the first time initialization and node restart cases separately. // This allows us to log the events and even run exclusive logic if any. @@ -315,7 +313,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { // when the node is started for the first time. err = node.UpdateStatusRunning() if err != nil { - panic("Failed to initialize the node status to running") + glog.Fatalf("Error running Node -- %v", err) } case STOPPED: // This case is called during a node restart. @@ -324,7 +322,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { glog.Info("Changing node status from STOP to RUNNING...") err = node.UpdateStatusRunning() if err != nil { - panic("Failed to initialize the node status to running") + glog.Fatalf("Error running Node -- %v", err) } default: // Rare occurrence. Happens if you set an invalid node status while restarting a node. @@ -376,7 +374,6 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { // Valid transition sequence is NEVERSTARTED->RUNNING->STOPPED. func (node *Node) UpdateStatusStopped() error { if err := node.changeNodeStatus(STOPPED); err != nil { - glog.Errorf("Error stopping Node -- %v", err) return err } return nil @@ -385,7 +382,6 @@ func (node *Node) UpdateStatusStopped() error { // Changes node status to RUNNING. func (node *Node) UpdateStatusRunning() error { if err := node.changeNodeStatus(RUNNING); err != nil { - glog.Errorf("Error stopping Node -- %v", err) return err } return nil @@ -486,10 +482,8 @@ func (node *Node) IsRunning() bool { if err != nil { return false } - if status == RUNNING { - return true - } - return false + + return status == RUNNING } func (node *Node) Stop() { From 716793a4610de539e2918034263ade0eacc238b8 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Tue, 15 Nov 2022 17:52:44 +0530 Subject: [PATCH 14/21] Optimizing the usage of locks in cmd/node.go - Removing runningMutex and replacing it with statusMutex - Adding statusMutex inside *Node.Start() and *Node.Stop() - Changing LoadStatus to GetStatus - Creating private functions getStatusWithoutLock and updateStatusWithoutLock to change node status without holding statusLocks. - Creating wrapper functions GetStatus and UpdateStatus to fetch and update node status while holding statusMutex locks. --- cmd/node.go | 113 ++++++++++++++++++++++++++++++---------------------- 1 file changed, 66 insertions(+), 47 deletions(-) diff --git a/cmd/node.go b/cmd/node.go index f322e1825..c1a5906fc 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -34,23 +34,21 @@ var ( // ErrAlreadyStopped is returned when somebody tries to stop an already // stopped service (without resetting it). ErrAlreadyStopped = errors.New("already stopped") - // ErrNodeNeverStarted is returned when somebody tries to find or changge the + // ErrNodeNeverStarted is returned when somebody tries to find or change the // running status of the server without never starting it once. ErrNodeNeverStarted = errors.New("never started the node instance") // Cannot set node status to NEVERSTARTED. // NEVERSTARTED is the default status, cannot set deliberately. ErrCannotSetToNeverStarted = errors.New("cannot set the node status to neverstarted") - // Error if the atomic compare of swap of the node status fails - ErrFailedtoChangeNodeStatus = errors.New("failed to change the status of the node") // Erorr when invalid status is set for node ErrInvalidNodeStatus = errors.New("invalid node status. check cmd/node.go for valid states") - // Invalid Status return value for the *Node.LoadStatus() helper function. + // Invalid Status return value for the *Node.GetStatus()/*Node.getStatusWithoutLock helper functions. // byte(255) is a reserved status for invalid status code for the Node INVALIDNODESTATUS = NodeStatus(255) ) // Valid status codes for the Node. -// Status should be retrieved using the helper functions *Node.LoadStatus() +// Status should be retrieved using the helper functions *Node.GetStatus() const ( // Status when the node is not initialized or started for the first time NEVERSTARTED NodeStatus = iota // byte(0) @@ -74,16 +72,13 @@ type Node struct { // status is nil when a NewNode is created, it is initialized and set to RUNNING [byte(1)] on node.Start(), // set to STOPPED [byte(2)] after Stop() is called. - // Use the convenience methods changeNodeStatus/UpdateStatusRunning/UpdateStatusStopped to change node status. + // Use the convenience methods UpdateStatus/UpdateStatusRunning/UpdateStatusStopped to change node status. // Use *Node.IsRunning() to check if the node is running. - // Use *Node.LoadStatus() to retrieve the status of the node. + // Use *Node.GetStatus() to retrieve the status of the node. + // Use *Node.getStatusWithoutLock() if the statusMutex is held by the caller. status *NodeStatus // Held whenever the status of the node is read or altered. - // Cannot use runningMutex to safe guard Node status due to deadlocks! statusMutex sync.Mutex - // runningMutex is held whenever we call Start() or Stop() on the node. - runningMutex sync.Mutex - // internalExitChan is used internally to signal that a node should close. internalExitChan chan struct{} // nodeMessageChan is passed to the core engine and used to trigger node actions such as a restart or database reset. @@ -113,8 +108,8 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { flag.Set("alsologtostderr", "true") flag.Parse() glog.CopyStandardLogTo("INFO") - node.runningMutex.Lock() - defer node.runningMutex.Unlock() + node.statusMutex.Lock() + defer node.statusMutex.Unlock() node.internalExitChan = make(chan struct{}) node.nodeMessageChan = make(chan lib.NodeMessage) @@ -297,10 +292,9 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { } } - node.statusMutex.Lock() // Load the node status. // This is to identify whether the node is initialized for the first time or it's a restart. - status, err := node.LoadStatus() + status, err := node.getStatusWithoutLock() if err != nil { glog.Fatal("Failed to load node status") } @@ -330,7 +324,6 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { panic(fmt.Sprintf("Cannot change node status to RUNNING from the current status %v", status)) } - node.statusMutex.Unlock() if shouldRestart { if node.nodeMessageChan != nil { @@ -356,9 +349,8 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { case <-syscallChannel: } - node.statusMutex.Lock() node.Stop() - node.statusMutex.Unlock() + for _, channel := range exitChannels { if *channel != nil { close(*channel) @@ -373,7 +365,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { // Cannot transition from NEVERSTARTED to STOPPED. // Valid transition sequence is NEVERSTARTED->RUNNING->STOPPED. func (node *Node) UpdateStatusStopped() error { - if err := node.changeNodeStatus(STOPPED); err != nil { + if err := node.updateStatusWithoutLock(STOPPED); err != nil { return err } return nil @@ -381,22 +373,22 @@ func (node *Node) UpdateStatusStopped() error { // Changes node status to RUNNING. func (node *Node) UpdateStatusRunning() error { - if err := node.changeNodeStatus(RUNNING); err != nil { + if err := node.updateStatusWithoutLock(RUNNING); err != nil { return err } return nil } // Loads the status of Node. -// Modifying LoadStatus() and the validateStatus() functions and their tests are hard requirements +// Modifying getStatusWithoutLock() and the validateStatus() functions and their tests are hard requirements // to add new status codes for the node. -func (node *Node) LoadStatus() (NodeStatus, error) { +func (node *Node) getStatusWithoutLock() (NodeStatus, error) { // Never initialized the server using *Node.Start() if node.status == nil { return NEVERSTARTED, nil } // using switch and case prevents from adding new invalid codes - // without changing the LoadStatus() function and its unit test. + // without changing the getStatusWithoutLock() function and its unit test. switch *node.status { // Node is started using *Node.Start() and not stopped yet. case RUNNING: @@ -411,9 +403,18 @@ func (node *Node) LoadStatus() (NodeStatus, error) { } } +// Wrapper function to get the status of the node with statusMutex. +// Use private method getStatusWithoutLock(), in case the locks are held by the caller. +func (node *Node) GetStatus() (NodeStatus, error) { + node.statusMutex.Lock() + defer node.statusMutex.Unlock() + + return node.getStatusWithoutLock() +} + // Verifies whether a status code of the node is a valid status code. // Used while changing the status of the node -// Modifying LoadStatus() and the validateStatus() functions and their tests are hard requirements +// Modifying getStatusWithoutLock() and the validateStatus() functions and their tests are hard requirements // to add new status codes for the node. func validateNodeStatus(status NodeStatus) error { switch status { @@ -435,7 +436,7 @@ func validateNodeStatus(status NodeStatus) error { // changes the running status of the node // Always use this function to change the status of the node. -func (node *Node) changeNodeStatus(newStatus NodeStatus) error { +func (node *Node) updateStatusWithoutLock(newStatus NodeStatus) error { if err := validateNodeStatus(newStatus); err != nil { return err } @@ -447,7 +448,7 @@ func (node *Node) changeNodeStatus(newStatus NodeStatus) error { return ErrCannotSetToNeverStarted } // Load the current status of the Node. - status, err := node.LoadStatus() + status, err := node.getStatusWithoutLock() if err != nil { return err } @@ -474,10 +475,21 @@ func (node *Node) changeNodeStatus(newStatus NodeStatus) error { return nil } +// Wrapper function for changing node status with statusMutex +// use updateStatusWithoutLock if the lock is held by the caller. +// calling this function while the statusMutex lock is already held will lead to deadlocks! +func (node *Node) UpdateStatus(newStatus NodeStatus) error { + node.statusMutex.Lock() + defer node.statusMutex.Unlock() + + return node.updateStatusWithoutLock(newStatus) + +} + // Helper function to check if the node is running. // returns false if node status is not set to RUNNING func (node *Node) IsRunning() bool { - status, err := node.LoadStatus() + status, err := node.GetStatus() if err != nil { return false @@ -486,16 +498,15 @@ func (node *Node) IsRunning() bool { return status == RUNNING } -func (node *Node) Stop() { - node.runningMutex.Lock() - defer node.runningMutex.Unlock() +func (node *Node) Stop() error { + node.statusMutex.Lock() + defer node.statusMutex.Unlock() // Change nodes running status to stop - // Node has to be in RUNNING status to be able to STOP! - // FIXME: Return error in cases where the the node fails to stop. This makes the function testable. + // Node's current status has to be RUNNING to be able to STOP! if err := node.UpdateStatusStopped(); err != nil { glog.Errorf("Error stopping Node -- %v", err) - return + return err } glog.Infof(lib.CLog(lib.Yellow, "Node is shutting down. This might take a minute. Please don't "+ @@ -533,6 +544,12 @@ func (node *Node) Stop() { close(node.internalExitChan) node.internalExitChan = nil } + return nil +} + +// Return internal exit channel to wait for the node to stop. +func (node *Node) Quit() chan struct{} { + return node.internalExitChan } // Close a database and handle the stopWaitGroup accordingly. We close databases in a go routine to speed up the process. @@ -562,15 +579,14 @@ func (node *Node) listenToNodeMessages(exitChannels ...*chan struct{}) { switch operation { case lib.NodeRestart: // Using Mutex while accessing the Node status to avoid any race conditions - node.statusMutex.Lock() - if !node.IsRunning() { - panic("Node.listenToNodeMessages: Node is currently not running, nodeMessageChan should've not been called!") - } - glog.Infof("Node.listenToNodeMessages: Restarting node.") glog.Infof("Node.listenToNodeMessages: Stopping node") - node.Stop() - node.statusMutex.Unlock() + // Stopping node + // Stop only works if the current state of the node is RUNNING. + if err := node.Stop(); err != nil { + panic(fmt.Sprintf("Error stopping node: %v", err)) + } + glog.Infof("Node.listenToNodeMessages: Finished stopping node") case lib.NodeErase: @@ -578,12 +594,17 @@ func (node *Node) listenToNodeMessages(exitChannels ...*chan struct{}) { glog.Infof("Node.listenToNodeMessages: Stopping node") // cannot stop the node if the node is not already in RUNNING state. // Stop the node and remove the data directory if the server is in RUNNING state. - node.statusMutex.Lock() - if node.IsRunning() { - node.Stop() + + // Stopping node. + // When restart with database erase fails when the node starts for the first time + // This is because the status of the node is still NEVERSTARTED. + // We log the Stop failure, and still go ahead with the hard restart, a.k.a restart with database erase! + if err := node.Stop(); err != nil { + glog.Infof("Node.listenToNodeMessages: Node Stop operation failed.") + glog.Infof("Still going ahead with the database erase.") + } else { + glog.Infof("Node.listenToNodeMessages: Finished stopping node") } - node.statusMutex.Unlock() - glog.Infof("Node.listenToNodeMessages: Finished stopping node") if err := os.RemoveAll(node.Config.DataDirectory); err != nil { glog.Fatal(lib.CLog(lib.Red, fmt.Sprintf("IMPORTANT: Problem removing the directory (%v), you "+ @@ -592,10 +613,8 @@ func (node *Node) listenToNodeMessages(exitChannels ...*chan struct{}) { } } - glog.Infof("Node.listenToNodeMessages: Restarting node") // Wait a few seconds so that all peer messages we've sent while closing the node get propagated in the network. go node.Start(exitChannels...) - break } } From 0cce3542181823cc66fbc978bc0948332caeaa37 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Tue, 15 Nov 2022 17:55:30 +0530 Subject: [PATCH 15/21] Updating tests for cmd/node.go - Updating function names to match the updates to utility functions in node.go - Adding test to verify graceful shutdown of the node. --- cmd/node_test.go | 75 +++++++++++++++++++++++++++++++++++------------- 1 file changed, 55 insertions(+), 20 deletions(-) diff --git a/cmd/node_test.go b/cmd/node_test.go index aff3768df..bf372a3ac 100644 --- a/cmd/node_test.go +++ b/cmd/node_test.go @@ -1,8 +1,11 @@ package cmd import ( + "fmt" "os" + "syscall" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -101,7 +104,7 @@ func TestNodeUpdateStatusStopped(t *testing.T) { // Node status is change in the following sequence, // NEVERSTARTED -> RUNNING -> STOP -> RUNNING // In each state change it's tested for valid change in status. -func TestNodechangeNodeStatus(t *testing.T) { +func TestNodeUpdateStatus(t *testing.T) { testDir := getTestDirectory(t, "test_node_change_running_status") defer os.RemoveAll(testDir) @@ -117,13 +120,13 @@ func TestNodechangeNodeStatus(t *testing.T) { // This is an invalid status transition. // Node status cannot needs to transitioned to RUNNING before changing to STOPPED expError := ErrNodeNeverStarted - actualError := testNode.changeNodeStatus(STOPPED) + actualError := testNode.UpdateStatus(STOPPED) require.ErrorIs(t, actualError, expError) // Cannot set the status to NEVERSTARTED, // It's the default value before the Node is initialized. expError = ErrCannotSetToNeverStarted - actualError = testNode.changeNodeStatus(NEVERSTARTED) + actualError = testNode.UpdateStatus(NEVERSTARTED) require.ErrorIs(t, actualError, expError) // Starting the node. // The current status of the node is RUNNING. @@ -131,47 +134,47 @@ func TestNodechangeNodeStatus(t *testing.T) { // The status should be changed to RUNNING. // This successfully tests the transition of status from NEVERSTARTED to RUNNING expectedStatus := RUNNING - actualStatus, err := testNode.LoadStatus() + actualStatus, err := testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) // Cannot set the status to NEVERSTARTED, // It's the default value before the Node is initialized. expError = ErrCannotSetToNeverStarted - actualError = testNode.changeNodeStatus(NEVERSTARTED) + actualError = testNode.UpdateStatus(NEVERSTARTED) require.ErrorIs(t, actualError, expError) // Cannot expect the Node status to changed from STOPPED to RUNNING, // while it's current state is RUNNING expError = ErrAlreadyStarted - actualError = testNode.changeNodeStatus(RUNNING) + actualError = testNode.UpdateStatus(RUNNING) require.ErrorIs(t, actualError, expError) // Stopping the node. // This should transition the Node state from RUNNING to STOPPED. testNode.Stop() expectedStatus = STOPPED - actualStatus, err = testNode.LoadStatus() + actualStatus, err = testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) // Cannot set the status to NEVERSTARTED, // It's the default value before the Node is initialized. expError = ErrCannotSetToNeverStarted - actualError = testNode.changeNodeStatus(NEVERSTARTED) + actualError = testNode.UpdateStatus(NEVERSTARTED) require.ErrorIs(t, actualError, expError) // Cannot expect the Node status to changed from NEVERSTARTED to STOPPED, // while it's current state is STOPPED expError = ErrAlreadyStopped - actualError = testNode.changeNodeStatus(STOPPED) + actualError = testNode.UpdateStatus(STOPPED) require.ErrorIs(t, actualError, expError) // Changing status from STOPPED to RUNNING testNode.Start() // The following tests validates a successful transition of state from STOP to RUNNING expectedStatus = RUNNING - actualStatus, err = testNode.LoadStatus() + actualStatus, err = testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) testNode.Stop() @@ -179,22 +182,22 @@ func TestNodechangeNodeStatus(t *testing.T) { // Set the Node status to an invalid status code // protects from the ignorance of adding new status codes to the iota sequence! expError = ErrInvalidNodeStatus - actualError = testNode.changeNodeStatus(NodeStatus(3)) + actualError = testNode.UpdateStatus(NodeStatus(3)) require.ErrorIs(t, actualError, expError) expError = ErrInvalidNodeStatus - actualError = testNode.changeNodeStatus(NodeStatus(4)) + actualError = testNode.UpdateStatus(NodeStatus(4)) require.ErrorIs(t, actualError, expError) expError = ErrInvalidNodeStatus - actualError = testNode.changeNodeStatus(NodeStatus(5)) + actualError = testNode.UpdateStatus(NodeStatus(5)) require.ErrorIs(t, actualError, expError) } -// Tests for *Node.LoadStatus() +// Tests for *Node.GetStatus() // Loads the status of node after node operations and tests its correctness. -func TestLoadStatus(t *testing.T) { +func TestGetStatus(t *testing.T) { testDir := getTestDirectory(t, "test_load_status") defer os.RemoveAll(testDir) @@ -206,7 +209,7 @@ func TestLoadStatus(t *testing.T) { // Status is set to NEVERSTARTED before the node is started. expectedStatus := NEVERSTARTED - actualStatus, err := testNode.LoadStatus() + actualStatus, err := testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) @@ -215,7 +218,7 @@ func TestLoadStatus(t *testing.T) { // The status is expected to be RUNNING once the node is started. expectedStatus = RUNNING - actualStatus, err = testNode.LoadStatus() + actualStatus, err = testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) @@ -224,7 +227,7 @@ func TestLoadStatus(t *testing.T) { // The status is expected to be STOPPED once the node is stopped. expectedStatus = STOPPED - actualStatus, err = testNode.LoadStatus() + actualStatus, err = testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) @@ -234,10 +237,9 @@ func TestLoadStatus(t *testing.T) { // expect error and invalid status code expectedStatus = INVALIDNODESTATUS - actualStatus, err = testNode.LoadStatus() + actualStatus, err = testNode.GetStatus() require.ErrorIs(t, err, ErrInvalidNodeStatus) require.Equal(t, actualStatus, expectedStatus) - } func TestValidateNodeStatus(t *testing.T) { @@ -251,3 +253,36 @@ func TestValidateNodeStatus(t *testing.T) { require.ErrorIs(t, err, errors[i]) } } + +// Stop the node and test whether the internalExitChan fires as expected. +func TestNodeStop(t *testing.T) { + testDir := getTestDirectory(t, "test_load_status") + defer os.RemoveAll(testDir) + + testConfig := GenerateTestConfig(t, 18000, testDir, 10) + + testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} + + testNode := NewNode(&testConfig) + testNode.Start() + + // stop the node + go func() { + err := testNode.Stop() + require.NoError(t, err) + }() + + // Test whether the node stops successfully under three seconds. + select { + case <-testNode.Quit(): + case <-time.After(3 * time.Second): + pid := os.Getpid() + p, err := os.FindProcess(pid) + if err != nil { + panic(err) + } + err = p.Signal(syscall.SIGABRT) + fmt.Println(err) + t.Fatal("timed out waiting for shutdown") + } +} From 10c76ed82e7f54a3e26ab1ddcb79ceb361f54fd1 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Tue, 15 Nov 2022 18:03:04 +0530 Subject: [PATCH 16/21] Renaming UpdateStatus to SetStatus --- cmd/node.go | 24 ++++++++++++------------ cmd/node_test.go | 36 ++++++++++++++++++------------------ 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/cmd/node.go b/cmd/node.go index c1a5906fc..c388dde9e 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -72,7 +72,7 @@ type Node struct { // status is nil when a NewNode is created, it is initialized and set to RUNNING [byte(1)] on node.Start(), // set to STOPPED [byte(2)] after Stop() is called. - // Use the convenience methods UpdateStatus/UpdateStatusRunning/UpdateStatusStopped to change node status. + // Use the convenience methods SetStatus/SetStatusRunning/SetStatusStopped to change node status. // Use *Node.IsRunning() to check if the node is running. // Use *Node.GetStatus() to retrieve the status of the node. // Use *Node.getStatusWithoutLock() if the statusMutex is held by the caller. @@ -305,7 +305,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { glog.Info("Changing node status from NEVERSTARTED to RUNNING...") // The node status is changed from NEVERSTARTED to RUNNING only once // when the node is started for the first time. - err = node.UpdateStatusRunning() + err = node.SetStatusRunning() if err != nil { glog.Fatalf("Error running Node -- %v", err) } @@ -314,7 +314,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { // During restart the Node is first STOPPED before setting the // status again to RUNNING. glog.Info("Changing node status from STOP to RUNNING...") - err = node.UpdateStatusRunning() + err = node.SetStatusRunning() if err != nil { glog.Fatalf("Error running Node -- %v", err) } @@ -364,16 +364,16 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { // Changes node status to STOPPED. // Cannot transition from NEVERSTARTED to STOPPED. // Valid transition sequence is NEVERSTARTED->RUNNING->STOPPED. -func (node *Node) UpdateStatusStopped() error { - if err := node.updateStatusWithoutLock(STOPPED); err != nil { +func (node *Node) SetStatusStopped() error { + if err := node.setStatusWithoutLock(STOPPED); err != nil { return err } return nil } // Changes node status to RUNNING. -func (node *Node) UpdateStatusRunning() error { - if err := node.updateStatusWithoutLock(RUNNING); err != nil { +func (node *Node) SetStatusRunning() error { + if err := node.setStatusWithoutLock(RUNNING); err != nil { return err } return nil @@ -436,7 +436,7 @@ func validateNodeStatus(status NodeStatus) error { // changes the running status of the node // Always use this function to change the status of the node. -func (node *Node) updateStatusWithoutLock(newStatus NodeStatus) error { +func (node *Node) setStatusWithoutLock(newStatus NodeStatus) error { if err := validateNodeStatus(newStatus); err != nil { return err } @@ -476,13 +476,13 @@ func (node *Node) updateStatusWithoutLock(newStatus NodeStatus) error { } // Wrapper function for changing node status with statusMutex -// use updateStatusWithoutLock if the lock is held by the caller. +// use SetStatusWithoutLock if the lock is held by the caller. // calling this function while the statusMutex lock is already held will lead to deadlocks! -func (node *Node) UpdateStatus(newStatus NodeStatus) error { +func (node *Node) SetStatus(newStatus NodeStatus) error { node.statusMutex.Lock() defer node.statusMutex.Unlock() - return node.updateStatusWithoutLock(newStatus) + return node.setStatusWithoutLock(newStatus) } @@ -504,7 +504,7 @@ func (node *Node) Stop() error { // Change nodes running status to stop // Node's current status has to be RUNNING to be able to STOP! - if err := node.UpdateStatusStopped(); err != nil { + if err := node.SetStatusStopped(); err != nil { glog.Errorf("Error stopping Node -- %v", err) return err } diff --git a/cmd/node_test.go b/cmd/node_test.go index bf372a3ac..d640d8781 100644 --- a/cmd/node_test.go +++ b/cmd/node_test.go @@ -45,31 +45,31 @@ func TestNodeStatusRunning(t *testing.T) { testNode := NewNode(&testConfig) // Can change the status to RUNNING from the state NEVERSTARTED - actualErr := testNode.UpdateStatusRunning() + actualErr := testNode.SetStatusRunning() require.NoError(t, actualErr) // Change status from RUNNING to STOPPED - actualErr = testNode.UpdateStatusStopped() + actualErr = testNode.SetStatusStopped() require.NoError(t, actualErr) // start the server // Cannot change status to RUNNING while the node is already RUNNING! testNode.Start() expErr := ErrAlreadyStarted - actualErr = testNode.UpdateStatusRunning() + actualErr = testNode.SetStatusRunning() require.ErrorIs(t, actualErr, expErr) // Stop the node testNode.Stop() // Should be able to change status to RUNNING from STOP. - actualErr = testNode.UpdateStatusRunning() + actualErr = testNode.SetStatusRunning() require.NoError(t, actualErr) // Once the running flag is changed, the isRunning function should return true require.True(t, testNode.IsRunning()) } -func TestNodeUpdateStatusStopped(t *testing.T) { +func TestNodeSetStatusStopped(t *testing.T) { testDir := getTestDirectory(t, "test_node_change_running_status") defer os.RemoveAll(testDir) @@ -82,7 +82,7 @@ func TestNodeUpdateStatusStopped(t *testing.T) { // Need to call node.start() to atleast once to be able to change node status // Cannot change status of node which never got initialized in the first place expErr := ErrNodeNeverStarted - actualErr := testNode.UpdateStatusStopped() + actualErr := testNode.SetStatusStopped() require.ErrorIs(t, actualErr, expErr) // start the node @@ -90,21 +90,21 @@ func TestNodeUpdateStatusStopped(t *testing.T) { // Once the server is started testNode.Start() - actualErr = testNode.UpdateStatusStopped() + actualErr = testNode.SetStatusStopped() require.NoError(t, actualErr) // stop the node testNode.Stop() expErr = ErrAlreadyStopped - actualErr = testNode.UpdateStatusStopped() + actualErr = testNode.SetStatusStopped() require.ErrorIs(t, actualErr, expErr) } // Node status is change in the following sequence, // NEVERSTARTED -> RUNNING -> STOP -> RUNNING // In each state change it's tested for valid change in status. -func TestNodeUpdateStatus(t *testing.T) { +func TestNodeSetStatus(t *testing.T) { testDir := getTestDirectory(t, "test_node_change_running_status") defer os.RemoveAll(testDir) @@ -120,13 +120,13 @@ func TestNodeUpdateStatus(t *testing.T) { // This is an invalid status transition. // Node status cannot needs to transitioned to RUNNING before changing to STOPPED expError := ErrNodeNeverStarted - actualError := testNode.UpdateStatus(STOPPED) + actualError := testNode.SetStatus(STOPPED) require.ErrorIs(t, actualError, expError) // Cannot set the status to NEVERSTARTED, // It's the default value before the Node is initialized. expError = ErrCannotSetToNeverStarted - actualError = testNode.UpdateStatus(NEVERSTARTED) + actualError = testNode.SetStatus(NEVERSTARTED) require.ErrorIs(t, actualError, expError) // Starting the node. // The current status of the node is RUNNING. @@ -141,13 +141,13 @@ func TestNodeUpdateStatus(t *testing.T) { // Cannot set the status to NEVERSTARTED, // It's the default value before the Node is initialized. expError = ErrCannotSetToNeverStarted - actualError = testNode.UpdateStatus(NEVERSTARTED) + actualError = testNode.SetStatus(NEVERSTARTED) require.ErrorIs(t, actualError, expError) // Cannot expect the Node status to changed from STOPPED to RUNNING, // while it's current state is RUNNING expError = ErrAlreadyStarted - actualError = testNode.UpdateStatus(RUNNING) + actualError = testNode.SetStatus(RUNNING) require.ErrorIs(t, actualError, expError) // Stopping the node. @@ -161,13 +161,13 @@ func TestNodeUpdateStatus(t *testing.T) { // Cannot set the status to NEVERSTARTED, // It's the default value before the Node is initialized. expError = ErrCannotSetToNeverStarted - actualError = testNode.UpdateStatus(NEVERSTARTED) + actualError = testNode.SetStatus(NEVERSTARTED) require.ErrorIs(t, actualError, expError) // Cannot expect the Node status to changed from NEVERSTARTED to STOPPED, // while it's current state is STOPPED expError = ErrAlreadyStopped - actualError = testNode.UpdateStatus(STOPPED) + actualError = testNode.SetStatus(STOPPED) require.ErrorIs(t, actualError, expError) // Changing status from STOPPED to RUNNING @@ -182,15 +182,15 @@ func TestNodeUpdateStatus(t *testing.T) { // Set the Node status to an invalid status code // protects from the ignorance of adding new status codes to the iota sequence! expError = ErrInvalidNodeStatus - actualError = testNode.UpdateStatus(NodeStatus(3)) + actualError = testNode.SetStatus(NodeStatus(3)) require.ErrorIs(t, actualError, expError) expError = ErrInvalidNodeStatus - actualError = testNode.UpdateStatus(NodeStatus(4)) + actualError = testNode.SetStatus(NodeStatus(4)) require.ErrorIs(t, actualError, expError) expError = ErrInvalidNodeStatus - actualError = testNode.UpdateStatus(NodeStatus(5)) + actualError = testNode.SetStatus(NodeStatus(5)) require.ErrorIs(t, actualError, expError) } From 062f84f768992f19aad733848a1e57d0aabf5ee4 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Fri, 25 Nov 2022 05:51:14 +0530 Subject: [PATCH 17/21] Minor changes - Removing the .DS files - Adding error wrapper %w - Removing the redundant lock --- .DS_Store | Bin 6148 -> 0 bytes .vscode/settings.json | 5 +++++ cmd/node.go | 4 ++-- cmd/run.go | 2 -- integration_testing/.DS_Store | Bin 6148 -> 0 bytes 5 files changed, 7 insertions(+), 4 deletions(-) delete mode 100644 .DS_Store create mode 100644 .vscode/settings.json delete mode 100644 integration_testing/.DS_Store diff --git a/.DS_Store b/.DS_Store deleted file mode 100644 index 9cbdf7378ee75eb330f890b50ff42acd5ebf06a4..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHKyGjH>5Ukb<4onVA4Cfd8gXK8C;66YFWf5c!&PTkF-{q%S{Xkgu#K=Udp}S^! zyJpx5wzmP;_U+*s*Z^459r58|Y5v@OVP};wBAsW9IN~0!c*0>(eLCUXOV)R?9%uZH z+sM7dc^%{X{@9-neazcVAE&GokOERb3P=Gd@QVW8dui*7L`5ke1*E{Y0{(qybjP7^ zN{mkjLyQ2#71Lo{$1Fi?o*)i|QzA1oODZv`RwITbo%vREL*bN|bXW}^RySKsC>FQ# z{1)Y~AyH8ZNP)Qm=egZ@|G%OCF#pd<+DQQ^@UIlG)o#Dr@|CK$E?&-iZKL1Oz2=kd q#&u8_q8$^X9dqOD_$rFBuKAkJhr%f_=*$P5sGkAXMJ5IQT7fT_i5Y?b diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 000000000..596fdd469 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,5 @@ +{ + "cSpell.words": [ + "Wrapf" + ] +} \ No newline at end of file diff --git a/cmd/node.go b/cmd/node.go index c388dde9e..f21ac097a 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -366,7 +366,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { // Valid transition sequence is NEVERSTARTED->RUNNING->STOPPED. func (node *Node) SetStatusStopped() error { if err := node.setStatusWithoutLock(STOPPED); err != nil { - return err + return fmt.Errorf("failed to set status to stop: %w", err) } return nil } @@ -374,7 +374,7 @@ func (node *Node) SetStatusStopped() error { // Changes node status to RUNNING. func (node *Node) SetStatusRunning() error { if err := node.setStatusWithoutLock(RUNNING); err != nil { - return err + return fmt.Errorf("failed to set status to running: %w", err) } return nil } diff --git a/cmd/run.go b/cmd/run.go index 6bc91fe59..57a03c8a3 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -29,9 +29,7 @@ func Run(cmd *cobra.Command, args []string) { node.Start(&shutdownListener) defer func() { - node.statusMutex.Lock() node.Stop() - node.statusMutex.Unlock() glog.Info("Shutdown complete") }() <-shutdownListener diff --git a/integration_testing/.DS_Store b/integration_testing/.DS_Store deleted file mode 100644 index 2c6db71aef675844f4e8feda6864ca011a308989..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHKOHRWu5PdFPR3Jf%Sg@&-6I9{`p(+d39H2C9RS+RYZIQ|@d+xv$I3m0mTUGkY z5+Rt8#?PO5W9Lb>V*sxII=KSc09tgx<}r&sCgb8e)`&ea=rqR@7I;H}F@`(Q)?o^m z0>4cG*}ENN$l}j5G2_>vb%68r9@z%(ilzYL0?= z3TCS1;+ou#iV|(k?5SqXZy%|m^?XsX%5QO{UNbBXDprj@WBf{2I-zQ-`qiphRW>I# z`?m`4&K7H(1hmo=Fa=D3wE}WJBy_VVgFZhg-mN1rHHq(V$1mI2vA PvmXJK!3tC0M-})6`+JZ$ From b32370f373ea1b755e6337fce7b79b36d70c2456 Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Fri, 25 Nov 2022 07:01:16 +0530 Subject: [PATCH 18/21] Deleting vs code settings file --- .vscode/settings.json | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .vscode/settings.json diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index 596fdd469..000000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "cSpell.words": [ - "Wrapf" - ] -} \ No newline at end of file From 957c0d3e492be94f61ac65135fc2bfe2a49631f7 Mon Sep 17 00:00:00 2001 From: Lazy Nina <> Date: Tue, 29 Nov 2022 16:14:17 -0800 Subject: [PATCH 19/21] Suggested changes to pr 414 to reduce file changes --- cmd/config.go | 60 +---------- cmd/node.go | 4 +- integration_testing/blocksync_test.go | 42 ++++---- integration_testing/hypersync_test.go | 94 ++++++++--------- integration_testing/migrations_test.go | 12 +-- integration_testing/mining_test.go | 6 +- {cmd => integration_testing}/node_test.go | 118 +++++++++++----------- integration_testing/rollback_test.go | 12 +-- integration_testing/tools.go | 42 +++++++- integration_testing/txindex_test.go | 12 +-- 10 files changed, 194 insertions(+), 208 deletions(-) rename {cmd => integration_testing}/node_test.go (70%) diff --git a/cmd/config.go b/cmd/config.go index 15afbc214..64b2e8c00 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -1,23 +1,13 @@ package cmd import ( - "io/ioutil" - "os" - "path/filepath" - "testing" - "github.com/deso-protocol/core/lib" "github.com/golang/glog" "github.com/spf13/viper" - "github.com/stretchr/testify/require" + "os" + "path/filepath" ) -// Global variable that determines the max tip blockheight of syncing nodes throughout test cases. -const MaxSyncBlockHeight = 1500 - -// Global variable that allows setting node configuration hypersync snapshot period. -const HyperSyncSnapshotPeriod = 1000 - type Config struct { // Core Params *lib.DeSoParams @@ -230,49 +220,3 @@ func (config *Config) Print() { glog.Infof("Rate Limit Feerate: %d", config.RateLimitFeerate) glog.Infof("Min Feerate: %d", config.MinFeerate) } - -// GenerateTestConfig creates a default config for a node, with provided port, db directory, and number of max peers. -// It's usually the first step to starting a node. -func GenerateTestConfig(t *testing.T, port uint32, dataDir string, maxPeers uint32) Config { - config := Config{} - params := lib.DeSoMainnetParams - - params.DNSSeeds = []string{} - config.Params = ¶ms - config.ProtocolPort = uint16(port) - // "/Users/piotr/data_dirs/n98_1" - config.DataDirectory = dataDir - if err := os.MkdirAll(config.DataDirectory, os.ModePerm); err != nil { - t.Fatalf("Could not create data directories (%s): %v", config.DataDirectory, err) - } - config.TXIndex = false - config.HyperSync = false - config.MaxSyncBlockHeight = 0 - config.ConnectIPs = []string{} - config.PrivateMode = true - config.GlogV = 0 - config.GlogVmodule = "*bitcoin_manager*=0,*balance*=0,*view*=0,*frontend*=0,*peer*=0,*addr*=0,*network*=0,*utils*=0,*connection*=0,*main*=0,*server*=0,*mempool*=0,*miner*=0,*blockchain*=0" - config.MaxInboundPeers = maxPeers - config.TargetOutboundPeers = maxPeers - config.StallTimeoutSeconds = 900 - config.MinFeerate = 1000 - config.OneInboundPerIp = false - config.MaxBlockTemplatesCache = 100 - config.MaxSyncBlockHeight = 100 - config.MinBlockUpdateInterval = 10 - config.SnapshotBlockHeightPeriod = HyperSyncSnapshotPeriod - config.MaxSyncBlockHeight = MaxSyncBlockHeight - config.SyncType = lib.NodeSyncTypeBlockSync - //config.ArchivalMode = true - - return config -} - -func getTestDirectory(t *testing.T, testName string) string { - require := require.New(t) - dbDir, err := ioutil.TempDir("", testName) - if err != nil { - require.NoError(err) - } - return dbDir -} diff --git a/cmd/node.go b/cmd/node.go index f21ac097a..5d46f051c 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -416,7 +416,7 @@ func (node *Node) GetStatus() (NodeStatus, error) { // Used while changing the status of the node // Modifying getStatusWithoutLock() and the validateStatus() functions and their tests are hard requirements // to add new status codes for the node. -func validateNodeStatus(status NodeStatus) error { +func ValidateNodeStatus(status NodeStatus) error { switch status { // Instance of the *Node is created, but not yet initialized using *Node.Start() case NEVERSTARTED: @@ -437,7 +437,7 @@ func validateNodeStatus(status NodeStatus) error { // changes the running status of the node // Always use this function to change the status of the node. func (node *Node) setStatusWithoutLock(newStatus NodeStatus) error { - if err := validateNodeStatus(newStatus); err != nil { + if err := ValidateNodeStatus(newStatus); err != nil { return err } diff --git a/integration_testing/blocksync_test.go b/integration_testing/blocksync_test.go index eb20a983b..c088a57a5 100644 --- a/integration_testing/blocksync_test.go +++ b/integration_testing/blocksync_test.go @@ -20,20 +20,20 @@ func TestSimpleBlockSync(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "block_sync_test") - dbDir2 := getTestDirectory(t, "block_sync_test_dir2") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -66,20 +66,20 @@ func TestSimpleSyncRestart(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_simple_sync_restart") - dbDir2 := getTestDirectory(t, "test_simple_sync_restart_2") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -118,26 +118,26 @@ func TestSimpleSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_simple_sync_disconnect_switch_new_peer") - dbDir2 := getTestDirectory(t, "test_simple_sync_disconnect_switch_new_peer_2") - dbDir3 := getTestDirectory(t, "test_simple_sync_disconnect_switch_new_peer_3") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) + dbDir3 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync - config3 := cmd.GenerateTestConfig(t, 18002, dbDir3, 10) + config3 := generateConfig(t, 18002, dbDir3, 10) config3.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} config3.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) - node3 := cmd.NewNode(&config3) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) + node3 := cmd.NewNode(config3) node1 = startNode(t, node1) node2 = startNode(t, node2) diff --git a/integration_testing/hypersync_test.go b/integration_testing/hypersync_test.go index 49e8ed110..1665c3d89 100644 --- a/integration_testing/hypersync_test.go +++ b/integration_testing/hypersync_test.go @@ -20,22 +20,22 @@ func TestSimpleHyperSync(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "simple_hyper_sync") - dbDir2 := getTestDirectory(t, "simple_hyper_sync_2") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSync config1.HyperSync = true config2.HyperSync = true config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -69,18 +69,18 @@ func TestHyperSyncFromHyperSyncedNode(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_hyper_synced_node") - dbDir2 := getTestDirectory(t, "test_hyper_synced_node_2") - dbDir3 := getTestDirectory(t, "test_hyper_synced_node_3") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) + dbDir3 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSyncArchival - config3 := cmd.GenerateTestConfig(t, 18002, dbDir3, 10) + config3 := generateConfig(t, 18002, dbDir3, 10) config3.SyncType = lib.NodeSyncTypeHyperSyncArchival config1.HyperSync = true @@ -88,9 +88,9 @@ func TestHyperSyncFromHyperSyncedNode(t *testing.T) { config3.HyperSync = true config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) - node3 := cmd.NewNode(&config3) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) + node3 := cmd.NewNode(config3) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -139,13 +139,13 @@ func TestSimpleHyperSyncRestart(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_hyper_sync_restart") - dbDir2 := getTestDirectory(t, "test_hyper_sync_restart") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config1.HyperSync = true config1.SyncType = lib.NodeSyncTypeBlockSync @@ -153,8 +153,8 @@ func TestSimpleHyperSyncRestart(t *testing.T) { config2.SyncType = lib.NodeSyncTypeHyperSyncArchival config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -194,18 +194,18 @@ func TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_hyper_sync_disconnect_switch_peer") - dbDir2 := getTestDirectory(t, "test_hyper_sync_disconnect_switch_peer_2") - dbDir3 := getTestDirectory(t, "test_hyper_sync_disconnect_switch_peer_3") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) + dbDir3 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSyncArchival - config3 := cmd.GenerateTestConfig(t, 18002, dbDir3, 10) + config3 := generateConfig(t, 18002, dbDir3, 10) config3.SyncType = lib.NodeSyncTypeBlockSync config1.HyperSync = true @@ -214,9 +214,9 @@ func TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { config1.ConnectIPs = []string{"deso-seed-2.io:17000"} config3.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) - node3 := cmd.NewNode(&config3) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) + node3 := cmd.NewNode(config3) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -271,8 +271,8 @@ func TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { // defer os.RemoveAll(dbDir1) // defer os.RemoveAll(dbDir2) // -// config1 := GenerateTestConfig(t, 18000, dbDir1, 10) -// config2 := GenerateTestConfig(t, 18001, dbDir2, 10) +// config1 := generateConfig(t, 18000, dbDir1, 10) +// config2 := generateConfig(t, 18001, dbDir2, 10) // // config1.HyperSync = true // config2.HyperSync = true @@ -315,13 +315,13 @@ func TestArchivalMode(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_archival_mode") - dbDir2 := getTestDirectory(t, "test_archival_mode_2") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config1.HyperSync = true config2.HyperSync = true @@ -329,8 +329,8 @@ func TestArchivalMode(t *testing.T) { config1.SyncType = lib.NodeSyncTypeBlockSync config2.SyncType = lib.NodeSyncTypeHyperSyncArchival - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) node1 = startNode(t, node1) node2 = startNode(t, node2) @@ -358,16 +358,16 @@ func TestBlockSyncFromArchivalModeHyperSync(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_block_sync_archival") - dbDir2 := getTestDirectory(t, "test_block_sync_archival_2") - dbDir3 := getTestDirectory(t, "test_block_sync_archival_3") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) + dbDir3 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) - config3 := cmd.GenerateTestConfig(t, 18002, dbDir3, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) + config3 := generateConfig(t, 18002, dbDir3, 10) config1.HyperSync = true config1.SyncType = lib.NodeSyncTypeBlockSync @@ -377,9 +377,9 @@ func TestBlockSyncFromArchivalModeHyperSync(t *testing.T) { config3.SyncType = lib.NodeSyncTypeBlockSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) - node3 := cmd.NewNode(&config3) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) + node3 := cmd.NewNode(config3) node1 = startNode(t, node1) node2 = startNode(t, node2) diff --git a/integration_testing/migrations_test.go b/integration_testing/migrations_test.go index 9a9e17a6a..be5113b64 100644 --- a/integration_testing/migrations_test.go +++ b/integration_testing/migrations_test.go @@ -16,22 +16,22 @@ func TestEncoderMigrations(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_encoder_migration") - dbDir2 := getTestDirectory(t, "test_encoder_migration_2") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeHyperSync config1.ConnectIPs = []string{"deso-seed-2.io:17000"} config1.HyperSync = true config2.HyperSync = true - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) node1 = startNode(t, node1) node2 = startNode(t, node2) diff --git a/integration_testing/mining_test.go b/integration_testing/mining_test.go index 737cdb3ee..2a8bf3b86 100644 --- a/integration_testing/mining_test.go +++ b/integration_testing/mining_test.go @@ -14,10 +14,10 @@ func TestRegtestMiner(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_regtest_miner") + dbDir1 := getTestDirectory(t) defer os.RemoveAll(dbDir1) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync config1.Params = &lib.DeSoTestnetParams config1.MaxSyncBlockHeight = 0 @@ -25,7 +25,7 @@ func TestRegtestMiner(t *testing.T) { config1.Regtest = true - node1 := cmd.NewNode(&config1) + node1 := cmd.NewNode(config1) node1 = startNode(t, node1) // wait for node1 to sync blocks diff --git a/cmd/node_test.go b/integration_testing/node_test.go similarity index 70% rename from cmd/node_test.go rename to integration_testing/node_test.go index d640d8781..af2a0e7a5 100644 --- a/cmd/node_test.go +++ b/integration_testing/node_test.go @@ -1,7 +1,8 @@ -package cmd +package integration_testing import ( "fmt" + "github.com/deso-protocol/core/cmd" "os" "syscall" "testing" @@ -11,14 +12,14 @@ import ( ) func TestNodeIsRunning(t *testing.T) { - testDir := getTestDirectory(t, "test_node_is_running") + testDir := getTestDirectory(t) defer os.RemoveAll(testDir) - testConfig := GenerateTestConfig(t, 18000, testDir, 10) + testConfig := generateConfig(t, 18000, testDir, 10) testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} - testNode := NewNode(&testConfig) + testNode := cmd.NewNode(testConfig) // expected running status should be false before the node is started require.False(t, testNode.IsRunning()) @@ -35,14 +36,14 @@ func TestNodeIsRunning(t *testing.T) { } func TestNodeStatusRunning(t *testing.T) { - testDir := getTestDirectory(t, "test_node_change_running_status") + testDir := getTestDirectory(t) defer os.RemoveAll(testDir) - testConfig := GenerateTestConfig(t, 18000, testDir, 10) + testConfig := generateConfig(t, 18000, testDir, 10) testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} - testNode := NewNode(&testConfig) + testNode := cmd.NewNode(testConfig) // Can change the status to RUNNING from the state NEVERSTARTED actualErr := testNode.SetStatusRunning() @@ -55,7 +56,7 @@ func TestNodeStatusRunning(t *testing.T) { // start the server // Cannot change status to RUNNING while the node is already RUNNING! testNode.Start() - expErr := ErrAlreadyStarted + expErr := cmd.ErrAlreadyStarted actualErr = testNode.SetStatusRunning() require.ErrorIs(t, actualErr, expErr) @@ -70,18 +71,18 @@ func TestNodeStatusRunning(t *testing.T) { } func TestNodeSetStatusStopped(t *testing.T) { - testDir := getTestDirectory(t, "test_node_change_running_status") + testDir := getTestDirectory(t) defer os.RemoveAll(testDir) - testConfig := GenerateTestConfig(t, 18000, testDir, 10) + testConfig := generateConfig(t, 18000, testDir, 10) testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} - testNode := NewNode(&testConfig) + testNode := cmd.NewNode(testConfig) // Need to call node.start() to atleast once to be able to change node status // Cannot change status of node which never got initialized in the first place - expErr := ErrNodeNeverStarted + expErr := cmd.ErrNodeNeverStarted actualErr := testNode.SetStatusStopped() require.ErrorIs(t, actualErr, expErr) @@ -96,7 +97,7 @@ func TestNodeSetStatusStopped(t *testing.T) { // stop the node testNode.Stop() - expErr = ErrAlreadyStopped + expErr = cmd.ErrAlreadyStopped actualErr = testNode.SetStatusStopped() require.ErrorIs(t, actualErr, expErr) } @@ -105,75 +106,75 @@ func TestNodeSetStatusStopped(t *testing.T) { // NEVERSTARTED -> RUNNING -> STOP -> RUNNING // In each state change it's tested for valid change in status. func TestNodeSetStatus(t *testing.T) { - testDir := getTestDirectory(t, "test_node_change_running_status") + testDir := getTestDirectory(t) defer os.RemoveAll(testDir) - testConfig := GenerateTestConfig(t, 18000, testDir, 10) + testConfig := generateConfig(t, 18000, testDir, 10) testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} - testNode := NewNode(&testConfig) + testNode := cmd.NewNode(testConfig) // Node is in NEVERSTARTED STATE // Changing status from NEVERSTARTED to STOPPED // This is an invalid status transition. // Node status cannot needs to transitioned to RUNNING before changing to STOPPED - expError := ErrNodeNeverStarted - actualError := testNode.SetStatus(STOPPED) + expError := cmd.ErrNodeNeverStarted + actualError := testNode.SetStatus(cmd.STOPPED) require.ErrorIs(t, actualError, expError) // Cannot set the status to NEVERSTARTED, // It's the default value before the Node is initialized. - expError = ErrCannotSetToNeverStarted - actualError = testNode.SetStatus(NEVERSTARTED) + expError = cmd.ErrCannotSetToNeverStarted + actualError = testNode.SetStatus(cmd.NEVERSTARTED) require.ErrorIs(t, actualError, expError) // Starting the node. // The current status of the node is RUNNING. testNode.Start() // The status should be changed to RUNNING. // This successfully tests the transition of status from NEVERSTARTED to RUNNING - expectedStatus := RUNNING + expectedStatus := cmd.RUNNING actualStatus, err := testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) // Cannot set the status to NEVERSTARTED, // It's the default value before the Node is initialized. - expError = ErrCannotSetToNeverStarted - actualError = testNode.SetStatus(NEVERSTARTED) + expError = cmd.ErrCannotSetToNeverStarted + actualError = testNode.SetStatus(cmd.NEVERSTARTED) require.ErrorIs(t, actualError, expError) // Cannot expect the Node status to changed from STOPPED to RUNNING, // while it's current state is RUNNING - expError = ErrAlreadyStarted - actualError = testNode.SetStatus(RUNNING) + expError = cmd.ErrAlreadyStarted + actualError = testNode.SetStatus(cmd.RUNNING) require.ErrorIs(t, actualError, expError) // Stopping the node. // This should transition the Node state from RUNNING to STOPPED. testNode.Stop() - expectedStatus = STOPPED + expectedStatus = cmd.STOPPED actualStatus, err = testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) // Cannot set the status to NEVERSTARTED, // It's the default value before the Node is initialized. - expError = ErrCannotSetToNeverStarted - actualError = testNode.SetStatus(NEVERSTARTED) + expError = cmd.ErrCannotSetToNeverStarted + actualError = testNode.SetStatus(cmd.NEVERSTARTED) require.ErrorIs(t, actualError, expError) // Cannot expect the Node status to changed from NEVERSTARTED to STOPPED, // while it's current state is STOPPED - expError = ErrAlreadyStopped - actualError = testNode.SetStatus(STOPPED) + expError = cmd.ErrAlreadyStopped + actualError = testNode.SetStatus(cmd.STOPPED) require.ErrorIs(t, actualError, expError) // Changing status from STOPPED to RUNNING testNode.Start() // The following tests validates a successful transition of state from STOP to RUNNING - expectedStatus = RUNNING + expectedStatus = cmd.RUNNING actualStatus, err = testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) @@ -181,16 +182,16 @@ func TestNodeSetStatus(t *testing.T) { // Set the Node status to an invalid status code // protects from the ignorance of adding new status codes to the iota sequence! - expError = ErrInvalidNodeStatus - actualError = testNode.SetStatus(NodeStatus(3)) + expError = cmd.ErrInvalidNodeStatus + actualError = testNode.SetStatus(cmd.NodeStatus(3)) require.ErrorIs(t, actualError, expError) - expError = ErrInvalidNodeStatus - actualError = testNode.SetStatus(NodeStatus(4)) + expError = cmd.ErrInvalidNodeStatus + actualError = testNode.SetStatus(cmd.NodeStatus(4)) require.ErrorIs(t, actualError, expError) - expError = ErrInvalidNodeStatus - actualError = testNode.SetStatus(NodeStatus(5)) + expError = cmd.ErrInvalidNodeStatus + actualError = testNode.SetStatus(cmd.NodeStatus(5)) require.ErrorIs(t, actualError, expError) } @@ -198,17 +199,17 @@ func TestNodeSetStatus(t *testing.T) { // Tests for *Node.GetStatus() // Loads the status of node after node operations and tests its correctness. func TestGetStatus(t *testing.T) { - testDir := getTestDirectory(t, "test_load_status") + testDir := getTestDirectory(t) defer os.RemoveAll(testDir) - testConfig := GenerateTestConfig(t, 18000, testDir, 10) + testConfig := generateConfig(t, 18000, testDir, 10) testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} - testNode := NewNode(&testConfig) + testNode := cmd.NewNode(testConfig) // Status is set to NEVERSTARTED before the node is started. - expectedStatus := NEVERSTARTED + expectedStatus := cmd.NEVERSTARTED actualStatus, err := testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) @@ -217,7 +218,7 @@ func TestGetStatus(t *testing.T) { testNode.Start() // The status is expected to be RUNNING once the node is started. - expectedStatus = RUNNING + expectedStatus = cmd.RUNNING actualStatus, err = testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) @@ -226,44 +227,47 @@ func TestGetStatus(t *testing.T) { testNode.Stop() // The status is expected to be STOPPED once the node is stopped. - expectedStatus = STOPPED + expectedStatus = cmd.STOPPED actualStatus, err = testNode.GetStatus() require.NoError(t, err) require.Equal(t, actualStatus, expectedStatus) // set an invalid status - wrongStatus := NodeStatus(5) - testNode.status = &wrongStatus - - // expect error and invalid status code - expectedStatus = INVALIDNODESTATUS - actualStatus, err = testNode.GetStatus() - require.ErrorIs(t, err, ErrInvalidNodeStatus) - require.Equal(t, actualStatus, expectedStatus) + wrongStatus := cmd.NodeStatus(5) + err = testNode.SetStatus(wrongStatus) + require.Error(t, err) + // Commenting this out as I can't set a wrong status + // maybe there's a way to do this to get more coverage, + // but I'd rather have everything in the integration testing file + //// expect error and invalid status code + //expectedStatus = cmd.INVALIDNODESTATUS + //actualStatus, err = testNode.GetStatus() + //require.ErrorIs(t, err, cmd.ErrInvalidNodeStatus) + //require.Equal(t, actualStatus, expectedStatus) } func TestValidateNodeStatus(t *testing.T) { - inputs := []NodeStatus{NEVERSTARTED, RUNNING, STOPPED, NodeStatus(3), NodeStatus(4)} - errors := []error{nil, nil, nil, ErrInvalidNodeStatus, ErrInvalidNodeStatus} + inputs := []cmd.NodeStatus{cmd.NEVERSTARTED, cmd.RUNNING, cmd.STOPPED, cmd.NodeStatus(3), cmd.NodeStatus(4)} + errors := []error{nil, nil, nil, cmd.ErrInvalidNodeStatus, cmd.ErrInvalidNodeStatus} var err error for i := 0; i < len(inputs); i++ { - err = validateNodeStatus(inputs[i]) + err = cmd.ValidateNodeStatus(inputs[i]) require.ErrorIs(t, err, errors[i]) } } // Stop the node and test whether the internalExitChan fires as expected. func TestNodeStop(t *testing.T) { - testDir := getTestDirectory(t, "test_load_status") + testDir := getTestDirectory(t) defer os.RemoveAll(testDir) - testConfig := GenerateTestConfig(t, 18000, testDir, 10) + testConfig := generateConfig(t, 18000, testDir, 10) testConfig.ConnectIPs = []string{"deso-seed-2.io:17000"} - testNode := NewNode(&testConfig) + testNode := cmd.NewNode(testConfig) testNode.Start() // stop the node diff --git a/integration_testing/rollback_test.go b/integration_testing/rollback_test.go index 8da094a6e..58deb9fba 100644 --- a/integration_testing/rollback_test.go +++ b/integration_testing/rollback_test.go @@ -15,14 +15,14 @@ func TestStateRollback(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "get_state_rollback") - dbDir2 := getTestDirectory(t, "get_state_rollback_2") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config2.SyncType = lib.NodeSyncTypeBlockSync config1.MaxSyncBlockHeight = 5000 @@ -32,8 +32,8 @@ func TestStateRollback(t *testing.T) { config1.ConnectIPs = []string{"deso-seed-2.io:17000"} config2.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) node1 = startNode(t, node1) node2 = startNode(t, node2) diff --git a/integration_testing/tools.go b/integration_testing/tools.go index ebe3cf0de..1c6a38878 100644 --- a/integration_testing/tools.go +++ b/integration_testing/tools.go @@ -4,6 +4,7 @@ import ( "encoding/hex" "fmt" "io/ioutil" + "os" "reflect" "sort" "testing" @@ -41,15 +42,52 @@ const MaxSyncBlockHeight = 1500 const HyperSyncSnapshotPeriod = 1000 // get a random temporary directory. -func getTestDirectory(t *testing.T, testName string) string { +func getTestDirectory(t *testing.T) string { require := require.New(t) - dbDir, err := ioutil.TempDir("", testName) + dbDir, err := ioutil.TempDir("", t.Name()) if err != nil { require.NoError(err) } return dbDir } +// generateConfig creates a default config for a node, with provided port, db directory, and number of max peers. +// It's usually the first step to starting a node. +func generateConfig(t *testing.T, port uint32, dataDir string, maxPeers uint32) *cmd.Config { + config := &cmd.Config{} + params := lib.DeSoMainnetParams + + params.DNSSeeds = []string{} + config.Params = ¶ms + config.ProtocolPort = uint16(port) + // "/Users/piotr/data_dirs/n98_1" + config.DataDirectory = dataDir + if err := os.MkdirAll(config.DataDirectory, os.ModePerm); err != nil { + t.Fatalf("Could not create data directories (%s): %v", config.DataDirectory, err) + } + config.TXIndex = false + config.HyperSync = false + config.MaxSyncBlockHeight = 0 + config.ConnectIPs = []string{} + config.PrivateMode = true + config.GlogV = 0 + config.GlogVmodule = "*bitcoin_manager*=0,*balance*=0,*view*=0,*frontend*=0,*peer*=0,*addr*=0,*network*=0,*utils*=0,*connection*=0,*main*=0,*server*=0,*mempool*=0,*miner*=0,*blockchain*=0" + config.MaxInboundPeers = maxPeers + config.TargetOutboundPeers = maxPeers + config.StallTimeoutSeconds = 900 + config.MinFeerate = 1000 + config.OneInboundPerIp = false + config.MaxBlockTemplatesCache = 100 + config.MaxSyncBlockHeight = 100 + config.MinBlockUpdateInterval = 10 + config.SnapshotBlockHeightPeriod = HyperSyncSnapshotPeriod + config.MaxSyncBlockHeight = MaxSyncBlockHeight + config.SyncType = lib.NodeSyncTypeBlockSync + //config.ArchivalMode = true + + return config +} + // waitForNodeToFullySync will busy-wait until provided node is fully current. func waitForNodeToFullySync(node *cmd.Node) { ticker := time.NewTicker(5 * time.Millisecond) diff --git a/integration_testing/txindex_test.go b/integration_testing/txindex_test.go index 44cc4ed2d..3778fab19 100644 --- a/integration_testing/txindex_test.go +++ b/integration_testing/txindex_test.go @@ -20,15 +20,15 @@ func TestSimpleTxIndex(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t, "test_simple_tx_index") - dbDir2 := getTestDirectory(t, "test_simple_tx_index_2") + dbDir1 := getTestDirectory(t) + dbDir2 := getTestDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) - config1 := cmd.GenerateTestConfig(t, 18000, dbDir1, 10) + config1 := generateConfig(t, 18000, dbDir1, 10) config1.HyperSync = true config1.SyncType = lib.NodeSyncTypeBlockSync - config2 := cmd.GenerateTestConfig(t, 18001, dbDir2, 10) + config2 := generateConfig(t, 18001, dbDir2, 10) config2.HyperSync = true config2.SyncType = lib.NodeSyncTypeHyperSyncArchival @@ -36,8 +36,8 @@ func TestSimpleTxIndex(t *testing.T) { config2.TXIndex = true config1.ConnectIPs = []string{"deso-seed-2.io:17000"} - node1 := cmd.NewNode(&config1) - node2 := cmd.NewNode(&config2) + node1 := cmd.NewNode(config1) + node2 := cmd.NewNode(config2) node1 = startNode(t, node1) node2 = startNode(t, node2) From 6e0c1ec06e10cfb3e9cd625b3a191964af243b10 Mon Sep 17 00:00:00 2001 From: Lazy Nina <> Date: Tue, 29 Nov 2022 16:16:11 -0800 Subject: [PATCH 20/21] getTestDirectory -> getDirectory --- integration_testing/blocksync_test.go | 14 +++++------ integration_testing/hypersync_test.go | 34 +++++++++++++------------- integration_testing/migrations_test.go | 4 +-- integration_testing/mining_test.go | 2 +- integration_testing/node_test.go | 12 ++++----- integration_testing/rollback_test.go | 4 +-- integration_testing/tools.go | 2 +- integration_testing/txindex_test.go | 4 +-- 8 files changed, 38 insertions(+), 38 deletions(-) diff --git a/integration_testing/blocksync_test.go b/integration_testing/blocksync_test.go index c088a57a5..435cfcf51 100644 --- a/integration_testing/blocksync_test.go +++ b/integration_testing/blocksync_test.go @@ -20,8 +20,8 @@ func TestSimpleBlockSync(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) @@ -66,8 +66,8 @@ func TestSimpleSyncRestart(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) @@ -118,9 +118,9 @@ func TestSimpleSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) - dbDir3 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) + dbDir3 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) diff --git a/integration_testing/hypersync_test.go b/integration_testing/hypersync_test.go index 1665c3d89..d413eeeb7 100644 --- a/integration_testing/hypersync_test.go +++ b/integration_testing/hypersync_test.go @@ -20,8 +20,8 @@ func TestSimpleHyperSync(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) @@ -69,9 +69,9 @@ func TestHyperSyncFromHyperSyncedNode(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) - dbDir3 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) + dbDir3 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) @@ -139,8 +139,8 @@ func TestSimpleHyperSyncRestart(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) @@ -194,9 +194,9 @@ func TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) - dbDir3 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) + dbDir3 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) @@ -266,8 +266,8 @@ func TestSimpleHyperSyncDisconnectWithSwitchingToNewPeer(t *testing.T) { // require := require.New(t) // _ = require // -// dbDir1 := getTestDirectory(t) -// dbDir2 := getTestDirectory(t) +// dbDir1 := getDirectory(t) +// dbDir2 := getDirectory(t) // defer os.RemoveAll(dbDir1) // defer os.RemoveAll(dbDir2) // @@ -315,8 +315,8 @@ func TestArchivalMode(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) @@ -358,9 +358,9 @@ func TestBlockSyncFromArchivalModeHyperSync(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) - dbDir3 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) + dbDir3 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) defer os.RemoveAll(dbDir3) diff --git a/integration_testing/migrations_test.go b/integration_testing/migrations_test.go index be5113b64..f9a74de19 100644 --- a/integration_testing/migrations_test.go +++ b/integration_testing/migrations_test.go @@ -16,8 +16,8 @@ func TestEncoderMigrations(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) diff --git a/integration_testing/mining_test.go b/integration_testing/mining_test.go index 2a8bf3b86..7ec262613 100644 --- a/integration_testing/mining_test.go +++ b/integration_testing/mining_test.go @@ -14,7 +14,7 @@ func TestRegtestMiner(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) + dbDir1 := getDirectory(t) defer os.RemoveAll(dbDir1) config1 := generateConfig(t, 18000, dbDir1, 10) diff --git a/integration_testing/node_test.go b/integration_testing/node_test.go index af2a0e7a5..49f7ce063 100644 --- a/integration_testing/node_test.go +++ b/integration_testing/node_test.go @@ -12,7 +12,7 @@ import ( ) func TestNodeIsRunning(t *testing.T) { - testDir := getTestDirectory(t) + testDir := getDirectory(t) defer os.RemoveAll(testDir) testConfig := generateConfig(t, 18000, testDir, 10) @@ -36,7 +36,7 @@ func TestNodeIsRunning(t *testing.T) { } func TestNodeStatusRunning(t *testing.T) { - testDir := getTestDirectory(t) + testDir := getDirectory(t) defer os.RemoveAll(testDir) testConfig := generateConfig(t, 18000, testDir, 10) @@ -71,7 +71,7 @@ func TestNodeStatusRunning(t *testing.T) { } func TestNodeSetStatusStopped(t *testing.T) { - testDir := getTestDirectory(t) + testDir := getDirectory(t) defer os.RemoveAll(testDir) testConfig := generateConfig(t, 18000, testDir, 10) @@ -106,7 +106,7 @@ func TestNodeSetStatusStopped(t *testing.T) { // NEVERSTARTED -> RUNNING -> STOP -> RUNNING // In each state change it's tested for valid change in status. func TestNodeSetStatus(t *testing.T) { - testDir := getTestDirectory(t) + testDir := getDirectory(t) defer os.RemoveAll(testDir) testConfig := generateConfig(t, 18000, testDir, 10) @@ -199,7 +199,7 @@ func TestNodeSetStatus(t *testing.T) { // Tests for *Node.GetStatus() // Loads the status of node after node operations and tests its correctness. func TestGetStatus(t *testing.T) { - testDir := getTestDirectory(t) + testDir := getDirectory(t) defer os.RemoveAll(testDir) testConfig := generateConfig(t, 18000, testDir, 10) @@ -260,7 +260,7 @@ func TestValidateNodeStatus(t *testing.T) { // Stop the node and test whether the internalExitChan fires as expected. func TestNodeStop(t *testing.T) { - testDir := getTestDirectory(t) + testDir := getDirectory(t) defer os.RemoveAll(testDir) testConfig := generateConfig(t, 18000, testDir, 10) diff --git a/integration_testing/rollback_test.go b/integration_testing/rollback_test.go index 58deb9fba..8b6808859 100644 --- a/integration_testing/rollback_test.go +++ b/integration_testing/rollback_test.go @@ -15,8 +15,8 @@ func TestStateRollback(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) diff --git a/integration_testing/tools.go b/integration_testing/tools.go index 1c6a38878..3e7c26be4 100644 --- a/integration_testing/tools.go +++ b/integration_testing/tools.go @@ -42,7 +42,7 @@ const MaxSyncBlockHeight = 1500 const HyperSyncSnapshotPeriod = 1000 // get a random temporary directory. -func getTestDirectory(t *testing.T) string { +func getDirectory(t *testing.T) string { require := require.New(t) dbDir, err := ioutil.TempDir("", t.Name()) if err != nil { diff --git a/integration_testing/txindex_test.go b/integration_testing/txindex_test.go index 3778fab19..f9bccbbd4 100644 --- a/integration_testing/txindex_test.go +++ b/integration_testing/txindex_test.go @@ -20,8 +20,8 @@ func TestSimpleTxIndex(t *testing.T) { require := require.New(t) _ = require - dbDir1 := getTestDirectory(t) - dbDir2 := getTestDirectory(t) + dbDir1 := getDirectory(t) + dbDir2 := getDirectory(t) defer os.RemoveAll(dbDir1) defer os.RemoveAll(dbDir2) From 967daa22d1be552947241c3b67e7c478742b568f Mon Sep 17 00:00:00 2001 From: Karthic Rao Date: Wed, 30 Nov 2022 06:30:43 +0530 Subject: [PATCH 21/21] Renaming status functions Renaming setstatus functions with a suffix of without locks. --- cmd/node.go | 12 ++++++------ integration_testing/node_test.go | 21 +++++++++++---------- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/cmd/node.go b/cmd/node.go index 5d46f051c..8c123066b 100644 --- a/cmd/node.go +++ b/cmd/node.go @@ -72,7 +72,7 @@ type Node struct { // status is nil when a NewNode is created, it is initialized and set to RUNNING [byte(1)] on node.Start(), // set to STOPPED [byte(2)] after Stop() is called. - // Use the convenience methods SetStatus/SetStatusRunning/SetStatusStopped to change node status. + // Use the convenience methods SetStatus/SetStatusRunningWithoutLock/SetStatusStoppedWithoutLock to change node status. // Use *Node.IsRunning() to check if the node is running. // Use *Node.GetStatus() to retrieve the status of the node. // Use *Node.getStatusWithoutLock() if the statusMutex is held by the caller. @@ -305,7 +305,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { glog.Info("Changing node status from NEVERSTARTED to RUNNING...") // The node status is changed from NEVERSTARTED to RUNNING only once // when the node is started for the first time. - err = node.SetStatusRunning() + err = node.SetStatusRunningWithoutLock() if err != nil { glog.Fatalf("Error running Node -- %v", err) } @@ -314,7 +314,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { // During restart the Node is first STOPPED before setting the // status again to RUNNING. glog.Info("Changing node status from STOP to RUNNING...") - err = node.SetStatusRunning() + err = node.SetStatusRunningWithoutLock() if err != nil { glog.Fatalf("Error running Node -- %v", err) } @@ -364,7 +364,7 @@ func (node *Node) Start(exitChannels ...*chan struct{}) { // Changes node status to STOPPED. // Cannot transition from NEVERSTARTED to STOPPED. // Valid transition sequence is NEVERSTARTED->RUNNING->STOPPED. -func (node *Node) SetStatusStopped() error { +func (node *Node) SetStatusStoppedWithoutLock() error { if err := node.setStatusWithoutLock(STOPPED); err != nil { return fmt.Errorf("failed to set status to stop: %w", err) } @@ -372,7 +372,7 @@ func (node *Node) SetStatusStopped() error { } // Changes node status to RUNNING. -func (node *Node) SetStatusRunning() error { +func (node *Node) SetStatusRunningWithoutLock() error { if err := node.setStatusWithoutLock(RUNNING); err != nil { return fmt.Errorf("failed to set status to running: %w", err) } @@ -504,7 +504,7 @@ func (node *Node) Stop() error { // Change nodes running status to stop // Node's current status has to be RUNNING to be able to STOP! - if err := node.SetStatusStopped(); err != nil { + if err := node.SetStatusStoppedWithoutLock(); err != nil { glog.Errorf("Error stopping Node -- %v", err) return err } diff --git a/integration_testing/node_test.go b/integration_testing/node_test.go index 49f7ce063..8d6c84c68 100644 --- a/integration_testing/node_test.go +++ b/integration_testing/node_test.go @@ -2,12 +2,13 @@ package integration_testing import ( "fmt" - "github.com/deso-protocol/core/cmd" "os" "syscall" "testing" "time" + "github.com/deso-protocol/core/cmd" + "github.com/stretchr/testify/require" ) @@ -35,7 +36,7 @@ func TestNodeIsRunning(t *testing.T) { } -func TestNodeStatusRunning(t *testing.T) { +func TestNodeStatusRunningWithoutLock(t *testing.T) { testDir := getDirectory(t) defer os.RemoveAll(testDir) @@ -46,31 +47,31 @@ func TestNodeStatusRunning(t *testing.T) { testNode := cmd.NewNode(testConfig) // Can change the status to RUNNING from the state NEVERSTARTED - actualErr := testNode.SetStatusRunning() + actualErr := testNode.SetStatusRunningWithoutLock() require.NoError(t, actualErr) // Change status from RUNNING to STOPPED - actualErr = testNode.SetStatusStopped() + actualErr = testNode.SetStatusStoppedWithoutLock() require.NoError(t, actualErr) // start the server // Cannot change status to RUNNING while the node is already RUNNING! testNode.Start() expErr := cmd.ErrAlreadyStarted - actualErr = testNode.SetStatusRunning() + actualErr = testNode.SetStatusRunningWithoutLock() require.ErrorIs(t, actualErr, expErr) // Stop the node testNode.Stop() // Should be able to change status to RUNNING from STOP. - actualErr = testNode.SetStatusRunning() + actualErr = testNode.SetStatusRunningWithoutLock() require.NoError(t, actualErr) // Once the running flag is changed, the isRunning function should return true require.True(t, testNode.IsRunning()) } -func TestNodeSetStatusStopped(t *testing.T) { +func TestNodeSetStatusStoppedWithoutLock(t *testing.T) { testDir := getDirectory(t) defer os.RemoveAll(testDir) @@ -83,7 +84,7 @@ func TestNodeSetStatusStopped(t *testing.T) { // Need to call node.start() to atleast once to be able to change node status // Cannot change status of node which never got initialized in the first place expErr := cmd.ErrNodeNeverStarted - actualErr := testNode.SetStatusStopped() + actualErr := testNode.SetStatusStoppedWithoutLock() require.ErrorIs(t, actualErr, expErr) // start the node @@ -91,14 +92,14 @@ func TestNodeSetStatusStopped(t *testing.T) { // Once the server is started testNode.Start() - actualErr = testNode.SetStatusStopped() + actualErr = testNode.SetStatusStoppedWithoutLock() require.NoError(t, actualErr) // stop the node testNode.Stop() expErr = cmd.ErrAlreadyStopped - actualErr = testNode.SetStatusStopped() + actualErr = testNode.SetStatusStoppedWithoutLock() require.ErrorIs(t, actualErr, expErr) }