Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
nkramer44 opened this issue Aug 10, 2023 · 3 comments
Open
Labels
bug Something isn't working

Comments

@nkramer44
Copy link
Collaborator

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:

  • DB7F3411A5F56726671CDC7E378ABDB852243A6AB3241195841FA278131895FF
  • A9E115D8E7399BABCE3A33AB7ACB7E542C170DB0FF2D8D813F64916966DBF596
  • 478A2D1485DA131AA9BE11D863B5198A23AFB06EB5026B544A9CF667FB906F3A
  • B18B5429A1C8FA5D523DA4B716734885AEA5675C9D99D5083CB7AA2111CD98F5
  • 8AC3BEA1A098A65FCAEC246510CC472784F370ECCA368CF1C6E5C3435BBFA48E
  • F8DBBD6BAE6E6D9A3734D7DCEFB66985614416C06A5585C82E58A0615BDF52C6
  • FB620BD9A9773022FEA5C9114BEA3F155252D405B2EDE3C3F29DB06F3FF6E452
  • E43F6FB3946CFB572E9DF822CBD449DB7A3B47A3089F60B8769793F3D6D539AB
  • 4944275473A983074C0C35CC83FC2CA41AE7E741209E187C7C2008587F74D035
  • 064E75D27F61D2EBB2949E4CAD9C2518C4ACDD0A5528F32F0B20B97B07D2ECAD
  • 028A7CE0A27759F02C94A34DE92675CC18A86DD89CCC39588621EE6440B92ED6
  • CF682E98547634E9C6D44652BADFC1E60283E5B8CE689EF75EA23C78C35931C4
  • 1FC05817B97101DE3967E1B747E9C0E03183058476F2CC09A7FAC06DB16B808A
  • C3FC0DAF5E757E1F427FFE5064FDC5285E27079847C6114315BACA694428A672
  • 4F5EA977211870A199394E75078671D55304904A4FAB7D9F8D01AD2E3040C085
  • 1FB3037BA5DC640CA5472879DF72B51F1223AC1EF48CD5B00E24101D8A3AE91C
  • 7D82E1AD813AAC4084BA93531EFFBD2CFA78664CAF1E5CBCDF0A35F15CEC8824
  • A10D808A82BAC00427968AB0EFD41704F192FE3DE8CCB7352833D53A4C7D9E6E
  • D0BE140FA177817F197B9198F061BEB95DD1CB42712AE62AB859B9094DFF34D9
  • DCBCA7F344D2CBB2FD1BF012DA866693E298EE1D68C94A3E64AD87710FBA1947
  • AE81015361B838B095509D682092AB366B6B6143574F8962E5FBA015C338834C
  • 5D608991E110767ABC179E9EC1FE8A1202CAC83AD51824ECFEEF8578A5AF229B
  • EB445FF448D30384467A8205B4442C5A05D4A61B622C25880F60D1B518476CA9
  • 32E31E23DA6E807F074190265F7B76FC83D7C627A67BEE7B9AB6EB7F630CE795
  • D44E0D70D12420764F355E146DF5B6D798DC7D5347DA128BF942BFEDFC9D3720
  • 7BBE2F553316FC60E0E97A8A38B15B49316977FF14B125C9AB07FC08E8C6536C
  • 2E8C0E2120C8F80CC581817A20E7795AFFDB958BB512722A036420EF06781CC6
@sappenin
Copy link
Collaborator

@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).

@sappenin sappenin added the bug Something isn't working label Aug 10, 2023
@nkramer44
Copy link
Collaborator Author

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
Copy link
Collaborator

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).

@sappenin sappenin mentioned this issue Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants