feat: XLS-78d Subscriptions

[dangell7]

High Level Overview of Change

Context of Change

Updated XLS Spec

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)

[codecov]

Codecov Report

❌ Patch coverage is 86.81319% with 72 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.8%. Comparing base (e67e039) to head (dc8a689).
⚠️ Report is 5 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/xrpld/app/tx/detail/SubscriptionClaim.cpp	82.4%	33 Missing ⚠️
src/xrpld/rpc/handlers/LedgerEntry.cpp	0.0%	13 Missing ⚠️
src/xrpld/app/tx/detail/SubscriptionSet.cpp	94.4%	10 Missing ⚠️
src/xrpld/app/misc/SubscriptionHelpers.h	92.5%	8 Missing ⚠️
src/xrpld/app/tx/detail/SubscriptionCancel.cpp	81.0%	8 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop   #5787    +/-   ##
========================================
  Coverage     78.7%   78.8%            
========================================
  Files          816     823     +7     
  Lines        71925   72470   +545     
  Branches      8460    8509    +49     
========================================
+ Hits         56638   57103   +465     
- Misses       15287   15367    +80     

Files with missing lines	Coverage Δ
include/xrpl/protocol/Indexes.h	100.0% <100.0%> (ø)
include/xrpl/protocol/detail/ledger_entries.macro	100.0% <100.0%> (ø)
include/xrpl/protocol/detail/transactions.macro	100.0% <100.0%> (ø)
src/libxrpl/protocol/Indexes.cpp	96.9% <100.0%> (+<0.1%)	⬆️
src/xrpld/app/tx/detail/InvariantCheck.cpp	89.3% <100.0%> (+<0.1%)	⬆️
src/xrpld/app/tx/detail/SubscriptionCancel.h	100.0% <100.0%> (ø)
src/xrpld/app/tx/detail/SubscriptionClaim.h	100.0% <100.0%> (ø)
src/xrpld/app/tx/detail/SubscriptionSet.h	100.0% <100.0%> (ø)
src/xrpld/app/tx/detail/applySteps.cpp	73.5% <ø> (ø)
src/xrpld/app/misc/SubscriptionHelpers.h	92.5% <92.5%> (ø)
... and 4 more

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[funkspock]

Hi @dangell7 @mvadari , comparing the standard to the code i noticed that:

32bit Unsigned Integers are used for:
sfFrequency , sfStartTime , sfNextClaimTime , sfExpiration

this can lead to an overflow condition in the year 2106, which is related to the year 2038 problem
https://en.wikipedia.org/wiki/Year_2038_problem
The Fix is using 64bit unsigned integers for all time components

Also it seems that the current code does not align with the standard specification for the following:
Standard mentions sfFrequency UInt64, while the code uses UInt32
Standard mentions sfNextPaymentTime, while the code uses sfNextClaimTime
Perhaps the standard and code can be aligned

[dangell7]

Hi @dangell7 @mvadari , comparing the standard to the code i noticed that:

32bit Unsigned Integers are used for:

sfFrequency , sfStartTime , sfNextClaimTime , sfExpiration

this can lead to an overflow condition in the year 2106, which is related to the year 2038 problem

https://en.wikipedia.org/wiki/Year_2038_problem

The Fix is using 64bit unsigned integers for all time components

Also it seems that the current code does not align with the standard specification for the following:

Standard mentions sfFrequency UInt64, while the code uses UInt32

Standard mentions sfNextPaymentTime, while the code uses sfNextClaimTime

Perhaps the standard and code can be aligned

Hey @funkspock you're absolutely right about the time. I was just using what already exists. I'll update and we should use UINT64 going forward. However sfExpiration already exists so I can't change that. There are a number of other time fields that will need to be changed in another PR.

As for the spec I will get that aligned too! Thanks for the comments and review.

[mvadari]

@funkspock rippled already has that problem with sequence numbers and rippled time. UInt64 numbers are a bit more user-unfriendly, since they're shown in JSON as hex.

I recommend having discussions about the spec to the XRPL-Standards repo.