LedgerIndex: UnsignedInteger or UnsignedLong?

[sappenin]

This function in LedgerIndex implies that a LedgerIndex should be typed as an UnsignedLong in the general case, at least when represented as a number. However, this implication contradicts how numbers are represented in the Transaction object.

Overall, we don't have a consistent style around whether a LedgerIndex should be an UnsignedInteger or an UnsignedLong, and when using this library, I'm tired of having to map back and forth. So, let's decide this once and for all:

Arguments in-favor of UnsignedInt

1. It's big enough. The maximum value of an UnsignedInteger is 4,294,967,295. The XRP Ledger mainnet has been running for 12 years, and is currently sitting at LedgerIndex 63,011,412. At ~1 ledger every 5 seconds, it will take ~26 years (4,231,955,883/5 secs / 31536000 (secs in a year)) before this value overflows. That's a very long time and maybe we don't need to worry about it.
2. No breaking change necessary in the Transaction class (but probably in LedgerIndex).

Arguments in-favor of UnsignedLong

1. On the off-chance that some ledger encounters a very large ledger number, the library will "just work."
2. It will take ~26 years before this value overflows (see above). While that is a very long time, changing old software is expensive, so let's do our descendants a favor now and not put them into a world of hurt in the year 2047.
3. We've become comfortable with more-aggressively using SemVer by releasing major-versions more frequently, and we expect a version 3.0 to come soon with various other changes, so the argument that "this will be breaking" is less substantive than it might have otherwise been.
4. Why not?

[sappenin]

After discussing with @nkramer44, we decided the following:

1. Introduce a LedgerIndex immutable that has a value() that returns UnsignedLong.
   1. Add plus functions only to this class.
2. Introduce a LedgerIndexShortcut that has 3 static values (current, validated, closed) with a private constructor so that only those 3 can ever be constructed. This class does not implement LedgerIndex (This is valid because shortcut-ledger indices are only ever used for requests, and for requests we always use LedgerIndexShortcut if desired.
3. In every client request object, introduce 3 mutually-exclusive properties (enforced by @Check precondition check) as follows:
   1. public abstract Optional<Hash256> ledgerHash();
   2. public Optional<LedgerIndex> ledgerIndex()
   3. public Optional<LedgerIndexShortcut> ledgerIndexShortcut()
4. In Transaction.java, change lastLedgerSequence() to be as follows:

   @JsonProperty("LastLedgerSequence")
  Optional<LedgerIndex> lastLedgerSequence();

5. Create a new Jackson (de)serializer for LedgerTransaction that picks the non-empty field from the 3 above.
6. Create a new Jackson serializer for LedgerTransactionShortcut that serializes the value to a String.

With the above changes, a developer will have clarity around what a LedgerIndex can and cannot do, and for any client request objects, object creation will fail-fast if a developer violates the precondition checks for ledger indices.

[nkramer44]

In every client request object, introduce 3 mutually-exclusive properties (enforced by @check precondition check)

I'm thinking of taking this one step further and creating an object LedgerSpecifier which has the three fields (ledgerHash, ledgerIndex and ledgerIndexShortcut), and using that class in each request object. I think this will make it more clear to developers that you can only specify one. As an added benefit, we can reduce code duplication by having a @Check precondition check in one place (LedgerSpecifier) rather than in each request object. We can also just write aLedgerSpecifier (de)serializer instead of a different, but functionally equivalent, (de)serializer for each request object.

[sappenin]

I like this!

[nkramer44]

@sappenin I'm having some trouble with this approach when it comes to deserializing objects that contain a LedgerSpecifier.

XrplRequestParams descendants will (for the most part) have a LedgerSpecifier ledgerSpecifier() field, which will get annotated with @JsonUnwrapped so that either ledger_hash or ledger_index ends up in the root JSON object when the params get serialized. This works fine with a custom serializer and the @JsonUnwrapped annotation. However, for some unknown reason, my custom deserializer for LedgerSpecifier does not get invoked when deserializing XrplRequestParams objects, so deserialization fails.

We have two options:

1. Struggle to get Jackson to invoke my custom deserializer for LedgerSpecifier fields annotated with @JsonUnwrapped (I haven't been able to find any solutions online)
2. Since the objects using LedgerSpecifiers are technically request objects, they should really only ever need to be serialized, not deserialized. Should we just assume this and not support deserialization?

[sappenin]

This has been implemented via #132.