x/bep3: genesis validation (#488)

* validate asset supply

* validate swap

* genesis tests

* swap and supply unit tests

* use Require() for suite

* fix tests

* Apply suggestions from code review


Co-authored-by: Denali Marsh <denali@kava.io>

x/bep3/genesis_test.go

@@ -278,19 +278,14 @@ func (suite *GenesisTestSuite) TestGenesisState() {
 	}
 
 	for _, tc := range testCases {
-		suite.SetupTest()
 		if tc.expectPass {
-			suite.Run(tc.name, func() {
+			suite.NotPanics(func() {
-				suite.NotPanics(func() {
+				suite.app.InitializeFromGenesisStates(tc.genState())
-					suite.app.InitializeFromGenesisStates(tc.genState())
+			}, tc.name)
-				})
-			})
 		} else {
-			suite.Run(tc.name, func() {
+			suite.Panics(func() {
-				suite.Panics(func() {
+				suite.app.InitializeFromGenesisStates(tc.genState())
-					suite.app.InitializeFromGenesisStates(tc.genState())
+			}, tc.name)
-				})
-			})
 		}
 	}
 }

x/bep3/integration_test.go

@@ -79,7 +79,7 @@ func loadSwapAndSupply(addr sdk.AccAddress, index int) (bep3.AtomicSwap, bep3.As
 	randomNumberHash := bep3.CalculateRandomHash(randomNumber.Bytes(), timestamp)
 	swap := bep3.NewAtomicSwap(cs(coin), randomNumberHash,
 		expireOffset, timestamp, addr, addr, TestSenderOtherChain,
-		TestRecipientOtherChain, 0, bep3.Open, true, bep3.Incoming)
+		TestRecipientOtherChain, 1, bep3.Open, true, bep3.Incoming)
 
 	supply := bep3.NewAssetSupply(coin.Denom, coin, c(coin.Denom, 0),
 		c(coin.Denom, 0), c(coin.Denom, StandardSupplyLimit.Int64()))

x/bep3/types/asset.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // AssetSupply contains information about an asset's supply
@@ -26,6 +27,23 @@ func NewAssetSupply(denom string, incomingSupply, outgoingSupply, currentSupply,
 	}
 }
 
+// Validate performs a basic validation of an asset supply fields.
+func (a AssetSupply) Validate() error {
+	if !a.IncomingSupply.IsValid() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "incoming supply %s", a.IncomingSupply)
+	}
+	if !a.OutgoingSupply.IsValid() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "outgoing supply %s", a.OutgoingSupply)
+	}
+	if !a.CurrentSupply.IsValid() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "current supply %s", a.CurrentSupply)
+	}
+	if !a.Limit.IsValid() {
+		return sdkerrors.Wrapf(sdkerrors.ErrInvalidCoins, "limit %s", a.Limit)
+	}
+	return sdk.ValidateDenom(a.Denom)
+}
+
 // String implements stringer
 func (a AssetSupply) String() string {
 	return fmt.Sprintf("Asset Supply"+

x/bep3/types/asset_test.go

@@ -0,0 +1,70 @@
+package types
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestAssetSupplyValidate(t *testing.T) {
+	coin := sdk.NewCoin("kava", sdk.OneInt())
+	invalidCoin := sdk.Coin{Denom: "Invalid Denom", Amount: sdk.NewInt(-1)}
+	testCases := []struct {
+		msg     string
+		asset   AssetSupply
+		expPass bool
+	}{
+		{
+			msg:     "valid asset",
+			asset:   NewAssetSupply("kava", coin, coin, coin, coin),
+			expPass: true,
+		},
+		{
+			"invalid incoming supply",
+			AssetSupply{IncomingSupply: invalidCoin},
+			false,
+		},
+		{
+			"invalid outgoing supply",
+			AssetSupply{
+				IncomingSupply: coin,
+				OutgoingSupply: invalidCoin,
+			},
+			false,
+		},
+		{
+			"invalid current supply",
+			AssetSupply{
+				IncomingSupply: coin,
+				OutgoingSupply: coin,
+				CurrentSupply:  invalidCoin,
+			},
+			false,
+		},
+		{
+			"invalid limit",
+			AssetSupply{
+				IncomingSupply: coin,
+				OutgoingSupply: coin,
+				CurrentSupply:  coin,
+				Limit:          invalidCoin,
+			},
+			false,
+		},
+		{
+			msg:     "invalid denom",
+			asset:   NewAssetSupply("Invalid Denom", coin, coin, coin, coin),
+			expPass: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		err := tc.asset.Validate()
+		if tc.expPass {
+			require.NoError(t, err, tc.msg)
+		} else {
+			require.Error(t, err, tc.msg)
+		}
+	}
+}

x/bep3/types/common_test.go

@@ -31,7 +31,7 @@ func atomicSwap(index int) types.AtomicSwap {
 	randomNumberHash := types.CalculateRandomHash(randomNumber.Bytes(), timestamp)
 
 	swap := types.NewAtomicSwap(cs(c("bnb", 50000)), randomNumberHash, expireOffset, timestamp, kavaAddrs[0],
-		kavaAddrs[1], binanceAddrs[0].String(), binanceAddrs[1].String(), 0, types.Open, true, types.Incoming)
+		kavaAddrs[1], binanceAddrs[0].String(), binanceAddrs[1].String(), 1, types.Open, true, types.Incoming)
 
 	return swap
 }

x/bep3/types/genesis.go

@@ -48,19 +48,31 @@ func (gs GenesisState) Validate() error {
 	if err := gs.Params.Validate(); err != nil {
 		return err
 	}
+
 	denoms := map[string]bool{}
-	for _, a := range gs.AssetSupplies {
+	for _, asset := range gs.AssetSupplies {
-		if denoms[a.Denom] {
+		if denoms[asset.Denom] {
-			return fmt.Errorf("found duplicate asset denom %s", a.Denom)
+			return fmt.Errorf("found duplicate asset denom %s", asset.Denom)
 		}
-		denoms[a.Denom] = true
+
+		if err := asset.Validate(); err != nil {
+			return err
+		}
+
+		denoms[asset.Denom] = true
 	}
+
 	ids := map[string]bool{}
-	for _, a := range gs.AtomicSwaps {
+	for _, swap := range gs.AtomicSwaps {
-		if ids[hex.EncodeToString(a.GetSwapID())] {
+		if ids[hex.EncodeToString(swap.GetSwapID())] {
-			return fmt.Errorf("found duplicate atomic swap ID %s", hex.EncodeToString(a.GetSwapID()))
+			return fmt.Errorf("found duplicate atomic swap ID %s", hex.EncodeToString(swap.GetSwapID()))
 		}
-		ids[hex.EncodeToString(a.GetSwapID())] = true
+
+		if err := swap.Validate(); err != nil {
+			return err
+		}
+
+		ids[hex.EncodeToString(swap.GetSwapID())] = true
 	}
 	return nil
 }

x/bep3/types/genesis_test.go

@@ -21,12 +21,10 @@ func (suite *GenesisTestSuite) SetupTest() {
 	config := sdk.GetConfig()
 	app.SetBech32AddressPrefixes(config)
 
-	count := 10
+	coin := sdk.NewCoin("kava", sdk.OneInt())
-	suite.swaps = atomicSwaps(count)
+	suite.swaps = atomicSwaps(10)
 
-	incomingSupply := int64(count * 50000)
+	supply := types.NewAssetSupply("kava", coin, coin, coin, coin)
-	supply := types.NewAssetSupply("bnb", c("bnb", incomingSupply),
-		c("bnb", 0), c("bnb", 0), c("bnb", 100000000000))
 	suite.supplies = types.AssetSupplies{supply}
 }
 
@@ -72,6 +70,22 @@ func (suite *GenesisTestSuite) TestValidate() {
 			},
 			false,
 		},
+		{
+			"invalid swap",
+			args{
+				swaps:    types.AtomicSwaps{types.AtomicSwap{Amount: sdk.Coins{sdk.Coin{Denom: "Invalid Denom", Amount: sdk.NewInt(-1)}}}},
+				supplies: types.AssetSupplies{},
+			},
+			false,
+		},
+		{
+			"invalid supply",
+			args{
+				swaps:    types.AtomicSwaps{},
+				supplies: types.AssetSupplies{types.AssetSupply{Denom: "Invalid Denom"}},
+			},
+			false,
+		},
 		{
 			"duplicate supplies",
 			args{
@@ -82,22 +96,19 @@ func (suite *GenesisTestSuite) TestValidate() {
 		}}
 
 	for _, tc := range testCases {
-		suite.SetupTest()
+		var gs types.GenesisState
-		suite.Run(tc.name, func() {
+		if tc.name == "default" {
-			var gs types.GenesisState
+			gs = types.DefaultGenesisState()
-			if tc.name == "default" {
+		} else {
-				gs = types.DefaultGenesisState()
+			gs = types.NewGenesisState(types.DefaultParams(), tc.args.swaps, tc.args.supplies)
-			} else {
+		}
-				gs = types.NewGenesisState(types.DefaultParams(), tc.args.swaps, tc.args.supplies)
-			}
 
-			err := gs.Validate()
+		err := gs.Validate()
-			if tc.expectPass {
+		if tc.expectPass {
-				suite.Nil(err)
+			suite.Require().NoError(err, tc.name)
-			} else {
+		} else {
-				suite.Error(err)
+			suite.Require().Error(err, tc.name)
-			}
+		}
-		})
 	}
 }
 

x/bep3/types/swap.go

@@ -3,11 +3,14 @@ package types
 import (
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"strings"
 
 	tmbytes "github.com/tendermint/tendermint/libs/bytes"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 // AtomicSwap contains the information for an atomic swap
@@ -56,19 +59,50 @@ func (a AtomicSwap) GetCoins() sdk.Coins {
 	return sdk.NewCoins(a.Amount...)
 }
 
-// Validate verifies that recipient is not empty
+// Validate performs a basic validation of an atomic swap fields.
 func (a AtomicSwap) Validate() error {
-	if len(a.Sender) != AddrByteCount {
+	if !a.Amount.IsValid() {
-		return fmt.Errorf(fmt.Sprintf("the expected address length is %d, actual length is %d", AddrByteCount, len(a.Sender)))
+		return fmt.Errorf("invalid amount: %s", a.Amount)
-	}
-	if len(a.Recipient) != AddrByteCount {
-		return fmt.Errorf(fmt.Sprintf("the expected address length is %d, actual length is %d", AddrByteCount, len(a.Recipient)))
-	}
-	if len(a.RandomNumberHash) != RandomNumberHashLength {
-		return fmt.Errorf(fmt.Sprintf("the length of random number hash should be %d", RandomNumberHashLength))
 	}
 	if !a.Amount.IsAllPositive() {
-		return fmt.Errorf(fmt.Sprintf("the swapped out coin must be positive"))
+		return fmt.Errorf("the swapped out coin must be positive: %s", a.Amount)
+	}
+	if len(a.RandomNumberHash) != RandomNumberHashLength {
+		return fmt.Errorf("the length of random number hash should be %d", RandomNumberHashLength)
+	}
+	if a.ExpireHeight == 0 {
+		return errors.New("expire height cannot be 0")
+	}
+	if a.Timestamp == 0 {
+		return errors.New("timestamp cannot be 0")
+	}
+	if a.Sender.Empty() {
+		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "sender cannot be empty")
+	}
+	if a.Recipient.Empty() {
+		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "recipient cannot be empty")
+	}
+	if len(a.Sender) != AddrByteCount {
+		return fmt.Errorf("the expected address length is %d, actual length is %d", AddrByteCount, len(a.Sender))
+	}
+	if len(a.Recipient) != AddrByteCount {
+		return fmt.Errorf("the expected address length is %d, actual length is %d", AddrByteCount, len(a.Recipient))
+	}
+	// NOTE: These adresses may not have a bech32 prefix.
+	if strings.TrimSpace(a.SenderOtherChain) == "" {
+		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "sender other chain cannot be blank")
+	}
+	if strings.TrimSpace(a.RecipientOtherChain) == "" {
+		return sdkerrors.Wrap(sdkerrors.ErrInvalidAddress, "recipient other chain cannot be blank")
+	}
+	if a.ClosedBlock == 0 {
+		return errors.New("closed block cannot be 0")
+	}
+	if a.Status == NULL || a.Status > 3 {
+		return errors.New("invalid swap status")
+	}
+	if a.Direction == INVALID || a.Direction > 2 {
+		return errors.New("invalid swap direction")
 	}
 	return nil
 }
@@ -111,6 +145,7 @@ func (swaps AtomicSwaps) String() string {
 // SwapStatus is the status of an AtomicSwap
 type SwapStatus byte
 
+// swap statuses
 const (
 	NULL      SwapStatus = 0x00
 	Open      SwapStatus = 0x01

x/bep3/types/swap_test.go

@@ -44,95 +44,205 @@ func (suite *AtomicSwapTestSuite) SetupTest() {
 }
 
 func (suite *AtomicSwapTestSuite) TestNewAtomicSwap() {
-	type args struct {
-		amount              sdk.Coins
-		randomNumberHash    tmbytes.HexBytes
-		expireHeight        int64
-		timestamp           int64
-		sender              sdk.AccAddress
-		recipient           sdk.AccAddress
-		recipientOtherChain string
-		senderOtherChain    string
-		closedBlock         int64
-		status              types.SwapStatus
-		crossChain          bool
-		direction           types.SwapDirection
-	}
 	testCases := []struct {
-		description string
+		msg        string
-		args        args
+		swap       types.AtomicSwap
-		expectPass  bool
+		expectPass bool
 	}{
 		{
-			"normal",
+			"valid Swap",
-			args{
+			types.AtomicSwap{
-				amount:              cs(c("bnb", 50000)),
+				Amount:              cs(c("bnb", 50000)),
-				randomNumberHash:    suite.randomNumberHashes[0],
+				RandomNumberHash:    suite.randomNumberHashes[0],
-				expireHeight:        int64(360),
+				ExpireHeight:        360,
-				timestamp:           suite.timestamps[0],
+				Timestamp:           suite.timestamps[0],
-				sender:              suite.addrs[0],
+				Sender:              suite.addrs[0],
-				recipient:           suite.addrs[5],
+				Recipient:           suite.addrs[5],
-				recipientOtherChain: "bnb1urfermcg92dwq36572cx4xg84wpk3lfpksr5g7",
+				RecipientOtherChain: "bnb1urfermcg92dwq36572cx4xg84wpk3lfpksr5g7",
-				senderOtherChain:    "bnb1uky3me9ggqypmrsvxk7ur6hqkzq7zmv4ed4ng7",
+				SenderOtherChain:    "bnb1uky3me9ggqypmrsvxk7ur6hqkzq7zmv4ed4ng7",
-				closedBlock:         0,
+				ClosedBlock:         1,
-				status:              types.Open,
+				Status:              types.Open,
-				crossChain:          true,
+				CrossChain:          true,
-				direction:           types.Incoming,
+				Direction:           types.Incoming,
 			},
 			true,
 		},
 		{
-			"invalid random number hash length",
+			"invalid amount",
-			args{
+			types.AtomicSwap{
-				amount:              cs(c("bnb", 50000)),
+				Amount: sdk.Coins{sdk.Coin{Denom: "BNB", Amount: sdk.NewInt(10)}},
-				randomNumberHash:    suite.randomNumberHashes[1][0:20],
-				expireHeight:        int64(360),
-				timestamp:           suite.timestamps[1],
-				sender:              suite.addrs[1],
-				recipient:           suite.addrs[5],
-				recipientOtherChain: "bnb1urfermcg92dwq36572cx4xg84wpk3lfpksr5g7",
-				senderOtherChain:    "bnb1uky3me9ggqypmrsvxk7ur6hqkzq7zmv4ed4ng7",
-				closedBlock:         0,
-				status:              types.Open,
-				crossChain:          true,
-				direction:           types.Incoming,
 			},
 			false,
 		},
 		{
-			"invalid amount",
+			"amount not positive",
-			args{
+			types.AtomicSwap{
-				amount:              cs(c("bnb", 0)),
+				Amount: cs(c("bnb", 0)),
-				randomNumberHash:    suite.randomNumberHashes[2],
+			},
-				expireHeight:        int64(360),
+			false,
-				timestamp:           suite.timestamps[2],
+		},
-				sender:              suite.addrs[2],
+		{
-				recipient:           suite.addrs[5],
+			"invalid random number hash length",
-				recipientOtherChain: "bnb1urfermcg92dwq36572cx4xg84wpk3lfpksr5g7",
+			types.AtomicSwap{
-				senderOtherChain:    "bnb1uky3me9ggqypmrsvxk7ur6hqkzq7zmv4ed4ng7",
+				Amount:           cs(c("bnb", 50000)),
-				closedBlock:         0,
+				RandomNumberHash: suite.randomNumberHashes[1][0:20],
-				status:              types.Open,
+			},
-				crossChain:          true,
+			false,
-				direction:           types.Incoming,
+		},
+		{
+			"exp height 0",
+			types.AtomicSwap{
+				Amount:           cs(c("bnb", 50000)),
+				RandomNumberHash: suite.randomNumberHashes[0],
+				ExpireHeight:     0,
+			},
+			false,
+		},
+		{
+			"timestamp 0",
+			types.AtomicSwap{
+				Amount:           cs(c("bnb", 50000)),
+				RandomNumberHash: suite.randomNumberHashes[0],
+				ExpireHeight:     10,
+				Timestamp:        0,
+			},
+			false,
+		},
+		{
+			"empty sender",
+			types.AtomicSwap{
+				Amount:           cs(c("bnb", 50000)),
+				RandomNumberHash: suite.randomNumberHashes[0],
+				ExpireHeight:     10,
+				Timestamp:        10,
+				Sender:           nil,
+			},
+			false,
+		},
+		{
+			"empty recipient",
+			types.AtomicSwap{
+				Amount:           cs(c("bnb", 50000)),
+				RandomNumberHash: suite.randomNumberHashes[0],
+				ExpireHeight:     10,
+				Timestamp:        10,
+				Sender:           suite.addrs[0],
+				Recipient:        nil,
+			},
+			false,
+		},
+		{
+			"invalid sender length",
+			types.AtomicSwap{
+				Amount:           cs(c("bnb", 50000)),
+				RandomNumberHash: suite.randomNumberHashes[0],
+				ExpireHeight:     10,
+				Timestamp:        10,
+				Sender:           suite.addrs[0][:10],
+				Recipient:        suite.addrs[5],
+			},
+			false,
+		},
+		{
+			"invalid recipient length",
+			types.AtomicSwap{
+				Amount:           cs(c("bnb", 50000)),
+				RandomNumberHash: suite.randomNumberHashes[0],
+				ExpireHeight:     10,
+				Timestamp:        10,
+				Sender:           suite.addrs[0],
+				Recipient:        suite.addrs[5][:10],
+			},
+			false,
+		},
+		{
+			"invalid sender other chain",
+			types.AtomicSwap{
+				Amount:           cs(c("bnb", 50000)),
+				RandomNumberHash: suite.randomNumberHashes[0],
+				ExpireHeight:     10,
+				Timestamp:        10,
+				Sender:           suite.addrs[0],
+				Recipient:        suite.addrs[5],
+				SenderOtherChain: "",
+			},
+			false,
+		},
+		{
+			"invalid recipient other chain",
+			types.AtomicSwap{
+				Amount:              cs(c("bnb", 50000)),
+				RandomNumberHash:    suite.randomNumberHashes[0],
+				ExpireHeight:        10,
+				Timestamp:           10,
+				Sender:              suite.addrs[0],
+				Recipient:           suite.addrs[5],
+				SenderOtherChain:    "bnb1uky3me9ggqypmrsvxk7ur6hqkzq7zmv4ed4ng7",
+				RecipientOtherChain: "",
+			},
+			false,
+		},
+		{
+			"closed block 0",
+			types.AtomicSwap{
+				Amount:              cs(c("bnb", 50000)),
+				RandomNumberHash:    suite.randomNumberHashes[0],
+				ExpireHeight:        10,
+				Timestamp:           10,
+				Sender:              suite.addrs[0],
+				Recipient:           suite.addrs[5],
+				SenderOtherChain:    "bnb1uky3me9ggqypmrsvxk7ur6hqkzq7zmv4ed4ng7",
+				RecipientOtherChain: "bnb1urfermcg92dwq36572cx4xg84wpk3lfpksr5g7",
+				ClosedBlock:         0,
+			},
+			false,
+		},
+		{
+			"invalid status 0",
+			types.AtomicSwap{
+				Amount:              cs(c("bnb", 50000)),
+				RandomNumberHash:    suite.randomNumberHashes[0],
+				ExpireHeight:        10,
+				Timestamp:           10,
+				Sender:              suite.addrs[0],
+				Recipient:           suite.addrs[5],
+				SenderOtherChain:    "bnb1uky3me9ggqypmrsvxk7ur6hqkzq7zmv4ed4ng7",
+				RecipientOtherChain: "bnb1urfermcg92dwq36572cx4xg84wpk3lfpksr5g7",
+				ClosedBlock:         1,
+				Status:              types.NULL,
+			},
+			false,
+		},
+		{
+			"invalid direction ",
+			types.AtomicSwap{
+				Amount:              cs(c("bnb", 50000)),
+				RandomNumberHash:    suite.randomNumberHashes[0],
+				ExpireHeight:        10,
+				Timestamp:           10,
+				Sender:              suite.addrs[0],
+				Recipient:           suite.addrs[5],
+				SenderOtherChain:    "bnb1uky3me9ggqypmrsvxk7ur6hqkzq7zmv4ed4ng7",
+				RecipientOtherChain: "bnb1urfermcg92dwq36572cx4xg84wpk3lfpksr5g7",
+				ClosedBlock:         1,
+				Status:              types.Open,
+				Direction:           types.INVALID,
 			},
 			false,
 		},
 	}
 
 	for _, tc := range testCases {
-		// Create atomic swap
-		swap := types.NewAtomicSwap(tc.args.amount, tc.args.randomNumberHash, tc.args.expireHeight,
-			tc.args.timestamp, tc.args.sender, tc.args.recipient, tc.args.senderOtherChain,
-			tc.args.recipientOtherChain, tc.args.closedBlock, tc.args.status, tc.args.crossChain,
-			tc.args.direction)
 
+		err := tc.swap.Validate()
 		if tc.expectPass {
-			suite.Nil(swap.Validate())
+			suite.Require().NoError(err, tc.msg)
-			suite.Equal(tc.args.amount, swap.GetCoins())
+			suite.Require().Equal(tc.swap.Amount, tc.swap.GetCoins())
-			expectedSwapID := types.CalculateSwapID(tc.args.randomNumberHash, tc.args.sender, tc.args.senderOtherChain)
+
-			suite.Equal(tmbytes.HexBytes(expectedSwapID), swap.GetSwapID())
+			expectedSwapID := types.CalculateSwapID(tc.swap.RandomNumberHash, tc.swap.Sender, tc.swap.SenderOtherChain)
+			suite.Require().Equal(tmbytes.HexBytes(expectedSwapID), tc.swap.GetSwapID())
 		} else {
-			suite.Error(swap.Validate())
+			suite.Require().Error(err)
 		}
 
 	}