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

Use bytes to represent memo text #259

Merged
merged 13 commits into from
Dec 13, 2019
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

As this project is pre 1.0, breaking changes may happen for minor version bumps. A breaking change will get clearly notified in this log.

## 0.12.0

* Represent memo text contents as bytes because a memo text may not be valid UTF-8 string.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's link to the issue (#257) from the changelog


## 0.11.0

* Fix bug in `org.stellar.sdk.requests.OperationsRequestBuilder.operation(long operationId)`. The method submitted an HTTP request to Horizon with the following path, /operation/<id> , but the correct path is /operations/<id>
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ apply plugin: 'com.github.ben-manes.versions' // gradle dependencyUpdates -Drevi
apply plugin: 'project-report' // gradle htmlDependencyReport

sourceCompatibility = 1.6
version = '0.11.0'
version = '0.12.0'
group = 'stellar'

jar {
Expand Down
9 changes: 9 additions & 0 deletions src/main/java/org/stellar/sdk/Memo.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ public static MemoText text(String text) {
return new MemoText(text);
}

/**
* Creates new {@link MemoText} instance.
* @param text
*/
public static MemoText text(byte[] text) {
return new MemoText(text);
}


/**
* Creates new {@link MemoId} instance.
* @param id
Expand Down
25 changes: 16 additions & 9 deletions src/main/java/org/stellar/sdk/MemoText.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,33 @@
import org.stellar.sdk.xdr.MemoType;

import java.nio.charset.Charset;
import java.util.Arrays;

import static com.google.common.base.Preconditions.checkNotNull;

/**
* Represents MEMO_TEXT.
*/
public class MemoText extends Memo {
private String text;
private byte[] text;
Copy link
Contributor

@leighmcculloch leighmcculloch Dec 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If text was an XdrString then we'd keep the UTF-8 conversion knowledge in one place, in the XdrString class. Right now that knowledge is spread out across XdrString and line 19 of this class, and any other class like MemoText that follows this pattern in the future.


public MemoText(String text) {
this.text = checkNotNull(text, "text cannot be null");
this(checkNotNull(text, "text cannot be null").getBytes((Charset.forName("UTF-8"))));
}

int length = text.getBytes((Charset.forName("UTF-8"))).length;
if (length > 28) {
throw new MemoTooLongException("text must be <= 28 bytes. length=" + String.valueOf(length));
public MemoText(byte[] text) {
this.text = checkNotNull(text, "text cannot be null");
if (this.text.length > 28) {
throw new MemoTooLongException("text must be <= 28 bytes. length=" + String.valueOf(this.text.length));
}
}

public String getText() {
return text;
return new String(text, Charset.forName("UTF-8"));
tamirms marked this conversation as resolved.
Show resolved Hide resolved
}

public byte[] getBytes() {
return this.text;
}

@Override
Expand All @@ -36,19 +43,19 @@ org.stellar.sdk.xdr.Memo toXdr() {

@Override
public int hashCode() {
return Objects.hashCode(this.text);
return Arrays.hashCode(this.text);
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
MemoText memoText = (MemoText) o;
return Objects.equal(this.text, memoText.text);
return Arrays.equals(this.text, memoText.text);
}

@Override
public String toString() {
return text == null ? "" : text;
return text == null ? "" : this.getText();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@
import com.google.gson.JsonParseException;

import org.stellar.sdk.Memo;
import org.stellar.sdk.xdr.TransactionEnvelope;
import org.stellar.sdk.xdr.XdrDataInputStream;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.lang.reflect.Type;

public class TransactionDeserializer implements JsonDeserializer<TransactionResponse> {
Expand All @@ -31,12 +35,19 @@ public TransactionResponse deserialize(JsonElement json, Type typeOfT, JsonDeser
// representation of a transaction. That's why we need to handle a special case
// here.
if (memoType.equals("text")) {
JsonElement memoField = json.getAsJsonObject().get("memo");
if (memoField != null) {
memo = Memo.text(memoField.getAsString());
} else {
memo = Memo.text("");
// we obtain the memo text from the xdr because the bytes may not be valid utf8
String envelopeXdr = json.getAsJsonObject().get("envelope_xdr").getAsString();
BaseEncoding base64Encoding = BaseEncoding.base64();
byte[] bytes = base64Encoding.decode(envelopeXdr);
TransactionEnvelope transactionEnvelope = null;
try {
transactionEnvelope = TransactionEnvelope.decode(new XdrDataInputStream(new ByteArrayInputStream(bytes)));
} catch (IOException e) {
// JsonDeserializer<TransactionResponse> cannot throw IOExceptions
// so we must throw it as a runtime exception
throw new RuntimeException(e);
}
memo = Memo.text(transactionEnvelope.getTx().getMemo().getText());
} else {
String memoValue = json.getAsJsonObject().get("memo").getAsString();
BaseEncoding base64Encoding = BaseEncoding.base64();
Expand Down
21 changes: 13 additions & 8 deletions src/main/java/org/stellar/sdk/xdr/Memo.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.io.IOException;

import com.google.common.base.Objects;
import java.util.Arrays;

// === xdr source ============================================================

Expand All @@ -15,7 +16,7 @@
// case MEMO_NONE:
// void;
// case MEMO_TEXT:
// string text<28>;
// opaque text<28>;
leighmcculloch marked this conversation as resolved.
Show resolved Hide resolved
// case MEMO_ID:
// uint64 id;
// case MEMO_HASH:
Expand All @@ -34,11 +35,11 @@ public MemoType getDiscriminant() {
public void setDiscriminant(MemoType value) {
this.type = value;
}
private String text;
public String getText() {
private byte[] text;
public byte[] getText() {
return this.text;
}
public void setText(String value) {
public void setText(byte[] value) {
this.text = value;
}
private Uint64 id;
Expand Down Expand Up @@ -70,7 +71,9 @@ public static void encode(XdrDataOutputStream stream, Memo encodedMemo) throws I
case MEMO_NONE:
break;
case MEMO_TEXT:
stream.writeString(encodedMemo.text);
int textsize = encodedMemo.text.length;
stream.writeInt(textsize);
stream.write(encodedMemo.getText(), 0, textsize);
break;
case MEMO_ID:
Uint64.encode(stream, encodedMemo.id);
Expand All @@ -94,7 +97,9 @@ public static Memo decode(XdrDataInputStream stream) throws IOException {
case MEMO_NONE:
break;
case MEMO_TEXT:
decodedMemo.text = stream.readString();
int textsize = stream.readInt();
decodedMemo.text = new byte[textsize];
stream.read(decodedMemo.text, 0, textsize);
break;
case MEMO_ID:
decodedMemo.id = Uint64.decode(stream);
Expand All @@ -110,7 +115,7 @@ public static Memo decode(XdrDataInputStream stream) throws IOException {
}
@Override
public int hashCode() {
return Objects.hashCode(this.text, this.id, this.hash, this.retHash, this.type);
return Objects.hashCode(Arrays.hashCode(this.text), this.id, this.hash, this.retHash, this.type);
}
@Override
public boolean equals(Object object) {
Expand All @@ -119,6 +124,6 @@ public boolean equals(Object object) {
}

Memo other = (Memo) object;
return Objects.equal(this.text, other.text) && Objects.equal(this.id, other.id) && Objects.equal(this.hash, other.hash) && Objects.equal(this.retHash, other.retHash) && Objects.equal(this.type, other.type);
return Arrays.equals(this.text, other.text) && Objects.equal(this.id, other.id) && Objects.equal(this.hash, other.hash) && Objects.equal(this.retHash, other.retHash) && Objects.equal(this.type, other.type);
}
}
18 changes: 18 additions & 0 deletions src/test/java/org/stellar/sdk/xdr/TransactionDecodeTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

import com.google.common.io.BaseEncoding;
import org.junit.Test;
import org.stellar.sdk.Memo;
import org.stellar.sdk.MemoText;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.util.Arrays;

Expand Down Expand Up @@ -59,4 +62,19 @@ public void testTransactionEnvelopeWithMemo() throws IOException {
assertTrue(Arrays.equals(new byte[]{'G', 'O', 'L', 'D'}, transactionEnvelope.getTx().getOperations()[0].getBody().getPaymentOp().getAsset().getAlphaNum4().getAssetCode().getAssetCode4()));
}

@Test
public void testRoundtrip() throws IOException {
String txBody = "AAAAAM6jLgjKjuXxWkir4M7v0NqoOfODXcFnn6AGlP+d4RxAAAAAZAAIiE4AAAABAAAAAAAAAAEAAAAcyKMl+WDSzuttWkF2DvzKAkkEqeSZ4cZihjGJEAAAAAEAAAAAAAAAAQAAAAAgECmBaDwiRPE1z2vAE36J+45toU/ZxdvpR38tc0HvmgAAAAAAAAAAAJiWgAAAAAAAAAABneEcQAAAAECeXDKebJoAbST1T2AbDBui9K0TbSM8sfbhXUAZ2ROAoCRs5cG1pRvY+ityyPWFEKPd7+3qEupavkAZ/+L7/28G";
BaseEncoding base64Encoding = BaseEncoding.base64();
byte[] bytes = base64Encoding.decode(txBody);

TransactionEnvelope transactionEnvelope = TransactionEnvelope.decode(new XdrDataInputStream(new ByteArrayInputStream(bytes)));
ByteArrayOutputStream byteOutputStream = new ByteArrayOutputStream();

transactionEnvelope.encode(new XdrDataOutputStream(byteOutputStream));
String serialized = base64Encoding.encode(byteOutputStream.toByteArray());
// fails here
assertEquals(serialized, txBody);
}

}
2 changes: 1 addition & 1 deletion xdr/Stellar-transaction.x
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ union Memo switch (MemoType type)
case MEMO_NONE:
void;
case MEMO_TEXT:
string text<28>;
opaque text<28>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where do we keep the source of truth for our .x files? Does this get updated there also so that all other SDKs also get updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they are kept here https://github.com/stellar/stellar-core/tree/master/src/xdr

I'm waiting for confirmation in stellar/go#2022 that we're going to change the type of the memo text field. Once we have confirmation we'll need to update the .x files in stellar core and in the go monorepo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Files in xdr should be exactly the same as in stellar-core repo, we shouldn't change them here nor autogenerated files in: src/test/java/org/stellar/sdk/xdr.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bartekn

I'm waiting for confirmation in stellar/go#2022 that we're going to change the type of the memo text field. Once we have confirmation we'll need to update the .x files in stellar core and in the go monorepo

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is merged after the changes are in stellar-core master then 👍.

case MEMO_ID:
uint64 id;
case MEMO_HASH:
Expand Down