Skip to content

Commit 838485a

Browse files
sappeninsappenin
authored
Handle and ignore invalid flags instead of throwing an exception (Fixes #640) (#647)
* Handle and ignore invalid flags instead of throwing an exception (Fixes #640)
1 parent 4ba4f31 commit 838485a

File tree

5 files changed

+290
-26
lines changed

5 files changed

+290
-26
lines changed

‎xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/AccountSet.java‎

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -146,17 +146,16 @@ default AccountSet normalizeClearFlag() {
146146
// This can happen if:
147147
// 1. A developer sets clearFlagRawValue manually in the builder
148148
// 2. JSON has ClearFlag and jackson sets clearFlagRawValue.
149-
// This value will never be negative due to XRPL representing this kind of flag as an unsigned number,
150-
// so no lower bound check is required.
151-
if (clearFlagRawValue().get().longValue() <= AccountSetFlag.MAX_VALUE) {
152-
// Set clearFlag to clearFlagRawValue if clearFlagRawValue matches a valid AccountSetFlag variant.
153-
return AccountSet.builder().from(this)
154-
.clearFlag(AccountSetFlag.forValue(clearFlagRawValue().get().intValue()))
155-
.build();
156-
} else {
157-
// Otherwise, leave clearFlag empty.
158-
return this;
159-
}
149+
// Try to set clearFlag to clearFlagRawValue if clearFlagRawValue matches a valid AccountSetFlag variant.
150+
// If the value doesn't correspond to a known AccountSetFlag (e.g., value 11 which is reserved for Hooks),
151+
// leave clearFlag empty.
152+
return AccountSetFlag.forValueIfValid(clearFlagRawValue().get().intValue())
153+
.map(flag -> AccountSet.builder()
154+
.from(this)
155+
.clearFlag(flag)
156+
.build()
157+
)
158+
.orElse((ImmutableAccountSet) this);
160159
}
161160
}
162161

@@ -238,27 +237,26 @@ default AccountSet normalizeSetFlag() {
238237
} else { // setFlag is empty and setFlagRawValue is present
239238
// This can happen if:
240239
// 1. A developer sets setFlagRawValue manually in the builder
241-
// 2. JSON has ClearFlag and jackson sets setFlagRawValue.
242-
// This value will never be negative due to XRPL representing this kind of flag as an unsigned number,
243-
// so no lower bound check is required.
244-
if (setFlagRawValue().get().longValue() <= AccountSetFlag.MAX_VALUE) {
245-
// Set setFlag to setFlagRawValue if setFlagRawValue matches a valid AccountSetFlag variant.
246-
return AccountSet.builder().from(this)
247-
.setFlag(AccountSetFlag.forValue(setFlagRawValue().get().intValue()))
248-
.build();
249-
} else {
250-
// Otherwise, leave setFlag empty.
251-
return this;
252-
}
240+
// 2. JSON has SetFlag and jackson sets setFlagRawValue.
241+
// Try to set setFlag to setFlagRawValue if setFlagRawValue matches a valid AccountSetFlag variant.
242+
// If the value doesn't correspond to a known AccountSetFlag (e.g., value 11 which is reserved for Hooks),
243+
// leave setFlag empty.
244+
return AccountSetFlag.forValueIfValid(setFlagRawValue().get().intValue())
245+
.map(flag -> AccountSet.builder()
246+
.from(this)
247+
.setFlag(flag)
248+
.build()
249+
)
250+
.orElse((ImmutableAccountSet) this);
253251
}
254252
}
255253

256254
/**
257255
* The hex string of the lowercase ASCII of the domain for the account. For example, the domain example.com would be
258256
* represented as "6578616D706C652E636F6D".
259257
*
260-
* <p>To remove the Domain field from an account, send an {@link AccountSet} with the {@link AccountSet#domain()}
261-
* set to an empty string.
258+
* <p>To remove the Domain field from an account, send an {@link AccountSet} with this domain field set to an empty
259+
* string.
262260
*
263261
* @return An {@link Optional} of type {@link String} containing the domain.
264262
*/
@@ -503,6 +501,7 @@ enum AccountSetFlag {
503501
*
504502
* @return The {@link AccountSetFlag} for the given integer value.
505503
*
504+
* @throws IllegalArgumentException if no matching {@link AccountSetFlag} exists for the given value.
506505
* @see "https://github.com/FasterXML/jackson-databind/issues/1850"
507506
*/
508507
@JsonCreator
@@ -516,6 +515,27 @@ public static AccountSetFlag forValue(int value) {
516515
throw new IllegalArgumentException("No matching AccountSetFlag enum value for int value " + value);
517516
}
518517

518+
/**
519+
* Get the {@link AccountSetFlag} for the given integer value, if one exists.
520+
*
521+
* <p>This method is useful when you need to check if a value corresponds to a known {@link AccountSetFlag}
522+
* without throwing an exception for unknown values (e.g., values reserved for future amendments like value 11 which
523+
* is reserved for the Hooks amendment).</p>
524+
*
525+
* @param value The int value of the flag.
526+
*
527+
* @return An {@link Optional} containing the {@link AccountSetFlag} if one exists for the given value, or
528+
* {@link Optional#empty()} if no matching flag exists.
529+
*/
530+
public static Optional<AccountSetFlag> forValueIfValid(int value) {
531+
for (AccountSetFlag flag : AccountSetFlag.values()) {
532+
if (flag.value == value) {
533+
return Optional.of(flag);
534+
}
535+
}
536+
return Optional.empty();
537+
}
538+
519539
/**
520540
* Get the underlying value of this {@link AccountSetFlag}.
521541
*

‎xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/AccountSetTests.java‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,4 +419,57 @@ public void testClearAllowTrustlineLockingFlag() {
419419
assertThat(accountSet.clearFlagRawValue()).isNotEmpty().get()
420420
.isEqualTo(UnsignedInteger.valueOf(17));
421421
}
422+
423+
@ParameterizedTest
424+
@MethodSource("accountSetFlags")
425+
void testForValueIfValidReturnsValueForKnownFlags(AccountSetFlag accountSetFlag) {
426+
assertThat(AccountSetFlag.forValueIfValid(accountSetFlag.getValue()))
427+
.isPresent()
428+
.hasValue(accountSetFlag);
429+
}
430+
431+
@Test
432+
void testForValueIfValidReturnsEmptyForUnknownFlags() {
433+
// Value 11 is reserved for Hooks amendment (asfTshCollect) but not yet a valid enum value
434+
assertThat(AccountSetFlag.forValueIfValid(11)).isEmpty();
435+
// Value 17 is greater than MAX_VALUE
436+
assertThat(AccountSetFlag.forValueIfValid(AccountSetFlag.MAX_VALUE + 1)).isEmpty();
437+
// Negative values should also return empty
438+
assertThat(AccountSetFlag.forValueIfValid(-1)).isEmpty();
439+
}
440+
441+
@Test
442+
void testForValueThrowsForUnknownFlags() {
443+
// Value 11 is reserved for Hooks amendment (asfTshCollect) but not yet a valid enum value
444+
assertThatThrownBy(() -> AccountSetFlag.forValue(11))
445+
.isInstanceOf(IllegalArgumentException.class)
446+
.hasMessage("No matching AccountSetFlag enum value for int value 11");
447+
}
448+
449+
/**
450+
* Tests that AccountSet transactions with flag value 11 (reserved for Hooks amendment) can be
451+
* properly constructed. This is a regression test for the bug where deserializing a ledger
452+
* containing an AccountSet with SetFlag=11 would throw an IllegalArgumentException.
453+
*/
454+
@Test
455+
void testWithReservedFlagValue11() {
456+
// Value 11 is reserved for the Hooks amendment (asfTshCollect) and is not yet a valid
457+
// AccountSetFlag enum value, but transactions with this flag value exist on testnet.
458+
AccountSet accountSet = AccountSet.builder()
459+
.account(Address.of("rf1BiGeXwwQoi8Z2ueFYTEXSwuJYfV2Jpn"))
460+
.fee(XrpCurrencyAmount.ofDrops(12))
461+
.sequence(UnsignedInteger.valueOf(5))
462+
.setFlagRawValue(UnsignedInteger.valueOf(11))
463+
.clearFlagRawValue(UnsignedInteger.valueOf(11))
464+
.build();
465+
466+
// setFlag and clearFlag should be empty since 11 is not a valid AccountSetFlag
467+
assertThat(accountSet.setFlag()).isEmpty();
468+
assertThat(accountSet.clearFlag()).isEmpty();
469+
// But the raw values should be preserved
470+
assertThat(accountSet.setFlagRawValue()).isPresent();
471+
assertThat(accountSet.setFlagRawValue().get()).isEqualTo(UnsignedInteger.valueOf(11));
472+
assertThat(accountSet.clearFlagRawValue()).isPresent();
473+
assertThat(accountSet.clearFlagRawValue().get()).isEqualTo(UnsignedInteger.valueOf(11));
474+
}
422475
}

‎xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/NegativeTransactionMetadataTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,8 @@ void deserializeLedgerResultWithNegativeAmounts(String ledgerResultFileName) thr
117117
*/
118118
@ParameterizedTest
119119
@ValueSource(strings = {
120-
"ledger-result-94084608.json" // <-- See https://github.com/XRPLF/xrpl4j/issues/590
120+
"ledger-result-94084608.json", // <-- See https://github.com/XRPLF/xrpl4j/issues/590
121+
"ledger-result-13037843.json" // <-- See https://github.com/XRPLF/xrpl4j/issues/640
121122
})
122123
void deserializeLedgerResultWithSpecialObjects(String ledgerResultFileName) throws IOException {
123124
Objects.requireNonNull(ledgerResultFileName);

‎xrpl4j-core/src/test/java/org/xrpl/xrpl4j/model/transactions/json/AccountSetJsonTests.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
* =========================LICENSE_END==================================
2121
*/
2222

23+
import static org.assertj.core.api.Assertions.assertThat;
24+
2325
import com.fasterxml.jackson.core.JsonProcessingException;
2426
import com.google.common.base.Strings;
2527
import com.google.common.primitives.UnsignedInteger;
@@ -311,6 +313,54 @@ void testJsonWithUnrecognizedClearAndSetFlag() throws JSONException, JsonProcess
311313
assertCanSerializeAndDeserialize(accountSet, json);
312314
}
313315

316+
/**
317+
* Tests that AccountSet transactions with flag value 11 (reserved for Hooks amendment) can be
318+
* properly deserialized and serialized. This is a regression test for the bug where deserializing
319+
* a ledger containing an AccountSet with SetFlag=11 would throw an IllegalArgumentException.
320+
*
321+
* @see <a href="https://github.com/XRPLF/xrpl4j/issues/640">GitHub Issue</a>
322+
*/
323+
@Test
324+
void testJsonWithReservedFlagValue11() throws JSONException, JsonProcessingException {
325+
// Value 11 is reserved for the Hooks amendment (asfTshCollect) and is not yet a valid
326+
// AccountSetFlag enum value, but transactions with this flag value exist on testnet.
327+
AccountSet accountSet = AccountSet.builder()
328+
.account(Address.of("rhyg7sn3ZQj9aja9CBuLETFKdkG9Fte7ck"))
329+
.fee(XrpCurrencyAmount.ofDrops(15))
330+
.sequence(UnsignedInteger.valueOf(40232131))
331+
.setFlagRawValue(UnsignedInteger.valueOf(11))
332+
.clearFlagRawValue(UnsignedInteger.valueOf(11))
333+
.signingPublicKey(PublicKey.fromBase16EncodedPublicKey(
334+
"ED03FCED79BB3699089BC3F0F57BBD9F9ABA4772C20EC14BFAE908B4299344194A"
335+
))
336+
.transactionSignature(Signature.fromBase16("8D0915449FB617234DD54E1BA79136B50C4439696BB4287F" +
337+
"CA25717F3FD4875D5A1FEE6269B91D71D9306B48DECAAE1F1C91CAD1AAD0E0D683DC11E68212740E"))
338+
.build();
339+
340+
// Verify that setFlag and clearFlag are empty (since 11 is not a valid AccountSetFlag)
341+
assertThat(accountSet.setFlag()).isEmpty();
342+
assertThat(accountSet.clearFlag()).isEmpty();
343+
// But the raw values should be preserved
344+
assertThat(accountSet.setFlagRawValue()).isPresent();
345+
assertThat(accountSet.setFlagRawValue().get()).isEqualTo(UnsignedInteger.valueOf(11));
346+
assertThat(accountSet.clearFlagRawValue()).isPresent();
347+
assertThat(accountSet.clearFlagRawValue().get()).isEqualTo(UnsignedInteger.valueOf(11));
348+
349+
String json = "{\n" +
350+
" \"Account\": \"rhyg7sn3ZQj9aja9CBuLETFKdkG9Fte7ck\",\n" +
351+
" \"Fee\": \"15\",\n" +
352+
" \"Sequence\": 40232131,\n" +
353+
" \"SetFlag\": 11,\n" +
354+
" \"ClearFlag\": 11,\n" +
355+
" \"SigningPubKey\": \"ED03FCED79BB3699089BC3F0F57BBD9F9ABA4772C20EC14BFAE908B4299344194A\",\n" +
356+
" \"TransactionType\": \"AccountSet\",\n" +
357+
" \"TxnSignature\": \"8D0915449FB617234DD54E1BA79136B50C4439696BB4287FCA25717F3FD4875D5A1FEE6269B" +
358+
"91D71D9306B48DECAAE1F1C91CAD1AAD0E0D683DC11E68212740E\"" +
359+
"}";
360+
361+
assertCanSerializeAndDeserialize(accountSet, json);
362+
}
363+
314364
@Test
315365
public void testJsonWithUnknownFields() throws JSONException, JsonProcessingException {
316366
AccountSet accountSet = AccountSet.builder()

‎xrpl4j-core/src/test/resources/special-object-ledgers/ledger-result-13037843.json‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
{
2+
"ledger_hash": "F672B5F8658D58639479A013225702CDC9BEDC55C482091ECB2F92210478B4DB",
3+
"ledger_index": 13037843,
4+
"validated": true,
5+
"ledger": {
6+
"account_hash": "BDC31E62B2453C710C88F3BF71080133A16E52843793DE05AA45F5099B046353",
7+
"close_flags": 0,
8+
"close_time": 483126290,
9+
"close_time_human": "2015-Apr-23 17:44:50.000000000 UTC",
10+
"close_time_resolution": 10,
11+
"close_time_iso": "2015-04-23T17:44:50Z",
12+
"ledger_hash": "F672B5F8658D58639479A013225702CDC9BEDC55C482091ECB2F92210478B4DB",
13+
"parent_close_time": 483126290,
14+
"parent_hash": "6AC76B4ED9DD853FF56FC8AECB6EA015A09025DA4BAC5E1BBA5F784705EE345B",
15+
"total_coins": "99999130104974366",
16+
"transaction_hash": "F856188565B8D2C2A3ECB30FEC0890C21D3C4CABEE0057BD3034500DA953F967",
17+
"ledger_index": "13037843",
18+
"closed": true,
19+
"transactions": [
20+
{
21+
"_description": "Set the asfTshCollect flag on an account to test #640",
22+
"TransactionType": "AccountSet",
23+
"Account": "rfCFLzNJYvvnoGHWQYACmJpTgkLUaugLEw",
24+
"Fee": "12",
25+
"Sequence": 5015086,
26+
"SetFlag": 11,
27+
"Flags": 2147483648,
28+
"OfferSequence": 17364889,
29+
"SigningPubKey": "025718736160FA6632F48EA4354A35AB0340F8D7DC7083799B9C57C3E937D71851",
30+
"TxnSignature": "304502210090986D64C16F4D0A3B8A40DD5E0DD3C30FA9688EFDD5F8AAB2D5AD3C9F2332BC02203256A4B280AE84660EAC05957AF9CF98A6B8B847F4B964FAF7A17052786557A6",
31+
"hash": "09BD561F981E9F29B8DA0BDE37AEF0559EDCB4E0F51520967E10A3072FA597A9",
32+
"metaData": {
33+
"AffectedNodes": [
34+
{
35+
"DeletedNode": {
36+
"FinalFields": {
37+
"ExchangeRate": "4f1e90a392ca26e2",
38+
"Flags": 0,
39+
"RootIndex": "1631FB5AAD5341DE2F2F71C99514111971A116EDDA351B7F4F1E90A392CA26E2",
40+
"TakerGetsCurrency": "0000000000000000000000000000000000000000",
41+
"TakerGetsIssuer": "0000000000000000000000000000000000000000",
42+
"TakerPaysCurrency": "0000000000000000000000004B52570000000000",
43+
"TakerPaysIssuer": "80DC3C16EAE6E6FEF45271D30CBDD1D1D0C73AA5"
44+
},
45+
"LedgerEntryType": "DirectoryNode",
46+
"LedgerIndex": "1631FB5AAD5341DE2F2F71C99514111971A116EDDA351B7F4F1E90A392CA26E2"
47+
}
48+
},
49+
{
50+
"ModifiedNode": {
51+
"FinalFields": {
52+
"Flags": 0,
53+
"IndexPrevious": "8b",
54+
"Owner": "rHsZHqa5oMQNL5hFm4kfLd47aEMYjPstpg",
55+
"RootIndex": "B65B458CE90B410C20AC46F87480569F661F3AB426FF773B2EEB4E75947A5FD6"
56+
},
57+
"LedgerEntryType": "DirectoryNode",
58+
"LedgerIndex": "4AA00809E029BFF3CE935377C975FE70E6DB404CBA808EA29D768B1D5ECA7B03"
59+
}
60+
},
61+
{
62+
"DeletedNode": {
63+
"FinalFields": {
64+
"Account": "rHsZHqa5oMQNL5hFm4kfLd47aEMYjPstpg",
65+
"BookDirectory": "1631FB5AAD5341DE2F2F71C99514111971A116EDDA351B7F4F1E90A392CA26E2",
66+
"BookNode": "0",
67+
"Flags": 0,
68+
"OwnerNode": "9c",
69+
"PreviousTxnID": "3D59942EF875ADAC648E5DAEF6BF9BDC7822AB817EE946AB6E52F05AABE99017",
70+
"PreviousTxnLgrSeq": 13037839,
71+
"Sequence": 17364889,
72+
"TakerGets": "4021962556",
73+
"TakerPays": {
74+
"currency": "KRW",
75+
"issuer": "rUkMKjQitpgAM5WTGk79xpjT38DEJY283d",
76+
"value": "34602.076124567"
77+
}
78+
},
79+
"LedgerEntryType": "Offer",
80+
"LedgerIndex": "6F57BA32AC3D00ADA8271B0B47A8CC872D46AE15D0BE5116F90AE8FD20BAB143"
81+
}
82+
},
83+
{
84+
"CreatedNode": {
85+
"LedgerEntryType": "DirectoryNode",
86+
"LedgerIndex": "7254404DF6B7FBFFEF34DC38867A7E7DE610B513997B78804D11B0687EEA4988",
87+
"NewFields": {
88+
"ExchangeRate": "4d11b0687eea4988",
89+
"RootIndex": "7254404DF6B7FBFFEF34DC38867A7E7DE610B513997B78804D11B0687EEA4988",
90+
"TakerPaysCurrency": "000000000000000000000000434E590000000000",
91+
"TakerPaysIssuer": "41C8BE2C0A6AA17471B9F6D0AF92AAB1C94D5A25"
92+
}
93+
}
94+
},
95+
{
96+
"CreatedNode": {
97+
"LedgerEntryType": "Offer",
98+
"LedgerIndex": "92345342A11CEAF9C8EACA8D4E03458A6261A4012E2E91AF2D0C34ECF580E8EC",
99+
"NewFields": {
100+
"Account": "rHsZHqa5oMQNL5hFm4kfLd47aEMYjPstpg",
101+
"BookDirectory": "7254404DF6B7FBFFEF34DC38867A7E7DE610B513997B78804D11B0687EEA4988",
102+
"OwnerNode": "9c",
103+
"Sequence": 17364891,
104+
"TakerGets": "39999999999",
105+
"TakerPays": {
106+
"currency": "CNY",
107+
"issuer": "razqQKzJRdB4UxFPWf5NEpEG3WMkmwgcXA",
108+
"value": "1991.614982531"
109+
}
110+
}
111+
}
112+
},
113+
{
114+
"ModifiedNode": {
115+
"FinalFields": {
116+
"Account": "rHsZHqa5oMQNL5hFm4kfLd47aEMYjPstpg",
117+
"Balance": "448562224804",
118+
"Flags": 0,
119+
"OwnerCount": 52,
120+
"Sequence": 17364892
121+
},
122+
"LedgerEntryType": "AccountRoot",
123+
"LedgerIndex": "B79D156918390AC41C4DDE5F181417D55B01D0D183ACBB1B892FF163C5BC8344",
124+
"PreviousFields": {
125+
"Balance": "448562235804",
126+
"Sequence": 17364891
127+
},
128+
"PreviousTxnID": "5A1061C98BF9BF1C12AB86261BD8AA1E58BFC8F79AA69FABE5408E9EB22B73A7",
129+
"PreviousTxnLgrSeq": 13037840
130+
}
131+
}
132+
],
133+
"TransactionIndex": 0,
134+
"TransactionResult": "tesSUCCESS"
135+
}
136+
}
137+
]
138+
},
139+
"status": "success"
140+
}

0 commit comments

Comments
 (0)