From ec105e88ad410c40e0d6ef8657900e9a6fa502d5 Mon Sep 17 00:00:00 2001 From: rhuairahrighairigh Date: Thu, 30 Aug 2018 13:43:15 -0400 Subject: [PATCH] remove unnecessary bidirectional features --- internal/x/paychan/README.md | 2 +- internal/x/paychan/endblocker_test.go | 1 - internal/x/paychan/keeper.go | 46 ++++++++++++++++----------- internal/x/paychan/keeper_test.go | 2 -- internal/x/paychan/types.go | 4 +-- 5 files changed, 30 insertions(+), 25 deletions(-) diff --git a/internal/x/paychan/README.md b/internal/x/paychan/README.md index 7cb74683..80a3492e 100644 --- a/internal/x/paychan/README.md +++ b/internal/x/paychan/README.md @@ -12,8 +12,8 @@ Simplifications: - 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, 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 + - tags - return channel id diff --git a/internal/x/paychan/endblocker_test.go b/internal/x/paychan/endblocker_test.go index 079cb7e7..d6e1d5dd 100644 --- a/internal/x/paychan/endblocker_test.go +++ b/internal/x/paychan/endblocker_test.go @@ -31,7 +31,6 @@ func TestEndBlocker(t *testing.T) { update := Update{ ChannelID: channelID, Payouts: payouts, - Sequence: 0, Sigs: [1]crypto.Signature{}, } sUpdate := SubmittedUpdate{ diff --git a/internal/x/paychan/keeper.go b/internal/x/paychan/keeper.go index d8520fb1..02c24e29 100644 --- a/internal/x/paychan/keeper.go +++ b/internal/x/paychan/keeper.go @@ -97,11 +97,18 @@ func (k Keeper) InitCloseChannelBySender(ctx sdk.Context, update Update) (sdk.Ta } if q.Contains(update.ChannelID) { // Someone has previously tried to update channel - existingSUpdate, found := k.getSubmittedUpdate(ctx, update.ChannelID) - if !found { - panic("can't find element in queue that should exist") - } - k.addToSubmittedUpdatesQueue(ctx, k.applyNewUpdate(existingSUpdate, update)) + // 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) + // if !found { + // panic("can't find element in queue that should exist") + // } + // k.addToSubmittedUpdatesQueue(ctx, k.applyNewUpdate(existingSUpdate, update)) + + // 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.") + panic("Sender can't submit an update for channel if one has already been submitted.") } else { // No one has tried to update channel submittedUpdate := SubmittedUpdate{ @@ -136,21 +143,22 @@ func (k Keeper) CloseChannelByReceiver(ctx sdk.Context, update Update) (sdk.Tags // Main function that compare updates against each other. // Pure function -func (k Keeper) applyNewUpdate(existingSUpdate SubmittedUpdate, proposedUpdate Update) SubmittedUpdate { - var returnUpdate SubmittedUpdate +// Not needed in unidirectional case. +// func (k Keeper) applyNewUpdate(existingSUpdate SubmittedUpdate, proposedUpdate Update) SubmittedUpdate { +// var returnUpdate SubmittedUpdate - if existingSUpdate.Sequence > proposedUpdate.Sequence { - // update accepted - returnUpdate = SubmittedUpdate{ - Update: proposedUpdate, - ExecutionTime: existingSUpdate.ExecutionTime, - } - } else { - // update rejected - returnUpdate = existingSUpdate - } - return returnUpdate -} +// if existingSUpdate.Sequence > proposedUpdate.Sequence { +// // update accepted +// returnUpdate = SubmittedUpdate{ +// Update: proposedUpdate, +// ExecutionTime: existingSUpdate.ExecutionTime, // FIXME any new update proposal should be subject to full dispute period from submission +// } +// } else { +// // update rejected +// returnUpdate = existingSUpdate +// } +// return returnUpdate +// } // 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) { diff --git a/internal/x/paychan/keeper_test.go b/internal/x/paychan/keeper_test.go index 392f24a8..9fd59a87 100644 --- a/internal/x/paychan/keeper_test.go +++ b/internal/x/paychan/keeper_test.go @@ -70,7 +70,6 @@ func TestKeeper(t *testing.T) { update := Update{ ChannelID: channelID, Payouts: payouts, - Sequence: 0, Sigs: [1]crypto.Signature{}, } // Set empty submittedUpdatesQueue TODO work out proper genesis initialisation @@ -118,7 +117,6 @@ func TestKeeper(t *testing.T) { update := Update{ ChannelID: channelID, Payouts: payouts, - Sequence: 0, Sigs: [1]crypto.Signature{}, } // Set empty submittedUpdatesQueue TODO work out proper genesis initialisation diff --git a/internal/x/paychan/types.go b/internal/x/paychan/types.go index a93454e3..3ed695b9 100644 --- a/internal/x/paychan/types.go +++ b/internal/x/paychan/types.go @@ -23,8 +23,8 @@ type ChannelID int64 // TODO should this be positive only type Update struct { 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 + //Sequence int64 Not needed for unidirectional channels + Sigs [1]crypto.Signature // only sender needs to sign in unidirectional } type Payout struct { Address sdk.AccAddress