Fix for deserialization of BroadcastTx which was missing the type byte.

[ratranqu]

This fixes an issue when deserialising a BroadcastTx json-rpc request over websockets.

Until 0.11.4, such json would be valid:

{
  "id": "57EC1D39-7B3D-4F96-B286-8FC128177AFC4",
  "jsonrpc": "2.0",
  "method": "erisdb.broadcastTx",
  "params": [
    2,
    {
      "address": "5A9083BB0EFFE4C8EB2ADD29174994F73E77D418",
      "data": "2F2397A00000000000000000000000000000000000000000000000000000000000003132",
      "fee": 1,
      "gas_limit": 1000000,
      "input": {
        "address": "BE18FDCBF12BF99F4D75325E17FF2E78F1A35FE8",
        "amount": 1,
        "pub_key": [
          1,
          "8D1611925948DC2EDDF739FB65CE517757D286155A039B28441C3349BE9A8C38"
        ],
        "sequence": 2,
        "signature": [
          1,
          "B090D622F143ECEDA9B9E7B15485CE7504453C05434951CF867B013D80ED1BD2A0CA32846FC175D234CDFB9D5C3D792759E8FE79FD4DB3006B24950EE3C37D00"
        ]
      }
    }
  ]
}

In 0.12.0, the "params" value is wrongly expected to be a dictionary instead of the correct [ type, {dict}].

[silasdavis]

Thanks for this. Indeed we seemed to be assuming a CallTx, whereas with this change it can be any transactions type.

I'm a little wary though because presumably it will break what our javascript libraries must be doing for 0.12. @NodeGuy - what do you provide for "params" when calling rpc/v0 from your libaries? @benjaminbollen can you remember why this was like this?

[ratranqu]

Silas, It's a bit more than just a change from always assuming a CallTx to handling any types of Tx. The json representation of the asummed CallTx is also not consistent with how it's expected in the wire protocol (see txs/tx.go file, where the concrete type is defined for the wire protocol). Ideally, this regression (introduced in 0.12.x) can be fixed for 0.16.0. The way transaction types are handled (array with type byte followed by payload) is exactly the same as how pubkeys and signatures are handled so it should not be too hard to bring in line. Let me know if/how I can help. Alex

…

On 25 Jan 2017, at 17:11, Silas Davis ***@***.***> wrote: Thanks for this. Indeed we seemed to be assuming a CallTx, whereas with this change it can be any transactions type. I'm a little wary of this change though because presumably it will break what our javascript libraries must be doing for 0.12. @NodeGuy - what do you provide for "params" when calling rpc/v0 from your libaries? @benjaminbollen can you remember why this was like this? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[NodeGuy]

Here's what's provided for params:

    this.server.transact({
      priv_key: privKey,
      address,
      data,
      gas_limit: gasLimit,
      fee
    }, callback)

https://github.com/eris-ltd/eris-db.js/blob/ab3c1b4cd34a4ab4526cfb9f6cab5abbbb1dce3e/lib/unsafe.js#L82-L88

[ratranqu]

David, awesome that you know exactly where the changes are. Could they be reverted for 0.16.0? Thx, Alex

…

On 25 Jan 2017, at 19:51, David Braun ***@***.***> wrote: Here's what's provided for params: this.server.transact({ priv_key: privKey, address, data, gas_limit: gasLimit, fee }, callback) https://github.com/eris-ltd/eris-db.js/blob/ab3c1b4cd34a4ab4526cfb9f6cab5abbbb1dce3e/lib/unsafe.js#L82-L88 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[benjaminbollen]

Hi Alex,

This is my fault, I got this mixed up in the 0.12 release. This RPC is marked for deprecation, but it is more complete to consider reverting it, before doing so. I will weigh in the opinion of the JS libs, but it seems a valid request.

[ratranqu]

Awesome. you make my evening :)

…

On 25 Jan 2017, at 22:56, Benjamin Bollen ***@***.***> wrote: Hi Alex, This is my fault, I got this mixed up in the 0.12 release. This RPC is marked for deprecation, but it is more complete to consider reverting it, before doing so. I will weigh in the opinion of the JS libs, but it seems a valid request. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#453 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AQPPLX8M8KWiWORY9N3SGPiwniXvVm8Nks5rV8UDgaJpZM4Ls2F9>.

[benjaminbollen]

Strictly speaking we can merge this without breaking the JS libs as they call the transact method - not the broadcast method - but it is better to reduce complexity and have it aligned for less headaches.

[NodeGuy]

Actually the JavaScript libraries never sent the type byte so there's nothing for me to revert.

[silasdavis]

Could you check if this works without the method, I think it should.

[silasdavis]

I'm not sure we need this. I think it will work with DecodeBytes, did you try that?

[silasdavis]

wire.ReadJSON should delegate to ReadJSONPtr if it gets a pointer.

[benjaminbollen]

@silasdavis I wanted to merge this into a feature branch and then we can work on it? cc @ratranqu

[silasdavis]

good idea

[ratranqu]

@silasdavis, yes, I think you are right, we can just use the DecodeBytes function.