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 fRelayTxes global to CConnman member #10915

Closed
wants to merge 1 commit into from

Conversation

benma
Copy link

@benma benma commented Jul 23, 2017

No description provided.

@@ -130,6 +130,7 @@ class CConnman

struct Options
{
bool m_relay_txes = false;
Copy link
Member

Choose a reason for hiding this comment

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

Why the different initial value?

Copy link
Author

Choose a reason for hiding this comment

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

Mostly because of this: #10596 (comment)

Personally, I also slightly prefer to put the correct default here, but it does not matter much as it gets overwritten anyway.

@@ -351,6 +354,7 @@ class CConnman
unsigned int nSendBufferMaxSize;
unsigned int nReceiveFloodSize;

bool m_relay_txes;
Copy link
Member

Choose a reason for hiding this comment

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

Missing initial value?

Copy link
Author

Choose a reason for hiding this comment

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

Is it needed? It gets set during Start() and isn't used before. Or is it a matter of good form?

Copy link
Member

Choose a reason for hiding this comment

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

Some future refactor can call GetRelayTxes (or other) before Start() and there is no way to know/prevent that.

Copy link
Author

Choose a reason for hiding this comment

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

Good point. Done.

@promag
Copy link
Member

promag commented Jul 23, 2017

Suggestion, rename PR and commit to Refactor fRelayTxes global to CConnman member

@benma
Copy link
Author

benma commented Jul 24, 2017

@promag thx for the review. I rewrote the commit msg as you suggested.

@benma benma changed the title get rid of the fRelayTxes global Refactor fRelayTxes global to CConnman member Jul 24, 2017
Copy link
Member

@promag promag left a comment

Choose a reason for hiding this comment

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

utACK 9c830bf.

@@ -351,6 +354,7 @@ class CConnman
unsigned int nSendBufferMaxSize;
unsigned int nReceiveFloodSize;

bool m_relay_txes;
Copy link
Member

Choose a reason for hiding this comment

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

Some future refactor can call GetRelayTxes (or other) before Start() and there is no way to know/prevent that.

@benma
Copy link
Author

benma commented Aug 3, 2017

ping @theuni

@benma
Copy link
Author

benma commented Aug 3, 2017

Rebased.

@theuni
Copy link
Member

theuni commented Aug 3, 2017

Err.. I'd rather not, as this really needs to be a member of the message processor instead. We'll likely start moving in that direction after #10756 goes in.

@TheBlueMatt agreed?

@TheBlueMatt
Copy link
Contributor

Yea, we should be moving fRelayTxes towards net_processing, not net, sorry for the churn in this area.

@benma benma closed this Sep 11, 2017
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants