-
Notifications
You must be signed in to change notification settings - Fork 37.7k
Move DEFAULT_MAX_ORPHAN_TRANSACTIONS to node/txorphanage.h #25564
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
The head ref may contain hidden characters: "2207-txorphan-move-\u{1F690}"
Conversation
fa17d03
to
dddd207
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review ACK dddd207
- Verified that the scripted diff script works correctly, and the consequent diff is the same as the first commit.
- Verified that the files are correctly sorted after the renaming.
- I agree with converting
const
->constexpr
, which makes the variable a compile-time constant.
dddd207
to
fa6e073
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK, light code review looks correct to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK fa6e073
- Verified
txorphan
fuzz target works properly - I think it can make writing fuzz target easier by modularizing
txorphanage
fa6e073
to
fa0096a
Compare
-BEGIN VERIFY SCRIPT- # Move module git mv src/txorphanage.cpp src/node/ git mv src/txorphanage.h src/node/ # Replacements sed -i 's:txorphanage\.h:node/txorphanage.h:g' $(git grep -l txorphanage) sed -i 's:txorphanage\.cpp:node/txorphanage.cpp:g' $(git grep -l txorphanage) sed -i 's:TXORPHANAGE_H:NODE_TXORPHANAGE_H:g' $(git grep -l TXORPHANAGE_H) -END VERIFY SCRIPT-
Also, sort file list after rename
fa0096a
to
fa999c3
Compare
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closing for now to open review for other pulls. |
It is not meaninful to run txorphanage without a node (validation, chainstate, mempool, rpc, ...). The module is in the node library and won't be added to the kernel library. Thus, it should be moved to
src/node
.Also, move
DEFAULT_MAX_ORPHAN_TRANSACTIONS
tonode/txorphanage.h
, as it logically belongs toTxOrphanage::m_orphans
and not toPeerManager
.