From 894509f1dab5fa6a3e1cdc29262192c04b59d297 Mon Sep 17 00:00:00 2001 From: rhuairahrighairigh Date: Sat, 1 Sep 2018 22:57:42 -0400 Subject: [PATCH] add usage instructions --- internal/x/paychan/README.md | 40 +++++- .../x/paychan/client/rest/channel-watcher.go | 4 +- internal/x/paychan/endblocker.go | 3 +- internal/x/paychan/keeper.go | 21 +-- internal/x/paychan/keeper_test.go | 128 ----------------- internal/x/paychan/test_common.go | 5 +- internal/x/paychan/types.go | 131 +++++++----------- internal/x/paychan/wire.go | 2 +- 8 files changed, 101 insertions(+), 233 deletions(-) diff --git a/internal/x/paychan/README.md b/internal/x/paychan/README.md index c380ba21..720832e2 100644 --- a/internal/x/paychan/README.md +++ b/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 - - no top ups or partial withdrawals (only opening and closing) +>Note: This is a work in progress. More feature planned. More test cases needed. + +# Usage + +## Create a channel + + kvcli paychan create --from --to --amount 100KVA --chain-id + +## Send an off-chain payment +Send a payment for 10 KVA. + + kvcli paychan pay --from --sen-amt 90KVA --rec-amt 10KVA --chan-id --filename payment.json --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 --payment payment.json --chain-id + +The sender can close subject to a dispute period during which the receiver can overrule them. + + kvcli paychan submit --from --payment payment.json --chain-id + +## Get info on a channel + + kvcli get --chan-id - 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 diff --git a/internal/x/paychan/client/rest/channel-watcher.go b/internal/x/paychan/client/rest/channel-watcher.go index d00c7f51..3f4d1c58 100644 --- a/internal/x/paychan/client/rest/channel-watcher.go +++ b/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 diff --git a/internal/x/paychan/endblocker.go b/internal/x/paychan/endblocker.go index d51a0c66..f56f5083 100644 --- a/internal/x/paychan/endblocker.go +++ b/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 } diff --git a/internal/x/paychan/keeper.go b/internal/x/paychan/keeper.go index af648315..47bb90d1 100644 --- a/internal/x/paychan/keeper.go +++ b/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 -// Not needed in unidirectional case. +// Pure function, 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 diff --git a/internal/x/paychan/keeper_test.go b/internal/x/paychan/keeper_test.go index 7435130b..145a4f28 100644 --- a/internal/x/paychan/keeper_test.go +++ b/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") - } - -} -*/ diff --git a/internal/x/paychan/test_common.go b/internal/x/paychan/test_common.go index ca9940f4..d102b27f 100644 --- a/internal/x/paychan/test_common.go +++ b/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 } diff --git a/internal/x/paychan/types.go b/internal/x/paychan/types.go index a1135282..76704ca7 100644 --- a/internal/x/paychan/types.go +++ b/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 -/*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 -}*/ - -const ChannelDisputeTime = int64(2000) // measured in blocks TODO pick reasonable time +type UpdateSignature struct { + PubKey crypto.PubKey + CryptoSignature crypto.Signature +} // 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 - /* - // check if all fields present / not 0 valued - if len(msg.Sender) == 0 { - return sdk.ErrInvalidAddress(msg.Sender.String()) - } - if len(msg.Receiver) == 0 { - return sdk.ErrInvalidAddress(msg.Receiver.String()) - } - if len(msg.Amount) == 0 { - return sdk.ErrInvalidCoins(msg.Amount.String()) - } - // Check if coins are sorted, non zero, non negative - if !msg.Amount.IsValid() { - return sdk.ErrInvalidCoins(msg.Amount.String()) - } - if !msg.Amount.IsPositive() { - return sdk.ErrInvalidCoins(msg.Amount.String()) - } - // TODO check if Address valid? + /* old logic + // check if all fields present / not 0 valued + if len(msg.Sender) == 0 { + return sdk.ErrInvalidAddress(msg.Sender.String()) + } + if len(msg.Receiver) == 0 { + return sdk.ErrInvalidAddress(msg.Receiver.String()) + } + if len(msg.Amount) == 0 { + return sdk.ErrInvalidCoins(msg.Amount.String()) + } + // Check if coins are sorted, non zero, non negative + if !msg.Amount.IsValid() { + return sdk.ErrInvalidCoins(msg.Amount.String()) + } + if !msg.Amount.IsPositive() { + return sdk.ErrInvalidCoins(msg.Amount.String()) + } + // 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 - /* - // check if all fields present / not 0 valued - if len(msg.Sender) == 0 { - return sdk.ErrInvalidAddress(msg.Sender.String()) - } - if len(msg.Receiver) == 0 { - return sdk.ErrInvalidAddress(msg.Receiver.String()) - } - if len(msg.ReceiverAmount) == 0 { - return sdk.ErrInvalidCoins(msg.ReceiverAmount.String()) - } - // check id ≥ 0 - if msg.Id < 0 { - return sdk.ErrInvalidAddress(strconv.Itoa(int(msg.Id))) // TODO implement custom errors - } - // Check if coins are sorted, non zero, non negative - if !msg.ReceiverAmount.IsValid() { - return sdk.ErrInvalidCoins(msg.ReceiverAmount.String()) - } - if !msg.ReceiverAmount.IsPositive() { - return sdk.ErrInvalidCoins(msg.ReceiverAmount.String()) - } - // TODO check if Address valid? + /* old logic + // check if all fields present / not 0 valued + if len(msg.Sender) == 0 { + return sdk.ErrInvalidAddress(msg.Sender.String()) + } + if len(msg.Receiver) == 0 { + return sdk.ErrInvalidAddress(msg.Receiver.String()) + } + if len(msg.ReceiverAmount) == 0 { + return sdk.ErrInvalidCoins(msg.ReceiverAmount.String()) + } + // check id ≥ 0 + if msg.Id < 0 { + return sdk.ErrInvalidAddress(strconv.Itoa(int(msg.Id))) // TODO implement custom errors + } + // Check if coins are sorted, non zero, non negative + if !msg.ReceiverAmount.IsValid() { + return sdk.ErrInvalidCoins(msg.ReceiverAmount.String()) + } + if !msg.ReceiverAmount.IsPositive() { + return sdk.ErrInvalidCoins(msg.ReceiverAmount.String()) + } + // 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} } diff --git a/internal/x/paychan/wire.go b/internal/x/paychan/wire.go index ee3da559..d8a745aa 100644 --- a/internal/x/paychan/wire.go +++ b/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() {