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

mempool: Add a skeleton mempool pkg #571

Merged
merged 1 commit into from
Apr 11, 2016
Merged

mempool: Add a skeleton mempool pkg #571

merged 1 commit into from
Apr 11, 2016

Conversation

dajohi
Copy link
Member

@dajohi dajohi commented Dec 1, 2015

Review on Reviewable

@davecgh
Copy link
Member

davecgh commented Dec 1, 2015

I'd prefer to not create the skeleton package unless it's absolutely needed. The mining case was required because mempool separation would be blocked. In this case, there is nothing else that needs mempoolPolicy. In other words, I'd just introduce type mempoolPolicy struct and switch the code to make use of it (very close to what you've done here, but without the package).

@dajohi
Copy link
Member Author

dajohi commented Dec 2, 2015

How does this look?

@@ -337,7 +337,7 @@ func loadConfig() (*config, []string, error) {
BlockMaxSize: defaultBlockMaxSize,
BlockPrioritySize: defaultBlockPrioritySize,
SigCacheMaxSize: defaultSigCacheMaxSize,
MaxOrphanTxs: maxOrphanTransactions,
MaxOrphanTxs: 1000,
Copy link
Member

Choose a reason for hiding this comment

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

This should be a defaultMaxOrphanTransactions constant to be consistent with the surrounding code.

@davecgh
Copy link
Member

davecgh commented Apr 7, 2016

This looks good overall. In addition to addressing the line comments, this needs a rebase since it can't be merged cleanly.

mempoolPolicy contains the values that configure the mempool policy.
This decouples the values from the internals of btcd to move closer
to a mempool package.
@dajohi
Copy link
Member Author

dajohi commented Apr 11, 2016

Rebased.

@davecgh
Copy link
Member

davecgh commented Apr 11, 2016

Thanks for addressing the review items. I've reviewed all of the updates and tested them.

OK

@davecgh davecgh merged commit 123ff36 into btcsuite:master Apr 11, 2016
jrick added a commit to jrick/btcd that referenced this pull request Feb 16, 2017
@dajohi dajohi deleted the mempoolPoliy branch February 28, 2019 00:38
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.

2 participants