Skip to content

feat: TEMP DRAFT: Convert CMake UNIT_TEST_REFERENCE_FEE to command line unittest-fee#5394

Draft
ximinez wants to merge 3 commits intodevelopfrom
ximinez/vault-test
Draft

feat: TEMP DRAFT: Convert CMake UNIT_TEST_REFERENCE_FEE to command line unittest-fee#5394
ximinez wants to merge 3 commits intodevelopfrom
ximinez/vault-test

Conversation

@ximinez
Copy link
Copy Markdown
Collaborator

@ximinez ximinez commented Apr 9, 2025

High Level Overview of Change

Context of Change

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

@ximinez ximinez changed the base branch from develop to ximinez/Bronek/vault April 9, 2025 22:04
@ximinez ximinez force-pushed the ximinez/vault-test branch from 1e82440 to 01b152d Compare April 9, 2025 22:41
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 9, 2025

Codecov Report

❌ Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.7%. Comparing base (ffeabc9) to head (233ad4a).

Files with missing lines Patch % Lines
src/xrpld/app/main/Main.cpp 55.6% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5394   +/-   ##
=======================================
  Coverage     78.7%   78.7%           
=======================================
  Files          816     816           
  Lines        72275   72285   +10     
  Branches      8542    8522   -20     
=======================================
+ Hits         56872   56882   +10     
  Misses       15403   15403           
Files with missing lines Coverage Δ
src/xrpld/app/main/Main.cpp 78.0% <55.6%> (-1.0%) ⬇️

... and 3 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.

@ximinez ximinez force-pushed the ximinez/vault-test branch 5 times, most recently from d6a2ce2 to 84b4e2a Compare April 11, 2025 23:34
@ximinez ximinez added DraftRunCI Normally CI does not run on draft PRs. This opts in. and removed DraftRunCI Normally CI does not run on draft PRs. This opts in. labels Apr 12, 2025
@ximinez ximinez force-pushed the ximinez/vault-test branch from 84b4e2a to 98bafb2 Compare April 14, 2025 19:49
ximinez added a commit that referenced this pull request Apr 15, 2025
- Takes a page from #5394, which may turn out to be useful!
@ximinez ximinez force-pushed the ximinez/vault-test branch from 98bafb2 to be219fe Compare April 24, 2025 14:51
@ximinez ximinez force-pushed the ximinez/Bronek/vault branch from 9ab36dd to fb2eb35 Compare April 24, 2025 16:01
@ximinez ximinez force-pushed the ximinez/vault-test branch 2 times, most recently from 8481ed6 to 7937823 Compare April 28, 2025 18:45
@ximinez ximinez force-pushed the ximinez/Bronek/vault branch from 40d80b8 to 01cc089 Compare April 29, 2025 16:23
@ximinez ximinez force-pushed the ximinez/vault-test branch from 7937823 to de0b4e1 Compare April 30, 2025 18:47
@ximinez ximinez force-pushed the ximinez/vault-test branch from de0b4e1 to b17e50d Compare May 7, 2025 18:52
@ximinez ximinez force-pushed the ximinez/vault-test branch from b17e50d to 4da5c72 Compare May 15, 2025 15:58
@ximinez ximinez deleted the branch develop July 2, 2025 23:25
@ximinez ximinez closed this Jul 2, 2025
@ximinez ximinez reopened this Jul 2, 2025
@ximinez ximinez changed the base branch from ximinez/Bronek/vault to develop July 2, 2025 23:28
@ximinez ximinez force-pushed the ximinez/vault-test branch 2 times, most recently from fa2bbf1 to ec11ae3 Compare July 10, 2025 16:35
@ximinez ximinez force-pushed the ximinez/vault-test branch 4 times, most recently from a9283e6 to a9ccb6a Compare July 18, 2025 22:35
@ximinez ximinez force-pushed the ximinez/vault-test branch 6 times, most recently from a4bd2ae to 3506237 Compare September 4, 2025 20:44
@ximinez ximinez force-pushed the ximinez/vault-test branch 6 times, most recently from b22b10c to bfc3e3c Compare September 11, 2025 14:36
@ximinez ximinez force-pushed the ximinez/vault-test branch 5 times, most recently from dd8a482 to 233ad4a Compare September 20, 2025 20:03
@ximinez ximinez force-pushed the ximinez/vault-test branch 2 times, most recently from eb000af to 0466a9d Compare September 25, 2025 17:28
Comment on lines 185 to 188
run: |
./xrpld --unittest --unittest-jobs "${BUILD_NPROC}"
./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" \
${{ inputs.unittest_args }}

Copy link
Copy Markdown

@semgrep-companion-app semgrep-companion-app bot Dec 11, 2025

Choose a reason for hiding this comment

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

Using variable interpolation ${{...}} with github context data in a run: step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code. github context data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable with env: to store the data and use the environment variable in the run: script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR".

🍰 Removed in commit 21315d6 🍰

Comment on lines 218 to 221
run: |
./xrpld --unittest --unittest-jobs "${BUILD_NPROC}"
./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" \
${{ inputs.unittest_args }}

Copy link
Copy Markdown

@semgrep-companion-app semgrep-companion-app bot Jan 15, 2026

Choose a reason for hiding this comment

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

Using variable interpolation ${{...}} with github context data in a run: step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code. github context data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable with env: to store the data and use the environment variable in the run: script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR".

Fixed in commit cdcc6b6

@github-actions
Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

@github-actions
Copy link
Copy Markdown

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

@github-actions
Copy link
Copy Markdown

⚠️ This PR contains unsigned commits. To get your PR merged, please sign them. ⚠️

If only the most recent commit is unsigned, you can run:

  1. Amend the commit: git commit --amend --no-edit -n -S
  2. Overwrite the commit: git push --force-with-lease

If multiple commits are unsigned, you can run:

  1. Go into interactive rebase mode: git rebase --interactive HEAD~<NUM_OF_COMMITS>,
    where NUM_OF_COMMITS is the number of most recent commits that will be available
    to edit.
  2. Change "pick" to "edit" for the commits you need to sign, and then save and exit.
  3. For each commit, run: git commit --amend --no-edit -n -S
  4. Continue the rebase: git rebase --continue
  5. Overwrite the commit(s): git push --force-with-lease

If you're new to commit signing, there are different ways to set it up:

Sign commits with gpg

Follow the steps below to set up commit signing with gpg:

  1. Generate a GPG key
  2. Add the GPG key to your GitHub account
  3. Configure git to use your GPG key for commit signing
Sign commits with ssh-agent

Follow the steps below to set up commit signing with ssh-agent:

  1. Generate an SSH key and add it to ssh-agent
  2. Add the SSH key to your GitHub account
  3. Configure git to use your SSH key for commit signing
Sign commits with 1Password

You can also sign commits using 1Password, which lets you sign commits with biometrics without the signing key leaving the local 1Password process.
See use 1Password to sign your commits.

@github-actions
Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 7, 2026

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

- Remove the cmake UNIT_TEST_REFERENCE_FEE setting
  - Replaced by --unittest-fee runtime param
@ximinez
Copy link
Copy Markdown
Collaborator Author

ximinez commented Apr 13, 2026

Squashed and rebased to resolve unsigned commit issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant