Encapsulate currency unit settings. #1023
Conversation
src/settings.cpp
Outdated
| @@ -41,7 +41,8 @@ settings::settings() | |||
| bip9_version_base(0x20000000), | |||
| satoshi_per_bitcoin(100000000), | |||
| initial_block_subsidy_bitcoin(50), | |||
| recursive_money(9999999989u) | |||
| recursive_money(max_money_recursive(bitcoin_to_satoshi( | |||
| initial_block_subsidy_bitcoin))) | |||
There was a problem hiding this comment.
This can’t be safely exposed as a property unless computed (vs. initialized). Same holds for other compued initializatuons above.
toxeus
force-pushed
the
recursive_money
branch
from
cebf133
to
9483190
Compare
|
I was waiting for the PR with the renaming to be merged before continuing here because both manipulate the settings.cpp file and hence might conflict. The renaming has a higher prio because it blocked the parser PR in the node repo. I have updated the PR now. |
toxeus
changed the title
toxeus
force-pushed
the
recursive_money
branch
from
9483190
to
01e7cb9
Compare
toxeus
changed the title
|
As discussed on slack, I have properly encapsulated the currency unit settings. I have successfully tested the setter methods with boost::program_options and can confirm that it works as expected. Other bc::settings members that are derived will be cleaned up in upcoming PRs. |
src/chain/block.cpp
Outdated
| settings.bitcoin_to_satoshi(settings.initial_block_subsidy_bitcoin)) | ||
| else if (!is_valid_coinbase_claim(state.height(), | ||
| settings.subsidy_interval(), settings.bitcoin_to_satoshi( | ||
| settings.initial_block_subsidy_bitcoin())) | ||
| ) |
There was a problem hiding this comment.
Style, don't balance parenthesis.
else if (!is_valid_coinbase_claim(state.height(),
settings.subsidy_interval(), settings.bitcoin_to_satoshi(
settings.initial_block_subsidy_bitcoin())))
src/settings.cpp
Outdated
| static const std::function<uint64_t(uint64_t)> recursive_money = | ||
| [](uint64_t money){ | ||
| return money > 0 ? money + recursive_money(money >> 1u) : 0; | ||
| }; |
There was a problem hiding this comment.
Style: balance braces, use auto, avoid static with closures:
const auto recursive_money = [](uint64_t money)
{
return money > 0 ? money + recursive_money(money >> 1u) : 0;
};There was a problem hiding this comment.
auto cannot deduce the type of recursive closures. Fixed the rest.
src/settings.cpp
Outdated
|
|
||
| uint64_t settings::bitcoin_to_satoshi(uint64_t bitcoin_units) const | ||
| { | ||
|
|
src/settings.cpp
Outdated
| max_money_ = recursive_money_ * subsidy_interval_; | ||
| } | ||
|
|
||
| uint64_t settings::bitcoin_to_satoshi(uint64_t bitcoin_units) const |
There was a problem hiding this comment.
For property methods I generally just use "value" for the parameter since the parameter name is redundant with the property name.
uint64_t settings::bitcoin_to_satoshi(uint64_t value) const
include/bitcoin/bitcoin/settings.hpp
Outdated
| uint64_t initial_block_subsidy_bitcoin() const; | ||
| void subsidy_interval(uint64_t subsidy_interval); | ||
| uint64_t subsidy_interval() const; | ||
| uint64_t max_money() const; |
There was a problem hiding this comment.
It seems inevitable when creating a class with fields that eventually proper behavior will drive it into properties.
toxeus
force-pushed
the
recursive_money
branch
2 times, most recently
from
2487771
to
bd5b43d
Compare
|
Looks good, please merge when ready. |
No description provided.