search

XRPLF / xrpl4j

A 100% Java implementation to interact with the XRP Ledger.
ISC License
86 stars 48 forks source link

LedgerResult Deserialization: Large XRP Amounts in TakerPays/TakerGets #475

Open nkramer44 opened 1 year ago

nkramer44 commented 1 year ago

There are several historical transactions where either the TakerPays or TakerGets fields have an XRP amount that is greater than the maximum XRP (100,000,000,000,000,000 drops). This causes a deserialization failure because XrpCurrencyAmount limits amounts to 100,000,000,000,000,000 drops. We should figure out how these transactions have such large amounts because this should not be allowed by rippled.

Ledgers where this is a problem:

sappenin commented 1 year ago

@nkramer44 Would simply removing the 100B limit precondition rectify this problem?

It occurs to me that the library tries to implement certain rippled rules (like this one) but in general our library should defer to ripple for "rules."

The counter argument here is, "if we keep the precondition, we'll help developers not make mistakes" but in this case I think the more appropriate solution is to (in version 4 I suppose) separate the request objects from response objects (so the precondition could exist on the request object, but not on the response object).

nkramer44 commented 1 year ago

Yes that would work I think. Developers who sign a transaction with an XrpCurrencyAmount > 100B XRP would still get a local error before being able to submit to rippled, as our binary encoder enforces that XRP amounts must be <= 100B.

I'm still curious to figure out how these transactions were successful in the first place. If the orders were filled, what happened to the buyer's XRP balance?

sappenin commented 1 month ago

Adding additional context here -- see this discussion about potentially allowing lower values of Drops. Allowing this in the XrpCurrencyAmount is likely untenable because this objects underlying core value is an UnsignedLong, and so the smallest unit here is 1 (i.e., 1 drop).

However, to support fractional drops of XRP, we could (1) introduce a new class maybe called FractionalXrpCurrencyAmount that would be backed by a BigDecimcal or (2) somehow update XrpCurrencyAmount to support fractional values (needs further investigation).