Sponsored Content
Skip to content

fix: Fix unity build for book step - #6942

Merged
ximinez merged 4 commits into
developfrom
a1q123456/fix-unity-build-for-bookstep
Apr 16, 2026
Merged

fix: Fix unity build for book step#6942
ximinez merged 4 commits into
developfrom
a1q123456/fix-unity-build-for-bookstep

Conversation

@a1q123456

@a1q123456 a1q123456 commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

High Level Overview of Change

This PR adds a static check in src/libxrpl/tx/paths/BookStep.cpp to avoid the case where both TIn and TOut are XRPAmount.

Context of Change

Because unity build puts OfferStream.cpp and BookStep.cpp in the same translation unit, the compiler sees TOfferStreamBase<XRPAmount, XRPAmount> which further triggers the compiler error.

With this fix, we explicitly disallow this combination so that we get rid of the build error.

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)

@a1q123456 a1q123456 added the Trivial Simple change with minimal effect, or already tested. Only needs one approval. label Apr 14, 2026
@a1q123456
a1q123456 requested a review from gregtatcam April 14, 2026 16:05
@a1q123456 a1q123456 changed the title Fix unity build for book step fiix: Fix unity build for book step Apr 14, 2026
@a1q123456 a1q123456 changed the title fiix: Fix unity build for book step fix: Fix unity build for book step Apr 14, 2026

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Had a look at this

One incorrect namespace string in the new UNREACHABLE tag — see inline.


Review by ReviewBot 🤖

Review by Claude Opus 4.6 · Prompt: V14

Comment thread src/libxrpl/tx/paths/BookStep.cpp Outdated
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V14

@codecov

codecov Bot commented Apr 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.6%. Comparing base (a873250) to head (e28bb83).
⚠️ Report is 6 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #6942   +/-   ##
=======================================
  Coverage     81.6%   81.6%           
=======================================
  Files         1010    1010           
  Lines        75982   75972   -10     
  Branches      7632    7606   -26     
=======================================
- Hits         61986   61984    -2     
+ Misses       13996   13988    -8     
Files with missing lines Coverage Δ
src/libxrpl/tx/paths/BookStep.cpp 95.9% <100.0%> (-<0.1%) ⬇️

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

Comment thread src/libxrpl/tx/paths/BookStep.cpp
Comment thread src/libxrpl/tx/paths/OfferStream.cpp Outdated
@a1q123456
a1q123456 requested a review from gregtatcam April 15, 2026 14:57

@xrplf-ai-reviewer xrplf-ai-reviewer Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues.

Review by Claude Opus 4.6 · Prompt: V15

@gregtatcam gregtatcam left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍 LGTM

@a1q123456 a1q123456 added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Apr 15, 2026
@ximinez
ximinez added this pull request to the merge queue Apr 16, 2026
Merged via the queue into develop with commit 4a73be4 Apr 16, 2026
3 checks passed
@ximinez
ximinez deleted the a1q123456/fix-unity-build-for-bookstep branch April 16, 2026 17:29
marek-foss-neti pushed a commit to marek-foss-neti/rippled that referenced this pull request May 5, 2026
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
beartec-jpg pushed a commit to beartec-jpg/FalconLedger that referenced this pull request Jun 1, 2026
Co-authored-by: xrplf-ai-reviewer[bot] <266832837+xrplf-ai-reviewer[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. Trivial Simple change with minimal effect, or already tested. Only needs one approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants