Proposed 1.10.0-b1 #4265
Closed
Proposed 1.10.0-b1 #4265
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Trustlines must be between two different accounts but two trustlines exist where an account extends trust to itself. They were created in the early days, likely because of bugs that have been fixed. The new fixTrustLinesToSelf amendment will remove those trustlines when it activates.
We profiled different algorithms and data structures to understand which strategy is best from a performance standpoint: - Linear search on an array; - Binary search on a sorted array; - Using `std::map`; and - Using `std::unordered_map`. Both linear search and std::unordered_map outperformed the other alternatives so no change to the existing data structure is justified. If more handers are added, this should be revisited. For some additional details and timings, please see: #3298 (comment)
Co-authored-by: Chenna Keshava B S <ckbs.keshava56@gmail.com>
The existing thread pool code uses several layers of indirection which uses a custom lock-free stack, and offers functionality that supports features that are never used (e.g. the ability to dynamically adjust the number of threads in the pool). This refactoring aims to simplify the code, making it easier to reason about (although lock-free multi-threaded code is always tricky) what is happening, and reduce the latency of the thread pool internals.
This commit cleans up and modernizes the JobQueue but does not change the queueing logic. It focuses on simplifying the code by eliminating awkward code constructs, like "invalid jobs" and the need for default constructors. It leverages modern C++ to initialize tables and data structures at compile time and replaces `std:map` instances with directly indexed arrays. Lastly, it restructures the load tracking infrastructure and reduces the need for dynamic memory allocations by supporting move semantics and value types.
Each node on the network is supposed to have a unique cryptographic identity. Typically, this identity is generated randomly at startup and stored for later reuse in the (poorly named) file `wallet.db`. If the file is copied, it is possible for two nodes to share the same node identity. This is generally not desirable and existing servers will detect and reject connections to other servers that have the same key. This commit achives three things: 1. It improves the detection code to pinpoint instances where two distinct servers with the same key connect with each other. In that case, servers will log an appropriate error and shut down pending intervention by the server's operator. 2. It makes it possible for server administrators to securely and easily generate new cryptographic identities for servers using the new `--newnodeid` command line arguments. When a server is started using this command, it will generate and save a random secure identity. 3. It makes it possible to configure the identity using a command line option, which makes it possible to derive it from data or parameters associated with the container or hardware where the instance is running by passing the `--nodeid` option, followed by a single argument identifying the infomation from which the node's identity is derived. For example, the following command will result in nodes with different hostnames having different node identities: `rippled --nodeid $HOSTNAME` The last option is particularly useful for automated cloud-based deployments that minimize the need for storing state and provide unique deployment identifiers. **Important note for server operators:** Depending on variables outside of the the control of this code, such as operating system version or configuration, permissions, and more, it may be possible for other users or programs to be able to access the command line arguments of other processes on the system. If you are operating in a shared environment, you should avoid using this option, preferring instead to use the `[node_seed]` option in the configuration file, and use permissions to limit exposure of the node seed. A user who gains access to the value used to derive the node's unique identity could impersonate that node. The commit also updates the minimum supported server protocol version to `XRPL/2.1`, which has been supported since version 1.5.0 and eliminates support for `XPRL/2.0`.
Caching the base58check encoded version of an `AccountID` has performance advantages, because because of the computationally heavy cost associated with the conversion, which requires the application of SHA-256 twice. This commit makes the cache significantly more efficient in terms of memory used: it eliminates the map, using a vector with a size that is determined by the configured size of the node, and a hash function to directly map any given `AccountID` to a specific slot in the cache; the eviction policy is simple: in case of collision the existing entry is removed and replaced with the new data. Previously, use of the cache was optional and required additional effort by the programmer. Now the cache is automatic and does not require any additional work or information. The new cache also utilizes a 64-way spinlock, to help reduce any contention that the pressure on the cache would impose.
When starting, the code generates a new ephemeral private key and a corresponding certificate to go along with it. This process can take time and, while this is unlikely to matter for normal server operations, it can have a significant impact for unit testing and development. Profiling data suggests that ~20% of the time needed for a unit test run can be attributed to this. This commit does several things: 1. It restructures the code so that a new self-signed certificate and its corresponding private key are only initialized once at startup; this has minimal impact on the operation of a regular server. 2. It provides new default DH parameters. This doesn't impact the security of the connection, but those who compile from scratch can generate new parameters if they so choose. 3. It properly sets the version number in the certificate, fixing issue #4007; thanks to @donovanhide for the report. 4. It uses SHA-256 instead of SHA-1 as the hash algorithm for the certificate and adds some X.509 extensions as well as a random 128-bit serial number. 5. It rounds the certificate's "start of validity" period so that the server's precise startup time cannot be easily deduced and limits the validity period to two years, down from ten years. 6. It removes some CBC-based ciphers from the default cipher list to avoid some potential security issues, such as CVE-2016-2107 and CVE-2013-0169.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If merged this PR will:
fixTrustLinesToSelf: Introduce amendment to handle trustlines to self #4105