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

Headers needed in Conan package to build xbridge witness server #4885

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

legleux
Copy link
Collaborator

@legleux legleux commented Jan 17, 2024

Headers required for the xrpl Conan package to build xbwd

  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)

Copy link
Collaborator

@thejohnfreeman thejohnfreeman left a comment

Choose a reason for hiding this comment

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

The general pattern for libxrpl headers is to export entire "modules" (directories under src/ripple) rather than, within the same module, export some files in libxrpl and keep the rest in rippled. I don't recall if there are any existing exceptions, but we should try to avoid adding new ones.

For server, this changeset excludes impl/JSONRPCUtil.h and its source file. They appear to have no dependencies outside of libxrpl, so I think we should include them too, even if the witness server doesn't use them.

The net and resource modules are different. Most of their files are excluded here. I think that net/RPCErr.h can fit into the protocol module that is already in libxrpl. Either way, we must not forget to add net/impl/RPCErr.cpp to the sources.

resource appears to have no rippled dependencies outside of the beast, json, and protocol modules which are already in libxrpl. Perhaps we can export the entire module in libxrpl.

Builds/CMake/RippledCore.cmake Show resolved Hide resolved
@intelliot
Copy link
Collaborator

note: legleux currently on PTO; to follow up after returning

@legleux legleux marked this pull request as ready for review January 23, 2024 19:39
@legleux
Copy link
Collaborator Author

legleux commented Jan 26, 2024

This is ready.
I did notice a few more things could be alphabetized in the CMakeLists.txt.

Copy link
Collaborator

@oleks-rip oleks-rip left a comment

Choose a reason for hiding this comment

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

Looks good

@intelliot
Copy link
Collaborator

intelliot commented Feb 1, 2024

  • Adds some headers to libxrpl
  • Reorders some includes

@legleux at your convenience, confirm that you still think this PR is ready to merge. (You can either comment or add the Passed label.)

@intelliot intelliot added Build System Perf impact not expected Change is not expected to improve nor harm performance. labels Feb 1, 2024
@codecov-commenter
Copy link

codecov-commenter commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (828bb64) 61.50% compared to head (7ef85cd) 61.49%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4885      +/-   ##
===========================================
- Coverage    61.50%   61.49%   -0.01%     
===========================================
  Files          797      797              
  Lines        70130    70131       +1     
  Branches     36244    36244              
===========================================
- Hits         43130    43128       -2     
- Misses       19758    19763       +5     
+ Partials      7242     7240       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@legleux legleux added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Feb 2, 2024
more headers
@legleux legleux added Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. and removed Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. labels Feb 2, 2024
@intelliot intelliot added this to the 2.1.0 (Mar 2024) milestone Feb 2, 2024
@intelliot
Copy link
Collaborator

I assume the force-push was just to rebase and resolve merge conflicts?

@legleux
Copy link
Collaborator Author

legleux commented Feb 2, 2024

I assume the force-push was just to rebase and resolve merge conflicts?

Yes, ready to go. :shipit:

@intelliot intelliot merged commit 1e96a1d into XRPLF:develop Feb 5, 2024
17 checks passed
legleux added a commit to legleux/rippled that referenced this pull request Mar 6, 2024
Adds STCurrency.h to libxrpl for Clio
Fixes PR XRPLF#4885 botched by force push
thejohnfreeman pushed a commit that referenced this pull request Mar 14, 2024
legleux added a commit to legleux/rippled that referenced this pull request Apr 12, 2024
* Price Oracle (XLS-47d): (XRPLF#4789) (XRPLF#4789)

Implement native support for Price Oracles.

 A Price Oracle is used to bring real-world data, such as market prices,
 onto the blockchain, enabling dApps to access and utilize information
 that resides outside the blockchain.

 Add Price Oracle functionality:
 - OracleSet: create or update the Oracle object
 - OracleDelete: delete the Oracle object

 To support this functionality add:
 - New RPC method, `get_aggregate_price`, to calculate aggregate price for a token pair of the specified oracles
 - `ltOracle` object

 The `ltOracle` object maintains:
 - Oracle Owner's account
 - Oracle's metadata
 - Up to ten token pairs with the scaled price
 - The last update time the token pairs were updated

 Add Oracle unit-tests

* fix compile error on gcc 13: (XRPLF#4932)

The compilation fails due to an issue in the initializer list
of an optional argument, which holds a vector of pairs.
The code compiles correctly on earlier gcc versions, but fails on gcc 13.

* Set version to 2.2.0-b1

* Remove default ctors from SecretKey and PublicKey: (XRPLF#4607)

* It is now an invariant that all constructed Public Keys are valid,
  non-empty and contain 33 bytes of data.
* Additionally, the memory footprint of the PublicKey class is reduced.
  The size_ data member is declared as static.
* Distinguish and identify the PublisherList retrieved from the local
  config file, versus the ones obtained from other validators.
* Fixes XRPLF#2942

* Fast base58 codec: (XRPLF#4327)

This algorithm is about an order of magnitude faster than the existing
algorithm (about 10x faster for encoding and about 15x faster for
decoding - including the double hash for the checksum). The algorithms
use gcc's int128 (fast MS version will have to wait, in the meantime MS
falls back to the slow code).

* feat: add user version of `feature` RPC (XRPLF#4781)

* uses same formatting as admin RPC
* hides potentially sensitive data

* build: add STCurrency.h to xrpl_core to fix clio build (XRPLF#4939)

* Embed patched recipe for RocksDB 6.29.5 (XRPLF#4947)

* fix: order book update variable swap: (XRPLF#4890)

This is likely the result of a typo when the code was simplified.

* Fix workflows (XRPLF#4948)

The problem was `CONAN_USERNAME` environment variable, which Conan 1.x uses as the default user in package references.

* Upgrade to xxhash 0.8.2 as a Conan requirement, enable SIMD hashing (XRPLF#4893)

We are currently using old version 0.6.2 of `xxhash`, as a verbatim copy and paste of its header file `xxhash.h`. Switch to the more recent version 0.8.2. Since this version is in Conan Center (and properly protects its ABI by keeping the state object incomplete), add it as a Conan requirement. Switch to the SIMD instructions (in the new `XXH3` family) supported by the new version.

* Update remaining actions (XRPLF#4949)

Downgrade {upload,download}-artifact action to v3 because of unreliability with v4.

* Install more public headers (XRPLF#4940)

Fixes some mistakes in XRPLF#4885

* test: Env unit test RPC errors return a unique result: (XRPLF#4877)

* telENV_RPC_FAILED is a new code, reserved exclusively
  for unit tests when RPC fails. This will
  make those types of errors distinct and easier to test
  for when expected and/or diagnose when not.
* Output RPC command result when result is not expected.

* Fix workflows (XRPLF#4951)

- Update container for Doxygen workflow. Matches Linux workflow, with newer GLIBC version required by newer actions.
- Fixes macOS workflow to install and configure Conan correctly. Still fails on tests, but that does not seem attributable to the workflow.

* perf: improve `account_tx` SQL query: (XRPLF#4955)

The witness server makes heavily use of the `account_tx` RPC command. Perf
testing showed that the SQL query used by `account_tx` became unacceptably slow
when the DB was large and there was a `marker` parameter. The plan for the query
showed only indexed reads. This appears to be an issue with the internal SQLite
optimizer. This patch rewrote the query to use `UNION` instead of `OR` and
significantly improves performance. See RXI-896 and RIPD-1847 for more details.

* `fixEmptyDID`: fix amendment to handle empty DID edge case: (XRPLF#4950)

This amendment fixes an edge case where an empty DID object can be
created. It adds an additional check to ensure that DIDs are
non-empty when created, and returns a `tecEMPTY_DID` error if the DID
would be empty.

* Enforce no duplicate slots from incoming connections: (XRPLF#4944)

We do not currently enforce that incoming peer connection does not have
remote_endpoint which is already used (either by incoming or outgoing
connection), hence already stored in slots_. If we happen to receive a
connection from such a duplicate remote_endpoint, it will eventually result in a
crash (when disconnecting) or weird behavior (when updating slot state), as a
result of an apparently matching remote_endpoint in slots_ being used by a
different connection.

* Remove zaphod.alloy.ee hub from default server list: (XRPLF#4903)

Remove the zaphod.alloy.ee hubs from the bootstrap and default configuration after 5 years. It has been an honor to run these servers, but it is now time for another entity to step into this role.

The zaphod servers will be taken offline in a phased manner keeping all those who have peering arrangements informed.

These would be the preferred attributes of a boostrap set of hubs:

    1. Commitment to run the hubs for a minimum of 2 years
    2. Highly available
    3. Geographically dispersed
    4. Secure and up to date
    5. Committed to ensure that peering information is kept private

* Write improved `forAllApiVersions` used in NetworkOPs (XRPLF#4833)

* Don't reach consensus as quickly if no other proposals seen: (XRPLF#4763)

This fixes a case where a peer can desync under a certain timing
circumstance--if it reaches a certain point in consensus before it receives
proposals. 

This was noticed under high transaction volumes. Namely, when we arrive at the
point of deciding whether consensus is reached after minimum establish phase
duration but before having received any proposals. This could be caused by
finishing the previous round slightly faster and/or having some delay in
receiving proposals. Existing behavior arrives at consensus immediately after
the minimum establish duration with no proposals. This causes us to desync
because we then close a non-validated ledger. The change in this PR causes us to
wait for a configured threshold before making the decision to arrive at
consensus with no proposals. This allows validators to catch up and for brief
delays in receiving proposals to be absorbed. There should be no drawback since,
with no proposals coming in, we needn't be in a huge rush to jump ahead.

* fixXChainRewardRounding: round reward shares down: (XRPLF#4933)

When calculating reward shares, the amount should always be rounded
down. If the `fixUniversalNumber` amendment is not active, this works
correctly. If it is not active, then the amount is incorrectly rounded
up. This patch introduces an amendment so it will be rounded down.

* Remove unused files

* Remove packaging scripts

* Consolidate external libraries

* Simplify protobuf generation

* Rename .hpp to .h

* Format formerly .hpp files

* Rewrite includes

$ find src/ripple/ src/test/ -type f -exec sed -i 's:include\s*["<]ripple/\(.*\)\.h\(pp\)\?[">]:include <ripple/\1.h>:' {} +

* Fix source lists

* Add markers around source lists

* fix: improper handling of large synthetic AMM offers:

A large synthetic offer was not handled correctly in the payment engine.
This patch fixes that issue and introduces a new invariant check while
processing synthetic offers.

* Set version to 2.1.1

* chore: change Github Action triggers for build/test jobs (XRPLF#4956)

Github Actions for the build/test jobs (nix.yml, mac.yml, windows.yml) will only run on branches that build packages (develop, release, master), and branches with names starting with "ci/". This is intended as a compromise between disabling CI jobs on personal forks entirely, and having the jobs run as a free-for-all. Note that it will not affect PR jobs at all.

* Address compiler warnings

* Fix search for protoc

* chore: Default validator-keys-tool to master branch: (XRPLF#4943)

* master is the default branch for that project. There's no point in
  using develop.

* Remove unused lambdas from MultiApiJson_test

* fix Conan component reference typo

* Set version to 2.2.0-b2

* bump version

* 2.2.3

* 2.2.4

* 2.2.5

---------

Co-authored-by: Gregory Tsipenyuk <gregtatcam@users.noreply.github.com>
Co-authored-by: seelabs <scott.determan@yahoo.com>
Co-authored-by: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com>
Co-authored-by: Mayukha Vadari <mvadari@gmail.com>
Co-authored-by: John Freeman <jfreeman08@gmail.com>
Co-authored-by: Bronek Kozicki <brok@incorrekt.com>
Co-authored-by: Ed Hennis <ed@ripple.com>
Co-authored-by: Olek <115580134+oleks-rip@users.noreply.github.com>
Co-authored-by: Alloy Networks <45832257+alloynetworks@users.noreply.github.com>
Co-authored-by: Mark Travis <mtrippled@users.noreply.github.com>
Co-authored-by: Gregory Tsipenyuk <gtsipenyuk@ripple.com>
sophiax851 pushed a commit to sophiax851/rippled that referenced this pull request Jun 12, 2024
These headers are required in the xrpl Conan package in order for
xbridge witness server (xbwd) to build. This change to libxrpl may help
any dependents of libxrpl. This addition does not change any C++ code.
sophiax851 pushed a commit to sophiax851/rippled that referenced this pull request Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Build System Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. Perf impact not expected Change is not expected to improve nor harm performance.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants