add minimal test coverage

internal/x/paychan/README.md

@@ -9,8 +9,11 @@ Simplifications:
  TODO
  - chnge module name to "channel"?
  - Find a better name for Queue - clarify distinction between int slice and abstract queue concept
+ - refactor queue into one object
  - Do all the small functions need to be methods on the keeper or can they just be floating around?
- - Tidy up - standardise var names, comments and method descriptions
+ - Tidy up - standardise var names, method descriptions, heading comments
  - is having all the get functions return a bool if not found reasonable?
  - any problem in signing your own address?
  - Gas
+ - Codespace
+ - find nicer name for payouts

internal/x/paychan/client/cmd/cmd.go

@@ -21,10 +21,10 @@ import (
 // list of functions that return pointers to cobra commands
 // No local storage needed for cli acting as a sender
 
-// Currently minimum set of cli commands are implemented:
-// create paychan - create and fund
-// generate new paychan state - print a half signed close tx (sender signs)
-// close paychan - close using state (receiver signs)
+// Current minimal set of cli commands:
+// create paychan - create and fund (sender signs tx)
+// generate new update - print a signed update (from sender)
+// submit update - send update to chain (either can sign tx)
 
 // Future cli commands:
 // create paychan

internal/x/paychan/endblocker.go

@@ -24,6 +24,7 @@ func EndBlocker(ctx sdk.Context, k Keeper) sdk.Tags {
 			panic("can't find element in queue that should exist")
 		}
 		if ctx.BlockHeight() >= sUpdate.ExecutionTime {
+			k.removeFromSubmittedUpdatesQueue(ctx, sUpdate.ChannelID)
 			channelTags, err = k.closeChannel(ctx, sUpdate.Update)
 			if err != nil {
 				panic(err)

internal/x/paychan/endblocker_test.go

@@ -0,0 +1,61 @@
+package paychan
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/stretchr/testify/assert"
+	"github.com/tendermint/tendermint/crypto"
+	"testing"
+)
+
+func TestEndBlocker(t *testing.T) {
+	// SETUP
+	ctx, _, channelKeeper, addrs, _ := createMockApp()
+	sender := addrs[0]
+	receiver := addrs[1]
+	coins := sdk.Coins{sdk.NewCoin("KVA", 10)}
+
+	// create new channel
+	channelID := ChannelID(0) // should be 0 as first channel
+	channel := Channel{
+		ID:           channelID,
+		Participants: [2]sdk.AccAddress{sender, receiver},
+		Coins:        coins,
+	}
+	channelKeeper.setChannel(ctx, channel)
+
+	// create closing update and submittedUpdate
+	payouts := Payouts{
+		{sender, sdk.Coins{sdk.NewCoin("KVA", 3)}},
+		{receiver, sdk.Coins{sdk.NewCoin("KVA", 7)}},
+	}
+	update := Update{
+		ChannelID: channelID,
+		Payouts:   payouts,
+		Sequence:  0,
+		Sigs:      [1]crypto.Signature{},
+	}
+	sUpdate := SubmittedUpdate{
+		Update:        update,
+		ExecutionTime: 0, // current blocktime
+	}
+	// Set empty submittedUpdatesQueue TODO work out proper genesis initialisation
+	channelKeeper.setSubmittedUpdatesQueue(ctx, SubmittedUpdatesQueue{})
+	// flag channel for closure
+	channelKeeper.addToSubmittedUpdatesQueue(ctx, sUpdate)
+
+	// ACTION
+	EndBlocker(ctx, channelKeeper)
+
+	// CHECK RESULTS
+	// ideally just check if keeper.channelClose was called, but can't
+	// writing endBlocker to accept an interface of which keeper is implementation would make this possible
+	// check channel is gone
+	_, found := channelKeeper.getChannel(ctx, channelID)
+	assert.False(t, found)
+	// check queue is empty, NOTE: due to encoding, an empty queue (underneath just an int slice) will be decoded as nil slice rather than an empty slice
+	suq, _ := channelKeeper.getSubmittedUpdatesQueue(ctx)
+	assert.Equal(t, SubmittedUpdatesQueue(nil), suq)
+	// check submittedUpdate is gone
+	_, found = channelKeeper.getSubmittedUpdate(ctx, channelID)
+	assert.False(t, found)
+}

internal/x/paychan/keeper.go

@@ -154,23 +154,17 @@ func (k Keeper) applyNewUpdate(existingSUpdate SubmittedUpdate, proposedUpdate U
 
 // unsafe close channel - doesn't check if update matches existing channel TODO make safer?
 func (k Keeper) closeChannel(ctx sdk.Context, update Update) (sdk.Tags, sdk.Error) {
-	var err error
-	var sdkErr sdk.Error
+	var err sdk.Error
 	var tags sdk.Tags
 
 	// Add coins to sender and receiver
 	// TODO check for possible errors first to avoid coins being half paid out?
-	var address sdk.AccAddress
-	for bech32Address, coins := range update.CoinsUpdate {
-		address, err = sdk.AccAddressFromBech32(bech32Address)
+	for _, payout := range update.Payouts {
+		// TODO check somewhere if coins are not negative?
+		_, tags, err = k.coinKeeper.AddCoins(ctx, payout.Address, payout.Coins)
 		if err != nil {
 			panic(err)
 		}
-		// TODO check somewhere if coins are not negative?
-		_, tags, sdkErr = k.coinKeeper.AddCoins(ctx, address, coins)
-		if sdkErr != nil {
-			panic(sdkErr)
-		}
 	}
 
 	k.deleteChannel(ctx, update.ChannelID)
@@ -224,10 +218,10 @@ func (k Keeper) getSubmittedUpdatesQueue(ctx sdk.Context) (SubmittedUpdatesQueue
 	// return
 	return suq, true
 }
-func (k Keeper) setSubmittedUpdatesQueue(ctx sdk.Context, q SubmittedUpdatesQueue) {
+func (k Keeper) setSubmittedUpdatesQueue(ctx sdk.Context, suq SubmittedUpdatesQueue) {
 	store := ctx.KVStore(k.storeKey)
 	// marshal
-	bz := k.cdc.MustMarshalBinary(q)
+	bz := k.cdc.MustMarshalBinary(suq)
 	// write to db
 	key := k.getSubmittedUpdatesQueueKey()
 	store.Set(key, bz)

internal/x/paychan/keeper_test.go

@@ -1,31 +1,153 @@
 package paychan
 
-/*
 import (
-	"testing"
-	//"github.com/stretchr/testify/assert"
-
-	abci "github.com/tendermint/abci/types"
-	dbm "github.com/tendermint/tmlibs/db"
-	"github.com/tendermint/tmlibs/log"
-
-	"github.com/cosmos/cosmos-sdk/store"
 	sdk "github.com/cosmos/cosmos-sdk/types"
-	"github.com/cosmos/cosmos-sdk/wire"
-	"github.com/cosmos/cosmos-sdk/x/auth"
-	"github.com/cosmos/cosmos-sdk/x/bank"
+	"github.com/stretchr/testify/assert"
+	//"github.com/stretchr/testify/require"
+	"github.com/tendermint/tendermint/crypto"
+	"testing"
 )
 
-// GetPaychan
-//  - gets a paychan if it exists, and not if it doesn't
-// setPaychan
-//  - sets a paychan
-// CreatePaychan
-//  - creates a paychan under normal conditions
-// ClosePaychan
-//  - closes a paychan under normal conditions
-// GetPaychans
-// paychanKey
+func TestKeeper(t *testing.T) {
+
+	t.Run("CreateChannel", func(t *testing.T) {
+
+		//
+		////// SETUP
+		// create basic mock app
+		ctx, coinKeeper, channelKeeper, addrs, genAccFunding := createMockApp()
+
+		sender := addrs[0]
+		receiver := addrs[1]
+		coins := sdk.Coins{sdk.NewCoin("KVA", 10)}
+
+		//
+		////// ACTION
+		_, err := channelKeeper.CreateChannel(ctx, sender, receiver, coins)
+
+		//
+		////// CHECK RESULTS
+		assert.Nil(t, err)
+		// channel exists with correct attributes
+		channelID := ChannelID(0) // should be 0 as first channel
+		expectedChan := Channel{
+			ID:           channelID,
+			Participants: [2]sdk.AccAddress{sender, receiver},
+			Coins:        coins,
+		}
+		createdChan, _ := channelKeeper.getChannel(ctx, channelID)
+		assert.Equal(t, expectedChan, createdChan)
+		// check coins deducted from sender
+		assert.Equal(t, genAccFunding.Minus(coins), coinKeeper.GetCoins(ctx, sender))
+		// check no coins deducted from receiver
+		assert.Equal(t, genAccFunding, coinKeeper.GetCoins(ctx, receiver))
+		// check next chan id
+		assert.Equal(t, ChannelID(1), channelKeeper.getNewChannelID(ctx))
+	})
+
+	t.Run("ReceiverCloseChannel", func(t *testing.T) {
+		// SETUP
+		ctx, coinKeeper, channelKeeper, addrs, genAccFunding := createMockApp()
+
+		sender := addrs[0]
+		receiver := addrs[1]
+		coins := sdk.Coins{sdk.NewCoin("KVA", 10)}
+
+		// create new channel
+		channelID := ChannelID(0) // should be 0 as first channel
+		channel := Channel{
+			ID:           channelID,
+			Participants: [2]sdk.AccAddress{sender, receiver},
+			Coins:        coins,
+		}
+		channelKeeper.setChannel(ctx, channel)
+
+		// create closing update
+		payouts := Payouts{
+			{sender, sdk.Coins{sdk.NewCoin("KVA", 3)}},
+			{receiver, sdk.Coins{sdk.NewCoin("KVA", 7)}},
+		}
+		update := Update{
+			ChannelID: channelID,
+			Payouts:   payouts,
+			Sequence:  0,
+			Sigs:      [1]crypto.Signature{},
+		}
+		// Set empty submittedUpdatesQueue TODO work out proper genesis initialisation
+		channelKeeper.setSubmittedUpdatesQueue(ctx, SubmittedUpdatesQueue{})
+
+		// ACTION
+		_, err := channelKeeper.CloseChannelByReceiver(ctx, update)
+
+		// CHECK RESULTS
+		// no error
+		assert.Nil(t, err)
+		// coins paid out
+		senderPayout, _ := payouts.Get(sender)
+		assert.Equal(t, genAccFunding.Plus(senderPayout), coinKeeper.GetCoins(ctx, sender))
+		receiverPayout, _ := payouts.Get(receiver)
+		assert.Equal(t, genAccFunding.Plus(receiverPayout), coinKeeper.GetCoins(ctx, receiver))
+		// channel deleted
+		_, found := channelKeeper.getChannel(ctx, channelID)
+		assert.False(t, found)
+
+	})
+
+	t.Run("SenderInitCloseChannel", func(t *testing.T) {
+		// SETUP
+		ctx, _, channelKeeper, addrs, _ := createMockApp()
+
+		sender := addrs[0]
+		receiver := addrs[1]
+		coins := sdk.Coins{sdk.NewCoin("KVA", 10)}
+
+		// create new channel
+		channelID := ChannelID(0) // should be 0 as first channel
+		channel := Channel{
+			ID:           channelID,
+			Participants: [2]sdk.AccAddress{sender, receiver},
+			Coins:        coins,
+		}
+		channelKeeper.setChannel(ctx, channel)
+
+		// create closing update
+		payouts := Payouts{
+			{sender, sdk.Coins{sdk.NewCoin("KVA", 3)}},
+			{receiver, sdk.Coins{sdk.NewCoin("KVA", 7)}},
+		}
+		update := Update{
+			ChannelID: channelID,
+			Payouts:   payouts,
+			Sequence:  0,
+			Sigs:      [1]crypto.Signature{},
+		}
+		// Set empty submittedUpdatesQueue TODO work out proper genesis initialisation
+		channelKeeper.setSubmittedUpdatesQueue(ctx, SubmittedUpdatesQueue{})
+
+		// ACTION
+		_, err := channelKeeper.InitCloseChannelBySender(ctx, update)
+
+		// CHECK RESULTS
+		// no error
+		assert.Nil(t, err)
+		// submittedupdate in queue and correct
+		suq, found := channelKeeper.getSubmittedUpdatesQueue(ctx)
+		assert.True(t, found)
+		assert.True(t, suq.Contains(channelID))
+
+		su, found := channelKeeper.getSubmittedUpdate(ctx, channelID)
+		assert.True(t, found)
+		expectedSubmittedUpdate := SubmittedUpdate{
+			Update:        update,
+			ExecutionTime: ChannelDisputeTime,
+		}
+		assert.Equal(t, expectedSubmittedUpdate, su)
+		// TODO check channel is still in db and coins haven't changed?
+	})
+
+}
+
+/*
 
 func setupMultiStore() (sdk.MultiStore, *sdk.KVStoreKey, *sdk.KVStoreKey) {
 	// create db

internal/x/paychan/test_common.go

@@ -0,0 +1,38 @@
+package paychan
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	"github.com/cosmos/cosmos-sdk/x/mock"
+	//"github.com/stretchr/testify/require"
+	"github.com/cosmos/cosmos-sdk/x/bank"
+	abci "github.com/tendermint/tendermint/abci/types"
+)
+
+// Setup an example app with an in memory DB and the required keepers
+// Also create two accounts with 1000KVA
+// Could do with refactoring
+func createMockApp() (sdk.Context, bank.Keeper, Keeper, []sdk.AccAddress, sdk.Coins) {
+	mApp := mock.NewApp() // creates a half complete app
+	coinKeeper := bank.NewKeeper(mApp.AccountMapper)
+
+	// create channel keeper
+	keyChannel := sdk.NewKVStoreKey("channel")
+	channelKeeper := NewKeeper(mApp.Cdc, keyChannel, coinKeeper)
+	// add router?
+	//mapp.Router().AddRoute("channel", NewHandler(channelKeeper))
+
+	mApp.CompleteSetup([]*sdk.KVStoreKey{keyChannel}) // needs to be called I think to finish setup
+
+	// create some accounts
+	numGenAccs := 2 // create two initial accounts
+	genAccFunding := sdk.Coins{sdk.NewCoin("KVA", 1000)}
+	genAccs, addrs, _, _ := mock.CreateGenAccounts(numGenAccs, genAccFunding)
+
+	// initialize the app with these accounts
+	mock.SetGenesis(mApp, genAccs)
+
+	mApp.BeginBlock(abci.RequestBeginBlock{}) // going off other module tests
+	ctx := mApp.BaseApp.NewContext(false, abci.Header{})
+
+	return ctx, coinKeeper, channelKeeper, addrs, genAccFunding
+}

internal/x/paychan/types.go

@@ -3,6 +3,7 @@ package paychan
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/tendermint/tendermint/crypto"
+	"reflect"
 )
 
 /*  CHANNEL TYPES  */
@@ -20,13 +21,28 @@ type ChannelID int64 // TODO should this be positive only
 
 // The data that is passed between participants as payments, and submitted to the blockchain to close a channel.
 type Update struct {
-	ChannelID   ChannelID
-	CoinsUpdate map[string]sdk.Coins // map of bech32 addresses to coins
-	Sequence    int64
-	Sigs        [1]{crypto.Signature} // only sender needs to sign
+	ChannelID ChannelID
+	Payouts   Payouts //map[string]sdk.Coins // map of bech32 addresses to coins
+	Sequence  int64
+	Sigs      [1]crypto.Signature // only sender needs to sign
+}
+type Payout struct {
+	Address sdk.AccAddress
+	Coins   sdk.Coins
+}
+type Payouts []Payout
+
+// Get the coins associated with payout address. TODO constrain payouts to only have one entry per address
+func (payouts Payouts) Get(addr sdk.AccAddress) (sdk.Coins, bool) {
+	for _, p := range payouts {
+		if reflect.DeepEqual(p.Address, addr) {
+			return p.Coins, true
+		}
+	}
+	return nil, false
 }
 
-var ChannelDisputeTime = int64(2000) // measured in blocks TODO pick reasonable time
+const ChannelDisputeTime = int64(2000) // measured in blocks TODO pick reasonable time
 
 // An update that has been submitted to the blockchain, but not yet acted on.
 type SubmittedUpdate struct {
@@ -49,15 +65,15 @@ func (suq SubmittedUpdatesQueue) Contains(channelID ChannelID) bool {
 }
 
 // Remove all values from queue that match argument
-func (suq SubmittedUpdatesQueue) RemoveMatchingElements(channelID ChannelID) {
+func (suq *SubmittedUpdatesQueue) RemoveMatchingElements(channelID ChannelID) {
 	newSUQ := SubmittedUpdatesQueue{}
 
-	for _, id := range suq {
+	for _, id := range *suq {
 		if id != channelID {
 			newSUQ = append(newSUQ, id)
 		}
 	}
-	suq = newSUQ
+	*suq = newSUQ
 }
 
 /*  MESSAGE TYPES  */

internal/x/paychan/types_test.go

@@ -0,0 +1,26 @@
+package paychan
+
+import (
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestSubmittedUpdatesQueue(t *testing.T) {
+	t.Run("RemoveMatchingElements", func(t *testing.T) {
+		// SETUP
+		q := SubmittedUpdatesQueue{4, 8, 23, 0, 5645657}
+		// ACTION
+		q.RemoveMatchingElements(23)
+		// CHECK RESULTS
+		expectedQ := SubmittedUpdatesQueue{4, 8, 0, 5645657}
+		assert.Equal(t, expectedQ, q)
+
+		// SETUP
+		q = SubmittedUpdatesQueue{0}
+		// ACTION
+		q.RemoveMatchingElements(0)
+		// CHECK RESULTS
+		expectedQ = SubmittedUpdatesQueue{}
+		assert.Equal(t, expectedQ, q)
+	})
+}