Clean up some older TODOs

app/bots/servermon/main.go

@@ -70,7 +70,7 @@ func main() {
 	timeout := 1 * time.Second
 	timeElapsed := 0 * time.Second
 	for {
-		err := botPeer.SendMessageToGroup(groupID, timeout.String())
+		_, err := botPeer.SendMessageToGroupTracked(groupID, timeout.String())
 		if err != nil {
 			fmt.Printf("Sent to group on server %v failed at interval %v of total %v with: %v\n", serverAddr, timeout, timeElapsed, err)
 			os.Exit(1)

model/message.go

@@ -103,7 +103,6 @@ func (t *Timeline) Less(i, j int) bool {
 }
 
 // Sort sorts the timeline in a canonical order.
-// TODO: There is almost definitely a more efficient way of doing things that involve not calling this method on every timeline load.
 func (t *Timeline) Sort() {
 	t.lock.Lock()
 	defer t.lock.Unlock()

model/profile.go

@@ -111,10 +111,15 @@ func GenerateNewProfile(name string) *Profile {
 func (p *Profile) AddContact(onion string, profile *PublicProfile) {
 	p.lock.Lock()
 	profile.init()
-	// TODO: More Robust V3 Onion Handling
-	decodedPub, _ := base32.StdEncoding.DecodeString(strings.ToUpper(onion[:56]))
-	profile.Ed25519PublicKey = ed25519.PublicKey(decodedPub[:32])
-	p.Contacts[onion] = profile
+	// We expect callers to verify addresses before we get to this point, so if this isn't a
+	// valid address this is a noop.
+	if tor.IsValidHostname(onion) {
+		decodedPub, err := base32.StdEncoding.DecodeString(strings.ToUpper(onion[:56]))
+		if err == nil {
+			profile.Ed25519PublicKey = ed25519.PublicKey(decodedPub[:32])
+			p.Contacts[onion] = profile
+		}
+	}
 	p.lock.Unlock()
 }
 

peer/cwtch_peer.go

@@ -63,7 +63,6 @@ type CwtchPeer interface {
 
 	AddServer(string) error
 	JoinServer(string) error
-	SendMessageToGroup(string, string) error
 	SendMessageToGroupTracked(string, string) (string, error)
 
 	GetName() string
@@ -393,14 +392,7 @@ func (cp *cwtchPeer) JoinServer(onion string) error {
 	return errors.New("no keys found for server connection")
 }
 
-// SendMessageToGroup attempts to sent the given message to the given group id.
-// TODO: Deprecate in favour of SendMessageToGroupTracked
-func (cp *cwtchPeer) SendMessageToGroup(groupid string, message string) error {
-	_, err := cp.SendMessageToGroupTracked(groupid, message)
-	return err
-}
-
-// SendMessageToGroup attempts to sent the given message to the given group id.
+// SendMessageToGroupTracked attempts to sent the given message to the given group id.
 // It returns the signature of the message which can be used to identify it in any UX layer.
 func (cp *cwtchPeer) SendMessageToGroupTracked(groupid string, message string) (string, error) {
 	cp.mutex.Lock()

peer/cwtch_peer_test.go

@@ -1,79 +0,0 @@
-package peer
-
-import (
-	"testing"
-)
-
-// TODO: Rewrite these tests (and others) using the news event bus interface.
-func TestCwtchPeerGenerate(t *testing.T) {
-	/**
-	alice := NewCwtchPeer("alice")
-
-	groupID, _, _ := alice.StartGroup("test.server")
-	exportedGroup, _ := alice.ExportGroup(groupID)
-	t.Logf("Exported Group: %v from %v", exportedGroup, alice.GetProfile().Onion)
-
-	importedGroupID, err := alice.ImportGroup(exportedGroup)
-	group := alice.GetGroup(importedGroupID)
-	t.Logf("Imported Group: %v, err := %v %v", group, err, importedGroupID)
-	*/
-}
-
-func TestTrustPeer(t *testing.T) {
-	/**
-	groupName := "2c3kmoobnyghj2zw6pwv7d57yzld753auo3ugauezzpvfak3ahc4bdyd"
-	alice := NewCwtchPeer("alice")
-	aem := new(event.Manager)
-	aem.Initialize()
-	alice.Init(connectivity.LocalProvider(),aem)
-	defer alice.Shutdown()
-	bob := NewCwtchPeer("bob")
-	bem := new(event.Manager)
-	bem.Initialize()
-	bob.Init(connectivity.LocalProvider(), bem)
-	defer bob.Shutdown()
-
-	bobOnion := bob.GetProfile().Onion
-	aliceOnion := alice.GetProfile().Onion
-
-	groupID, _, err := alice.StartGroup(groupName)
-	if err != nil {
-		t.Error(err)
-	}
-
-	groupAlice := alice.GetGroup(groupID)
-	if groupAlice.GroupID != groupID {
-		t.Errorf("Alice should be part of group %v, got %v instead", groupID, groupAlice)
-	}
-
-	exportedGroup, err := alice.ExportGroup(groupID)
-	if err != nil {
-		t.Error(err)
-	}
-
-	err = alice.InviteOnionToGroup(bobOnion, groupID)
-	if err == nil {
-		t.Errorf("onion invitation should fail since alice does no trust bob")
-	}
-
-	err = alice.TrustPeer(bobOnion)
-	if err == nil {
-		t.Errorf("trust peer should fail since alice does not know about bob")
-	}
-
-	// bob adds alice contact by importing serialized group created by alice
-	_, err = bob.ImportGroup(exportedGroup)
-	if err != nil {
-		t.Error(err)
-	}
-	err = bob.TrustPeer(aliceOnion)
-	if err != nil {
-		t.Errorf("bob must be able to trust alice, got %v", err)
-	}
-
-	err = bob.InviteOnionToGroup(aliceOnion, groupID)
-	if err == nil {
-		t.Errorf("bob trusts alice but peer connection is not ready yet. should not be able to invite her to group, instead got: %v", err)
-	}
-	*/
-}

server/app/main.go

@@ -67,7 +67,6 @@ func main() {
 	server := new(cwtchserver.Server)
 	log.Infoln("starting cwtch server...")
 
-	// TODO load params from .cwtch/server.conf or command line flag
 	// TODO: respond to HUP so t.Close is gracefully called
 	server.Setup(serverConfig)
 

server/metrics/monitors.go

@@ -18,16 +18,16 @@ const (
 
 // Monitors is a package of metrics for a Cwtch Server including message count, CPU, Mem, and conns
 type Monitors struct {
-	MessageCounter Counter
+	MessageCounter      Counter
 	TotalMessageCounter Counter
-	Messages       MonitorHistory
-	CPU            MonitorHistory
-	Memory         MonitorHistory
-	ClientConns    MonitorHistory
-	starttime      time.Time
-	breakChannel   chan bool
-	log            bool
-	configDir      string
+	Messages            MonitorHistory
+	CPU                 MonitorHistory
+	Memory              MonitorHistory
+	ClientConns         MonitorHistory
+	starttime           time.Time
+	breakChannel        chan bool
+	log                 bool
+	configDir           string
 }
 
 // Start initializes a Monitors's monitors
@@ -40,7 +40,12 @@ func (mp *Monitors) Start(ts tapir.Service, configDir string, log bool) {
 
 	// Maintain a count of total messages
 	mp.TotalMessageCounter = NewCounter()
-	mp.Messages = NewMonitorHistory(Count, Cumulative, func() (c float64) { c = float64(mp.MessageCounter.Count()); mp.TotalMessageCounter.Add(int(c)); mp.MessageCounter.Reset(); return })
+	mp.Messages = NewMonitorHistory(Count, Cumulative, func() (c float64) {
+		c = float64(mp.MessageCounter.Count())
+		mp.TotalMessageCounter.Add(int(c))
+		mp.MessageCounter.Reset()
+		return
+	})
 
 	var pidUsageLock sync.Mutex
 	mp.CPU = NewMonitorHistory(Percent, Average, func() float64 {

server/server.go

@@ -16,6 +16,7 @@ import (
 	"git.openprivacy.ca/openprivacy/connectivity/tor"
 	"git.openprivacy.ca/openprivacy/log"
 	"path"
+	"sync"
 )
 
 // Server encapsulates a complete, compliant Cwtch server.
@@ -31,6 +32,7 @@ type Server struct {
 	onionServiceStopped  bool
 	running              bool
 	existingMessageCount int
+	lock 	sync.RWMutex
 }
 
 // Setup initialized a server from a given configuration
@@ -84,7 +86,10 @@ func (s *Server) Run(acn connectivity.ACN) error {
 		s.service.Listen(NewTokenBoardServer(ms, s.tokenServer))
 		s.onionServiceStopped = true
 	}()
+
+	s.lock.Lock()
 	s.running = true
+	s.lock.Unlock()
 	return nil
 }
 
@@ -101,7 +106,8 @@ func (s *Server) KeyBundle() *model.KeyBundle {
 
 // CheckStatus returns true if the server is running and/or an error if any part of the server needs to be restarted.
 func (s *Server) CheckStatus() (bool, error) {
-
+	s.lock.RLock()
+	defer s.lock.RUnlock()
 	if s.onionServiceStopped == true || s.tokenServiceStopped == true {
 		return s.running, fmt.Errorf("one of more server components are down: onion:%v token service: %v", s.onionServiceStopped, s.tokenServiceStopped)
 	}
@@ -110,10 +116,13 @@ func (s *Server) CheckStatus() (bool, error) {
 
 // Shutdown kills the app closing all connections and freeing all goroutines
 func (s *Server) Shutdown() {
+	s.lock.Lock()
+	defer s.lock.Unlock()
 	s.service.Shutdown()
 	s.tokenTapirService.Shutdown()
 	s.metricsPack.Stop()
-	s.running = false
+	s.running = true
+
 }
 
 // Statistics is an encapsulation of information about the server that an operator might want to know at a glance.

testing/cwtch_peer_server_integration_test.go

@@ -319,25 +319,25 @@ func TestCwtchPeerIntegration(t *testing.T) {
 	fmt.Println("Starting conversation in group...")
 	// Conversation
 	fmt.Printf("%v> %v\n", aliceName, aliceLines[0])
-	err = alice.SendMessageToGroup(groupID, aliceLines[0])
+	_, err = alice.SendMessageToGroupTracked(groupID, aliceLines[0])
 	if err != nil {
 		t.Fatalf("Alice failed to send a message to the group: %v", err)
 	}
 	time.Sleep(time.Second * 10)
 
 	fmt.Printf("%v> %v\n", bobName, bobLines[0])
-	err = bob.SendMessageToGroup(groupID, bobLines[0])
+	_, err = bob.SendMessageToGroupTracked(groupID, bobLines[0])
 	if err != nil {
 		t.Fatalf("Bob failed to send a message to the group: %v", err)
 	}
 	time.Sleep(time.Second * 10)
 
 	fmt.Printf("%v> %v\n", aliceName, aliceLines[1])
-	alice.SendMessageToGroup(groupID, aliceLines[1])
+	alice.SendMessageToGroupTracked(groupID, aliceLines[1])
 	time.Sleep(time.Second * 10)
 
 	fmt.Printf("%v> %v\n", bobName, bobLines[1])
-	bob.SendMessageToGroup(groupID, bobLines[1])
+	bob.SendMessageToGroupTracked(groupID, bobLines[1])
 	time.Sleep(time.Second * 10)
 
 	fmt.Println("Alice inviting Carol to group...")
@@ -367,12 +367,12 @@ func TestCwtchPeerIntegration(t *testing.T) {
 	numGoRotinesPostCarolConnect := runtime.NumGoroutine()
 
 	fmt.Printf("%v> %v", bobName, bobLines[2])
-	bob.SendMessageToGroup(groupID, bobLines[2])
+	bob.SendMessageToGroupTracked(groupID, bobLines[2])
 	// Bob should have enough tokens so we don't need to account for
 	// token acquisition here...
 
 	fmt.Printf("%v> %v", carolName, carolLines[0])
-	carol.SendMessageToGroup(groupID, carolLines[0])
+	carol.SendMessageToGroupTracked(groupID, carolLines[0])
 	time.Sleep(time.Second * 30) // we need to account for spam-based token acquisition, but everything should
 	// be warmed-up and delays should be pretty small.