add usage instructions

internal/x/paychan/README.md

@@ -1,12 +1,40 @@
-Payment channel implementation sketch
+# Unidrectional Payment Channels
 
-Simplifications:
+This module implements simple but feature complete unidirectional payment channels. Channels can be opened by a sender and closed immediately by the receiver, or by the sender subject to a dispute period. There are no top-ups or partial withdrawals (yet). Channels support multiple currencies.
 
- - unidirectional paychans
+>Note: This is a work in progress. More feature planned. More test cases needed.
- - no top ups or partial withdrawals (only opening and closing)
+
+# Usage
+
+## Create a channel
+
+	kvcli paychan create --from <your account name> --to <receivers address> --amount 100KVA --chain-id <your chain ID>
+
+## Send an off-chain payment
+Send a payment for 10 KVA.
+
+	kvcli paychan pay --from <your account name> --sen-amt 90KVA --rec-amt 10KVA --chan-id <ID of channel> --filename payment.json --chain-id <your chain ID>
+
+Send the file payment.json to your receiver. Then they run the following to verify.
+
+	kvcli paychan verify --filename payment.json
+
+## Close a channel
+The receiver can close immediately at any time.
+
+	kvcli paychan submit --from <receiver's account name> --payment payment.json --chain-id <your chain ID>
+
+The sender can close subject to a dispute period during which the receiver can overrule them.
+
+	kvcli paychan submit --from <receiver's account name> --payment payment.json --chain-id <your chain ID>
+
+## Get info on a channel
+
+	kvcli get --chan-id <ID of channel>
 
 
- TODO
+# TODOs
+
  - in code TODOs
  - Tidy up - method descriptions, heading comments, remove uneccessary comments, README/docs
  - Find a better name for Queue - clarify distinction between int slice and abstract queue concept
@@ -22,5 +50,5 @@ Simplifications:
  - use custom errors instead of using sdk.ErrInternal
  - split off signatures from update as with txs/msgs - testing easier, code easier to use, doesn't store sigs unecessarily on chain
  - consider removing pubKey from UpdateSignature - instead let channel module access accountMapper
- - remove printout from tests when app initialised
  - refactor queue into one object
+ - remove printout during tests caused by mock app initialisation

internal/x/paychan/client/rest/channel-watcher.go

@@ -2,6 +2,6 @@ package lcd
 
 import ()
 
-// implement thing that polls blockchain and handles paychan disputes
+// implement a thing to poll blockchain and handles paychan disputes
 // needs plugged into LCD - add a "background processes" slot in the LCD run function?
-// eventually LCD evolves into paychan (network) daemon
+// eventually LCD could evolve into paychan (network) daemon

internal/x/paychan/endblocker.go

@@ -10,7 +10,7 @@ func EndBlocker(ctx sdk.Context, k Keeper) sdk.Tags {
 	tags := sdk.EmptyTags()
 
 	// Iterate through submittedUpdatesQueue
-	// TODO optimise so it doesn't pull every update from DB every block
+	// TODO optimise so it doesn't pull every channel update from DB every block
 	q := k.getSubmittedUpdatesQueue(ctx)
 	var sUpdate SubmittedUpdate
 	var found bool
@@ -31,6 +31,5 @@ func EndBlocker(ctx sdk.Context, k Keeper) sdk.Tags {
 			tags.AppendTags(channelTags)
 		}
 	}
-
 	return tags
 }

internal/x/paychan/keeper.go

@@ -11,12 +11,13 @@ import (
 // Keeper of the paychan store
 // Handles validation internally. Does not rely on calling code to do validation.
 // Aim to keep public methods safe, private ones not necessaily.
+// Keepers contain main business logic of the module.
 type Keeper struct {
 	storeKey   sdk.StoreKey
 	cdc        *wire.Codec // needed to serialize objects before putting them in the store
 	coinKeeper bank.Keeper
 
-	//codespace sdk.CodespaceType
+	//codespace sdk.CodespaceType TODO custom errors
 }
 
 // Called when creating new app.
@@ -30,12 +31,10 @@ func NewKeeper(cdc *wire.Codec, key sdk.StoreKey, ck bank.Keeper) Keeper {
 	return keeper
 }
 
-// ============================================== Main Business Logic
-
 // Create a new payment channel and lock up sender funds.
 func (k Keeper) CreateChannel(ctx sdk.Context, sender sdk.AccAddress, receiver sdk.AccAddress, coins sdk.Coins) (sdk.Tags, sdk.Error) {
 
-	// Check addresses valid (Technicaly don't need to check sender address is valid as SubtractCoins does that)
+	// Check addresses valid (Technicaly don't need to check sender address is valid as SubtractCoins checks)
 	if len(sender) == 0 {
 		return nil, sdk.ErrInvalidAddress(sender.String())
 	}
@@ -71,6 +70,7 @@ func (k Keeper) CreateChannel(ctx sdk.Context, sender sdk.AccAddress, receiver s
 	return tags, err
 }
 
+// Initiate the close of a payment channel, subject to dispute period.
 func (k Keeper) InitCloseChannelBySender(ctx sdk.Context, update Update) (sdk.Tags, sdk.Error) {
 	// This is roughly the default path for non unidirectional channels
 
@@ -87,6 +87,7 @@ func (k Keeper) InitCloseChannelBySender(ctx sdk.Context, update Update) (sdk.Ta
 	q := k.getSubmittedUpdatesQueue(ctx)
 	if q.Contains(update.ChannelID) {
 		// Someone has previously tried to update channel
+
 		// In bidirectional channels the new update is compared against existing and replaces it if it has a higher sequence number.
 
 		// existingSUpdate, found := k.getSubmittedUpdate(ctx, update.ChannelID)
@@ -97,7 +98,7 @@ func (k Keeper) InitCloseChannelBySender(ctx sdk.Context, update Update) (sdk.Ta
 
 		// However in unidirectional case, only the sender can close a channel this way. No clear need for them to be able to submit an update replacing a previous one they sent, so don't allow it.
 		// TODO tags
-		// TODO custom errors return sdk.EmptyTags(), sdk.NewError("Sender can't submit an update for channel if one has already been submitted.")
+		// TODO custom errors
 		sdk.ErrInternal("Sender can't submit an update for channel if one has already been submitted.")
 	} else {
 		// No one has tried to update channel
@@ -113,6 +114,7 @@ func (k Keeper) InitCloseChannelBySender(ctx sdk.Context, update Update) (sdk.Ta
 	return tags, nil
 }
 
+// Immediately close a channel.
 func (k Keeper) CloseChannelByReceiver(ctx sdk.Context, update Update) (sdk.Tags, sdk.Error) {
 
 	// get the channel
@@ -138,8 +140,7 @@ func (k Keeper) CloseChannelByReceiver(ctx sdk.Context, update Update) (sdk.Tags
 }
 
 // Main function that compare updates against each other.
-// Pure function
+// Pure function, Not needed in unidirectional case.
-// Not needed in unidirectional case.
 // func (k Keeper) applyNewUpdate(existingSUpdate SubmittedUpdate, proposedUpdate Update) SubmittedUpdate {
 // 	var returnUpdate SubmittedUpdate
 
@@ -183,7 +184,8 @@ func VerifyUpdate(channel Channel, update Update) sdk.Error {
 	return nil
 }
 
-// unsafe close channel - doesn't check if update matches existing channel TODO make safer?
+// 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 sdk.Error
 	var tags sdk.Tags
@@ -194,7 +196,6 @@ func (k Keeper) closeChannel(ctx sdk.Context, update Update) (sdk.Tags, sdk.Erro
 	// Add coins to sender and receiver
 	// TODO check for possible errors first to avoid coins being half paid out?
 	for i, coins := range update.Payout {
-		// TODO check somewhere if coins are not negative?
 		_, tags, err = k.coinKeeper.AddCoins(ctx, channel.Participants[i], coins)
 		if err != nil {
 			panic(err)
@@ -223,7 +224,7 @@ func verifySignatures(channel Channel, update Update) bool {
 
 }
 
-// =========================================== QUEUE
+// =========================================== SUBMITTED UPDATES QUEUE
 
 func (k Keeper) addToSubmittedUpdatesQueue(ctx sdk.Context, sUpdate SubmittedUpdate) {
 	// always overwrite prexisting values - leave paychan logic to higher levels

internal/x/paychan/keeper_test.go

@@ -3,8 +3,6 @@ package paychan
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/stretchr/testify/assert"
-	//"github.com/stretchr/testify/require"
-	//"github.com/tendermint/tendermint/crypto"
 	"testing"
 )
 
@@ -315,129 +313,3 @@ func TestKeeper(t *testing.T) {
 	})
 
 }
-
-/*
-func privAndAddr() (crypto.PrivKey, sdk.AccAddress) {
-	priv := ed25519.GenPrivKey()
-	addr := sdk.AccAddress(priv.PubKey().Address())
-	return priv, addr
-
-sig, err := priv.Sign(signBytes)
-*/
-
-/*
-
-func setupMultiStore() (sdk.MultiStore, *sdk.KVStoreKey, *sdk.KVStoreKey) {
-	// create db
-	db := dbm.NewMemDB()
-	// create keys
-	authKey := sdk.NewKVStoreKey("authkey")
-	paychanKey := sdk.NewKVStoreKey("paychankey")
-	// create new multistore around db
-	ms := store.NewCommitMultiStore(db) // DB handle plus store key maps
-	// register separate stores in the multistore
-	ms.MountStoreWithDB(authKey, sdk.StoreTypeIAVL, db) // sets store key map
-	ms.MountStoreWithDB(paychanKey, sdk.StoreTypeIAVL, db)
-	ms.LoadLatestVersion()
-
-	return ms, authKey, paychanKey
-}
-
-func setupCodec() *wire.Codec {
-	cdc := wire.NewCodec()
-	auth.RegisterBaseAccount(cdc)
-	// TODO might need to register paychan struct
-	return cdc
-}
-
-func TestKeeper(t *testing.T) {
-	// Setup
-
-	// create multistore and key
-	ms, authKey, paychanKey := setupMultiStore()
-
-	// create and initialise codec(s)
-	cdc := setupCodec()
-
-	// create context
-	ctx := sdk.NewContext(ms, abci.Header{}, false, nil, log.NewNopLogger())
-
-	// create accountMapper
-	accountMapper := auth.NewAccountMapper(cdc, authKey, &auth.BaseAccount{})
-
-	// create coinkeeper
-	coinKeeper := bank.NewKeeper(accountMapper)
-
-	// create keeper
-	paychanKeeper := NewKeeper(cdc, paychanKey, coinKeeper)
-
-	// Test no paychans exist
-	_, exists := paychanKeeper.GetPaychan(ctx, sdk.Address{}, sdk.Address{}, 0)
-	if exists {
-		t.Error("payment channel found when none exist")
-	}
-
-	// Test paychan can be set and get
-	p := Paychan{
-		Sender:   sdk.Address([]byte("senderAddress")),
-		Receiver: sdk.Address([]byte("receiverAddress")),
-		Id:       0,
-		Balance:  sdk.Coins{{"KVA", 100}},
-	}
-	paychanKeeper.setPaychan(ctx, p)
-
-	_, exists = paychanKeeper.GetPaychan(ctx, p.Sender, p.Receiver, p.Id)
-	if !exists {
-		t.Error("payment channel not found")
-	}
-
-	// Test create paychan under normal conditions
-	senderAddress := sdk.Address([]byte("senderAddress"))
-	senderFunds := sdk.Coins{{"KVA", 100}}
-	receiverAddress := sdk.Address([]byte("receiverAddress"))
-	balance := sdk.Coins{{"KVA", 10}}
-
-	coinKeeper.SetCoins(ctx, senderAddress, senderFunds)
-
-	_, err := paychanKeeper.CreatePaychan(ctx, senderAddress, receiverAddress, balance)
-	if err != nil {
-		t.Error("unexpected error created payment channel", err)
-	}
-
-	p, exists = paychanKeeper.GetPaychan(ctx, senderAddress, receiverAddress, 0)
-	if !exists {
-		t.Error("payment channel missing")
-	}
-	if !p.Balance.IsEqual(balance) {
-		t.Error("payment channel balance incorrect", p.Balance, balance)
-	}
-	expectedNewSenderFunds := senderFunds.Minus(balance)
-	if !coinKeeper.GetCoins(ctx, senderAddress).IsEqual(expectedNewSenderFunds) {
-		t.Error("sender has incorrect balance after paychan creation")
-	}
-
-	// Test close paychan under normal conditions
-	senderFunds = coinKeeper.GetCoins(ctx, senderAddress)
-	receiverAmount := sdk.Coins{{"KVA", 9}}
-	_, err = paychanKeeper.ClosePaychan(ctx, senderAddress, receiverAddress, 0, receiverAmount)
-	if err != nil {
-		t.Error("unexpected error closing payment channel", err)
-	}
-	// paychan shouldn't exist
-	_, exists = paychanKeeper.GetPaychan(ctx, senderAddress, receiverAddress, 0)
-	if exists {
-		t.Error("payment channel should not exist")
-	}
-	// sender's funds should have increased
-	expectedNewSenderFunds = senderFunds.Plus(balance.Minus(receiverAmount))
-	if !coinKeeper.GetCoins(ctx, senderAddress).IsEqual(expectedNewSenderFunds) {
-		t.Error("sender has incorrect balance after paychan creation", expectedNewSenderFunds)
-	}
-	// receiver's funds should have increased
-	expectedNewReceiverFunds := receiverAmount // started at zero
-	if !coinKeeper.GetCoins(ctx, receiverAddress).IsEqual(expectedNewReceiverFunds) {
-		t.Error("receiver has incorrect balance after paychan creation")
-	}
-
-}
-*/

internal/x/paychan/test_common.go

@@ -21,10 +21,10 @@ func createMockApp(accountSeeds []string) (sdk.Context, bank.Keeper, Keeper, []s
 	// create channel keeper
 	keyChannel := sdk.NewKVStoreKey("channel")
 	channelKeeper := NewKeeper(mApp.Cdc, keyChannel, coinKeeper)
-	// add router?
+	// could add router for msg tests
 	//mapp.Router().AddRoute("channel", NewHandler(channelKeeper))
 
-	mApp.CompleteSetup([]*sdk.KVStoreKey{keyChannel}) // needs to be called I think to finish setup
+	mApp.CompleteSetup([]*sdk.KVStoreKey{keyChannel})
 
 	// create some accounts
 	genAccFunding := sdk.Coins{sdk.NewCoin("KVA", 1000)}
@@ -57,6 +57,5 @@ func createTestGenAccounts(accountSeeds []string, genCoins sdk.Coins) (genAccs [
 		pubKeys = append(pubKeys, pubKey)
 		addrs = append(addrs, addr)
 	}
-
 	return
 }

internal/x/paychan/types.go

@@ -16,12 +16,14 @@ type Channel struct {
 	Coins        sdk.Coins
 }
 
-type ChannelID int64 // TODO should this be positive only
+const ChannelDisputeTime = int64(2000) // measured in blocks TODO pick reasonable time
+
+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
-	Payout    Payout //map[string]sdk.Coins // map of bech32 addresses to coins
+	Payout    Payout
 	//Sequence  int64 Not needed for unidirectional channels
 	Sigs [1]UpdateSignature // only sender needs to sign in unidirectional
 }
@@ -40,11 +42,6 @@ func (u Update) GetSignBytes() []byte {
 	return sdk.MustSortJSON(bz)
 }
 
-type UpdateSignature struct {
-	PubKey          crypto.PubKey
-	CryptoSignature crypto.Signature
-}
-
 type Payout [2]sdk.Coins // a list of coins to be paid to each of Channel.Participants
 func (p Payout) IsNotNegative() bool {
 	result := true
@@ -62,17 +59,10 @@ func (p Payout) Sum() sdk.Coins {
 	return total
 }
 
-// Get the coins associated with payout address. TODO constrain payouts to only have one entry per address
+type UpdateSignature struct {
-/*func (payouts Payouts) Get(addr sdk.AccAddress) (sdk.Coins, bool) {
+	PubKey          crypto.PubKey
-	for _, p := range payouts {
+	CryptoSignature crypto.Signature
-		if reflect.DeepEqual(p.Address, addr) {
+}
-			return p.Coins, true
-		}
-	}
-	return nil, false
-}*/
-
-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 {
@@ -135,19 +125,6 @@ type MsgCreate struct {
 	Coins        sdk.Coins
 }
 
-//Create a new message.
-/*
-Called in client code when constructing transaction from cli args to send to the network.
-maybe just a placeholder for more advanced future functionality?
-func (msg CreatMsg) NewMsgCreate(sender sdk.Address, receiver sdk.Address, amount sdk.Coins) MsgCreate {
-	return MsgCreate{
-		sender
-		receiver
-		amount
-	}
-}
-*/
-
 func (msg MsgCreate) Type() string { return "paychan" }
 
 func (msg MsgCreate) GetSignBytes() []byte {
@@ -164,25 +141,25 @@ func (msg MsgCreate) ValidateBasic() sdk.Error {
 
 	//TODO implement
 
-	/*
+	/* old logic
-		// check if all fields present / not 0 valued
+	// check if all fields present / not 0 valued
-		if len(msg.Sender) == 0 {
+	if len(msg.Sender) == 0 {
-			return sdk.ErrInvalidAddress(msg.Sender.String())
+		return sdk.ErrInvalidAddress(msg.Sender.String())
-		}
+	}
-		if len(msg.Receiver) == 0 {
+	if len(msg.Receiver) == 0 {
-			return sdk.ErrInvalidAddress(msg.Receiver.String())
+		return sdk.ErrInvalidAddress(msg.Receiver.String())
-		}
+	}
-		if len(msg.Amount) == 0 {
+	if len(msg.Amount) == 0 {
-			return sdk.ErrInvalidCoins(msg.Amount.String())
+		return sdk.ErrInvalidCoins(msg.Amount.String())
-		}
+	}
-		// Check if coins are sorted, non zero, non negative
+	// Check if coins are sorted, non zero, non negative
-		if !msg.Amount.IsValid() {
+	if !msg.Amount.IsValid() {
-			return sdk.ErrInvalidCoins(msg.Amount.String())
+		return sdk.ErrInvalidCoins(msg.Amount.String())
-		}
+	}
-		if !msg.Amount.IsPositive() {
+	if !msg.Amount.IsPositive() {
-			return sdk.ErrInvalidCoins(msg.Amount.String())
+		return sdk.ErrInvalidCoins(msg.Amount.String())
-		}
+	}
-		// TODO check if Address valid?
+	// TODO check if Address valid?
 	*/
 	return nil
 }
@@ -198,12 +175,6 @@ type MsgSubmitUpdate struct {
 	Submitter sdk.AccAddress
 }
 
-// func (msg MsgSubmitUpdate) NewMsgSubmitUpdate(update Update) MsgSubmitUpdate {
-// 	return MsgSubmitUpdate{
-// 		update
-// 	}
-// }
-
 func (msg MsgSubmitUpdate) Type() string { return "paychan" }
 
 func (msg MsgSubmitUpdate) GetSignBytes() []byte {
@@ -217,35 +188,33 @@ func (msg MsgSubmitUpdate) GetSignBytes() []byte {
 func (msg MsgSubmitUpdate) ValidateBasic() sdk.Error {
 
 	// TODO implement
-	/*
+	/* old logic
-		// check if all fields present / not 0 valued
+	// check if all fields present / not 0 valued
-		if len(msg.Sender) == 0 {
+	if len(msg.Sender) == 0 {
-			return sdk.ErrInvalidAddress(msg.Sender.String())
+		return sdk.ErrInvalidAddress(msg.Sender.String())
-		}
+	}
-		if len(msg.Receiver) == 0 {
+	if len(msg.Receiver) == 0 {
-			return sdk.ErrInvalidAddress(msg.Receiver.String())
+		return sdk.ErrInvalidAddress(msg.Receiver.String())
-		}
+	}
-		if len(msg.ReceiverAmount) == 0 {
+	if len(msg.ReceiverAmount) == 0 {
-			return sdk.ErrInvalidCoins(msg.ReceiverAmount.String())
+		return sdk.ErrInvalidCoins(msg.ReceiverAmount.String())
-		}
+	}
-		// check id ≥ 0
+	// check id ≥ 0
-		if msg.Id < 0 {
+	if msg.Id < 0 {
-			return sdk.ErrInvalidAddress(strconv.Itoa(int(msg.Id))) // TODO implement custom errors
+		return sdk.ErrInvalidAddress(strconv.Itoa(int(msg.Id))) // TODO implement custom errors
-		}
+	}
-		// Check if coins are sorted, non zero, non negative
+	// Check if coins are sorted, non zero, non negative
-		if !msg.ReceiverAmount.IsValid() {
+	if !msg.ReceiverAmount.IsValid() {
-			return sdk.ErrInvalidCoins(msg.ReceiverAmount.String())
+		return sdk.ErrInvalidCoins(msg.ReceiverAmount.String())
-		}
+	}
-		if !msg.ReceiverAmount.IsPositive() {
+	if !msg.ReceiverAmount.IsPositive() {
-			return sdk.ErrInvalidCoins(msg.ReceiverAmount.String())
+		return sdk.ErrInvalidCoins(msg.ReceiverAmount.String())
-		}
+	}
-		// TODO check if Address valid?
+	// TODO check if Address valid?
 	*/
 	return nil
 }
 
 func (msg MsgSubmitUpdate) GetSigners() []sdk.AccAddress {
-	// Signing not strictly necessary as signatures contained within the channel update.
-	// TODO add signature by submitting address
 	return []sdk.AccAddress{msg.Submitter}
 }

internal/x/paychan/wire.go

@@ -9,7 +9,7 @@ func RegisterWire(cdc *wire.Codec) {
 	cdc.RegisterConcrete(MsgSubmitUpdate{}, "paychan/MsgSubmitUpdate", nil)
 }
 
-// TODO move this to near the msg definitions
+// TODO move this to near the msg definitions?
 var msgCdc = wire.NewCodec()
 
 func init() {