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

txs encode restful api return wrong response #4141

Closed
1 of 4 tasks
bitrocks opened this issue Apr 17, 2019 · 7 comments · Fixed by #4212
Closed
1 of 4 tasks

txs encode restful api return wrong response #4141

bitrocks opened this issue Apr 17, 2019 · 7 comments · Fixed by #4212

Comments

@bitrocks
Copy link

bitrocks commented Apr 17, 2019

Summary of Bug

thetxs/encode api return different txbytes from gaiacli tx encode signed.json when apply the same json file.

Version

0.34.0-rc2

Steps to Reproduce

my signed.json:

{
    "type":"auth/StdTx",
    "value":{
        "msg":[
            {
                "type":"cosmos-sdk/MsgSend",
                "value":{
                    "from_address":"cosmos1rpsdl0wdk42jjf9sp57d3eflm8q8ektuz66at3",
                    "to_address":"cosmos1e4lhppa79t40eqdgl5her60p4zqtenyj839y8z",
                    "amount":[
                        {
                            "denom":"muon",
                            "amount":"1000"
                        }
                    ]
                }
            }
        ],
        "fee":{
            "amount":[
                {
                    "denom":"muon",
                    "amount":"5000"
                }
            ],
            "gas":"200000"
        },
        "signatures":[
            {
                "pub_key":{
                    "type":"tendermint/PubKeySecp256k1",
                    "value":"A9dZ7e2azCHXp9hoTbXxTfiH2elBIet4Crj/4t5vecX0"
                },
                "signature":"TSNafIKca3+hEA48Fbf66gPJJscXkVvqactKHZOMPnBzB0PtNeHSFXWPl1lr1OR5p01V6eaz8zuJH7ukvTRGog=="
            }
        ],
        "memo":"userid001"
    }
}

CLI: gaiacli tx encode signed.json

zwHwYl3uCj6oo2GaChQYYN+9zbVVKSSwDTzY5T/ZwHzZfBIUzX9wh74q6vyBqP0vkenhqIC8zJIaDAoEbXVvbhIEMTAwMBISCgwKBG11b24SBDUwMDAQwJoMGmoKJuta6YchA9dZ7e2azCHXp9hoTbXxTfiH2elBIet4Crj/4t5vecX0EkBNI1p8gpxrf6EQDjwVt/rqA8kmxxeRW+ppy0odk4w+cHMHQ+014dIVdY+XWWvU5HmnTVXp5rPzO4kfu6S9NEaiIgl1c2VyaWQwMDE=

API: curl -d @signed.json -X POST http://localhost:1317/txs/encode

{"tx":"BPBiXe4="}

The former is right. I guess there are some cdc.UnmarshalJson problem. But I failed to get it right.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@alexanderbez
Copy link
Contributor

This is most likely because the CLI reads the tx directly from file and decodes it. The REST client attempts to first decode into the EncodeReq type. I think we can ditch the EncodeReq type and just attempt to decode into a sdk.StdTx.

@yangyanqing
Copy link
Contributor

yangyanqing commented Apr 18, 2019

I suddenly realized there are several things to discuss:

  1. Even if we define type EncodeReq auth.StdTx, response expected will still NOT appear through curl -d @signed.json -X POST http://localhost:1317/txs/encode. Because what rest /txs/encode wants is the subobject value of CLI output. The CLI output should NOT be applied to REST Input
  2. If we define type EncodeReq auth.StdTx, the definition of /txs/encode/ in swagger-ui.yaml can not use $ref: "#/definitions/StdTx". We have to define every properties one by one directly. I don't think this is a good solution.
  3. If we define type EncodeReq auth.StdTx but don't modify swagger-ui.yaml, documents will be outdate or inconformity and we can NOT call it in swagger-ui just like GET /txs.

So, the current solution may be the best one, this is not a bug.

@alexanderbez @alessio

@bitrocks
Copy link
Author

Thanks for your quick response. I make a small change in the codebase and now the /txs/encode api works nicely for me.

@yangyanqing
Copy link
Contributor

Thanks for your quick response. I make a small change in the codebase and now the /txs/encode api works nicely for me.

Can your share your solution ?

@alessio
Copy link
Contributor

alessio commented Apr 18, 2019

@yangyanqing

The CLI output should NOT be applied to REST Input

I don't disagree to that. I'm fine with any solution that meets the following acceptance criteria:

Given A StdTx X
When I call the function Y = decode(encode(X))
Then Y equals X

As long as REST and command-line respective encoding and decoding functionalities' simmetry is preserved, I'm fine with any working solution.

@yangyanqing
Copy link
Contributor

I agree with your Given..When..Then.... But there are too many constraints in reality. For example, a one-way street, we can only go from one side to the other, and can not go in the opposite direction.
@alessio

@alessio
Copy link
Contributor

alessio commented Apr 19, 2019

I am afraid I don't follow you here @yangyanqing. I'm just saying that what I generally expect from a decoding/encoding functionality is to provide me with the ability to decode what had been encoded before.
To clarify: I do not expect REST and CLI to be conform to the same output format.

alessio pushed a commit that referenced this issue May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants