Skip to content

Commit

Permalink
Allow for No-Op AccountSet SetFlag and ClearFlag values (#479)
Browse files Browse the repository at this point in the history 
* fix AccountSet desrialization for ClearFlags or SetFlags with large numbers

* use if-else

* fix typos

* add comment
  • Loading branch information
@nkramer44
nkramer44 authored Aug 14, 2023
1 parent 42047e1 commit 96650dd
Show file tree
Hide file tree
Showing 3 changed files with 418 additions and 13 deletions.
187 changes: 176 additions & 11 deletions xrpl4j-core/src/main/java/org/xrpl/xrpl4j/model/transactions/AccountSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
*
* http://www.apache.org/licenses/LICENSE-2.0
*
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
Expand All @@ -21,6 +21,7 @@
*/

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.annotation.JsonValue;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
Expand All @@ -29,7 +30,6 @@
import com.google.common.primitives.UnsignedInteger;
import org.immutables.value.Value;
import org.xrpl.xrpl4j.model.flags.AccountSetTransactionFlags;
import org.xrpl.xrpl4j.model.flags.TransactionFlags;

import java.util.Optional;

Expand Down Expand Up @@ -65,25 +65,189 @@ default AccountSetTransactionFlags flags() {
/**
* Unique identifier of a flag to disable for this account.
*
* <p>If this field is empty, developers should check if {@link #clearFlagRawValue()} is also empty. If
* {@link #clearFlagRawValue()} is present, it means that the {@code ClearFlag} field of the transaction was not a
* valid {@link AccountSetFlag} but was still present in a validated transaction on ledger.</p>
*
* <p>Because the preferred way of setting account flags is with {@link AccountSetFlag}s, this field should
* not be set in conjunction with the {@link AccountSet#flags()} field.
*
* @return An {@link Optional} of type {@link AccountSetFlag} representing the flag to disable on this account.
*/
@JsonProperty("ClearFlag")
@JsonIgnore
Optional<AccountSetFlag> clearFlag();

/**
* A flag to disable for this account, as an {@link UnsignedInteger}.
*
* <p>Developers should prefer setting {@link #clearFlag()} and leaving this field empty when constructing
* a new {@link AccountSet}. This field is used to serialize and deserialize the {@code "ClearFlag"} field in JSON,
* as some {@link AccountSet} transactions on the XRPL set the "ClearFlag" field to a number that is not recognized as
* an asf flag by rippled. Without this field, xrpl4j would fail to deserialize those transactions, as
* {@link AccountSetFlag} does not support arbitrary integer values.</p>
*
* <p>Additionally, using this field as the source of truth for JSON serialization/deserialization rather than
* {@link #clearFlag()} allows developers to recompute the hash of a transaction that was deserialized from a rippled
* RPC/WS result accurately. An alternative to this field would be to add an enum variant to {@link AccountSetFlag}
* for unknown values, but binary serializing an {@link AccountSet} that was constructed by deserializing JSON would
* result in a different binary blob than what exists on ledger.</p>
*
* @return An {@link Optional} {@link UnsignedInteger}.
*/
@JsonProperty("ClearFlag")
Optional<UnsignedInteger> clearFlagRawValue();

/**
* Normalization method to try to get {@link #clearFlag()}and {@link #clearFlagRawValue()} to match.
*
* <p>If neither field is present, there is nothing to do.</p>
* <p>If both fields are present, there is nothing to do, but we will check that {@link #clearFlag()}'s
* underlying value equals {@link #clearFlagRawValue()}.</p>
* <p>If {@link #clearFlag()} is present but {@link #clearFlagRawValue()} is empty, we set
* {@link #clearFlagRawValue()} to the underlying value of {@link #clearFlag()}.</p>
* <p>If {@link #clearFlag()} is empty and {@link #clearFlagRawValue()} is present, we will set
* {@link #clearFlag()} to the {@link AccountSetFlag} variant associated with {@link #clearFlagRawValue()}, or leave
* {@link #clearFlag()} empty if {@link #clearFlagRawValue()} does not map to an {@link AccountSetFlag}.</p>
*
* @return A normalized {@link AccountSet}.
*/
@Value.Check
default AccountSet normalizeClearFlag() {
if (!clearFlag().isPresent() && !clearFlagRawValue().isPresent()) {
// If both are empty, nothing to do.
return this;
} else if (clearFlag().isPresent() && clearFlagRawValue().isPresent()) {
// Both will be present if:
// 1. A developer set them both manually (in the builder)
// 2. This normalize method has already been called.

// We should still check that the clearFlagRawValue matches the inner value of AccountSetFlag.
Preconditions.checkState(
clearFlag().get().getValue() == clearFlagRawValue().get().longValue(),
String.format("clearFlag and clearFlagRawValue should be equivalent, but clearFlag's underlying " +
"value was %s and clearFlagRawValue was %s",
clearFlag().get().getValue(),
clearFlagRawValue().get().longValue()
)
);
return this;
} else if (clearFlag().isPresent() && !clearFlagRawValue().isPresent()) {
// This can only happen if the developer only set clearFlag(). In this case, we need to set clearFlagRawValue to
// match clearFlag.
return AccountSet.builder().from(this)
.clearFlagRawValue(UnsignedInteger.valueOf(clearFlag().get().getValue()))
.build();
} else { // clearFlag not present and clearFlagRawValue is present
// This can happen if:
// 1. A developer sets clearFlagRawValue manually in the builder
// 2. JSON has ClearFlag and jackson sets clearFlagRawValue.
// This value will never be negative due to XRPL representing this kind of flag as an unsigned number,
// so no lower bound check is required.
if (clearFlagRawValue().get().longValue() <= 15) {
// Set clearFlag to clearFlagRawValue if clearFlagRawValue matches a valid AccountSetFlag variant.
return AccountSet.builder().from(this)
.clearFlag(AccountSetFlag.forValue(clearFlagRawValue().get().intValue()))
.build();
} else {
// Otherwise, leave clearFlag empty.
return this;
}
}
}

/**
* Unique identifier of a flag to enable for this account.
*
* <p>Because the preferred way of setting account flags is with {@link AccountSetFlag}s, this field should not be set
* in conjunction with the {@link AccountSet#flags()} field.
* <p>If this field is empty, developers should check if {@link #setFlagRawValue()} is also empty. If
* {@link #setFlagRawValue()} is present, it means that the {@code ClearFlag} field of the transaction was not a
* valid {@link AccountSetFlag} but was still present in a validated transaction on ledger.</p>
*
* <p>Because the preferred way of setting account flags is with {@link AccountSetFlag}s, this field should not be
* set in conjunction with the {@link AccountSet#flags()} field.
*
* @return An {@link Optional} of type {@link AccountSetFlag} representing the flag to enable on this account.
*/
@JsonProperty("SetFlag")
@JsonIgnore
Optional<AccountSetFlag> setFlag();

/**
* A flag to disable for this account, as an {@link UnsignedInteger}.
*
* <p>Developers should prefer setting {@link #setFlag()} and leaving this field empty when constructing
* a new {@link AccountSet}. This field is used to serialize and deserialize the {@code "ClearFlag"} field in JSON,
* as some {@link AccountSet} transactions on the XRPL set the "ClearFlag" field to a number that is not recognized as
* an asf flag by rippled. Without this field, xrpl4j would fail to deserialize those transactions, as
* {@link AccountSetFlag} does not support arbitrary integer values.</p>
*
* <p>Additionally, using this field as the source of truth for JSON serialization/deserialization rather than
* {@link #setFlag()} allows developers to recompute the hash of a transaction that was deserialized from a rippled
* RPC/WS result accurately. An alternative to this field would be to add an enum variant to {@link AccountSetFlag}
* for unknown values, but binary serializing an {@link AccountSet} that was constructed by deserializing JSON would
* result in a different binary blob than what exists on ledger.</p>
*
* @return An {@link Optional} {@link UnsignedInteger}
*/
@JsonProperty("SetFlag")
Optional<UnsignedInteger> setFlagRawValue();

/**
* Normalization method to try to get {@link #setFlag()}and {@link #setFlagRawValue()} to match.
*
* <p>If neither field is present, there is nothing to do.</p>
* <p>If both fields are present, there is nothing to do, but we will check that {@link #setFlag()}'s
* underlying value equals {@link #setFlagRawValue()}.</p>
* <p>If {@link #setFlag()} is present but {@link #setFlagRawValue()} is empty, we set
* {@link #setFlagRawValue()} to the underlying value of {@link #setFlag()}.</p>
* <p>If {@link #setFlag()} is empty and {@link #setFlagRawValue()} is present, we will set
* {@link #setFlag()} to the {@link AccountSetFlag} variant associated with {@link #setFlagRawValue()}, or leave
* {@link #setFlag()} empty if {@link #setFlagRawValue()} does not map to an {@link AccountSetFlag}.</p>
*
* @return A normalized {@link AccountSet}.
*/
@Value.Check
default AccountSet normalizeSetFlag() {
if (!setFlag().isPresent() && !setFlagRawValue().isPresent()) {
// If both are empty, nothing to do.
return this;
} else if (setFlag().isPresent() && setFlagRawValue().isPresent()) {
// Both will be present if:
// 1. A developer set them both manually (in the builder)
// 2. This normalize method has already been called.

// We should still check that the setFlagRawValue matches the inner value of AccountSetFlag.
Preconditions.checkState(
setFlag().get().getValue() == setFlagRawValue().get().longValue(),
String.format("setFlag and setFlagRawValue should be equivalent, but setFlag's underlying " +
"value was %s and setFlagRawValue was %s",
setFlag().get().getValue(),
setFlagRawValue().get().longValue()
)
);
return this;
} else if (setFlag().isPresent() && !setFlagRawValue().isPresent()) {
// This can only happen if the developer only set setFlag(). In this case, we need to set setFlagRawValue to
// match setFlag.
return AccountSet.builder().from(this)
.setFlagRawValue(UnsignedInteger.valueOf(setFlag().get().getValue()))
.build();
} else { // setFlag is empty and setFlagRawValue is present
// This can happen if:
// 1. A developer sets setFlagRawValue manually in the builder
// 2. JSON has ClearFlag and jackson sets setFlagRawValue.
// This value will never be negative due to XRPL representing this kind of flag as an unsigned number,
// so no lower bound check is required.
if (setFlagRawValue().get().longValue() <= 15) {
// Set setFlag to setFlagRawValue if setFlagRawValue matches a valid AccountSetFlag variant.
return AccountSet.builder().from(this)
.setFlag(AccountSetFlag.forValue(setFlagRawValue().get().intValue()))
.build();
} else {
// Otherwise, leave setFlag empty.
return this;
}
}
}

/**
* The hex string of the lowercase ASCII of the domain for the account. For example, the domain example.com would be
* represented as "6578616D706C652E636F6D".
Expand Down Expand Up @@ -132,8 +296,8 @@ default AccountSetTransactionFlags flags() {
Optional<UnsignedInteger> tickSize();

/**
* Sets an alternate account that is allowed to mint NFTokens on this
* account's behalf using NFTokenMint's `Issuer` field.
* Sets an alternate account that is allowed to mint NFTokens on this account's behalf using NFTokenMint's `Issuer`
* field.
*
* @return An {@link Optional} field of type {@link Address}.
*/
Expand Down Expand Up @@ -206,8 +370,8 @@ default void checkTickSize() {
*/
enum AccountSetFlag {
/**
* This flag will do nothing but exists to accurately deserialize AccountSet transactions whose {@code SetFlag}
* or {@code ClearFlag} fields are zero.
* This flag will do nothing but exists to accurately deserialize AccountSet transactions whose {@code SetFlag} or
* {@code ClearFlag} fields are zero.
*/
NONE(0),
/**
Expand Down Expand Up @@ -291,6 +455,7 @@ enum AccountSetFlag {
* @param value The int value of the flag.
*
* @return The {@link AccountSetFlag} for the given integer value.
*
* @see "https://github.com/FasterXML/jackson-databind/issues/1850"
*/
@JsonCreator
Expand Down
Loading

0 comments on commit 96650dd

Please sign in to comment.