-
Notifications
You must be signed in to change notification settings - Fork 36.2k
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
consensus: move amount.h into consensus #22951
Conversation
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. |
Concept ACK. I think this is organizationally the right thing to do. Nearly everything in
Even |
166ed04
to
3cc3e89
Compare
3cc3e89
to
2c1554a
Compare
Concept ACK. |
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.
ACK 2c1554a.
I see the last two commits as a way to modernize amount.h
. Here are some additional suggestions:
- use type alias instead of
typedef
- use separators for integer literals
- s/
nValue
/value
/ to follow our name convention - pass
CAmount
as a non-reference parameter: s/const CAmount&
/CAmount
/. Also refactor: pass CTxMemPool and CFeeRate in-param objects by const reference #23076 (comment).
Concept ACK |
Move amount.h to consensus/amount.h. Renames, adds missing and removes uneeded includes.
2c1554a
to
9d0379c
Compare
concept ACK 9d0379c 🏝 Show signature and timestampSignature:
Timestamp of file with hash |
Summary: Move amount.h to consensus/amount.h. Renames, adds missing and removes uneeded includes. Also make MAX_MONEY constexpr This is a backport of [[bitcoin/bitcoin#22951 | core#22951]] Test Plan: `ninja all check-all bitcoin-fuzzers bench-bitcoin` `grep -r "<amount.h>" src/` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Subscribers: Fabien Differential Revision: https://reviews.bitcoinabc.org/D12164
A first step (of a few) towards some source code reorganization, as well as making libbitcoinconsensus slightly more self contained.
Related to #15732.