Cosmos-sdk: Add tests to REST client for simulate and generate_only

[okwme]

This issue is a reproduction of the issue https://github.com/cosmos/gaia/issues/335 filed by @feri42

Summary of Bug

Calling the Gaia-Lite for Cosmos API calls:

POST /staking/delegators/{delegatorAddr}/delegations
POST /staking/delegators/{delegatorAddr}/unbonding_delegations
POST /staking/delegators/{delegatorAddr}/redelegations

with simulate=true or with the query paramter generate_only=true returns a Internal Server Error with the following response body:

{
  "jsonrpc": "2.0",
  "id": "",
  "error": {
    "code": -32603,
    "message": "Internal error",
    "data": "runtime error: invalid memory address or nil pointer dereference"
  }
}

Version

Gaiacli and gaiad version is 2.0.7

Steps to Reproduce

Unpack the attached body.json file and run:

curl -X POST -H "Content-Type: application/json" https://<gaiad>/staking/delegators/cosmos1j4heycy25ersvld236f3ckpn9avjjt4p3tmg23/delegations -d @body.json

body.json.gz

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[jgimeno]

I can check this one.

[jgimeno]

The problem is that amount arrives nil:

https://github.com/cosmos/cosmos-sdk/blob/53bf2271d5bac054a8f74723732f21055c1b72d4/x/staking/types/msg.go#L187

[jgimeno]

The problem is the request, it has:

      "delegation": {
        "amount": "10000",
        "denom": "uatom"
      }

but should be:

"amount": {
        "amount": "10000",
        "denom": "uatom"
      }

[jgimeno]

Created a test to show it in https://github.com/cosmos/cosmos-sdk/pull/5906/files

[okwme]

Thank you for looking into this @jgimeno ! I've notified the original poster of the issue, probably good to have tests for this regardless though?

[jgimeno]

@okwme I think part of the problem comes with the Type Int

type Int struct {
    i *big.Int
}

In this case it happens that the request does not find the field but sets the default

        Amount              sdk.Coin       `json:"amount" yaml:"amount"`

Coin has

type Coin struct {
    Denom  string `protobuf:"bytes,1,opt,name=denom,proto3" json:"denom,omitempty"`
    Amount Int    `protobuf:"bytes,2,opt,name=amount,proto3,customtype=Int" json:"amount"`
}

Which the default Int has a nil pointer inside as default value. So I think having the option to set an Int{} that the default value is nil and all the methods fail maybe needs a rethink. Thoughts? :D @okwme @alexanderbez @alessio

[alexanderbez]

What's the issue exactly? The Int type should have nothing to do with this AFAIK. Seems like a poorly
constructed DelegateRequest? Or is there an actual bug?

[jgimeno]

Well was just a suggestion, the problem is the msgValidate:

func (msg MsgDelegate) ValidateBasic() error {
    if msg.DelegatorAddress.Empty() {
        return ErrEmptyDelegatorAddr
    }
    if msg.ValidatorAddress.Empty() {
        return ErrEmptyValidatorAddr
    }
    if !msg.Amount.Amount.IsPositive() {
        return ErrBadDelegationAmount
    }
    return nil
}

on a bad constructed request it checks if it is positive, but msg.Amount.Amount is nil and it panics.
Amount.Amount exists, but the inner i is nil. So I dont know if this is a good design on Int. I cannot check just if Amount.Amount.i is nil. Do you know what I mean? The problem is the panic.

[alexanderbez]

We already have checks like this elsewhere. The check should be:

if msg.Amount.IsNil() || !msg.Amount.Amount.IsPositive() {
    return ErrBadDelegationAmount
}

@alessio did this with the x/distribution params validation helpers. I recommend we go through all the param validation functions and REST handlers to enforce this 👍

[jgimeno]

ah ok! Thanks @alexanderbez I was checking the Int for Nil and did not see that Amount has it!

[feri42]

@jgimeno Thank you very much for looking into this. I can confirm that is the case with

POST /staking/delegators/{delegatorAddr}/delegations
POST /staking/delegators/{delegatorAddr}/unbonding_delegations

I have been relying on the documentation at https://cosmos.network/rpc/#/ , which is incorrect. May I also point out that I did not find specs on how to construct transactions, so I resorted to guessing (incorrectly).

[feri42]

@jgimeno Can you please let me know how to construct the correct request for the remaining call:

POST /staking/delegators/{delegatorAddr}/redelegations

Please ignore this. I have figured it out. It is the same as for the other two.

For me, this issue is solved.

[okwme]

Was this issue with swagger a problem because the swagger file uses a version of the SDK that is different from the one run on the current cosmoshub-3?

(if so may be avoided by something like: https://github.com/cosmos/cosmos-sdk/issues/3988)