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

Remove transactions from orphan pool. #167

Closed
wants to merge 1 commit into from
Closed

Remove transactions from orphan pool. #167

wants to merge 1 commit into from

Conversation

dajohi
Copy link
Member

@dajohi dajohi commented Aug 27, 2014

This change removes transactions in a newly connected block
from the orphan pool if they exist. This helps keep memory
usage lower.

@dajohi
Copy link
Member Author

dajohi commented Aug 28, 2014

Another bonus with this fix, when the txs are removed from the orphan pool, orphans that are no longer orphans are moved to mempool and inv'd to peers.

@davecgh
Copy link
Member

davecgh commented Sep 10, 2014

This logically reads well, but I want to manually test it before merging.

@davecgh
Copy link
Member

davecgh commented Sep 11, 2014

While I know Go doesn't care about the function order, I'd prefer the unexported version comes before the exported version which depends on it. This is more consistent with the rest of the code.

This change removes transactions from a newly connected block
from the orphan pool if they exist.  Additionally, any orphan
transactions that are no longer orphan transactions are moved
to the mempool and inv'd to the currently connected peers.
@davecgh
Copy link
Member

davecgh commented Sep 19, 2014

After reviewing this and thinking about it some more, I think a better approach to do this is to modify the removeTransaction function to also operate on the orphan pool. This would have the benefit of ensuring that any transactions which are also recursively removed also remove any orphans that depend on them. It also would only be a change in a single existing function instead of having to also modify the block manager, and will catch any other future cases of transaction removal.

I also noticed that, unlike normal transactions, orphans that now contain double spends due to newly connected transactions aren't currently removed either which this pull request doesn't address.

@davecgh
Copy link
Member

davecgh commented Sep 20, 2014

I'm going to go ahead and merge this and open another issue to address the remaining cases. While this doesn't catch all of the cases, it is an improvement over the current code, works properly, and will allow the release to proceed.

@davecgh
Copy link
Member

davecgh commented Sep 20, 2014

This has been rebased and merged as commit 4ad8622.

@davecgh davecgh closed this Sep 20, 2014
cjepson added a commit to cjepson/btcd that referenced this pull request May 13, 2016
cjepson added a commit to cjepson/btcd that referenced this pull request May 16, 2016
cjepson added a commit to cjepson/btcd that referenced this pull request May 16, 2016
kcalvinalvin added a commit to kcalvinalvin/btcd that referenced this pull request Nov 29, 2024
…empt-to-prune-proofs-if-were-a-prune-node

indexers: don't prune proofs if the node is pruned
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