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

Add staking functions to cosmos-sdk #82

Merged
merged 16 commits into from
Jun 22, 2021

Conversation

kimonp
Copy link
Contributor

@kimonp kimonp commented May 14, 2021

Add staking functions to the cosmos-sdk by extending to existing protobuf methods:

  • MsgDelegate
  • MsgUnDelegate
  • MsgBeginRedelegate

@kimonp
Copy link
Contributor Author

kimonp commented Jun 2, 2021

@tony-iqlusion , @jkilpatr : this pull request adds three staking functions to the cosmos-sdk by extending to existing protobuf methods. Hopefully others will find this useful as well, please take a look.

pub validator_address: AccountId,

/// Amount to send
pub amount: Option<Coin>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious... when would this ever be None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proto::cosmos::staking::v1beta1::MsgDelegate's amount can be None, so I'm mirroring that here.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Rust code generated from the protos has no way to express non-optional fields for messages, so that's not a great thing to go off of.

Looking at the raw Protobuf definition, it's:

https://github.com/cosmos/cosmos-sdk/blob/9fd866e3820b3510010ae172b682d71594cd8c14/proto/cosmos/staking/v1beta1/tx.proto#L89

message MsgDelegate {
  option (gogoproto.equal)           = false;
  option (gogoproto.goproto_getters) = false;

  string                   delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""];
  string                   validator_address = 2 [(gogoproto.moretags) = "yaml:\"validator_address\""];
  cosmos.base.v1beta1.Coin amount            = 3 [(gogoproto.nullable) = false];
}

It's annotated as non-nullable (using a custom gogoproto directive which the Rust proto parser doesn't understand), so this should be changed to pub amount: Coin and messages missing this field should be rejected.

Copy link
Member

@tony-iqlusion tony-iqlusion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this took so long to review! LGTM

@tony-iqlusion
Copy link
Member

@kimonp this just needs a rustfmt to be mergeable, but also there is one open question it'd be good to get an answer to

@tony-iqlusion
Copy link
Member

Going to merge this. I'll correct the rustfmt issue locally (I'm opening another PR anyway)

@tony-iqlusion tony-iqlusion merged commit 17c3908 into cosmos:main Jun 22, 2021
tony-iqlusion added a commit that referenced this pull request Aug 25, 2021
As noted on #82, the `amount` field of `MsgDelegate` is annotated as
non-nullable in the upstream cosmos-sdk:

https://github.com/cosmos/cosmos-sdk/blob/9fd866e3820b3510010ae172b682d71594cd8c14/proto/cosmos/staking/v1beta1/tx.proto#L89

```proto
message MsgDelegate {
  option (gogoproto.equal)           = false;
  option (gogoproto.goproto_getters) = false;

  string                   delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""];
  string                   validator_address = 2 [(gogoproto.moretags) = "yaml:\"validator_address\""];
  cosmos.base.v1beta1.Coin amount            = 3 [(gogoproto.nullable) = false];
}
```

This commit changes the `amount` field of the domain type from
`Option<Coin>` to `Coin` to reflect that.
tony-iqlusion added a commit that referenced this pull request Aug 25, 2021
As noted on #82, the `amount` field of `MsgDelegate` is annotated as
non-nullable in the upstream cosmos-sdk:

https://github.com/cosmos/cosmos-sdk/blob/9fd866e3820b3510010ae172b682d71594cd8c14/proto/cosmos/staking/v1beta1/tx.proto#L89

```proto
message MsgDelegate {
  option (gogoproto.equal)           = false;
  option (gogoproto.goproto_getters) = false;

  string                   delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""];
  string                   validator_address = 2 [(gogoproto.moretags) = "yaml:\"validator_address\""];
  cosmos.base.v1beta1.Coin amount            = 3 [(gogoproto.nullable) = false];
}
```

This commit changes the `amount` field of the domain type from
`Option<Coin>` to `Coin` to reflect that.
tony-iqlusion added a commit that referenced this pull request Aug 25, 2021
As noted on #82, the `amount` field of `MsgDelegate` is annotated as
non-nullable in the upstream cosmos-sdk:

https://github.com/cosmos/cosmos-sdk/blob/9fd866e3820b3510010ae172b682d71594cd8c14/proto/cosmos/staking/v1beta1/tx.proto#L89

```proto
message MsgDelegate {
  option (gogoproto.equal)           = false;
  option (gogoproto.goproto_getters) = false;

  string                   delegator_address = 1 [(gogoproto.moretags) = "yaml:\"delegator_address\""];
  string                   validator_address = 2 [(gogoproto.moretags) = "yaml:\"validator_address\""];
  cosmos.base.v1beta1.Coin amount            = 3 [(gogoproto.nullable) = false];
}
```

This commit changes the `amount` field of the domain type from
`Option<Coin>` to `Coin` to reflect that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants