Go: JSON: Memo: An empty "memo" is not being generated for a transaction even if "memo_type" is "text"

[michaeljmonte]

I'm going to start with an example JSON returned from Horizon using the following transaction:

1113f23c225495534b2fd589a037798155ea73ee68a418e74364c1a3be4a20d8

JSON:

{
  "_links": {
    "self": {
      "href": "https://horizon.stellar.org/transactions/1113f23c225495534b2fd589a037798155ea73ee68a418e74364c1a3be4a20d8"
    },
    "account": {
      "href": "https://horizon.stellar.org/accounts/GALPCCZN4YXA3YMJHKL6CVIECKPLJJCTVMSNYWBTKJW4K5HQLYLDMZTB"
    },
    "ledger": {
      "href": "https://horizon.stellar.org/ledgers/7871"
    },
    "operations": {
      "href": "https://horizon.stellar.org/transactions/1113f23c225495534b2fd589a037798155ea73ee68a418e74364c1a3be4a20d8/operations{?cursor,limit,order}",
      "templated": true
    },
    "effects": {
      "href": "https://horizon.stellar.org/transactions/1113f23c225495534b2fd589a037798155ea73ee68a418e74364c1a3be4a20d8/effects{?cursor,limit,order}",
      "templated": true
    },
    "precedes": {
      "href": "https://horizon.stellar.org/transactions?order=asc\u0026cursor=33805687590912"
    },
    "succeeds": {
      "href": "https://horizon.stellar.org/transactions?order=desc\u0026cursor=33805687590912"
    }
  },
  "id": "1113f23c225495534b2fd589a037798155ea73ee68a418e74364c1a3be4a20d8",
  "paging_token": "33805687590912",
  "hash": "1113f23c225495534b2fd589a037798155ea73ee68a418e74364c1a3be4a20d8",
  "ledger": 7871,
  "created_at": "2015-10-01T04:17:31Z",
  "source_account": "GALPCCZN4YXA3YMJHKL6CVIECKPLJJCTVMSNYWBTKJW4K5HQLYLDMZTB",
  "source_account_sequence": "12884901893",
  "fee_paid": 100,
  "operation_count": 1,
  "envelope_xdr": "AAAAABbxCy3mLg3hiTqX4VUEEp60pFOrJNxYM1JtxXTwXhY2AAAAZAAAAAMAAAAFAAAAAAAAAAEAAAAAAAAAAQAAAAAAAAAJAAAAAAAAAAHwXhY2AAAAQGCy99saUm6alXIRyp0NNh2OFSCybp1JGPDN2pb/+Fw07/X7y4lPEp/B6WIV130a+2eY+5T3ujbiKa6TIcUaNwQ=",
  "result_xdr": "AAAAAAAAAGQAAAAAAAAAAQAAAAAAAAAJAAAAAAAAAAEAAAAAH6Ue1GOPj6Hb/ROPyIFCJpQPMujihEIvJSfK0UfMDIgAAK11sXTKRwAAAAA=",
  "result_meta_xdr": "AAAAAAAAAAEAAAACAAAAAwAAHrcAAAAAAAAAAB+lHtRjj4+h2/0Tj8iBQiaUDzLo4oRCLyUnytFHzAyIAACtdb1ePEoAAB6hAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAAAQAAHr8AAAAAAAAAAB+lHtRjj4+h2/0Tj8iBQiaUDzLo4oRCLyUnytFHzAyIAAFa627TBpEAAB6hAAAAAAAAAAAAAAAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAA",
  "fee_meta_xdr": "AAAAAgAAAAMAAB63AAAAAAAAAAAW8Qst5i4N4Yk6l+FVBBKetKRTqyTcWDNSbcV08F4WNg3gtrN3tPO0AAAAAwAAAAQAAAAAAAAAAQAAAAAfpR7UY4+Podv9E4/IgUImlA8y6OKEQi8lJ8rRR8wMiAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAAAAAAEAAB6/AAAAAAAAAAAW8Qst5i4N4Yk6l+FVBBKetKRTqyTcWDNSbcV08F4WNg3gtrN3tPNQAAAAAwAAAAUAAAAAAAAAAQAAAAAfpR7UY4+Podv9E4/IgUImlA8y6OKEQi8lJ8rRR8wMiAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAAA==",
  "memo_type": "text",
  "signatures": [
    "YLL32xpSbpqVchHKnQ02HY4VILJunUkY8M3alv/4XDTv9fvLiU8Sn8HpYhXXfRr7Z5j7lPe6NuIprpMhxRo3BA=="
  ]
}

Notice there is the "memo_type" element with the value of "text". This suggests there should be a "memo" element somewhere(because client SDKs don't allow null memos of type text). If we decode the envelope_xdr we can see there is indeed a MemoText in there, with a value of empty string.

This will cause issues when client SDKs try to deserialize this transaction from JSON. I have tested this in the official java-stellar-sdk and it does indeed throw a null reference exception.

[bartekn]

I created this test:

    @Test
    public void testTransactionEnvelopeWithEmptyMemoText() throws IOException {
        String transactionEnvelopeToDecode = "AAAAABbxCy3mLg3hiTqX4VUEEp60pFOrJNxYM1JtxXTwXhY2AAAAZAAAAAMAAAAFAAAAAAAAAAEAAAAAAAAAAQAAAAAAAAAJAAAAAAAAAAHwXhY2AAAAQGCy99saUm6alXIRyp0NNh2OFSCybp1JGPDN2pb/+Fw07/X7y4lPEp/B6WIV130a+2eY+5T3ujbiKa6TIcUaNwQ=";
        Base64 base64Codec = new Base64();
        byte[] bytes = base64Codec.decode(transactionEnvelopeToDecode);

        TransactionEnvelope transactionEnvelope = TransactionEnvelope.decode(new XdrDataInputStream(new ByteArrayInputStream(bytes)));
        assertEquals(MemoType.MEMO_TEXT, transactionEnvelope.getTx().getMemo().getDiscriminant());
        assertEquals("", transactionEnvelope.getTx().getMemo().getText());
    }

and it's working fine. Could you provide a code that throws null reference exception?

[bartekn]

OK, I understand what's the issue. This is happening because of the way encoding/json works on structs in Go, if transaction has an empty memo_text value, the memo field won't be present in a JSON representation of a transaction. I don't think we want to change this behaviour in Horizon. @nullstyle @tomerweller @nikhilsaraf any opinion here?

This is fixed in https://github.com/stellar/java-stellar-sdk/pull/54.

[nullstyle]

IMO, the memo field should only be missing when the memo type is "none". This seems like a bug in horizon to me.

[elucidsoft]

I agree, if the memo_type is "none", then and only then should the memo field be missing. Seems like a bug in Horizon.

[nikhilsaraf]

+1 on the memo_type="none" -> missing memo field

to @bartekn's comment, should we disallow an empty value for the memo_text field?

[bartekn]

to @bartekn's comment, should we disallow an empty value for the memo_text field?

Core accepts such transactions so we probably shouldn't do it. I was thinking more like: there's no easy way to fix this in encoding/json with structs and omitempty fields in Go but then I realized we can probably create an intermediate struct in such cases or a map so it shouldn't be that hard to fix.

[nullstyle]

@nikhilsaraf we can't disallow empty text memos. It's valid in the core protocol and I imagine it will remain that way. No memo is a distinct value from a text memo that's an empty string.

Basically, we need to extract https://github.com/stellar/go/blob/master/services/horizon/internal/resource/main.go#L238-L239 into another struct that can more tightly control the encoding behavior by defining an implementation of https://golang.org/pkg/encoding/json/#Marshaler

edit: yeah, what @bartekn said 😛

[nikhilsaraf]

Cool, thanks for clarifying. Agree with the separated struct with marshaler approach.

[kr]

Hey, @lechengfan and I would like to work on this!