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

Proposed 1.6.0-b5 #3388

Merged
merged 18 commits into from
May 6, 2020
Merged

Proposed 1.6.0-b5 #3388

merged 18 commits into from
May 6, 2020

Conversation

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
@codecov-io
Copy link

codecov-io commented May 4, 2020

Codecov Report

Merging #3388 into develop will increase coverage by 0.20%.
The diff coverage is 74.55%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #3388      +/-   ##
===========================================
+ Coverage    70.23%   70.44%   +0.20%     
===========================================
  Files          683      682       -1     
  Lines        54630    54392     -238     
===========================================
- Hits         38370    38315      -55     
+ Misses       16260    16077     -183     
Impacted Files Coverage Δ
src/ripple/app/consensus/RCLConsensus.h 80.00% <ø> (ø)
src/ripple/app/ledger/InboundLedgers.h 100.00% <ø> (ø)
src/ripple/app/ledger/InboundTransactions.h 100.00% <ø> (ø)
src/ripple/app/ledger/LedgerMaster.h 53.33% <ø> (ø)
src/ripple/app/ledger/impl/InboundLedgers.cpp 28.82% <0.00%> (+2.04%) ⬆️
src/ripple/app/ledger/impl/TransactionAcquire.cpp 0.00% <0.00%> (ø)
src/ripple/app/ledger/impl/TransactionAcquire.h 0.00% <0.00%> (ø)
src/ripple/app/main/BasicApp.h 100.00% <ø> (ø)
src/ripple/app/misc/AmendmentTable.h 100.00% <ø> (ø)
src/ripple/app/misc/FeeVote.h 100.00% <ø> (ø)
... and 96 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2bf3b19...9771210. Read the comment docs.

@carlhua carlhua self-requested a review May 5, 2020 19:15
carlhua
carlhua previously approved these changes May 5, 2020
Copy link
Contributor

@carlhua carlhua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

nbougalis and others added 14 commits May 5, 2020 16:05
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.
The script, when invoked by a server operator can collect information
useful for debugging, while attempting to redact potentially sensitive
data.

It contained no explanation or other exposition to allow people who
look at the file but aren't familiar with shell scripts to understand
its purpose.
This commit introduces no functional changes but cleans up the
code and shrinks the surface area by removing dead and unused
code, leveraging std:: alternatives to hand-rolled code and
improving comments and documentation.
The unit test now verifies that if an account is not present in the
starting account_tx ledger, account_tx still iterates down and finds
the transaction that deletes the account (and earlier transactions).
@nbougalis nbougalis merged commit 9771210 into XRPLF:develop May 6, 2020
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.