Skip to content

Commit

Permalink
feat: improve rewards messages checks (#199)
Browse files Browse the repository at this point in the history
## Description

This PR improves the `x/rewards` messages by adding missing
`ValidateBasic`, `GetSignBytes` and `GetSigners` methods. It also
updates the registered codec values and adds missing ``UnpackInterfaces`
implementations to make sure all messages can be encoded using Amino
(and thus be signed using a Ledger devices).

<!-- Add a description of the changes that this PR introduces and the
files that
are the most critical to review. -->

---

### Author Checklist

*All items are required. Please add a note to the item if the item is
not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type
prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json)
in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR
Targeting](https://github.com/milkyway-labs/milkyway/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building
modules](https://docs.cosmos.network/v0.44/building-modules/intro.html)
- [ ] included the necessary unit and integration
[tests](https://github.com/milkyway-labs/milkyway/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go
code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable
and please add
your handle next to the items reviewed if you only reviewed selected
items.*

I have...

- [ ] confirmed the correct [type
prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json)
in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
RiccardoM authored Nov 25, 2024
1 parent 24d4086 commit 0e95a3a
Show file tree
Hide file tree
Showing 10 changed files with 770 additions and 64 deletions.
6 changes: 0 additions & 6 deletions app/encoding.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@ import (
"github.com/milkyway-labs/milkyway/app/params"
)

var encodingConfig params.EncodingConfig = MakeEncodingConfig()

func GetEncodingConfig() params.EncodingConfig {
return encodingConfig
}

// MakeEncodingConfig creates an EncodingConfig.
func MakeEncodingConfig() params.EncodingConfig {
encodingConfig := params.MakeEncodingConfig()
Expand Down
12 changes: 12 additions & 0 deletions testutils/codec.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package testutils

import (
"github.com/cosmos/cosmos-sdk/codec"
codectestutil "github.com/cosmos/cosmos-sdk/codec/testutil"
)

// MakeCodecs constructs the *codec.Codec and *codec.LegacyAmino instances that can be used inside tests
func MakeCodecs() (codec.Codec, *codec.LegacyAmino) {
interfaceRegistry := codectestutil.CodecOptions{AccAddressPrefix: "cosmos", ValAddressPrefix: "cosmosvaloper"}.NewInterfaceRegistry()
return codec.NewProtoCodec(interfaceRegistry), codec.NewLegacyAmino()
}
13 changes: 13 additions & 0 deletions x/rewards/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ Where rewards_plan.json contains:
if err != nil {
return fmt.Errorf("parsing rewards plan json: %w", err)
}

err = rewardsPlan.Validate(clientCtx.Codec)
if err != nil {
return fmt.Errorf("invalid rewards plan json: %w", err)
Expand All @@ -104,6 +105,12 @@ Where rewards_plan.json contains:
creator,
)

// Validate the message
err = msg.ValidateBasic()
if err != nil {
return fmt.Errorf("invalid message: %w", err)
}

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
},
}
Expand Down Expand Up @@ -197,6 +204,12 @@ Where rewards_plan.json contains:
sender,
)

// Validate the message
err = msg.ValidateBasic()
if err != nil {
return err
}

return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
},
}
Expand Down
9 changes: 4 additions & 5 deletions x/rewards/client/cli/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"

milkyway "github.com/milkyway-labs/milkyway/app"
milkywayapp "github.com/milkyway-labs/milkyway/app"
"github.com/milkyway-labs/milkyway/x/rewards/client/cli"
"github.com/milkyway-labs/milkyway/x/rewards/types"
)

func TestCliUtils_parseRewardsPlan(t *testing.T) {
encodingConfig := milkyway.MakeEncodingConfig()
codec := encodingConfig.Marshaler
func TestCLIUtils_parseRewardsPlan(t *testing.T) {
cdc, _ := milkywayapp.MakeCodecs()

testCases := []struct {
name string
Expand Down Expand Up @@ -191,7 +190,7 @@ func TestCliUtils_parseRewardsPlan(t *testing.T) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
require.NotNil(t, tc.jsonFile)
plan, err := cli.ParseRewardsPlan(codec, tc.jsonFile.Name())
plan, err := cli.ParseRewardsPlan(cdc, tc.jsonFile.Name())
if tc.shouldErr {
require.Error(t, err)
} else {
Expand Down
20 changes: 20 additions & 0 deletions x/rewards/types/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/legacy"
"github.com/cosmos/cosmos-sdk/codec/types"
cryptocodec "github.com/cosmos/cosmos-sdk/crypto/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/msgservice"
)
Expand All @@ -15,6 +16,11 @@ func RegisterLegacyAminoCodec(cdc *codec.LegacyAmino) {
legacy.RegisterAminoMsg(cdc, &MsgWithdrawDelegatorReward{}, "milkyway/MsgWithdrawDelegatorReward")
legacy.RegisterAminoMsg(cdc, &MsgWithdrawOperatorCommission{}, "milkyway/MsgWithdrawOperatorCommission")
legacy.RegisterAminoMsg(cdc, &MsgUpdateParams{}, "milkyway/rewards/MsgUpdateParams")

cdc.RegisterInterface((*DistributionType)(nil), nil)
cdc.RegisterConcrete(&DistributionTypeBasic{}, "milkyway/DistributionTypeBasic", nil)
cdc.RegisterConcrete(&DistributionTypeWeighted{}, "milkyway/DistributionTypeWeighted", nil)
cdc.RegisterConcrete(&DistributionTypeEgalitarian{}, "milkyway/DistributionTypeEgalitarian", nil)
}

func RegisterInterfaces(registry types.InterfaceRegistry) {
Expand All @@ -41,3 +47,17 @@ func RegisterInterfaces(registry types.InterfaceRegistry) {
)
msgservice.RegisterMsgServiceDesc(registry, &_Msg_serviceDesc)
}

// AminoCdc references the global x/rewards module codec. Note, the codec should
// ONLY be used in certain instances of tests and for JSON encoding as Amino is
// still used for that purpose.
//
// The actual codec used for serialization should be provided to x/rewards and
// defined at the application level.
var AminoCdc = codec.NewLegacyAmino()

func init() {
RegisterLegacyAminoCodec(AminoCdc)
cryptocodec.RegisterCrypto(AminoCdc)
sdk.RegisterLegacyAminoCodec(AminoCdc)
}
5 changes: 3 additions & 2 deletions x/rewards/types/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (

"github.com/stretchr/testify/require"

milkyway "github.com/milkyway-labs/milkyway/app"
"github.com/milkyway-labs/milkyway/testutils"
"github.com/milkyway-labs/milkyway/utils"
"github.com/milkyway-labs/milkyway/x/rewards/types"
)
Expand Down Expand Up @@ -69,7 +69,8 @@ func TestGenesisState_Validate(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
cdc, _ := milkyway.MakeCodecs()
cdc, _ := testutils.MakeCodecs()

err := tc.genesis.Validate(cdc)
if tc.shouldErr {
require.Error(t, err)
Expand Down
Loading

0 comments on commit 0e95a3a

Please sign in to comment.