Can I create slice of interface?

Question

Can I create slice of interface?

Closed this issue 6 years ago · 15 comments

I'm trying to make tx in JS.

This is cosmos-sdk's standard tx...

type StdTx struct {  
	Msgs       []sdk.Msg      `json:"msg"`  // <- sdk.Msg is interface
	Fee        StdFee         `json:"fee"`
	Signatures []StdSignature `json:"signatures"`
	Memo       string         `json:"memo"`
}
cdc.RegisterConcrete(StdTx{}, "auth/StdTx", nil)

I think I can make this structure like this...

let StdTx = TypeFactory.create('auth/StdTx', [
    {
        name: 'msg',
        type: Types.Array,
    },
    {
        name: 'fee',
        type: Types.Struct,
    },
    /*{
        name: 'signatures',
        type: Types.Array,
    },*/
    {
        name: 'memo',
        type: Types.String,
    },
])
let WriteMsg = TypeFactory.create('test/MsgWrite', [
    {
        name: 'payload',
        type: Types.String,
    }
])

and make StdTX like...

codec.registerConcrete(new StdTx(), 'auth/StdTx', {})
codec.registerConcrete(new WriteMsg(), 'test/MsgWrite', {})
let writeMsg = new WriteMsg('payload')
let stdTx = new StdTx([writeMsg], fee, 'test')

and output is

let binary = codec.marshalBinary(stdTx)
console.log(toHex(binary))
// -> 1ff0625deea9a77061796c6f61641281263230303030301a474657374

auth/StdTx's prefix is f0625dee.
test/MsgWrite's prefix is e7a5c8ff.
It seemed that StdTx's prefix is well managed...
But It seemed that MsgWrite's prefix is not included...

Currently, It seemed that Js-Amino manages Array type as only struct array.
So I can't make slice of interface of Msg...
Is there a way to solve this problem?

Answer 1 · 2018-11-03T03:12:12.000Z

Thanks for raising issue, I will check it

Answer 2 · 2018-11-05T04:07:17.000Z

auth/StdTx's prefix is f0625dee.
test/MsgWrite's prefix is e7a5c8ff.
It seemed that StdTx's prefix is well managed...
But It seemed that MsgWrite's prefix is not included...

Since the amino with version greater than 0.99, it seems that the Go code to encode/decode does not include the prefix of sub struct into binary. If this is the problem in encoding, the 'decoding part' in Golang will give you a panic. Please feel free to use it if it does not raise any panic in decoding

Answer 3 · 2018-11-05T04:22:59.000Z

I'm not sure with the 'slice of interface' in your case. But I have an example like this.
In golang, If I defined :

type PubKeySecp256k1 [65]byte
type PubKey interface {
}
var _ PubKey = PubKeySecp256k1{}
cdc.RegisterInterface((*PubKey)(nil), nil)
cdc.RegisterConcrete(PubKeySecp256k1{}, "shareledger/PubSecp256k1", nil)

in JS you will do the similar things like this:

let PubSecp256k1 = TypeFactory.create('PubSecp256k1', [{
    name: "bytes",
    type: Types.ByteSlice
}],Types.ByteSlice) //-> **remeber to add "Types.ByteSlice" here (default is "Types.Struct")**

To init the byte slide directly :

codec.registerConcrete(new PubSecp256k1(), "shareledger/PubSecp256k1", {})
let pubKey = new PubSecp256k1([1,2,3])

pubKey is a 'byteslice' (not Struct)
You can refer the example in examples/interfaceTest.go and example/interfaceTest.js

Hope it help you

Answer 4 · 2018-11-06T03:20:39.000Z

@TanNgocDo Thank you for your reply.
In my case,

type StdTx struct {  
	Msgs       []sdk.Msg      `json:"msg"`  // <- sdk.Msg is interface
	Fee        StdFee         `json:"fee"`
	Signatures []StdSignature `json:"signatures"`
	Memo       string         `json:"memo"`
}
cdc.RegisterConcrete(StdTx{}, "auth/StdTx", nil)

this should be encoded like this...

a501f0625dee0a20e7a5c8ff0a14749c54393ae8c4a6a3b06f60e1b361b48ad1c25a1a047465737412090a031201301080b5181a720a26eb5ae9872102745e346835ef675e880413ed29303e9e41cff37079525868ae986ee613b3f542124630440220673134b5da9b221221206e67473351929e3463d83280b6588f87c6cecf767220022035ff3de1b1cb3b5c4484b9a5a7645815be47eae54ddadd932a599ba6d150b0252012

{"type":"auth/StdTx","value":{"msg":[{"type":"test/MsgWrite","value":{"writer":"cosmosaccaddr1wjw9gwf6arz2dgasdaswrvmpkj9drsj6w2cn40","parent":"","payload":"dGVzdA=="}}],"fee":{"amount":[{"denom":"","amount":"0"}],"gas":"200000"},"signatures":[{"pub_key":{"type":"tendermint/PubKeySecp256k1","value":"AnReNGg172deiAQT7SkwPp5Bz/NweVJYaK6YbuYTs/VC"},"signature":"MEQCIGcxNLXamyISISBuZ0czUZKeNGPYMoC2WI+Hxs7PdnIgAiA1/z3hscs7XESEuaWnZFgVvkfq5U3a3ZMqWZum0VCwJQ==","account_number":"0","sequence":"9"}],"memo":""}}

e7a5c8ff is prefix of test/MsgWrite.
It seemed that this prefix is included because sdk.Msg is interface.

In cosmos-sdk, transactions are passed to each handler according to type of Msg.
Therefore, it is very important that Msg is converted to the appropriate concrete type.

Since cosmos-sdk is a main project of the Tendermint, I think it's important to support this feature in js-amino.

Answer 5 · 2018-11-06T03:58:04.000Z

Hi @Thunnini , which version of Go-amino are you using ? If you use the master branch in js-amino, can you unMarsaBinary successfully with Go-amino ?

Answer 6 · 2018-11-06T04:05:45.000Z

Could you please send me the code that you encode in JS and decode in Go ?

Answer 7 · 2018-11-06T06:43:04.000Z

go-amino version is v0.12.0-rc0.
https://gist.github.com/Thunnini/b6a20639bde7d2674a0efe75d2ca5b92
In golang, result is
87010a41c06abad60a145051d57ff4af941b4a4b22e6bb783d7e2e105cc112250a145051d57ff4af941b4a4b22e6bb783d7e2e105cc1120d0a047465737412053130303030120f0a090a04746573741201301080b5181a2b0a26eb5ae9872103dac7b7f9df94578a3648f1c07c60de0a9a53c79255388e358b7e2368f0c94c16120100220474657374
{"msg":[{"type":"cosmos-sdk/Issue","value":{"banker":"cosmosaccaddr12pga2ll5472pkjjtytntk7pa0chpqhxprex49p","outputs":[{"address":"cosmosaccaddr12pga2ll5472pkjjtytntk7pa0chpqhxprex49p","coins":[{"denom":"test","amount":"10000"}]}]}}],"fee":{"amount":[{"denom":"test","amount":"0"}],"gas":"200000"},"signatures":[{"pub_key":{"type":"tendermint/PubKeySecp256k1","value":"A9rHt/nflFeKNkjxwHxg3gqaU8eSVTiONYt+I2jwyUwW"},"signature":"AA==","account_number":"0","sequence":"0"}],"memo":"test"}

"cosmos-sdk/Issue"'s prefix is included.

https://gist.github.com/Thunnini/6d2a9933d3e550c68c5bb73597dcc6c8
In JS, result is
6ff0625dee0a190a010012140a0100120f0a0474657374101027000000000000120910400d0300000000001a3d0a26eb5ae9872102745e346835ef675e880413ed29303e9e41cff37079525868ae986ee613b3f542120100180000000000000000200000000000000000220474657374
{"type":"142,254,71,240,98,93,238","value":{"idx":3,"msg":[{"idx":1,"banker":[0],"outputs":[{"idx":1,"address":[0],"coins":[{"idx":1,"denom":"test","amount":10000}]}]}],"fee":{"idx":1,"amount":{"idx":1,"denom":"test","amount":0},"gas":200000},"signatures":[{"idx":3,"pub_key":{"idx":0,"s":[2,116,94,52,104,53,239,103,94,136,4,19,237,41,48,62,158,65,207,243,112,121,82,88,104,174,152,110,230,19,179,245,66]},"signature":[0],"account_number":0,"sequence":0}],"memo":"test"}}

"cosmos-sdk/Issue"'s prefix is not included.

I think issue msg's prefix sholud be included because sdk.Msg is interface.
But, currently, it seemed that js-amino misses feature that manages slice of interface...

I couldn't succeed to make transaction that is able to be decoded in go-amino yet...
I think we need more attempts.
One of the problem is that we can not create the slice of interface.

Answer 8 · 2018-11-06T06:54:09.000Z

If I try to decode in go-amino with result of JS, there are no explicit errors. But it was not decoded at all...
I'll go into some more research on this part...

Answer 9 · 2018-11-06T07:24:49.000Z

thanks @Thunnini for reporting. I'm checking this issues.

Answer 10 · 2018-11-12T05:07:15.000Z

Hi @Thunnini , I updated the code. Now, if you want to encode array of interface, you should specify as Types.ArrayInterface, the same case for Struct with Types.ArrayStruct.
I add examples/slice_bug.go and slice_bug.js as the examples derived from your examples. Please remember that: I use the lastest Amino-Go implementation for reference and testing decoding -> it's a little bit different from your code. I don't use directly cosmos type, instead I re-define them for easy testing and I'm not sure that it will be work well with Cosmos-sdk(due to different go-amino version they are using). Also, beware that: I use 'Int8' not 'Int64', now Go-amino use opts to check the condition before decoding int64(you can encouter error if using encoding with int64). If possible, can you create more test-case/example. I absolutely needs to provide documentation and refactor code before releasing on npm. Go-Amino also changes a lot from some months ago, I'm trying to keep up to date with them

Answer 11 · 2018-11-16T07:52:14.000Z

@Thunnini I see you have some update on your fork. Could you please make an PR ?

Answer 12 · 2018-11-18T13:00:11.000Z

@TanNgocDo Could you tell me which version you use? I can't understand why I may encounter error if encoding/decoding int64.

package main

import (
	"fmt"

	amino "github.com/tendermint/go-amino"
)

type IntStruct struct {
	Int int64 `json:"int"`
}

func main() {
	var cdc = amino.NewCodec()

	cdc.RegisterConcrete(IntStruct{}, "test/IntStruct", nil)

	intStruct := IntStruct{
		Int: 10000000000,
	}

	bz, err := cdc.MarshalBinaryLengthPrefixed(intStruct)
	if err != nil {
		panic(err)
	}
	fmt.Println(bz)

	intStruct2 := IntStruct{}
	err = cdc.UnmarshalBinaryLengthPrefixed(bz, &intStruct2)
	if err != nil {
		panic(err)
	}
	fmt.Println(intStruct2)
}

In go-amino v0.14.1, no error happened with such simple example.
And yes, I will try to make more test/examples.

Answer 13 · 2018-11-18T13:08:47.000Z

I used the lastest 0.14.1. The problem occurred due to fixedByte decode with int64. I tested with 0.9.9. It works, but with some recent version, it fails. Could you please bypass with comment out the checking Binfixed64 : https://github.com/tendermint/go-amino/blob/v0.14.0/binary-decode.go#L196

Answer 14 · 2018-11-18T13:20:31.000Z

@TanNgocDo Could you please check this issue #9? In go-amino v0.9.7 https://github.com/tendermint/go-amino/blob/v0.9.7/binary-decode.go#L214, Int64 is encoded as bin-fixed(?) defaultly. But in go-amino v0.14.0, https://github.com/tendermint/go-amino/blob/v0.14.0/binary-decode.go#L122, Int64 is encoded as varint defaultly. Is this a problem caused by this? I think we can solve this problem by encoding/decoding (u)int32/64 as (u)varint defaultly in js-amino.

Answer 15 · 2018-11-18T13:46:51.000Z

@Thunnini , yes, you're right. You can by pass by add these lines in file : BinaryEncoder.js for encoding Int64 :

case Types.Int64:
            {   data = Encoder.encodeUVarint(tmpInstance)
                //data = Encoder.encodeInt64(tmpInstance) //temporary comment out this line
                break;
            }

I will add the checking condition later when I finish adding the 'opts' field