Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

77 Cumulus Console and Peer updates #82

Merged
merged 13 commits into from
Jul 11, 2017
Merged

77 Cumulus Console and Peer updates #82

merged 13 commits into from
Jul 11, 2017

Conversation

bfbachmann
Copy link
Member

What Changed:

  • Added exchangeListenAddrs to peer as a way of handshaking when peers establish a connection. We didn't do this before and it caused us to store incorrect peer addresses in the peerstore.
  • Added the console. Run it using cumulus run -c.
  • Added the word Test to exported blockchain test fixtures so they don't confuse people.
  • Remove the test package.

// Start a goroutine that waits for program termination. Before the program
// exits it will flush logs and save the blockchain.
c := make(chan os.Signal, 1)
signal.Notify(c, os.Interrupt, syscall.SIGTERM)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

@@ -34,14 +55,25 @@ func Run(cfg conf.Config) {
// Start listening on the given interface and port so we can receive
// conenctions from other peers
log.Infof("Starting listener on %s:%d", cfg.Interface, cfg.Port)
peer.LocalAddr = fmt.Sprintf("%s:%d", cfg.Interface, cfg.Port)
peer.ListenAddr = fmt.Sprintf("%s:%d", cfg.Interface, cfg.Port)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

// RunConsole starts the Cumulus console. This should be run only once as a
// goroutine, and logging should be redirected away from stdout before it is run.
func RunConsole() {
shell = ishell.New()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome

"Wallet",
"Transaction",
}, "What would you like to create?")
if choice == 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

symbolic constant for this?

} else {
shell.Println("Connected to", addr)
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This console looks great. Nicely done Bruno

@@ -1,33 +0,0 @@
package cmd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return
}
p := New(c, PStore, addr)

// If we are already at MaxPeers, disconnect from a peer to connect to a new
// one. This way nobody gets choked out of the network because everybody
// happens to be fully connected.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems like a good policy

if err != nil {
return false
}
return port > 1024 && port < 65536
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Port can be under 1024 technically

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.4%) to 68.633% when pulling 0a8520d on 77-console into 0ce1330 on dev.

@coveralls
Copy link

Coverage Status

Coverage decreased (-8.4%) to 68.633% when pulling 5f868ea on 77-console into 0ce1330 on dev.

@bfbachmann bfbachmann merged commit 2fef206 into dev Jul 11, 2017
@jordanschalm jordanschalm deleted the 77-console branch July 15, 2017 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants