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

Refactor: Move banning logic into peer package #1042

Open
jimpo opened this issue Aug 31, 2017 · 1 comment
Open

Refactor: Move banning logic into peer package #1042

jimpo opened this issue Aug 31, 2017 · 1 comment

Comments

@jimpo
Copy link
Contributor

jimpo commented Aug 31, 2017

In an attempt to remove code from server.go and from the serverPeer struct specifically, I think it makes sense to move addBanScore from serverPeer to a public method IncreaseBanScore on peer.Peer. Furthermore, we should move dynamicbanscore.go and the associated test from the connmgr package to the peer package.

Thoughts on this?

@davecgh
Copy link
Member

davecgh commented Aug 31, 2017

I'm not really sure on this one. It seems to me like banning is very much a connection management related issue. I suspect the bigger issue, which is something I've commented on before, is that more of the connection-related logic should move into connection manager.

More specifically, it seems to me like the code that handles maintaining persistent peers, adding peers, removing peers, banning peers, and querying of the state of connections, should all be refactored from the server to the connection manager. Of course we want to be careful to ensure the connection manager remains usable by more than btcd's server though since it is highly useful code for other things such as SPV wallets as well.

The reason it is sort of split is that originally everything was in server and the connmanager package was created to help refactor some of that code out. I think the real driving issue you're running into here is simply that the connmanager isn't handling all of the connections as it name implies.

I'm definitely open to further discussion here.

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

No branches or pull requests

2 participants