diff --git a/swap/config.go b/swap/config.go index ab645d0a89..ee66e8ad25 100644 --- a/swap/config.go +++ b/swap/config.go @@ -27,6 +27,9 @@ const ( // DefaultPaymentThreshold is set to be equivalent to requesting and serving 10mb of data (2441 chunks (4096 bytes) = 10 mb, 10^7 bytes = 10 mb) DefaultPaymentThreshold = 2441*RetrieveRequestPrice + (10^7)*ChunkDeliveryPrice // 4096 * 2441 = 10 mb, DefaultDisconnectThreshold = 20 * DefaultPaymentThreshold + // ChequeDebtTolerance is the lowest resulting balance a node is willing to accept when receiving a cheque + // the value is meant to be used below 0, as positive resulting balances should always be accepted when receiving cheques + ChequeDebtTolerance = DefaultPaymentThreshold * 20 / 100 // roughly 20% of the payment threshold // DefaultDepositAmount is the default amount to send to the contract when initially deploying // NOTE: deliberate value for now; needs experimentation DefaultDepositAmount = 0 diff --git a/swap/peer.go b/swap/peer.go index 174d71c953..83b8ad6f9c 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -212,7 +212,7 @@ func (p *Peer) sendCheque() error { honeyAmount := int64(cheque.Honey) err = p.updateBalance(honeyAmount) if err != nil { - return fmt.Errorf("error while creating cheque: %v", err) + return fmt.Errorf("error while updating balance: %v", err) } metrics.GetOrRegisterCounter("swap.cheques.emitted.num", nil).Inc(1) diff --git a/swap/protocol_test.go b/swap/protocol_test.go index 49dfa2c3bc..1b3ad93764 100644 --- a/swap/protocol_test.go +++ b/swap/protocol_test.go @@ -358,7 +358,9 @@ func TestTriggerPaymentThreshold(t *testing.T) { // set the balance to manually be at PaymentThreshold overDraft := 42 expectedAmount := uint64(overDraft) + DefaultPaymentThreshold - creditor.setBalance(-int64(DefaultPaymentThreshold)) + if err = creditor.setBalance(-int64(DefaultPaymentThreshold)); err != nil { + t.Fatal(err) + } // we expect a cheque at the end of the test, but not yet if creditor.getLastSentCheque() != nil { @@ -493,7 +495,9 @@ func TestTriggerDisconnectThreshold(t *testing.T) { overDraft := 42 expectedBalance := int64(DefaultDisconnectThreshold) // we don't expect any change after the test - debitor.setBalance(expectedBalance) + if err = debitor.setBalance(expectedBalance); err != nil { + t.Fatal(err) + } // we also don't expect any cheques yet if debitor.getPendingCheque() != nil { t.Fatalf("Expected no cheques yet, but there is %v", debitor.getPendingCheque()) diff --git a/swap/simulations_test.go b/swap/simulations_test.go index 581a2e222e..523dbd78c1 100644 --- a/swap/simulations_test.go +++ b/swap/simulations_test.go @@ -74,7 +74,7 @@ type swapSimulationParams struct { // define test message types type testMsgBySender struct{} type testMsgByReceiver struct{} -type testMsgBigPrice struct{} +type testMsgSmallPrice struct{} // create a test Spec; every node has its Spec and its accounting Hook func newTestSpec() *protocols.Spec { @@ -85,7 +85,7 @@ func newTestSpec() *protocols.Spec { Messages: []interface{}{ testMsgBySender{}, testMsgByReceiver{}, - testMsgBigPrice{}, + testMsgSmallPrice{}, }, } } @@ -106,9 +106,9 @@ func (m *testMsgByReceiver) Price() *protocols.Price { } } -func (m *testMsgBigPrice) Price() *protocols.Price { +func (m *testMsgSmallPrice) Price() *protocols.Price { return &protocols.Price{ - Value: DefaultPaymentThreshold + 1, + Value: DefaultPaymentThreshold / 100, // ensures that the message won't put nodes into debt PerByte: false, Payer: protocols.Sender, } @@ -250,135 +250,6 @@ func newSharedBackendSwaps(t *testing.T, nodeCount int) (*swapSimulationParams, return params, nil } -// TestPingPongChequeSimulation just launches two nodes and sends each other -// messages which immediately crosses the PaymentThreshold and triggers cheques -// to each other. Checks that accounting and cheque handling works across multiple -// cheques and also if cheques are mutually sent -func TestPingPongChequeSimulation(t *testing.T) { - nodeCount := 2 - // create the shared backend and params - params, err := newSharedBackendSwaps(t, nodeCount) - if err != nil { - t.Fatal(err) - } - // cleanup backend - defer params.backend.Close() - - // setup the wait for mined transaction function for testing - cleanup := setupContractTest() - defer cleanup() - - params.backend.cashDone = make(chan struct{}, 1) - defer close(params.backend.cashDone) - - // initialize the simulation - sim := simulation.NewBzzInProc(newSimServiceMap(params), false) - defer sim.Close() - - log.Info("Initializing") - - // we are going to use the metrics system to sync the test - // we are only going to continue with the next iteration after the message - // has been received on the other side - metricsReg := metrics.AccountingRegistry - // testMsgBigPrice is paid by the sender, so the credit counter will only be - // increased when receiving the message, which is what we want for this test - cter := metricsReg.Get("account.msg.credit") - counter := cter.(metrics.Counter) - counter.Clear() - var lastCount int64 - expectedPayout1, expectedPayout2 := DefaultPaymentThreshold+1, DefaultPaymentThreshold+1 - - _, err = sim.AddNodesAndConnectFull(nodeCount) - if err != nil { - t.Fatal(err) - } - - p1 := sim.UpNodeIDs()[0] - p2 := sim.UpNodeIDs()[1] - ts1 := sim.Service("swap", p1).(*testService) - ts2 := sim.Service("swap", p2).(*testService) - - var ts1Len, ts2Len, ts1sLen, ts2sLen int - timeout := time.After(10 * time.Second) - - for { - // let's always be nice and allow a time out to be catched - select { - case <-timeout: - t.Fatal("Timed out waiting for all swap peer connections to be established") - default: - } - // the node has all other peers in its peer list - ts1.swap.peersLock.Lock() - ts1Len = len(ts1.swap.peers) - ts1sLen = len(ts1.peers) - ts1.swap.peersLock.Unlock() - - ts2.swap.peersLock.Lock() - ts2Len = len(ts2.swap.peers) - ts2sLen = len(ts2.peers) - ts2.swap.peersLock.Unlock() - - if ts1Len == 1 && ts2Len == 1 && ts1sLen == 1 && ts2sLen == 1 { - break - } - // don't overheat the CPU... - time.Sleep(5 * time.Millisecond) - } - - maxCheques := 42 - - ts1.lock.Lock() - p2Peer := ts1.peers[p2] - ts1.lock.Unlock() - - ts2.lock.Lock() - p1Peer := ts2.peers[p1] - ts2.lock.Unlock() - - for i := 0; i < maxCheques; i++ { - if i%2 == 0 { - if err := p2Peer.Send(context.Background(), &testMsgBigPrice{}); err != nil { - t.Fatal(err) - } - if err := waitForChequeProcessed(t, params.backend, counter, lastCount, ts1.swap.peers[p2], expectedPayout1); err != nil { - t.Fatal(err) - } - lastCount += 1 - expectedPayout1 += DefaultPaymentThreshold + 1 - } else { - if err := p1Peer.Send(context.Background(), &testMsgBigPrice{}); err != nil { - t.Fatal(err) - } - if err := waitForChequeProcessed(t, params.backend, counter, lastCount, ts2.swap.peers[p1], expectedPayout2); err != nil { - t.Fatal(err) - } - lastCount += 1 - expectedPayout2 += DefaultPaymentThreshold + 1 - } - } - - ch1, err := ts2.swap.loadLastReceivedCheque(p1) - if err != nil { - t.Fatal(err) - } - ch2, err := ts1.swap.loadLastReceivedCheque(p2) - if err != nil { - t.Fatal(err) - } - - expected := uint64(maxCheques) / 2 * (DefaultPaymentThreshold + 1) - if !ch1.CumulativePayout.Equals(uint256.FromUint64(expected)) { - t.Fatalf("expected cumulative payout to be %d, but is %v", expected, ch1.CumulativePayout) - } - if !ch2.CumulativePayout.Equals(uint256.FromUint64(expected)) { - t.Fatalf("expected cumulative payout to be %d, but is %v", expected, ch2.CumulativePayout) - } - - log.Info("Simulation ended") -} - // TestMultiChequeSimulation just launches two nodes, a creditor and a debitor. // The debitor is the one owing to the creditor, so the debitor is the one sending cheques // It sends multiple cheques in a sequence to the same node. @@ -405,18 +276,18 @@ func TestMultiChequeSimulation(t *testing.T) { defer sim.Close() log.Info("Initializing") - + msgPrice := (&testMsgSmallPrice{}).Price().Value // we are going to use the metrics system to sync the test // we are only going to continue with the next iteration after the message // has been received on the other side metricsReg := metrics.AccountingRegistry - // testMsgBigPrice is paid by the sender, so the credit counter will only be + // testMsgSmallPrice is paid by the sender, so the credit counter will only be // increased when receiving the message, which is what we want for this test cter := metricsReg.Get("account.msg.credit") counter := cter.(metrics.Counter) counter.Clear() var lastCount int64 - expectedPayout := DefaultPaymentThreshold + 1 + var expectedPayout uint64 _, err = sim.AddNodesAndConnectFull(nodeCount) if err != nil { @@ -434,7 +305,7 @@ func TestMultiChequeSimulation(t *testing.T) { var debLen, credLen, debSwapLen, credSwapLen int timeout := time.After(10 * time.Second) for { - // let's always be nice and allow a time out to be catched + // let's always be nice and allow a time out to be caught select { case <-timeout: t.Fatal("Timed out waiting for all swap peer connections to be established") @@ -458,27 +329,68 @@ func TestMultiChequeSimulation(t *testing.T) { time.Sleep(5 * time.Millisecond) } - // we will send just maxCheques number of cheques - maxCheques := 10 + paymentThreshold := debitorSvc.swap.params.PaymentThreshold + + chequesAmount := 4 + msgsPerCheque := (uint64(paymentThreshold) / msgPrice) + 1 // +1 to round up without casting to float + msgAmount := int(msgsPerCheque) * chequesAmount + log.Debug("sending %d messages", msgAmount) // the peer object used for sending debitorSvc.lock.Lock() creditorPeer := debitorSvc.peers[creditor] debitorSvc.lock.Unlock() - // send maxCheques number of cheques - for i := 0; i < maxCheques; i++ { - // use a price which will trigger a cheque each time - if err := creditorPeer.Send(context.Background(), &testMsgBigPrice{}); err != nil { + allMessagesArrived := make(chan struct{}) + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + + go func() { + for { + select { + case <-ctx.Done(): + return + default: + } + // all messages have been received + if counter.Count() == int64(msgAmount) { + close(allMessagesArrived) + return + } + time.Sleep(10 * time.Millisecond) + } + }() + + for i := 0; i < msgAmount; i++ { + debitorBalance, err := debitorSvc.swap.loadBalance(creditor) + if err != nil { t.Fatal(err) } - // we need to wait a bit in order to give time for the cheque to be processed - if err := waitForChequeProcessed(t, params.backend, counter, lastCount, debitorSvc.swap.peers[creditor], expectedPayout); err != nil { + + if err := creditorPeer.Send(ctx, &testMsgSmallPrice{}); err != nil { t.Fatal(err) } - lastCount += 1 - expectedPayout += DefaultPaymentThreshold + 1 + + // check if cheque should have been sent + balanceAfterMessage := debitorBalance - int64(msgPrice) + if balanceAfterMessage <= -paymentThreshold { + // we need to wait a bit in order to give time for the cheque to be processed + if err := waitForChequeProcessed(t, params.backend, counter, lastCount, debitorSvc.swap.peers[creditor], expectedPayout); err != nil { + t.Fatal(err) + } + expectedPayout += uint64(-balanceAfterMessage) + } + + lastCount++ + } + // give enough time for all messages to be processed + select { + case <-ctx.Done(): + t.Fatal("timed out waiting for all messages to arrive, aborting") + case <-allMessagesArrived: } + log.Debug("all messages arrived") // check balances: b1, err := debitorSvc.swap.loadBalance(creditor) @@ -507,9 +419,6 @@ func TestMultiChequeSimulation(t *testing.T) { t.Fatalf("Expected symmetric cheques payout, but they are not: %v vs %v", cheque1.CumulativePayout, cheque2.CumulativePayout) } - // check also the actual expected amount - expectedPayout = uint64(maxCheques) * (DefaultPaymentThreshold + 1) - if !cheque2.CumulativePayout.Equals(uint256.FromUint64(expectedPayout)) { t.Fatalf("Expected %d in cumulative payout, got %v", expectedPayout, cheque1.CumulativePayout) } @@ -722,7 +631,7 @@ func waitForChequeProcessed(t *testing.T, backend *swapTestBackend, counter metr select { case <-ctx.Done(): lock.Lock() - errs = append(errs, "Timed out waiting for cheque to have been cached") + errs = append(errs, "Timed out waiting for cheque to be cashed") lock.Unlock() wg.Done() return @@ -762,7 +671,7 @@ func waitForChequeProcessed(t *testing.T, backend *swapTestBackend, counter metr select { case <-ctx.Done(): lock.Lock() - errs = append(errs, "Timed out waiting for peer to have processed accounted message") + errs = append(errs, "Timed out waiting for peer to process accounted message") lock.Unlock() wg.Done() return diff --git a/swap/swap.go b/swap/swap.go index bd2d5c8dd9..657f619748 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -521,6 +521,13 @@ func (s *Swap) processAndVerifyCheque(cheque *Cheque, p *Peer) (*uint256.Uint256 return nil, err } + // calculate tentative new balance after cheque is processed + newBalance := p.getBalance() - int64(cheque.Honey) + // check if this new balance would put creditor into debt + if newBalance < -int64(ChequeDebtTolerance) { + return nil, fmt.Errorf("received cheque would result in balance %d which exceeds tolerance %d and would cause debt", newBalance, ChequeDebtTolerance) + } + if err := p.setLastReceivedCheque(cheque); err != nil { p.logger.Error("error while saving last received cheque", "err", err.Error()) // TODO: what do we do here? Related issue: https://github.com/ethersphere/swarm/issues/1515 diff --git a/swap/swap_test.go b/swap/swap_test.go index 225bd85038..cc0656bdb1 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -634,15 +634,21 @@ func TestResetBalance(t *testing.T) { } // set balances arbitrarily - debitor.setBalance(int64(testAmount)) - creditor.setBalance(int64(-testAmount)) + if err = debitor.setBalance(int64(testAmount)); err != nil { + t.Fatal(err) + } + if err = creditor.setBalance(int64(-testAmount)); err != nil { + t.Fatal(err) + } // setup the wait for mined transaction function for testing cleanup := setupContractTest() defer cleanup() // now simulate sending the cheque to the creditor from the debitor - creditor.sendCheque() + if err = creditor.sendCheque(); err != nil { + t.Fatal(err) + } debitorSwap.handleConfirmChequeMsg(ctx, creditor, &ConfirmChequeMsg{ Cheque: creditor.getPendingCheque(), @@ -682,6 +688,64 @@ func TestResetBalance(t *testing.T) { } } +// TestDebtCheques verifies that cheques that would put a node in debt past the defined tolerance are rejected +// and that ones within the tolerance are accepted +func TestDebtCheques(t *testing.T) { + testBackend := newTestBackend(t) + defer testBackend.Close() + + creditorSwap, cleanup := newTestSwap(t, beneficiaryKey, testBackend) + defer cleanup() + + ctx := context.Background() + if err := testDeploy(ctx, creditorSwap, uint256.FromUint64(0)); err != nil { + t.Fatal(err) + } + + debitorChequebook, err := testDeployWithPrivateKey(ctx, testBackend, ownerKey, ownerAddress, uint256.FromUint64((DefaultPaymentThreshold * 2))) + if err != nil { + t.Fatal(err) + } + + debitorDummyPeer := newDummyPeerWithSpec(Spec) + debitorPeer, err := creditorSwap.addPeer(debitorDummyPeer.Peer, ownerAddress, debitorChequebook.ContractParams().ContractAddress) + if err != nil { + t.Fatal(err) + } + + // create debt cheque + chequeAmount := uint256.FromUint64(ChequeDebtTolerance * 2) + cheque, err := newSignedTestCheque(debitorChequebook.ContractParams().ContractAddress, creditorSwap.owner.address, chequeAmount, ownerKey) + if err != nil { + t.Fatal(err) + } + + // simulate cheque handling + err = creditorSwap.handleEmitChequeMsg(ctx, debitorPeer, &EmitChequeMsg{ + Cheque: cheque, + }) + // cheque should not have gone through as it would put the creditor in debt + if err == nil || !strings.Contains(err.Error(), "cause debt") { + t.Fatalf("expected invalid cheque to trigger debt cheque error, but got: %v", err) + } + + // now create a (barely) admissible cheque + chequeAmount = uint256.FromUint64(ChequeDebtTolerance) + cheque, err = newSignedTestCheque(debitorChequebook.ContractParams().ContractAddress, creditorSwap.owner.address, chequeAmount, ownerKey) + if err != nil { + t.Fatal(err) + } + + // simulate cheque handling + err = creditorSwap.handleEmitChequeMsg(ctx, debitorPeer, &EmitChequeMsg{ + Cheque: cheque, + }) + // cheque should have gone through + if err != nil { + t.Fatal(err) + } +} + // generate bookings based on parameters, apply them to a Swap struct and verify the result // append generated bookings to slice pointer func testPeerBookings(t *testing.T, s *Swap, bookings *[]booking, bookingAmount int64, bookingQuantity int, peer *protocols.Peer) { @@ -766,7 +830,9 @@ func TestRestoreBalanceFromStateStore(t *testing.T) { if err != nil { t.Fatal(err) } - testPeer.setBalance(-8888) + if err = testPeer.setBalance(-8888); err != nil { + t.Fatal(err) + } tmpBalance := testPeer.getBalance() swap.store.Put(testPeer.ID().String(), &tmpBalance) @@ -1221,15 +1287,21 @@ func TestSwapLogToFile(t *testing.T) { } // set balances arbitrarily - debitor.setBalance(int64(testAmount)) - creditor.setBalance(int64(-testAmount)) + if err = debitor.setBalance(int64(testAmount)); err != nil { + t.Fatal(err) + } + if err = creditor.setBalance(int64(-testAmount)); err != nil { + t.Fatal(err) + } // setup the wait for mined transaction function for testing cleanup := setupContractTest() defer cleanup() // now simulate sending the cheque to the creditor from the debitor - creditor.sendCheque() + if err = creditor.sendCheque(); err != nil { + t.Fatal(err) + } if logDirDebitor == "" { t.Fatal("Swap Log Dir is not defined") @@ -1333,7 +1405,9 @@ func TestAvailableBalance(t *testing.T) { // send a cheque worth 42 chequeAmount := uint64(42) // create a dummy peer. Note: the peer's contract address and the peers address are resp the swap contract and the swap owner - peer.setBalance(int64(-chequeAmount)) + if err = peer.setBalance(int64(-chequeAmount)); err != nil { + t.Fatal(err) + } if err = peer.sendCheque(); err != nil { t.Fatal(err) }