8000 Merge develop into ripple/smart-escrow by mvadari · Pull Request #5412 · XRPLF/rippled · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Merge develop into ripple/smart-escrow #5412

8000 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

Merged
merged 42 commits into from
May 6, 2025

Conversation

mvadari
Copy link
Collaborator
@mvadari mvadari commented Apr 28, 2025

High Level Overview of Change

Title says it all.

Context of Change

Keep the ripple/smart-escrow branch relatively in-sync with develop, so the merge isn't too gnarly when it's time for it

Type of Change

  • Chore

legleux and others added 30 commits March 6, 2025 10:41
The codebase is filled with includes that are unused, and which thus can be removed. At the same time, the files often do not include all headers that contain the definitions used in those files. This change uses clang-format and clang-tidy to clean up the includes, with minor manual intervention to ensure the code compiles on all platforms.
Requiring manual updates of numFeatures is an annoying manual process that is easily forgotten, and leads to frequent merge conflicts. This change takes advantage of the `XRPL_FEATURE` and `XRPL_FIX` macros, and adds a new `XRPL_RETIRE` macro to automatically set `numFeatures`.
Removes all manual header groupings from source and header files by leveraging clang-format options.
What the LoadManager class does is stall detection, which is not the same as deadlock detection. In the condition of severe CPU starvation, LoadManager will currently intentionally crash rippled reporting `LogicError: Deadlock detected`. This error message is misleading as the condition being detected is not a deadlock. This change fixes and refactors the code in response.
Changes the error to `malformedAddress` for `permissioned_domain` in the `ledger_entry` rpc, when the account is not a string. This change makes it more clear to a user what is wrong with their request.
The `end_marker` is used to limit the range of ledger entries to fetch. If `end_marker` is less than `marker`, a crash can occur. This change adds an additional check.
The Trustline RPC `no_ripple` flag gets set depending on `lsfDefaultRipple` flag, which is not a flag of a trustline but of the account root. The `lsfDefaultRipple` flag does not provide any insight if this particular trust line has `lsfLowNoRipple` or `lsfHighNoRipple` flag set, so it should not be used here at all. This change simplifies the logic.
Updates RocksDB to version 9.7.3, the latest version supported in Conan 1.x. A patch for 9.7.4 that fixes a memory leak is included.
This change removes the existing undefined behavior from `LogicError`, so we can be certain that there will be always a stacktrace.

De-referencing a null pointer is an old trick to generate `SIGSEGV`, which would typically also create a stacktrace. However it is also an undefined behaviour and compilers can do something else. A more robust way to create a stacktrace while crashing the program is to use `std::abort`, which we have also used in this location for a long time. If we combine the two, we might not get the expected behaviour - namely, the nullpointer deref followed by `std::abort`, as handled in certain compiler versions may not immediately cause a crash. We have observed stacktrace being wiped instead, and thread put in indeterminate state, then stacktrace created without any useful information.
This PR adds one more payload field to the libXRPL compatibility check workflow - the PR number itself.
The link to ripple-binary-codec's definitions.json appears to be outdated. The updated link is also documented here: https://xrpl.org/docs/references/protocol/binary-format#definitions-file
- Detects if the consensus process is "stalled". If it is, then we can declare a 
  consensus and end successfully even if we do not have 80% agreement on
  our proposal.
  - "Stalled" is defined as:
    - We have a close time consensus
    - Each disputed transaction is individually stalled:
      - It has been in the final "stuck" 95% requirement for at least 2
        (avMIN_ROUNDS) "inner rounds" of phaseEstablish,
      - and either all of the other trusted proposers or this validator, if proposing,
        have had the same vote(s) for at least 4 (avSTALLED_ROUNDS) "inner
        rounds", and at least 80% of the validators (including this one, if
        appropriate) agree about the vote (whether yes or no).
- If we have been in the establish phase for more than 10x the previous
  consensus establish phase's time, then consensus is considered "expired",
  and we will leave the round, which sends a partial validation (indicating
  that the node is moving on without validating). Two restrictions avoid
  prematurely exiting, or having an extended exit in extreme situations.
  - The 10x time is clamped to be within a range of 15s
    (ledgerMAX_CONSENSUS) to 120s (ledgerABANDON_CONSENSUS).
  - If consensus has not had an opportunity to walk through all avalanche
    states (defined as not going through 8 "inner rounds" of phaseEstablish),
    then ConsensusState::Expired is treated as ConsensusState::No.
- When enough nodes leave the round, any remaining nodes will see they've
  fallen behind, and move on, too, generally before hitting the timeout. Any
  validations or partial validations sent during this time will help the
  consensus process bring the nodes back together.
)

In preparation for a potential reference fee change we would like to verify that fee change works as expected. The first step is to fix all unit tests to be able to work with different reference fee values.
Fix remaining unit tests to be able to process reference fee values other than 10.
…e synchronization (XRPLF#5152)

The main goal of this optimisation is memory reduction in SHAMapTreeNodes by introducing intrusive pointers instead of standard std::shared_ptr and std::weak_ptr.
…F#5367)

This change moves `examples/example` into `tests/conan` to make it clear it is an integration test, and adjusts the `conan` CI job accordingly
…5159)

Adds an extra CI pipeline to perform unit tests using different values for fees.
As part of import optimization, a transitive include had been removed that defined `BOOST_COMP_MSVC` on Windows. In unity builds, this definition was pulled in, but in non-unity builds it was not - causing a compilation error. An inspection of the Boost code revealed that we can just gate the statements by `_MS_VER` instead. A `#pragma message` is added to verify that the statement is only printed on Windows builds.
…the intrusive reference counting logic (XRPLF#5381)

This change addresses a memory ordering assertion failure observed on one of the Windows test machines during the IntrusiveShared_test suite.
The ci pipelines are constantly hitting Docker Hub's public rate limiting since increasing the number of jobs we're running. This change switches over to images hosted in GitHub's registry.
…F#5387)

It’s possible for this to happen legitimately if a set of peers, including a validator, are connected in a cycle, and the latency and message processing time between those peers is significantly less than the latency between the validator and the last peer. It’s unlikely in the real world, but obviously easy to simulate with Antithesis.
Adds metric counters for the following P2P message types:

* Untrusted proposal and validation messages
* Duplicate proposal, validation and transaction messages
…XRPLF#5376)

This PR splits out `ledger_entry` tests into its own file (`LedgerEntry_test.cpp`) and alphabetizes the helper functions in `LedgerEntry.cpp`. These commits were split out of XRPLF#5237 to make that PR a little more manageable, since these basic trivial changes are most of the diff. There is no code change, just moving code around.
This change introduces a new fix amendment (`fixPayChanV1`) that prevents the creation of new `PaymentChannelCreate` transaction with a `CancelAfter` time less than the current ledger time. It piggy backs off of fix1571.

Once the amendment is activated, creating a new `PaymentChannel` will require that if you specify the `CancelAfter` time/value, that value must be greater than or equal to the current ledger time.

Currently users can create a payment channel where the `CancelAfter` time is before the current ledger time. This results in the payment channel being immediately closed on the next PaymentChannel transaction.
…LF#5163)

When using subscribe at admin RPC port to send webhooks for the transaction stream to a backend, on large(r) ledgers the endpoint receives fewer HTTP POSTs with TX information than the amount of transactions in a ledger. This change removes the hardcoded queue length to avoid dropping TX notifications for the admin-only command. In addition, the per-request TTL for outgoing RPC HTTP calls has been reduced from 10 minutes to 30 seconds.
This change fixes a number of issues involved with CTID:
* CTID is not present on all RPC tx transactions.
* rpcWRONG_NETWORK is missing in the ErrorCodes.cpp
vlntb and others added 7 commits April 10, 2025 21:58
We temporarily disable running unit tests on macOS on the CI pipeline while we are investigating the delays.
This PR replaces the word `failed` with `failure` in any test names and renames some test files to fix MSVC warnings, so that it is easier to search through the test output to find tests that failed.
…RPLF#5400)

- Avoids costly overhead for idle PRs where the CI results don't add any
  value.
This change addresses an issue where `rippled` attempts to connect to an IPv6 address, even when the local network lacks IPv6 support, resulting in a "Network is unreachable" error.

The fix replaces the custom endpoint selection logic with `boost::async_connect`, which sequentially attempts to connect to available endpoints until one succeeds o
8000
r all fail.
@mvadari mvadari marked this pull request as ready for review April 28, 2025 21:25
Copy link
codecov bot commented Apr 28, 2025

Codecov Report

Attention: Patch coverage is 79.65971% with 263 lines in your changes missing coverage. Please review.

Project coverage is 78.0%. Comparing base (b4b53a6) to head (bb9bb5f).
Report is 3 commits behind head on ripple/smart-escrow.

Files with missing lines Patch % Lines
src/xrpld/overlay/detail/TrafficCount.h 56.0% 55 Missing ⚠️
src/xrpld/rpc/handlers/LedgerEntry.cpp 81.4% 40 Missing ⚠️
include/xrpl/basics/IntrusivePointer.ipp 86.4% 39 Missing ⚠️
include/xrpl/basics/TaggedCache.ipp 85.6% 38 Missing ⚠️
src/xrpld/overlay/detail/PeerImp.cpp 0.0% 28 Missing ⚠️
include/xrpl/basics/SharedWeakCachePointer.ipp 75.9% 13 Missing ⚠️
src/xrpld/overlay/detail/OverlayImpl.cpp 26.7% 11 Missing ⚠️
src/xrpld/overlay/detail/OverlayImpl.h 16.7% 10 Missing ⚠️
include/xrpl/basics/IntrusiveRefCounts.h 91.8% 8 Missing ⚠️
src/xrpld/shamap/detail/SHAMapInnerNode.cpp 58.3% 5 Missing ⚠️
... and 10 more
Additional details and impacted files

Impacted file tree graph

@@                  Coverage Diff                  @@
##           ripple/smart-escrow   #5412     +/-   ##
=====================================================
- Coverage                 78.0%   78.0%   -0.0%     
=====================================================
  Files                      799     802      +3     
  Lines                    69261   69563    +302     
  Branches                  8408    8441     +33     
=====================================================
+ Hits                     54026   54233    +207     
- Misses                   15235   15330     +95     
Files with missing lines Coverage Δ
include/xrpl/basics/TaggedCache.h 100.0% <100.0%> (ø)
src/libxrpl/basics/Number.cpp 99.7% <ø> (ø)
src/libxrpl/basics/mulDiv.cpp 100.0% <100.0%> (ø)
src/libxrpl/protocol/ErrorCodes.cpp 85.7% <ø> (ø)
src/xrpld/app/ledger/detail/LedgerMaster.cpp 44.0% <ø> (ø)
src/xrpld/app/ledger/detail/TransactionMaster.cpp 73.8% <100.0%> (ø)
src/xrpld/app/main/Application.h 100.0% <ø> (ø)
src/xrpld/app/misc/Transaction.h 100.0% <ø> (ø)
src/xrpld/app/misc/detail/Transaction.cpp 83.3% <100.0%> (ø)
src/xrpld/app/misc/detail/ValidatorSite.cpp 92.5% <100.0%> (ø)
... and 44 more

... and 5 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mvadari mvadari requested review from pwang200 and oleks-rip April 29, 2025 19:43
@ximinez ximinez marked this pull request as draft May 6, 2025 15:55
@ximinez ximinez marked this pull request as ready for review May 6, 2025 15:55
@mvadari mvadari merged commit bb9bb5f into XRPLF:ripple/smart-escrow May 6, 2025
24 of 26 checks passed
@mvadari mvadari deleted the develop2 branch May 6, 2025 17:19
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.

0