Settle channel when funding fails (Variant B) (hyperledger-labs#349)

* client/proposal: Settle channel on funding error

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* client/client_role_test: NewClients expects setups

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* client/test: Move handlers from multiledger.go -> handler.go

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* Add failing funder test

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* PR Feedback: Fix typos

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* PR Feedback: Move constant into test

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* client/test/backend: Add mock Funder to MockBackend

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* PR Feedback: Move Increment usage to completeCPP

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* ProposeChannel does not settle automatically

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* channel/funder: Adapt doc

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* client/test/backend: Mock Funder waits for challenge duration

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* Test both sides with failing funders

Signed-off-by: Philipp-Florens Lehwalder <[email protected]>

* Add ChannelFundingError

Signed-off-by: Matthias Geihs <[email protected]>

* Revise test

Signed-off-by: Matthias Geihs <[email protected]>

Co-authored-by: Matthias Geihs <[email protected]>

channel/funder.go

@@ -31,10 +31,9 @@ type (
 		// because a peer did not fund the channel in time.
 		// Depending on the funding protocol, if we fund first and then the peer does
 		// not fund in time, a dispute process needs to be initiated to get back the
-		// funds from the partially funded channel. In this case, the user should
-		// return a PeerTimedOutFundingError containing the index of the peer who
-		// did not fund in time. The framework will then initiate the dispute
-		// process.
+		// funds from the partially funded channel. In this case, it should
+		// return a FundingTimeoutError containing the index of the peer who
+		// did not fund in time.
 		Fund(context.Context, FundingReq) error
 	}
 

client/client_role_test.go

@@ -75,9 +75,8 @@ type Client struct {
 	ctest.RoleSetup
 }
 
-func NewClients(t *testing.T, rng *rand.Rand, names []string) []*Client {
+func NewClients(t *testing.T, rng *rand.Rand, setups []ctest.RoleSetup) []*Client {
 	t.Helper()
-	setups := NewSetups(rng, names)
 	clients := make([]*Client, len(setups))
 	for i, setup := range setups {
 		setup.Identity = setup.Wallet.NewRandomAccount(rng)

client/failing_funding_test.go

@@ -0,0 +1,126 @@
+// Copyright 2022 - See NOTICE file for copyright holders.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package client_test
+
+import (
+	"context"
+	"errors"
+	"math/big"
+	"math/rand"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"perun.network/go-perun/channel"
+	chtest "perun.network/go-perun/channel/test"
+	"perun.network/go-perun/client"
+	ctest "perun.network/go-perun/client/test"
+	"perun.network/go-perun/wire"
+	"polycry.pt/poly-go/test"
+)
+
+func TestFailingFunding(t *testing.T) {
+	rng := test.Prng(t)
+
+	t.Run("failing funder proposer", func(t *testing.T) {
+		setups := NewSetups(rng, []string{"Frida", "Fred"})
+		setups[0].Funder = FailingFunder{}
+
+		runFredFridaTest(t, rng, setups)
+	})
+
+	t.Run("failing funder proposee", func(t *testing.T) {
+		setups := NewSetups(rng, []string{"Frida", "Fred"})
+		setups[1].Funder = FailingFunder{}
+
+		runFredFridaTest(t, rng, setups)
+	})
+
+	t.Run("failing funder both sides", func(t *testing.T) {
+		setups := NewSetups(rng, []string{"Frida", "Fred"})
+		setups[0].Funder = FailingFunder{}
+		setups[1].Funder = FailingFunder{}
+
+		runFredFridaTest(t, rng, setups)
+	})
+}
+
+//nolint:thelper // The linter thinks this is a helper function, but it is not.
+func runFredFridaTest(t *testing.T, rng *rand.Rand, setups []ctest.RoleSetup) {
+	const (
+		challengeDuration = 1
+		fridaIdx          = 0
+		fredIdx           = 1
+		fridaInitBal      = 100
+		fredInitBal       = 50
+	)
+
+	ctx, cancel := context.WithTimeout(context.Background(), twoPartyTestTimeout)
+	defer cancel()
+
+	clients := NewClients(t, rng, setups)
+	frida, fred := clients[fridaIdx], clients[fredIdx]
+	fridaAddr, fredAddr := frida.Identity.Address(), fred.Identity.Address()
+
+	// The channel into which Fred's created ledger channel is sent into.
+	chsFred := make(chan *client.Channel, 1)
+	errsFred := make(chan error, 1)
+	go fred.Handle(
+		ctest.AlwaysAcceptChannelHandler(ctx, fredAddr, chsFred, errsFred),
+		ctest.AlwaysRejectUpdateHandler(ctx, errsFred),
+	)
+
+	// Create the proposal.
+	asset := chtest.NewRandomAsset(rng)
+	initAlloc := channel.NewAllocation(2, asset)
+	initAlloc.SetAssetBalances(asset, []*big.Int{big.NewInt(fridaInitBal), big.NewInt(fredInitBal)})
+	parts := []wire.Address{fridaAddr, fredAddr}
+	prop, err := client.NewLedgerChannelProposal(
+		challengeDuration,
+		fridaAddr,
+		initAlloc,
+		parts,
+	)
+	require.NoError(t, err, "creating ledger channel proposal")
+
+	// Frida sends the proposal.
+	chFrida, err := frida.ProposeChannel(ctx, prop)
+	require.IsType(t, &client.ChannelFundingError{}, err)
+	require.NotNil(t, chFrida)
+	// Frida settles the channel.
+	require.NoError(t, chFrida.Settle(ctx, false))
+
+	// Fred gets the channel and settles it afterwards.
+	chFred := <-chsFred
+	err = <-errsFred
+	require.IsType(t, &client.ChannelFundingError{}, err)
+	require.NotNil(t, chFred)
+	// Fred settles the channel.
+	require.NoError(t, chFred.Settle(ctx, false))
+
+	// Test the final balances.
+	fridaFinalBal := frida.BalanceReader.Balance(fridaAddr, asset)
+	assert.Truef(t, fridaFinalBal.Cmp(big.NewInt(fridaInitBal)) == 0, "frida: wrong final balance: got %v, expected %v", fridaFinalBal, fridaInitBal)
+	fredFinalBal := fred.BalanceReader.Balance(fredAddr, asset)
+	assert.Truef(t, fredFinalBal.Cmp(big.NewInt(fredInitBal)) == 0, "fred: wrong final balance: got %v, expected %v", fredFinalBal, fredInitBal)
+}
+
+type FailingFunder struct{}
+
+// Fund returns an error to simulate failed funding.
+func (m FailingFunder) Fund(ctx context.Context, req channel.FundingReq) error {
+	return errors.New("funding failed")
+}

client/proposal.go

@@ -75,6 +75,11 @@ type (
 		ItemType string // ItemType indicates the type of item rejected (channel proposal or channel update).
 		Reason   string // Reason sent by the peer for the rejection.
 	}
+
+	// ChannelFundingError indicates an error during channel funding.
+	ChannelFundingError struct {
+		Err error
+	}
 )
 
 // HandleProposal calls the proposal handler function.
@@ -93,16 +98,22 @@ func (f ProposalHandlerFunc) HandleProposal(p ChannelProposal, r *ProposalRespon
 // callback registered with Client.OnNewChannel. Accept returns after this
 // callback has run.
 //
-// It is important that the passed context does not cancel before twice the
-// ChallengeDuration has passed (at least for real blockchain backends with wall
-// time), or the channel cannot be settled if a peer times out funding.
+// It is important that the passed context does not cancel before the
+// ChallengeDuration has passed. Otherwise funding may not complete.
+//
+// If funding fails, ChannelFundingError is thrown and an unfunded channel
+// object is returned, which can be used for withdrawing the funds.
 //
 // After the channel got successfully created, the user is required to start the
 // channel watcher with Channel.Watch() on the returned channel controller.
 //
-// Returns TxTimedoutError when the program times out waiting for a transaction
-// to be mined.
-// Returns ChainNotReachableError if the connection to the blockchain network
+// Returns ChannelFundingError if an error happened during funding. The internal
+// error gives more information.
+// - Contains FundingTimeoutError if any of the participants do not fund the
+// channel in time.
+// - Contains TxTimedoutError when the program times out waiting for a
+// transaction to be mined.
+// - Contains ChainNotReachableError if the connection to the blockchain network
 // fails when sending a transaction to / reading from the blockchain.
 func (r *ProposalResponder) Accept(ctx context.Context, acc ChannelProposalAccept) (*Channel, error) {
 	if ctx == nil {
@@ -136,9 +147,11 @@ func (r *ProposalResponder) Reject(ctx context.Context, reason string) error {
 // callback registered with Client.OnNewChannel. Accept returns after this
 // callback has run.
 //
-// It is important that the passed context does not cancel before twice the
-// ChallengeDuration has passed (at least for real blockchain backends with wall
-// time), or the channel cannot be settled if a peer times out funding.
+// It is important that the passed context does not cancel before the
+// ChallengeDuration has passed. Otherwise funding may not complete.
+//
+// If funding fails, ChannelFundingError is thrown and an unfunded channel
+// object is returned, which can be used for withdrawing the funds.
 //
 // After the channel got successfully created, the user is required to start the
 // channel watcher with Channel.Watch() on the returned channel
@@ -147,11 +160,13 @@ func (r *ProposalResponder) Reject(ctx context.Context, reason string) error {
 // Returns PeerRejectedProposalError if the channel is rejected by the peer.
 // Returns RequestTimedOutError if the peer did not respond before the context
 // expires or is cancelled.
-// Returns FundingTimeoutError if any of the participants do not fund the
+// Returns ChannelFundingError if an error happened during funding. The internal
+// error gives more information.
+// - Contains FundingTimeoutError if any of the participants do not fund the
 // channel in time.
-// Returns TxTimedoutError when the program times out waiting for a transaction
-// to be mined.
-// Returns ChainNotReachableError if the connection to the blockchain network
+// - Contains TxTimedoutError when the program times out waiting for a
+// transaction to be mined.
+// - Contains ChainNotReachableError if the connection to the blockchain network
 // fails when sending a transaction to / reading from the blockchain.
 func (c *Client) ProposeChannel(ctx context.Context, prop ChannelProposal) (*Channel, error) {
 	if ctx == nil {
@@ -182,8 +197,12 @@ func (c *Client) ProposeChannel(ctx context.Context, prop ChannelProposal) (*Cha
 	}
 
 	// 3. fund
-	fundingErr := c.fundChannel(ctx, ch, prop)
-	return ch, fundingErr
+	err = c.fundChannel(ctx, ch, prop)
+	if err != nil {
+		return ch, newChannelFundingError(err)
+	}
+
+	return ch, nil
 }
 
 func (c *Client) prepareChannelOpening(ctx context.Context, prop ChannelProposal, ourIdx channel.Index) (err error) {
@@ -254,8 +273,11 @@ func (c *Client) handleChannelProposalAcc(
 		return ch, errors.WithMessage(err, "accept channel proposal")
 	}
 
-	fundingErr := c.fundChannel(ctx, ch, prop)
-	return ch, fundingErr
+	err = c.fundChannel(ctx, ch, prop)
+	if err != nil {
+		return ch, newChannelFundingError(err)
+	}
+	return ch, nil
 }
 
 func (c *Client) acceptChannelProposal(
@@ -582,6 +604,7 @@ func (c *Client) completeCPP(
 		return ch, errors.WithMessage(err, "exchanging initial sigs and enabling state")
 	}
 
+	c.wallet.IncrementUsage(params.Parts[partIdx])
 	return ch, nil
 }
 
@@ -753,3 +776,11 @@ func (e PeerRejectedError) Error() string {
 func newPeerRejectedError(rejectedItemType, reason string) error {
 	return errors.WithStack(PeerRejectedError{rejectedItemType, reason})
 }
+
+func newChannelFundingError(err error) *ChannelFundingError {
+	return &ChannelFundingError{err}
+}
+
+func (e ChannelFundingError) Error() string {
+	return fmt.Sprintf("channel funding failed: %v", e.Err.Error())
+}

client/test/backend.go

@@ -20,13 +20,13 @@ import (
 	"fmt"
 	"math/big"
 	"math/rand"
-	"sync"
 	"time"
 
 	"perun.network/go-perun/channel"
 	"perun.network/go-perun/channel/multi"
 	"perun.network/go-perun/log"
 	"perun.network/go-perun/wallet"
+	"polycry.pt/poly-go/sync"
 )
 
 type (
@@ -35,6 +35,7 @@ type (
 		log          log.Logger
 		rng          rng
 		mu           sync.Mutex
+		funder       *funder
 		latestEvents map[channel.ID]channel.AdjudicatorEvent
 		eventSubs    map[channel.ID][]*MockSubscription
 		balances     map[addressMapKey]map[assetMapKey]*big.Int
@@ -65,6 +66,7 @@ func NewMockBackend(rng *rand.Rand, id string) *MockBackend {
 	return &MockBackend{
 		log:          log.Default(),
 		rng:          newThreadSafePrng(backendRng),
+		funder:       newFunder(newThreadSafePrng(backendRng)),
 		latestEvents: make(map[channel.ID]channel.AdjudicatorEvent),
 		eventSubs:    make(map[channel.ID][]*MockSubscription),
 		balances:     make(map[string]map[string]*big.Int),
@@ -97,10 +99,11 @@ func (g *threadSafeRng) Intn(n int) int {
 }
 
 // Fund funds the channel.
-func (b *MockBackend) Fund(_ context.Context, req channel.FundingReq) error {
-	time.Sleep(time.Duration(b.rng.Intn(fundMaxSleepMs+1)) * time.Millisecond)
+func (b *MockBackend) Fund(ctx context.Context, req channel.FundingReq) error {
 	b.log.Infof("Funding: %+v", req)
-	return nil
+	b.funder.InitFunder(req)
+	b.funder.Fund(req)
+	return b.funder.WaitForFunding(ctx, req)
 }
 
 // Register registers the channel.
@@ -394,6 +397,56 @@ func (b *MockBackend) removeSubscription(ch channel.ID, sub *MockSubscription) {
 	b.eventSubs[ch] = append(subs[:i], subs[i+1:]...)
 }
 
+// funder mocks a funder for the MockBackend.
+type funder struct {
+	rng       rng
+	mtx       sync.Mutex
+	fundedWgs map[channel.ID]*sync.WaitGroup
+}
+
+// newFunder returns a new funder.
+func newFunder(rng rng) *funder {
+	return &funder{
+		rng:       rng,
+		fundedWgs: make(map[channel.ID]*sync.WaitGroup),
+	}
+}
+
+// InitFunder initializes the funded WaitGroups for a channel if not already
+// initialized.
+//
+// Must be called before using the Funder for a funding request.
+func (f *funder) InitFunder(req channel.FundingReq) {
+	f.mtx.Lock()
+	defer f.mtx.Unlock()
+
+	if f.fundedWgs[req.Params.ID()] == nil {
+		f.fundedWgs[req.Params.ID()] = &sync.WaitGroup{}
+		f.fundedWgs[req.Params.ID()].Add(len(req.Params.Parts))
+	}
+}
+
+// Fund simulates funding the channel.
+func (f *funder) Fund(req channel.FundingReq) {
+	time.Sleep(time.Duration(f.rng.Intn(fundMaxSleepMs+1)) * time.Millisecond)
+	f.fundedWgs[req.Params.ID()].Done()
+}
+
+// WaitForFunding waits until all participants have funded the channel.
+func (f *funder) WaitForFunding(ctx context.Context, req channel.FundingReq) error {
+	challengeDuration := time.Duration(req.Params.ChallengeDuration) * time.Second
+	fundCtx, cancel := context.WithTimeout(ctx, challengeDuration)
+	defer cancel()
+
+	select {
+	case <-f.fundedWgs[req.Params.ID()].WaitCh():
+		log.Infof("Funded: %+v", req)
+		return nil
+	case <-fundCtx.Done():
+		return channel.FundingTimeoutError{}
+	}
+}
+
 // MockSubscription is a subscription for MockBackend.
 type MockSubscription struct {
 	ctx     context.Context //nolint:containedctx // This is just done for testing. Could be revised.