irisnet / irisnet-crypto

JavaScript library for IRIS Hub and Cosmos Hub
Apache License 2.0
21 stars 28 forks source link

Amino-js support #48

Open ethanfrey opened 6 years ago

ethanfrey commented 6 years ago

Hi, I have been asking around for amino codecs in javascript, and irisnet comes up a lot. Today, someone sent me a link to your implementation: https://github.com/irisnet/irisnet-crypto/blob/master/chains/iris/amino.js and I'd like to figure out the status and if it is ready for outside use.

As a background, I used to work at cosmos on the sdk itself and understand the golang side quite well. I left some months ago, and am working on a pure javascript (actually typescript) client library that can speak to multiple modern blockchains. I would like to add support for cosmos-sdk, but the only blocker for me is the amino codec support in javascript, which seems to be a huge effort, and which makes the cost/benefit of integration not worth it currently.

I took a look at the code in https://github.com/irisnet/irisnet-crypto/tree/master/chains/iris and it seems like pretty solid implementations of GetSignBytes(), but I only see two usages of MarshalBinary(), which are for the pubkey. This is a relatively simple example, an interface (needing the proper amino prefix) along with a byte array (needing a length prefix). This is a great step forward to implement the interface switching, but I don't see how any larger structs are handled.

In particular, how would in MarshalBinary() on a Coin, a Msg or a Tx? Which I need to post it to the blockchain as a valid tx and not require any golang daemon between the js client and the tendermint node?

If this is not yet done, that is a decent answer. I would also be willing to collaborate a bit if there is some working code.

haifengxi commented 6 years ago

Hi Ethan! Nice to hear from you after a while.

Tx related encoding logic can be found in chains/iris/tx/tx_serializer.js and tx.js, which in turn uses protobufjs for most of the actual encoding.

ethanfrey commented 6 years ago

Awesome, thanks for the tip. I didn't see it, but then switched to develop and there it was: https://github.com/irisnet/irisnet-crypto/blob/develop/chains/iris/tx/tx_serializer.js

Glad to see you can make use of protobufjs for most of the encoding. I use that myself for other projects and am quite happy with it. It seems that you "manually" encode the amino sections (public key, signature, message), then wrap that all up into a protobuf object that can be encoded directly with protobuf.js. I like the approach for minimal code needed to maintain.

I will study the code a bit more, now that I find the working implementation. But one question, where is the .proto file that you used to generate tx.js, I didn't find it in the repo. Do you manually create it from the go structs?

dreamer-zq commented 6 years ago

@ethanfrey Please switch to the develop branch, in the [irisnet-crypto/chains/iris/proto/tx.proto] path, you will find it, use the command [pbjs -t static-module -w commonjs -o tx.js tx.proto 】 to generate the corresponding tx.js file, but the generated code can not be used directly, because amino changes on the basis of protobuf3, the difference is expressed in: the processing of null values (this is what I found now, the details are not complete clear).

ethanfrey commented 6 years ago

Thank you. I have been using protobuf.js against a golang backend that uses pure protobuf, not cosmos-sdk, but weave. The generated code works great, but since the server insists on the "canonical" protobuf representation to avoid attack vectors, the server tries to parse and reserialize the tx and ensure the same binary representation, which cosmos-sdk may do as well.

The one issue there is "zero value" fields. That is in go, there is no distinction between a number field that is 0 or undefined, as it may not have a pointer value. And thus the 0 value is just not encoded (skip the field if it is zero value/default). I had the same for strings.

I have yet to write a generic solution, but when building the protobuf objects, I try to replace all zero values with undefined. Luckily the golang zero values: 0, false, "" are also zero values in javascript. And empty arrays are not encoded by either js or golang codec.

I write code like:

export function encodeAmount(amount: Amount): codecImpl.x.Coin {
  return codecImpl.x.Coin.create({
    // use null instead of 0 to not encode zero fields
    // for compatibility with golang encoder
    whole: amount.whole || null,
    fractional: amount.fractional || null,
    ticker: amount.tokenTicker,  // enforced to be at least 3 characters long, never ""
  });
}

Maybe this helps. I was thinking of writing a wrapper function that "go-ified" a protobuf struct, recursing through the struct and replacing all 0 values with undefined, but haven't had the time to make it generic yet.

dreamer-zq commented 6 years ago

I have always had an idea to rewrite the source code of protobuf.js (1: handle special values, 2: prefixes are registered by the caller via a function such as RegisterConcrete). This way you can make no changes to the js code generated by proto, but only if you have a clear understanding of the difference between go-amino and protobuf.

hleb-albau commented 5 years ago

Also, you can get addition community hype, if you will release standalone amino.js lib(under your name). A lot of people looking for it, but tendermint do nothing :(