Skip to content

Commit

Permalink
pr feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
nkramer44 committed Oct 29, 2024
1 parent b8911c0 commit ae996c6
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -300,13 +300,13 @@ public JsonNode toJson() {
*
* @return {@code true} if this AmountType is native; {@code false} otherwise.
*/
private boolean isNative() {
public boolean isNative() {
// 1st bit in 1st byte is set to 0 for native XRP, 3rd bit is also 0.
byte leadingByte = toBytes()[0];
return (leadingByte & 0x80) == 0 && (leadingByte & 0x20) == 0;
}

private boolean isMpt() {
public boolean isMpt() {
// 1st bit in 1st byte is 0, and 3rd bit is 1
byte leadingByte = toBytes()[0];
return (leadingByte & 0x80) == 0 && (leadingByte & 0x20) != 0;
Expand All @@ -317,9 +317,9 @@ private boolean isMpt() {
*
* @return {@code true} if this AmountType is positive; {@code false} otherwise.
*/
private boolean isPositive() {
public boolean isPositive() {
// 2nd bit in 1st byte is set to 1 for positive amounts
return (toBytes()[0] & 0x40) > 0;
return toHex().startsWith("00000000000000", 2) || (toBytes()[0] & 0x40) > 0;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ public T fromParser(BinaryParser parser, int lengthHint) {
}

/**
* Obtain a {@link T} using the supplied {@code node}.
* Obtain a {@link T} using the supplied {@code node}. Prefer using {@link #fromJson(JsonNode, FieldInstance)} over
* this method, as some {@link SerializedType}s require a {@link FieldInstance} to accurately serialize and
* deserialize.
*
* @param node A {@link JsonNode} to use.
*
Expand Down Expand Up @@ -208,7 +210,9 @@ public byte[] toBytes() {
}

/**
* Convert this {@link SerializedType} to a {@link JsonNode}.
* Convert this {@link SerializedType} to a {@link JsonNode}. Prefer using {@link #toJson(FieldInstance)} over this
* method where possible, as some {@link SerializedType}s require a {@link FieldInstance} to accurately serialize and
* deserialize.
*
* @return A {@link JsonNode}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,17 @@ protected static Stream<Arguments> dataDrivenFixturesForType(SerializedType seri
@ParameterizedTest
@MethodSource("dataDrivenFixtures")
void fixtureTests(ValueTest fixture) throws IOException {
SerializedType serializedType = getType();
SerializedType<?> serializedType = getType();
JsonNode value = getValue(fixture);
if (fixture.error() != null) {
assertThrows(Exception.class, () -> serializedType.fromJson(value));
} else {
assertThat(serializedType.fromJson(value).toHex()).isEqualTo(fixture.expectedHex());
SerializedType<?> serialized = serializedType.fromJson(value);
if (fixture.type().equals("Amount")) {
assertThat(((AmountType) serialized).isPositive()).isEqualTo(!fixture.isNegative());
assertThat(((AmountType) serialized).isNative()).isEqualTo(fixture.isNative());
}
assertThat(serialized.toHex()).isEqualTo(fixture.expectedHex());
}
}

Expand Down
4 changes: 2 additions & 2 deletions xrpl4j-core/src/test/resources/data-driven-tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -3704,7 +3704,7 @@
"is_native": false,
"type": "Amount",
"expected_hex": "20000000000000000100002403C84A0A28E0190E208E982C352BBD5006600555CF",
"is_negative": false
"is_negative": true
},
{
"test_json": {
Expand Down Expand Up @@ -3811,7 +3811,7 @@
"is_native": false,
"type": "Amount",
"expected_hex": "20000000000000000000002403C84A0A28E0190E208E982C352BBD5006600555CF",
"is_negative": false
"is_negative": true
},
{
"test_json": {
Expand Down

0 comments on commit ae996c6

Please sign in to comment.