rippled icon indicating copy to clipboard operation
rippled copied to clipboard

Fix: XRPAmount Overflow

Open dangell7 opened this issue 10 months ago • 3 comments

High Level Overview of Change

Context of Change

Type of Change

  • [x] 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)

dangell7 avatar Mar 04 '25 08:03 dangell7

Codecov Report

:x: Patch coverage is 81.25000% with 6 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 78.1%. Comparing base (d22a505) to head (0d0dda9). :warning: Report is 253 commits behind head on develop.

Files with missing lines Patch % Lines
include/xrpl/protocol/XRPAmount.h 81.2% 6 Missing :warning:
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5330   +/-   ##
=======================================
  Coverage     78.1%   78.1%           
=======================================
  Files          790     790           
  Lines        67988   68013   +25     
  Branches      8255    8260    +5     
=======================================
+ Hits         53084   53104   +20     
- Misses       14904   14909    +5     
Files with missing lines Coverage Δ
include/xrpl/protocol/XRPAmount.h 93.8% <81.2%> (-5.1%) :arrow_down:

... and 3 files with indirect coverage changes

Impacted file tree graph

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Mar 04 '25 08:03 codecov[bot]

@dangell7 Find a Windows equivalent and use as appropriate, please

Bronek avatar Mar 04 '25 11:03 Bronek

Can you pls also increase the unit test coverage here ? No need to go through Env , just create XRPAmount with inputs to cover all the interesting cases.

This aside this looks good to me.

Bronek avatar Jun 06 '25 15:06 Bronek