Support XRPFees amendment #508
Conversation
…eFee is hex and BaseFeeDrops is decimal
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #508 +/- ##
============================================
+ Coverage 91.65% 91.66% +0.01%
- Complexity 1774 1781 +7
============================================
Files 365 366 +1
Lines 4948 4957 +9
Branches 408 408
============================================
+ Hits 4535 4544 +9
Misses 280 280
Partials 133 133 ☔ View full report in Codecov by Sentry. |
| @Value.Default | ||
| @JsonIgnore | ||
| default UnsignedInteger referenceFeeUnits() { | ||
| return maybeReferenceFeeUnits().orElse(null); |
There was a problem hiding this comment.
This feels like a breaking change.
E.g., imagine an application using an older version of xrpl4j is calling referenceFeeUnits. Then, that code upgrades to the version of xrpl4j that has this change (but the application usage of xrpl4j isn't changed because there's just a deprecation warning). If that application starts talking to a server with XRPFees amendment enabled, that code will throw a NPE likely.
I need to think more about this one -- unless the argument is that what I wrote above can't happen? Or maybe just is unlikely to happen?
There was a problem hiding this comment.
I think you're right. Anyone using this method and not checking for null will get a NPE if XRPFees is enabled. Unfortunately I can't think of a way to support the XRPFees change without making this @Nullable. We could @Value.Default it to UnsignedInteger.ZERO to avoid creating NPE, but that almost seems harder to catch as a developer than getting a bunch of NPE in your logs.
Adjusts the
SetFeepseudo-transaction to work when theXRPFeesamendment goes live.