Cleanup #3326
Cleanup #3326
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3326 +/- ##
===========================================
+ Coverage 70.23% 70.38% +0.14%
===========================================
Files 683 682 -1
Lines 54630 54449 -181
===========================================
- Hits 38370 38323 -47
+ Misses 16260 16126 -134
Continue to review full report at Codecov.
|
src/ripple/protocol/impl/Indexes.cpp
Outdated
| { | ||
| return { ltCHILD, key }; | ||
| } | ||
|
|
||
| Keylet skip_t::operator()() const | ||
| Keylet const& skip() noexcept |
There was a problem hiding this comment.
Using a static and returning a const& instead of value may be a pessimization because of RVO, it's going to depend on how it's called. If it's used in a function call and bound to a const& paramter (foo(skip())) then this is an optimization. If it's used to initialize a value (auto const a = skip()) then it's a pessimization (there's extra coping because of missed RVO).
I think I'd keep this as Keylet skip() noexcept, but I'm OK if you want to keep the const&. I really just wanted to make sure you knew there was a trade-off here to consider. (same somment for fees as well)
There was a problem hiding this comment.
The thing is that these particular keylets are actually constants; they accept no arguments and their value is a fixed part of the protocol and will never change. From that perspective, I think that the return value being a const& is relevant.
However, you raise a good point that it may, actually, result in a pessimization. I'll take a look.
src/ripple/ledger/impl/View.cpp
Outdated
| saReceiverLimit, | ||
| 0, | ||
| 0, | ||
| j); |
carlhua
added
Passed
|
I am going to rebase this against #3291, which is rebased against 1.6.0-b4. |
nbougalis
dismissed stale reviews from seelabs, HowardHinnant, and undertome
via
da75e5b
| constexpr std::uint32_t vfFullValidation = 0x00000001; | ||
|
|
||
| // The signature is fully canonical | ||
| constexpr std::uint32_t vfFullyCanonicalSig = 0x80000000; |
There was a problem hiding this comment.
May be have a sense to use prefix 'u' because 0x80000000 is too big?
There was a problem hiding this comment.
Hmm, I think that's safe, but let me ask the gurus: @HowardHinnant, @scottschurr, any suggestions here?
There was a problem hiding this comment.
It's safe as is, but I like using the u suffix when initializing unsigned. But I also alphabetize my socks. So you can't always go by me. :-)
There was a problem hiding this comment.
🤣 All of my socks are identical so I don't have to alphabetize them. I also never need to match them.
| getEncodedVersion() | ||
| { | ||
| static std::uint64_t const cookie = []() { | ||
| std::uint64_t c = 0x183B000000000000; |
There was a problem hiding this comment.
@HowardHinnant, @scottschurr, any suggestions here?
There was a problem hiding this comment.
I like the ull suffix here. You could try it as a prefix, but I don't think it will compile. :-) That being said, it's optional. Just a readability issue.
This commit introduces the "HardenedValidations" amendment which, if enabled, allows validators to include additional information in their validations that can increase the robustness of consensus. Specifically, the commit introduces a new optional field that can be set in validation messages can be used to attest to the hash of the latest ledger that a validator considers to be fully validated. Additionally, the commit leverages the previously introduced "cookie" field to improve the robustness of the network by making it possible for servers to automatically detect accidental misconfiguration which results in two or more validators using the same validation key.
Currently there is no mechanism for a validator to report the
version of the software it is currently running. Such reports
can be useful for those who are developing network monitoring
dashboards and server operators in general.
This commit, if merged, defines an encoding scheme to encode
a version string into a 64-bit unsigned integer and adds an
additional optional field to validations.
This commit piggybacks on "HardenedValidations" amendment to
determine whether version information should be propagated
or not.
The general encoding scheme is:
XXXXXXXX-XXXXXXXX-YYYYYYYY-YYYYYYYY-YYYYYYYY-YYYYYYYY-YYYYYYYY-YYYYYYYY
X: 16 bits identifying the particular implementation
Y: 48 bits of data specific to the implementation
The rippled-specific format (implementation ID is: 0x18 0x3B) is:
00011000-00111011-MMMMMMMM-mmmmmmmm-pppppppp-TTNNNNNN-00000000-00000000
M: 8-bit major version (0-255)
m: 8-bit minor version (0-255)
p: 8-bit patch version (0-255)
T: 11 if neither an RC nor a beta
10 if an RC
01 if a beta
N: 6-bit rc/beta number (1-63)
A deliberately malformed token can cause the server to crash during
startup. This is not remotely exploitable and would require someone
with access to the configuration file of the server to make changes
and then restart the server.
Acknowledgements:
Guido Vranken for responsibly disclosing this issue.
Bug Bounties and Responsible Disclosures:
We welcome reviews of the rippled code and urge researchers to
responsibly disclose any issues they may find.
Ripple is generously sponsoring a bug bounty program for the
rippled project. For more information please visit:
https://ripple.com/bug-bounty
This commit removes obsolete comments, dead or no longer useful code, and workarounds for several issues that were present in older compilers that we no longer support. Specifically: - It improves the transaction metadata handling class, simplifying its use and making it less error-prone. - It reduces the footprint of the Serializer class by consolidating code and leveraging templates. - It cleanups the ST* class hierarchy, removing dead code, improving and consolidating code to reduce complexity and code duplication. - It shores up the handling of currency codes and the conversation between 160-bit currency codes and their string representation. - It migrates beast::secure_erase to the ripple namespace and uses a call to OpenSSL_cleanse instead of the custom implementation.
Entries in the ledger are located using 256-bit locators. The locators are calculated using a wide range of parameters specific to the entry whose locator we are calculating (e.g. an account's locator is derived from the account's address, whereas the locator for an offer is derived from the account and the offer sequence.) Keylets enhance type safety during lookup and make the code more robust, so this commit removes most of the earlier code, which used naked uint256 values.
The built-in watchdog is simplistic and can, sometimes, cause problems especially on systems that have the ability to automatically start and monitor processes. This commit removes the sustain system entirely, changes the handling of the SIGTERM signal to properly terminate the process and improves the error message reported to the user when the command line used to start `rippled` is incorrect and malformed.
This PR includes commits that: