Skip to content

Commit

Permalink
PR feedback:
Browse files Browse the repository at this point in the history
Use enum for status
use int64 for height
add distributions response proto
add address parsing check
remove imports on expected_keepers
  • Loading branch information
timlind committed Apr 26, 2021
1 parent 4d1be94 commit 83fa95e
Show file tree
Hide file tree
Showing 19 changed files with 539 additions and 346 deletions.
6 changes: 3 additions & 3 deletions cmd/ebrelayer/txs/relayToCosmos.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,10 @@ func RelayToCosmos(moniker, password string, claims []*types.EthBridgeClaim, cli
// Packages the claim as a Tendermint message
msg := types.NewMsgCreateEthBridgeClaim(claim)

e := msg.ValidateBasic()
if e != nil {
err := msg.ValidateBasic()
if err != nil {
sugaredLogger.Errorw("failed to get message from claim.",
errorMessageKey, e.Error())
errorMessageKey, err.Error())
continue
} else {
messages = append(messages, &msg)
Expand Down
7 changes: 2 additions & 5 deletions proto/sifnode/dispensation/v1/querier.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ syntax = "proto3";
package sifnode.dispensation.v1;

import "gogoproto/gogo.proto";
import "sifnode/dispensation/v1/types.proto";

option go_package = "github.com/Sifchain/sifnode/x/dispensation/types";

Expand All @@ -11,9 +12,5 @@ message QueryRecordsByRecipientAddr {

message QueryRecordsByDistributionName {
string distribution_name = 1;
string status = 2 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Uint",
(gogoproto.nullable) = false,
(gogoproto.moretags) = "yaml:\"status\""
];
sifnode.dispensation.v1.ClaimStatus status = 2;
}
6 changes: 6 additions & 0 deletions proto/sifnode/dispensation/v1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@ syntax = "proto3";
package sifnode.dispensation.v1;

import "gogoproto/gogo.proto";
import "sifnode/dispensation/v1/types.proto";

option go_package = "github.com/Sifchain/sifnode/x/dispensation/types";

service Query {
}

message DistributionsResponse {
repeated Distribution distributions = 1;
int64 height = 2;
}
28 changes: 8 additions & 20 deletions proto/sifnode/dispensation/v1/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ syntax = "proto3";
package sifnode.dispensation.v1;

import "gogoproto/gogo.proto";
import "cosmos/base/coin.proto";

option go_package = "github.com/Sifchain/sifnode/x/dispensation/types";

Expand All @@ -21,31 +20,24 @@ enum DistributionType {

// Claim status enum
enum ClaimStatus {
// Unspecified
CLAIM_STATUS_UNSPECIFIED = 0;
// Pending claim status
CLAIM_STATUS_PENDING = 0;
CLAIM_STATUS_PENDING = 1;
// Completed claim status
CLAIM_STATUS_COMPLETED = 1;
CLAIM_STATUS_COMPLETED = 2;
}

message DistributionRecord {
ClaimStatus claim_status = 1;
string distribution_name = 2;
string recipient_address = 3;
repeated cosmos.base.v1beta1.Coin coins = 4 [
(gogoproto.nullable) = false,
repeated string coins = 4 [
(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins",
(gogoproto.moretags) = "yaml:\"coins\""
];
string distribution_start_height = 5 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
(gogoproto.nullable) = false,
(gogoproto.moretags) = "yaml:\"distribution_start_height\""
];
string distribution_completed_height = 6 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
(gogoproto.nullable) = false,
(gogoproto.moretags) = "yaml:\"distribution_completed_height\""
];
int64 distribution_start_height = 5;
int64 distribution_completed_height = 6;
}

message DistributionRecords {
Expand All @@ -54,11 +46,7 @@ message DistributionRecords {

message DistributionRecordsResponse {
DistributionRecords distribution_records = 1;
string height = 2 [
(gogoproto.customtype) = "github.com/cosmos/cosmos-sdk/types.Int",
(gogoproto.nullable) = false,
(gogoproto.moretags) = "yaml:\"height\""
];
int64 height = 2;
}

message Distributions {
Expand Down
40 changes: 0 additions & 40 deletions third_party/proto/cosmos/base/coin.proto

This file was deleted.

16 changes: 0 additions & 16 deletions third_party/proto/cosmos/cosmos.proto

This file was deleted.

14 changes: 7 additions & 7 deletions x/dispensation/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func GetCmdDistributionRecordForRecipient(queryRoute string) *cobra.Command {
}
var drs types.DistributionRecords
types.ModuleCdc.MustUnmarshalJSON(res, &drs)
out := types.NewDistributionRecordsResponse(drs, sdk.NewInt(height))
out := types.NewDistributionRecordsResponse(drs, height)
return clientCtx.PrintProto(&out)
},
}
Expand All @@ -97,7 +97,7 @@ func GetCmdDistributionRecordForDistNameAll(queryRoute string) *cobra.Command {
return err
}
name := args[0]
params := types.NewQueryRecordsByDistributionName(name, sdk.NewUint(2))
params := types.NewQueryRecordsByDistributionName(name, types.ClaimStatus_CLAIM_STATUS_UNSPECIFIED)
bz, err := clientCtx.LegacyAmino.MarshalJSON(params)
if err != nil {
return err
Expand All @@ -109,7 +109,7 @@ func GetCmdDistributionRecordForDistNameAll(queryRoute string) *cobra.Command {
}
var drs types.DistributionRecords
types.ModuleCdc.MustUnmarshalJSON(res, &drs)
out := types.NewDistributionRecordsResponse(drs, sdk.NewInt(height))
out := types.NewDistributionRecordsResponse(drs, height)
return clientCtx.PrintProto(&out)
},
}
Expand All @@ -127,7 +127,7 @@ func GetCmdDistributionRecordForDistNamePending(queryRoute string) *cobra.Comman
return err
}
name := args[0]
params := types.NewQueryRecordsByDistributionName(name, sdk.NewUint(1))
params := types.NewQueryRecordsByDistributionName(name, types.ClaimStatus_CLAIM_STATUS_PENDING)
bz, err := clientCtx.LegacyAmino.MarshalJSON(params)
if err != nil {
return err
Expand All @@ -139,7 +139,7 @@ func GetCmdDistributionRecordForDistNamePending(queryRoute string) *cobra.Comman
}
var drs types.DistributionRecords
types.ModuleCdc.MustUnmarshalJSON(res, &drs)
out := types.NewDistributionRecordsResponse(drs, sdk.NewInt(height))
out := types.NewDistributionRecordsResponse(drs, height)
return clientCtx.PrintProto(&out)
},
}
Expand All @@ -157,7 +157,7 @@ func GetCmdDistributionRecordForDistNameCompleted(queryRoute string) *cobra.Comm
return err
}
name := args[0]
params := types.NewQueryRecordsByDistributionName(name, sdk.NewUint(2))
params := types.NewQueryRecordsByDistributionName(name, types.ClaimStatus_CLAIM_STATUS_COMPLETED)
bz, err := clientCtx.LegacyAmino.MarshalJSON(params)
if err != nil {
return err
Expand All @@ -169,7 +169,7 @@ func GetCmdDistributionRecordForDistNameCompleted(queryRoute string) *cobra.Comm
}
var drs types.DistributionRecords
types.ModuleCdc.MustUnmarshalJSON(res, &drs)
out := types.NewDistributionRecordsResponse(drs, sdk.NewInt(height))
out := types.NewDistributionRecordsResponse(drs, height)
return clientCtx.PrintProto(&out)
},
}
Expand Down
6 changes: 3 additions & 3 deletions x/dispensation/keeper/executors.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ import (
// Each Recipient and DropName generate a unique Record
func (k Keeper) CreateDrops(ctx sdk.Context, output []banktypes.Output, name string) error {
for _, receiver := range output {
distributionRecord := types.NewDistributionRecord(name, receiver.Address, receiver.Coins, sdk.NewInt(ctx.BlockHeight()), sdk.NewInt(1))
distributionRecord := types.NewDistributionRecord(name, receiver.Address, receiver.Coins, ctx.BlockHeight(), int64(1))
if k.ExistsDistributionRecord(ctx, name, receiver.Address) {
oldRecord, err := k.GetDistributionRecord(ctx, name, receiver.Address)
if err != nil {
return errors.Wrapf(types.ErrDistribution, "failed appending record for : %s", distributionRecord.RecipientAddress)
}
distributionRecord.Add(*oldRecord)
}
distributionRecord.ClaimStatus = types.ClaimStatus_CLAIM_STATUS_COMPLETED
distributionRecord.ClaimStatus = types.ClaimStatus_CLAIM_STATUS_PENDING
err := k.SetDistributionRecord(ctx, distributionRecord)
if err != nil {
return errors.Wrapf(types.ErrFailedOutputs, "error setting distibution record : %s", distributionRecord.String())
Expand All @@ -40,7 +40,7 @@ func (k Keeper) DistributeDrops(ctx sdk.Context, height int64) error {
return errors.Wrapf(types.ErrFailedOutputs, "for address : %s", record.RecipientAddress)
}
record.ClaimStatus = types.ClaimStatus_CLAIM_STATUS_COMPLETED
record.DistributionCompletedHeight = sdk.NewInt(height)
record.DistributionCompletedHeight = height
err = k.SetDistributionRecord(ctx, *record)
if err != nil {
return errors.Wrapf(types.ErrFailedOutputs, "error setting distibution record : %s", record)
Expand Down
4 changes: 2 additions & 2 deletions x/dispensation/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestKeeper_GetRecordsForName(t *testing.T) {
outList := test.GenerateOutputList("1000000000")
name := uuid.New().String()
for _, rec := range outList {
record := types.NewDistributionRecord(name, rec.Address, rec.Coins, sdk.NewInt(ctx.BlockHeight()), sdk.NewInt(-1))
record := types.NewDistributionRecord(name, rec.Address, rec.Coins, ctx.BlockHeight(), int64(-1))
err := keeper.SetDistributionRecord(ctx, record)
assert.NoError(t, err)
_, err = keeper.GetDistributionRecord(ctx, name, rec.Address)
Expand All @@ -48,7 +48,7 @@ func TestKeeper_GetRecordsForRecipient(t *testing.T) {
outList := test.GenerateOutputList("1000000000")
name := uuid.New().String()
for _, rec := range outList {
record := types.NewDistributionRecord(name, rec.Address, rec.Coins, sdk.NewInt(ctx.BlockHeight()), sdk.NewInt(-1))
record := types.NewDistributionRecord(name, rec.Address, rec.Coins, ctx.BlockHeight(), int64(-1))
err := keeper.SetDistributionRecord(ctx, record)
assert.NoError(t, err)
_, err = keeper.GetDistributionRecord(ctx, name, rec.Address)
Expand Down
9 changes: 6 additions & 3 deletions x/dispensation/keeper/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ func queryDistributionRecordsForName(ctx sdk.Context, req abci.RequestQuery, kee
}
records := new(types.DistributionRecords)
switch params.Status {
case sdk.NewUint(1):
case types.ClaimStatus_CLAIM_STATUS_PENDING:
*records = keeper.GetRecordsForNamePending(ctx, params.DistributionName)
case sdk.NewUint(2):
case types.ClaimStatus_CLAIM_STATUS_COMPLETED:
*records = keeper.GetRecordsForNameCompleted(ctx, params.DistributionName)
default:
*records = keeper.GetRecordsForNameAll(ctx, params.DistributionName)
Expand All @@ -57,7 +57,10 @@ func queryDistributionRecordsForRecipient(ctx sdk.Context, req abci.RequestQuery
if err != nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrJSONUnmarshal, err.Error())
}
addr, _ := sdk.AccAddressFromBech32(params.Address)
addr, err := sdk.AccAddressFromBech32(params.Address)
if err != nil {
return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, err.Error())
}
records := keeper.GetRecordsForRecipient(ctx, addr)
res, err := types.ModuleCdc.MarshalJSON(records)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion x/dispensation/keeper/querier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func GenerateQueryData(app *app.SifchainApp, ctx sdk.Context, name string, outLi
}

for _, rec := range outList {
record := types.NewDistributionRecord(name, rec.Address, rec.Coins, sdk.NewInt(ctx.BlockHeight()), sdk.NewInt(-1))
record := types.NewDistributionRecord(name, rec.Address, rec.Coins, ctx.BlockHeight(), int64(-1))
_ = keeper.SetDistributionRecord(ctx, record)
}

Expand Down
6 changes: 0 additions & 6 deletions x/dispensation/types/expected_keepers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package types

import (
sdk "github.com/cosmos/cosmos-sdk/types"
authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
bankkeeper "github.com/cosmos/cosmos-sdk/x/bank/keeper"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
Expand All @@ -17,8 +15,6 @@ type ParamSubspace interface {
SetParamSet(ctx sdk.Context, ps paramtypes.ParamSet)
}

var _ BankKeeper = &bankkeeper.BaseKeeper{}

type BankKeeper interface {
SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) error
SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error
Expand All @@ -31,8 +27,6 @@ type BankKeeper interface {
HasBalance(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coin) bool
}

var _ AccountKeeper = &authkeeper.AccountKeeper{}

type AccountKeeper interface {
SetModuleAccount(sdk.Context, authtypes.ModuleAccountI)
GetModuleAccount(ctx sdk.Context, moduleName string) authtypes.ModuleAccountI
Expand Down
5 changes: 3 additions & 2 deletions x/dispensation/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,16 @@ func (m MsgDistribution) Type() string {
}

func (m MsgDistribution) ValidateBasic() error {
if m.Signer == "" {
_, err := sdk.AccAddressFromBech32(m.Signer)
if err != nil {
return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, m.Signer)
}

if m.DistributionName == "" {
return sdkerrors.Wrap(ErrInvalid, "Name cannot be empty")
}

err := types.ValidateInputsOutputs(m.Input, m.Output)
err = types.ValidateInputsOutputs(m.Input, m.Output)
if err != nil {
return err
}
Expand Down
4 changes: 1 addition & 3 deletions x/dispensation/types/querier.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
package types

import sdk "github.com/cosmos/cosmos-sdk/types"

const (
QueryAllDistributions = "distributions"
QueryRecordsByDistrName = "records_by_name"
QueryRecordsByRecipient = "records_by_recipient"
)

func NewQueryRecordsByDistributionName(distributionName string, status sdk.Uint) QueryRecordsByDistributionName {
func NewQueryRecordsByDistributionName(distributionName string, status ClaimStatus) QueryRecordsByDistributionName {
return QueryRecordsByDistributionName{DistributionName: distributionName, Status: status}
}

Expand Down
Loading

0 comments on commit 83fa95e

Please sign in to comment.