From c8727147e3bbdbbd64d397118cb11ec1a2014408 Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Fri, 11 Oct 2019 13:16:27 +0900 Subject: [PATCH 01/26] swap: introduce pending cheques, drop peers on io errors --- swap/peer.go | 27 ++++++++++--- swap/protocol.go | 1 + swap/protocol_test.go | 10 ++++- swap/simulations_test.go | 1 - swap/swap.go | 83 ++++++++++++++++++++++++++++++++++++++-- swap/swap_test.go | 6 ++- swap/types.go | 4 ++ 7 files changed, 120 insertions(+), 12 deletions(-) diff --git a/swap/peer.go b/swap/peer.go index c4cd5b33cf..d1e4934653 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -40,6 +40,7 @@ type Peer struct { contractAddress common.Address lastReceivedCheque *Cheque lastSentCheque *Cheque + pendingCheque *Cheque balance int64 logger log.Logger // logger for swap related messages and audit trail with peer identifier } @@ -65,6 +66,11 @@ func NewPeer(p *protocols.Peer, s *Swap, beneficiary common.Address, contractAdd if peer.balance, err = s.loadBalance(p.ID()); err != nil { return nil, err } + + if peer.pendingCheque, err = s.loadPendingCheque(p.ID()); err != nil { + return nil, err + } + return peer, nil } @@ -76,6 +82,10 @@ func (p *Peer) getLastSentCheque() *Cheque { return p.lastSentCheque } +func (p *Peer) getPendingCheque() *Cheque { + return p.pendingCheque +} + func (p *Peer) setLastReceivedCheque(cheque *Cheque) error { p.lastReceivedCheque = cheque return p.swap.saveLastReceivedCheque(p.ID(), cheque) @@ -86,6 +96,11 @@ func (p *Peer) setLastSentCheque(cheque *Cheque) error { return p.swap.saveLastSentCheque(p.ID(), cheque) } +func (p *Peer) setPendingCheque(cheque *Cheque) error { + p.pendingCheque = cheque + return p.swap.savePendingCheque(p.ID(), cheque) +} + func (p *Peer) getLastSentCumulativePayout() uint64 { lastCheque := p.getLastSentCheque() if lastCheque != nil { @@ -154,16 +169,18 @@ func (p *Peer) createCheque() (*Cheque, error) { // To be called with mutex already held // Caller must be careful that the same resources aren't concurrently read and written by multiple routines func (p *Peer) sendCheque() error { + if p.getPendingCheque() != nil { + p.logger.Warn("old cheque still pending, resending cheque") + return p.Send(context.Background(), &EmitChequeMsg{ + Cheque: p.getPendingCheque(), + }) + } cheque, err := p.createCheque() if err != nil { return fmt.Errorf("error while creating cheque: %v", err) } - if err := p.setLastSentCheque(cheque); err != nil { - return fmt.Errorf("error while storing the last cheque: %v", err) - } - - if err := p.updateBalance(int64(cheque.Honey)); err != nil { + if err = p.setPendingCheque(cheque); err != nil { return err } diff --git a/swap/protocol.go b/swap/protocol.go index 34dcdf9fc9..8940b3482b 100644 --- a/swap/protocol.go +++ b/swap/protocol.go @@ -46,6 +46,7 @@ var ( Messages: []interface{}{ HandshakeMsg{}, EmitChequeMsg{}, + ConfirmChequeMsg{}, }, } ) diff --git a/swap/protocol_test.go b/swap/protocol_test.go index 319cd513a9..efdac037dc 100644 --- a/swap/protocol_test.go +++ b/swap/protocol_test.go @@ -143,7 +143,7 @@ func TestEmitCheque(t *testing.T) { // create the debitor peer dPtpPeer := p2p.NewPeer(enode.ID{}, "debitor", []p2p.Cap{}) - dProtoPeer := protocols.NewPeer(dPtpPeer, nil, Spec) + dProtoPeer := protocols.NewPeer(dPtpPeer, &dummyMsgRW{}, Spec) debitor, err := creditorSwap.addPeer(dProtoPeer, debitorSwap.owner.address, debitorSwap.GetParams().ContractAddress) if err != nil { t.Fatal(err) @@ -245,6 +245,10 @@ func TestTriggerPaymentThreshold(t *testing.T) { t.Fatal(err) } + debitorSwap.handleConfirmChequeMsg(ctx, creditor, &ConfirmChequeMsg{ + Cheque: creditor.getPendingCheque(), + }) + // we should now have a cheque if creditor.getLastSentCheque() == nil { t.Fatal("Expected one cheque, but there is none") @@ -266,6 +270,10 @@ func TestTriggerPaymentThreshold(t *testing.T) { t.Fatal(err) } + debitorSwap.handleConfirmChequeMsg(ctx, creditor, &ConfirmChequeMsg{ + Cheque: creditor.getPendingCheque(), + }) + if creditor.getBalance() != 0 { t.Fatalf("Expected debitorSwap balance to be 0, but is %d", creditor.getBalance()) } diff --git a/swap/simulations_test.go b/swap/simulations_test.go index cb6a86379c..62fb141edb 100644 --- a/swap/simulations_test.go +++ b/swap/simulations_test.go @@ -58,7 +58,6 @@ and is purely meant for testing the accounting functionality across nodes. For integration tests, run test cluster deployments with all integration modueles (blockchains, oracles, etc.) */ - // swapSimulationParams allows to avoid global variables for the test type swapSimulationParams struct { swaps map[int]*Swap diff --git a/swap/swap.go b/swap/swap.go index 5ad25bfd82..887e474b1c 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -206,6 +206,7 @@ const ( balancePrefix = "balance_" sentChequePrefix = "sent_cheque_" receivedChequePrefix = "received_cheque_" + pendingChequePrefix = "pending_cheque_" connectedChequebookKey = "connected_chequebook" connectedBlockchainKey = "connected_blockchain" ) @@ -263,6 +264,10 @@ func receivedChequeKey(peer enode.ID) string { return receivedChequePrefix + peer.String() } +func pendingChequeKey(peer enode.ID) string { + return pendingChequePrefix + peer.String() +} + func keyToID(key string, prefix string) enode.ID { return enode.HexID(key[len(prefix):]) } @@ -297,14 +302,17 @@ func (s *Swap) Add(amount int64, peer *protocols.Peer) (err error) { return err } - // Check if balance with peer crosses the payment threshold - // It is the peer with a negative balance who sends a cheque, thus we check - // that the balance is *below* the threshold + return s.checkPaymentThreshold(swapPeer) +} + +// Check if balance with peer crosses the payment threshold +// It is the peer with a negative balance who sends a cheque, thus we check +// that the balance is *below* the threshold +func (s *Swap) checkPaymentThreshold(swapPeer *Peer) error { if swapPeer.getBalance() <= -s.params.PaymentThreshold { swapPeer.logger.Info("balance for peer went over the payment threshold, sending cheque", "payment threshold", s.params.PaymentThreshold) return swapPeer.sendCheque() } - return nil } @@ -314,6 +322,8 @@ func (s *Swap) handleMsg(p *Peer) func(ctx context.Context, msg interface{}) err switch msg := msg.(type) { case *EmitChequeMsg: go s.handleEmitChequeMsg(ctx, p, msg) + case *ConfirmChequeMsg: + go s.handleConfirmChequeMsg(ctx, p, msg) } return nil } @@ -329,6 +339,14 @@ func (s *Swap) handleEmitChequeMsg(ctx context.Context, p *Peer, msg *EmitCheque cheque := msg.Cheque p.logger.Info("received cheque from peer", "honey", cheque.Honey) + + if cheque == p.getLastReceivedCheque() { + p.logger.Warn("received old cheque from peer", "cumulativePayout", cheque.CumulativePayout) + return p.Send(ctx, &ConfirmChequeMsg{ + Cheque: cheque, + }) + } + _, err := s.processAndVerifyCheque(cheque, p) if err != nil { log.Error("error processing and verifying cheque", "err", err) @@ -345,6 +363,12 @@ func (s *Swap) handleEmitChequeMsg(ctx context.Context, p *Peer, msg *EmitCheque return err } + if err := p.Send(ctx, &ConfirmChequeMsg{ + Cheque: cheque, + }); err != nil { + return err + } + otherSwap, err := contract.InstanceAt(cheque.Contract, s.backend) if err != nil { log.Error("error getting contract", "err", err) @@ -371,6 +395,42 @@ func (s *Swap) handleEmitChequeMsg(ctx context.Context, p *Peer, msg *EmitCheque return err } +func (s *Swap) handleConfirmChequeMsg(ctx context.Context, p *Peer, msg *ConfirmChequeMsg) { + p.lock.Lock() + defer p.lock.Unlock() + cheque := msg.Cheque + + if p.getPendingCheque() == nil { + p.logger.Warn("ignoring confirm msg, no pending cheque") + } + + if !cheque.Equal(p.getPendingCheque()) { + p.logger.Warn("ignoring confirm msg, wrong cheque", "got", cheque, "expected", p.getPendingCheque()) + return + } + + if err := p.setLastSentCheque(cheque); err != nil { + p.Drop("persistence error") + return + } + + if err := p.setPendingCheque(nil); err != nil { + p.Drop("persistence error") + return + } + + if err := p.updateBalance(int64(cheque.Honey)); err != nil { + p.Drop("persistence error") + return + } + + // since more swap traffic might have occurred since this cheque was already sent, we redo the payment threshold check + if err := s.checkPaymentThreshold(p); err != nil { + p.logger.Warn("failed to send already due cheque", "error", err) + return + } +} + // cashCheque should be called async as it blocks until the transaction(s) are mined // The function cashes the cheque by sending it to the blockchain func cashCheque(s *Swap, otherSwap contract.Contract, opts *bind.TransactOpts, cheque *Cheque) { @@ -563,6 +623,16 @@ func (s *Swap) loadLastSentCheque(p enode.ID) (cheque *Cheque, err error) { return cheque, err } +// loadPendingCheque loads the current pending cheque for the peer from the store +// and returns nil when there never was a pending cheque saved +func (s *Swap) loadPendingCheque(p enode.ID) (cheque *Cheque, err error) { + err = s.store.Get(pendingChequeKey(p), &cheque) + if err == state.ErrNotFound { + return nil, nil + } + return cheque, err +} + // loadBalance loads the current balance for the peer from the store // and returns 0 if there was no prior balance saved func (s *Swap) loadBalance(p enode.ID) (balance int64, err error) { @@ -583,6 +653,11 @@ func (s *Swap) saveLastSentCheque(p enode.ID, cheque *Cheque) error { return s.store.Put(sentChequeKey(p), cheque) } +// savePendingCheque saves cheque as the last pending cheque for peer +func (s *Swap) savePendingCheque(p enode.ID, cheque *Cheque) error { + return s.store.Put(pendingChequeKey(p), cheque) +} + // saveBalance saves balance as the current balance for peer func (s *Swap) saveBalance(p enode.ID, balance int64) error { return s.store.Put(balanceKey(p), balance) diff --git a/swap/swap_test.go b/swap/swap_test.go index 020a5692f8..5643fe1696 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -925,7 +925,7 @@ func TestPaymentThreshold(t *testing.T) { } var cheque *Cheque - _ = swap.store.Get(sentChequeKey(testPeer.Peer.ID()), &cheque) + _ = swap.store.Get(pendingChequeKey(testPeer.Peer.ID()), &cheque) if cheque.CumulativePayout != DefaultPaymentThreshold { t.Fatal() } @@ -982,6 +982,10 @@ func TestResetBalance(t *testing.T) { // now simulate sending the cheque to the creditor from the debitor creditor.sendCheque() + + debitorSwap.handleConfirmChequeMsg(ctx, creditor, &ConfirmChequeMsg{ + Cheque: creditor.getPendingCheque(), + }) // the debitor should have already reset its balance if creditor.getBalance() != 0 { t.Fatalf("unexpected balance to be 0, but it is %d", creditor.getBalance()) diff --git a/swap/types.go b/swap/types.go index b97beaf207..2338e7eed4 100644 --- a/swap/types.go +++ b/swap/types.go @@ -43,3 +43,7 @@ type HandshakeMsg struct { type EmitChequeMsg struct { Cheque *Cheque } + +type ConfirmChequeMsg struct { + Cheque *Cheque +} From c8aebdd93e0155c7173639573f0d30cd20f7731c Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Tue, 22 Oct 2019 13:28:45 +0200 Subject: [PATCH 02/26] swap: return if confirm while no pending, add comments to peer getters --- swap/peer.go | 14 ++++++++++++++ swap/swap.go | 1 + 2 files changed, 15 insertions(+) diff --git a/swap/peer.go b/swap/peer.go index d1e4934653..53a4f035e8 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -74,33 +74,44 @@ func NewPeer(p *protocols.Peer, s *Swap, beneficiary common.Address, contractAdd return peer, nil } +// getLastReceivedCheque returns the last cheque we received for this peer +// To be called with mutex already held func (p *Peer) getLastReceivedCheque() *Cheque { return p.lastReceivedCheque } +// getLastSentCheque returns the last cheque we sent and got confirmed for this peer +// To be called with mutex already held func (p *Peer) getLastSentCheque() *Cheque { return p.lastSentCheque } +// getPendingCheque returns the last cheque we sent but that is not yet confirmed for this peer +// To be called with mutex already held func (p *Peer) getPendingCheque() *Cheque { return p.pendingCheque } +// To be called with mutex already held func (p *Peer) setLastReceivedCheque(cheque *Cheque) error { p.lastReceivedCheque = cheque return p.swap.saveLastReceivedCheque(p.ID(), cheque) } +// To be called with mutex already held func (p *Peer) setLastSentCheque(cheque *Cheque) error { p.lastSentCheque = cheque return p.swap.saveLastSentCheque(p.ID(), cheque) } +// To be called with mutex already held func (p *Peer) setPendingCheque(cheque *Cheque) error { p.pendingCheque = cheque return p.swap.savePendingCheque(p.ID(), cheque) } +// getLastSentCumulativePayout returns the cumulative payout of the last sent cheque or 0 if there is none +// To be called with mutex already held func (p *Peer) getLastSentCumulativePayout() uint64 { lastCheque := p.getLastSentCheque() if lastCheque != nil { @@ -109,11 +120,14 @@ func (p *Peer) getLastSentCumulativePayout() uint64 { return 0 } +// To be called with mutex already held func (p *Peer) setBalance(balance int64) error { p.balance = balance return p.swap.saveBalance(p.ID(), balance) } +// getBalance returns the current balance for this peer +// To be called with mutex already held func (p *Peer) getBalance() int64 { return p.balance } diff --git a/swap/swap.go b/swap/swap.go index 887e474b1c..191545816a 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -402,6 +402,7 @@ func (s *Swap) handleConfirmChequeMsg(ctx context.Context, p *Peer, msg *Confirm if p.getPendingCheque() == nil { p.logger.Warn("ignoring confirm msg, no pending cheque") + return } if !cheque.Equal(p.getPendingCheque()) { From ff1bbfc870b0a4ffe25282d9e49669a71c98b953 Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Tue, 22 Oct 2019 15:03:33 +0200 Subject: [PATCH 03/26] swap: add comments for pending cheques --- swap/peer.go | 16 +++++++++------- swap/types.go | 3 ++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/swap/peer.go b/swap/peer.go index 53a4f035e8..254f8be428 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -36,13 +36,13 @@ type Peer struct { *protocols.Peer lock sync.RWMutex swap *Swap - beneficiary common.Address - contractAddress common.Address - lastReceivedCheque *Cheque - lastSentCheque *Cheque - pendingCheque *Cheque - balance int64 - logger log.Logger // logger for swap related messages and audit trail with peer identifier + beneficiary common.Address // address of the peers chequebook owner + contractAddress common.Address // address of the peers chequebook + lastReceivedCheque *Cheque // last cheque we received from the peer + lastSentCheque *Cheque // last cheque that was sent to peer that was confirmed + pendingCheque *Cheque // last cheque that was sent to peer but is not yet confirmed + balance int64 // current balance of the peer + logger log.Logger // logger for swap related messages and audit trail with peer identifier } // NewPeer creates a new swap Peer instance @@ -180,6 +180,8 @@ func (p *Peer) createCheque() (*Cheque, error) { } // sendCheque sends a cheque to peer +// if there is already a pending cheque it will resend that one +// otherwise it will create a new cheque and save it as the pending cheque // To be called with mutex already held // Caller must be careful that the same resources aren't concurrently read and written by multiple routines func (p *Peer) sendCheque() error { diff --git a/swap/types.go b/swap/types.go index 2338e7eed4..246d4cf547 100644 --- a/swap/types.go +++ b/swap/types.go @@ -39,11 +39,12 @@ type HandshakeMsg struct { ContractAddress common.Address } -// EmitChequeMsg is sent from the debitor to the creditor with the actual check +// EmitChequeMsg is sent from the debitor to the creditor with the actual cheque type EmitChequeMsg struct { Cheque *Cheque } +// ConfirmChequeMsg is sent from the creditor to the debitor with the cheque to confirm successful processing type ConfirmChequeMsg struct { Cheque *Cheque } From ea751978e36cb8f35cc7c9b822654a87db646249 Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Tue, 22 Oct 2019 15:03:50 +0200 Subject: [PATCH 04/26] swap: add pending cheque key to TestStoreKeys --- swap/swap_test.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/swap/swap_test.go b/swap/swap_test.go index 5643fe1696..328f03101e 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -475,15 +475,16 @@ type storeKeysTestCases struct { expectedBalanceKey string expectedSentChequeKey string expectedReceivedChequeKey string + expectedPendingChequeKey string expectedUsedChequebookKey string } // Test the getting balance and cheques store keys based on a node ID, and the reverse process as well func TestStoreKeys(t *testing.T) { testCases := []storeKeysTestCases{ - {enode.HexID("f6876a1f73947b0495d36e648aeb74f952220c3b03e66a1cc786863f6104fa56"), "balance_f6876a1f73947b0495d36e648aeb74f952220c3b03e66a1cc786863f6104fa56", "sent_cheque_f6876a1f73947b0495d36e648aeb74f952220c3b03e66a1cc786863f6104fa56", "received_cheque_f6876a1f73947b0495d36e648aeb74f952220c3b03e66a1cc786863f6104fa56", "connected_chequebook"}, - {enode.HexID("93a3309412ff6204ec9b9469200742f62061932009e744def79ef96492673e6c"), "balance_93a3309412ff6204ec9b9469200742f62061932009e744def79ef96492673e6c", "sent_cheque_93a3309412ff6204ec9b9469200742f62061932009e744def79ef96492673e6c", "received_cheque_93a3309412ff6204ec9b9469200742f62061932009e744def79ef96492673e6c", "connected_chequebook"}, - {enode.HexID("c19ecf22f02f77f4bb320b865d3f37c6c592d32a1c9b898efb552a5161a1ee44"), "balance_c19ecf22f02f77f4bb320b865d3f37c6c592d32a1c9b898efb552a5161a1ee44", "sent_cheque_c19ecf22f02f77f4bb320b865d3f37c6c592d32a1c9b898efb552a5161a1ee44", "received_cheque_c19ecf22f02f77f4bb320b865d3f37c6c592d32a1c9b898efb552a5161a1ee44", "connected_chequebook"}, + {enode.HexID("f6876a1f73947b0495d36e648aeb74f952220c3b03e66a1cc786863f6104fa56"), "balance_f6876a1f73947b0495d36e648aeb74f952220c3b03e66a1cc786863f6104fa56", "sent_cheque_f6876a1f73947b0495d36e648aeb74f952220c3b03e66a1cc786863f6104fa56", "received_cheque_f6876a1f73947b0495d36e648aeb74f952220c3b03e66a1cc786863f6104fa56", "pending_cheque_f6876a1f73947b0495d36e648aeb74f952220c3b03e66a1cc786863f6104fa56", "connected_chequebook"}, + {enode.HexID("93a3309412ff6204ec9b9469200742f62061932009e744def79ef96492673e6c"), "balance_93a3309412ff6204ec9b9469200742f62061932009e744def79ef96492673e6c", "sent_cheque_93a3309412ff6204ec9b9469200742f62061932009e744def79ef96492673e6c", "received_cheque_93a3309412ff6204ec9b9469200742f62061932009e744def79ef96492673e6c", "pending_cheque_93a3309412ff6204ec9b9469200742f62061932009e744def79ef96492673e6c", "connected_chequebook"}, + {enode.HexID("c19ecf22f02f77f4bb320b865d3f37c6c592d32a1c9b898efb552a5161a1ee44"), "balance_c19ecf22f02f77f4bb320b865d3f37c6c592d32a1c9b898efb552a5161a1ee44", "sent_cheque_c19ecf22f02f77f4bb320b865d3f37c6c592d32a1c9b898efb552a5161a1ee44", "received_cheque_c19ecf22f02f77f4bb320b865d3f37c6c592d32a1c9b898efb552a5161a1ee44", "pending_cheque_c19ecf22f02f77f4bb320b865d3f37c6c592d32a1c9b898efb552a5161a1ee44", "connected_chequebook"}, } testStoreKeys(t, testCases) } @@ -494,6 +495,7 @@ func testStoreKeys(t *testing.T, testCases []storeKeysTestCases) { actualBalanceKey := balanceKey(testCase.nodeID) actualSentChequeKey := sentChequeKey(testCase.nodeID) actualReceivedChequeKey := receivedChequeKey(testCase.nodeID) + actualPendingChequeKey := pendingChequeKey(testCase.nodeID) actualUsedChequebookKey := connectedChequebookKey if actualBalanceKey != testCase.expectedBalanceKey { @@ -506,9 +508,12 @@ func testStoreKeys(t *testing.T, testCases []storeKeysTestCases) { t.Fatalf("Expected received cheque key to be %s, but is %s instead.", testCase.expectedReceivedChequeKey, actualReceivedChequeKey) } + if actualPendingChequeKey != testCase.expectedPendingChequeKey { + t.Fatalf("Expected received cheque key to be %s, but is %s instead.", testCase.expectedReceivedChequeKey, actualReceivedChequeKey) + } + if actualUsedChequebookKey != testCase.expectedUsedChequebookKey { t.Fatalf("Expected used chequebook key to be %s, but is %s instead.", testCase.expectedUsedChequebookKey, actualUsedChequebookKey) - } nodeID := keyToID(actualBalanceKey, balancePrefix) From b80884a9b6bdce001591ef6ecef552905e70e9b6 Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Tue, 22 Oct 2019 15:10:00 +0200 Subject: [PATCH 05/26] swap: use same err in same block --- swap/swap.go | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/swap/swap.go b/swap/swap.go index 191545816a..5faa9b0b8e 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -358,14 +358,14 @@ func (s *Swap) handleEmitChequeMsg(ctx context.Context, p *Peer, msg *EmitCheque // reset balance by amount // as this is done by the creditor, receiving the cheque, the amount should be negative, // so that updateBalance will calculate balance + amount which result in reducing the peer's balance - if err := p.updateBalance(-int64(cheque.Honey)); err != nil { + err = p.updateBalance(-int64(cheque.Honey)) + if err != nil { log.Error("error updating balance", "err", err) return err } - if err := p.Send(ctx, &ConfirmChequeMsg{ - Cheque: cheque, - }); err != nil { + err = p.Send(ctx, &ConfirmChequeMsg{Cheque: cheque}) + if err != nil { return err } @@ -410,23 +410,27 @@ func (s *Swap) handleConfirmChequeMsg(ctx context.Context, p *Peer, msg *Confirm return } - if err := p.setLastSentCheque(cheque); err != nil { + err := p.setLastSentCheque(cheque) + if err != nil { p.Drop("persistence error") return } - if err := p.setPendingCheque(nil); err != nil { + err = p.setPendingCheque(nil) + if err != nil { p.Drop("persistence error") return } - if err := p.updateBalance(int64(cheque.Honey)); err != nil { + err = p.updateBalance(int64(cheque.Honey)) + if err != nil { p.Drop("persistence error") return } // since more swap traffic might have occurred since this cheque was already sent, we redo the payment threshold check - if err := s.checkPaymentThreshold(p); err != nil { + err = s.checkPaymentThreshold(p) + if err != nil { p.logger.Warn("failed to send already due cheque", "error", err) return } From 7c928ac4fe65fae334f937dc7004fdc0e2358b4f Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Thu, 24 Oct 2019 19:29:21 +0200 Subject: [PATCH 06/26] swap: check for pending cheque in payment threshold test --- swap/protocol_test.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/swap/protocol_test.go b/swap/protocol_test.go index efdac037dc..56eb15a228 100644 --- a/swap/protocol_test.go +++ b/swap/protocol_test.go @@ -233,6 +233,7 @@ func TestTriggerPaymentThreshold(t *testing.T) { // set the balance to manually be at PaymentThreshold overDraft := 42 + expectedAmount := uint64(overDraft) + DefaultPaymentThreshold creditor.setBalance(-int64(DefaultPaymentThreshold)) // we expect a cheque at the end of the test, but not yet @@ -245,6 +246,21 @@ func TestTriggerPaymentThreshold(t *testing.T) { t.Fatal(err) } + // balance should only be adjusted by overDraft prior to confirm msg + if creditor.getBalance() != -int64(expectedAmount) { + t.Fatalf("Expected debitorSwap balance to be 0, but is %d", creditor.getBalance()) + } + + // pending cheque should now be set + pending := creditor.getPendingCheque() + if pending == nil { + t.Fatal("Expected pending cheque") + } + + if pending.CumulativePayout != expectedAmount { + t.Fatalf("Expected cheque cumulative payout to be %d, but is %d", expectedAmount, pending.CumulativePayout) + } + debitorSwap.handleConfirmChequeMsg(ctx, creditor, &ConfirmChequeMsg{ Cheque: creditor.getPendingCheque(), }) @@ -255,7 +271,7 @@ func TestTriggerPaymentThreshold(t *testing.T) { } cheque := creditor.getLastSentCheque() - expectedAmount := uint64(overDraft) + DefaultPaymentThreshold + if cheque.CumulativePayout != expectedAmount { t.Fatalf("Expected cheque cumulative payout to be %d, but is %d", expectedAmount, cheque.CumulativePayout) } From 906f5b47a73de4acf260e2061e44795124a4c928 Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Fri, 25 Oct 2019 13:49:13 +0200 Subject: [PATCH 07/26] swap: rename checkPaymentThreshold and add comment about lock --- swap/peer.go | 26 ++++++++++++-------------- swap/swap.go | 11 ++++++----- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/swap/peer.go b/swap/peer.go index 254f8be428..f5adca2a98 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -75,43 +75,43 @@ func NewPeer(p *protocols.Peer, s *Swap, beneficiary common.Address, contractAdd } // getLastReceivedCheque returns the last cheque we received for this peer -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) getLastReceivedCheque() *Cheque { return p.lastReceivedCheque } // getLastSentCheque returns the last cheque we sent and got confirmed for this peer -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) getLastSentCheque() *Cheque { return p.lastSentCheque } // getPendingCheque returns the last cheque we sent but that is not yet confirmed for this peer -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) getPendingCheque() *Cheque { return p.pendingCheque } -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) setLastReceivedCheque(cheque *Cheque) error { p.lastReceivedCheque = cheque return p.swap.saveLastReceivedCheque(p.ID(), cheque) } -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) setLastSentCheque(cheque *Cheque) error { p.lastSentCheque = cheque return p.swap.saveLastSentCheque(p.ID(), cheque) } -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) setPendingCheque(cheque *Cheque) error { p.pendingCheque = cheque return p.swap.savePendingCheque(p.ID(), cheque) } // getLastSentCumulativePayout returns the cumulative payout of the last sent cheque or 0 if there is none -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) getLastSentCumulativePayout() uint64 { lastCheque := p.getLastSentCheque() if lastCheque != nil { @@ -120,19 +120,19 @@ func (p *Peer) getLastSentCumulativePayout() uint64 { return 0 } -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) setBalance(balance int64) error { p.balance = balance return p.swap.saveBalance(p.ID(), balance) } // getBalance returns the current balance for this peer -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) getBalance() int64 { return p.balance } -// To be called with mutex already held +// the caller is expected to hold p.lock func (p *Peer) updateBalance(amount int64) error { //adjust the balance //if amount is negative, it will decrease, otherwise increase @@ -147,8 +147,7 @@ func (p *Peer) updateBalance(amount int64) error { // createCheque creates a new cheque whose beneficiary will be the peer and // whose amount is based on the last cheque and current balance for this peer // The cheque will be signed and point to the issuer's contract -// To be called with mutex already held -// Caller must be careful that the same resources aren't concurrently read and written by multiple routines +// the caller is expected to hold p.lock func (p *Peer) createCheque() (*Cheque, error) { var cheque *Cheque var err error @@ -182,8 +181,7 @@ func (p *Peer) createCheque() (*Cheque, error) { // sendCheque sends a cheque to peer // if there is already a pending cheque it will resend that one // otherwise it will create a new cheque and save it as the pending cheque -// To be called with mutex already held -// Caller must be careful that the same resources aren't concurrently read and written by multiple routines +// the caller is expected to hold p.lock func (p *Peer) sendCheque() error { if p.getPendingCheque() != nil { p.logger.Warn("old cheque still pending, resending cheque") diff --git a/swap/swap.go b/swap/swap.go index 5faa9b0b8e..ee51f9656e 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -302,13 +302,14 @@ func (s *Swap) Add(amount int64, peer *protocols.Peer) (err error) { return err } - return s.checkPaymentThreshold(swapPeer) + return s.checkPaymentThresholdAndSendCheque(swapPeer) } -// Check if balance with peer crosses the payment threshold +// checkPaymentThresholdAndSendCheque checks if balance with peer crosses the payment threshold and attempts to send a cheque if so // It is the peer with a negative balance who sends a cheque, thus we check // that the balance is *below* the threshold -func (s *Swap) checkPaymentThreshold(swapPeer *Peer) error { +// the caller is expected to hold swapPeer.lock +func (s *Swap) checkPaymentThresholdAndSendCheque(swapPeer *Peer) error { if swapPeer.getBalance() <= -s.params.PaymentThreshold { swapPeer.logger.Info("balance for peer went over the payment threshold, sending cheque", "payment threshold", s.params.PaymentThreshold) return swapPeer.sendCheque() @@ -401,7 +402,7 @@ func (s *Swap) handleConfirmChequeMsg(ctx context.Context, p *Peer, msg *Confirm cheque := msg.Cheque if p.getPendingCheque() == nil { - p.logger.Warn("ignoring confirm msg, no pending cheque") + p.logger.Warn("ignoring confirm msg, no pending cheque", "got", cheque) return } @@ -429,7 +430,7 @@ func (s *Swap) handleConfirmChequeMsg(ctx context.Context, p *Peer, msg *Confirm } // since more swap traffic might have occurred since this cheque was already sent, we redo the payment threshold check - err = s.checkPaymentThreshold(p) + err = s.checkPaymentThresholdAndSendCheque(p) if err != nil { p.logger.Warn("failed to send already due cheque", "error", err) return From af27bb81565b0c714e7539ecc321790cd426405b Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Mon, 28 Oct 2019 16:32:53 +0100 Subject: [PATCH 08/26] swap: adjust comments and error message --- swap/peer.go | 12 ++++++++---- swap/swap.go | 4 +++- swap/swap_test.go | 2 +- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/swap/peer.go b/swap/peer.go index f5adca2a98..887480c5a8 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -92,18 +92,21 @@ func (p *Peer) getPendingCheque() *Cheque { return p.pendingCheque } +// setLastReceivedCheque sets the given cheque as the last one received from this peer // the caller is expected to hold p.lock func (p *Peer) setLastReceivedCheque(cheque *Cheque) error { p.lastReceivedCheque = cheque return p.swap.saveLastReceivedCheque(p.ID(), cheque) } +// setLastReceivedCheque sets the given cheque as the last sent cheque for this peer // the caller is expected to hold p.lock func (p *Peer) setLastSentCheque(cheque *Cheque) error { p.lastSentCheque = cheque return p.swap.saveLastSentCheque(p.ID(), cheque) } +// setLastReceivedCheque sets the given cheque as the pending cheque for this peer // the caller is expected to hold p.lock func (p *Peer) setPendingCheque(cheque *Cheque) error { p.pendingCheque = cheque @@ -178,13 +181,13 @@ func (p *Peer) createCheque() (*Cheque, error) { return cheque, err } -// sendCheque sends a cheque to peer +// sendCheque creates and sends a cheque to peer // if there is already a pending cheque it will resend that one // otherwise it will create a new cheque and save it as the pending cheque // the caller is expected to hold p.lock func (p *Peer) sendCheque() error { if p.getPendingCheque() != nil { - p.logger.Warn("old cheque still pending, resending cheque") + p.logger.Info("previous cheque still pending, resending cheque", "pending", p.getPendingCheque()) return p.Send(context.Background(), &EmitChequeMsg{ Cheque: p.getPendingCheque(), }) @@ -194,8 +197,9 @@ func (p *Peer) sendCheque() error { return fmt.Errorf("error while creating cheque: %v", err) } - if err = p.setPendingCheque(cheque); err != nil { - return err + err = p.setPendingCheque(cheque) + if err != nil { + return fmt.Errorf("error while saving pending cheque: %v", err) } p.logger.Info("sending cheque to peer", "honey", cheque.Honey, "cumulativePayout", cheque.ChequeParams.CumulativePayout, "beneficiary", cheque.Beneficiary, "contract", cheque.Contract) diff --git a/swap/swap.go b/swap/swap.go index ee51f9656e..5b274486c7 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -365,7 +365,9 @@ func (s *Swap) handleEmitChequeMsg(ctx context.Context, p *Peer, msg *EmitCheque return err } - err = p.Send(ctx, &ConfirmChequeMsg{Cheque: cheque}) + err = p.Send(ctx, &ConfirmChequeMsg{ + Cheque: cheque, + }) if err != nil { return err } diff --git a/swap/swap_test.go b/swap/swap_test.go index 328f03101e..3932a2af2e 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -509,7 +509,7 @@ func testStoreKeys(t *testing.T, testCases []storeKeysTestCases) { } if actualPendingChequeKey != testCase.expectedPendingChequeKey { - t.Fatalf("Expected received cheque key to be %s, but is %s instead.", testCase.expectedReceivedChequeKey, actualReceivedChequeKey) + t.Fatalf("Expected pending cheque key to be %s, but is %s instead.", testCase.expectedPendingChequeKey, actualPendingChequeKey) } if actualUsedChequebookKey != testCase.expectedUsedChequebookKey { From afda8db80e3cb487f7ccc30ca38abd7945244054 Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Tue, 29 Oct 2019 13:55:56 +0100 Subject: [PATCH 09/26] swap: dont crash if there is no last received cheque --- swap/swap.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/swap/swap.go b/swap/swap.go index 5b274486c7..c15efe606e 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -341,8 +341,8 @@ func (s *Swap) handleEmitChequeMsg(ctx context.Context, p *Peer, msg *EmitCheque cheque := msg.Cheque p.logger.Info("received cheque from peer", "honey", cheque.Honey) - if cheque == p.getLastReceivedCheque() { - p.logger.Warn("received old cheque from peer", "cumulativePayout", cheque.CumulativePayout) + if p.getLastReceivedCheque() != nil && cheque.Equal(p.getLastReceivedCheque()) { + p.logger.Warn("cheque sent by peer has already been received in the past", "cumulativePayout", cheque.CumulativePayout) return p.Send(ctx, &ConfirmChequeMsg{ Cheque: cheque, }) From 0d20163e1d50b0e293e7370e3dcd11de34d9d4f7 Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Tue, 29 Oct 2019 13:56:30 +0100 Subject: [PATCH 10/26] swap: improve logs and error messages --- swap/swap.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/swap/swap.go b/swap/swap.go index c15efe606e..a3e2f95c84 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -350,7 +350,7 @@ func (s *Swap) handleEmitChequeMsg(ctx context.Context, p *Peer, msg *EmitCheque _, err := s.processAndVerifyCheque(cheque, p) if err != nil { - log.Error("error processing and verifying cheque", "err", err) + log.Error("error processing and verifying received cheque", "err", err) return err } @@ -404,30 +404,30 @@ func (s *Swap) handleConfirmChequeMsg(ctx context.Context, p *Peer, msg *Confirm cheque := msg.Cheque if p.getPendingCheque() == nil { - p.logger.Warn("ignoring confirm msg, no pending cheque", "got", cheque) + p.logger.Warn("ignoring confirm msg, no pending cheque", "confirm message cheque", cheque) return } if !cheque.Equal(p.getPendingCheque()) { - p.logger.Warn("ignoring confirm msg, wrong cheque", "got", cheque, "expected", p.getPendingCheque()) + p.logger.Warn("ignoring confirm msg, unexpected cheque", "got", cheque, "expected", p.getPendingCheque()) return } err := p.setLastSentCheque(cheque) if err != nil { - p.Drop("persistence error") + p.Drop(fmt.Sprintf("persistence error: %v", err)) return } err = p.setPendingCheque(nil) if err != nil { - p.Drop("persistence error") + p.Drop(fmt.Sprintf("persistence error: %v", err)) return } err = p.updateBalance(int64(cheque.Honey)) if err != nil { - p.Drop("persistence error") + p.Drop(fmt.Sprintf("persistence error: %v", err)) return } From a498e937dee2198835d49d9511039b8ed79d2add Mon Sep 17 00:00:00 2001 From: mortelli Date: Fri, 1 Nov 2019 17:02:16 -0300 Subject: [PATCH 11/26] swap: check available balance before sending cheque, add placeholder sentCheques function --- swap/peer.go | 10 +++++++++ swap/simulations_test.go | 2 +- swap/swap.go | 47 ++++++++++++++++++++++++++++++++++------ swap/swap_test.go | 1 - 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/swap/peer.go b/swap/peer.go index 887480c5a8..a064fcab3a 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -197,6 +197,16 @@ func (p *Peer) sendCheque() error { return fmt.Errorf("error while creating cheque: %v", err) } + chequebookBalance, err := p.swap.AvailableBalance() + if err != nil { + return fmt.Errorf("error while getting available balance: %v", err) + } + + chequeAmount := uint64(-p.getBalance()) + if chequebookBalance < chequeAmount { + return fmt.Errorf("cannot send created cheque, amount %d is greater than chequebook available balance %d", chequeAmount, chequebookBalance) + } + err = p.setPendingCheque(cheque) if err != nil { return fmt.Errorf("error while saving pending cheque: %v", err) diff --git a/swap/simulations_test.go b/swap/simulations_test.go index 62fb141edb..b04f5e7a41 100644 --- a/swap/simulations_test.go +++ b/swap/simulations_test.go @@ -413,7 +413,7 @@ func TestMultiChequeSimulation(t *testing.T) { } // we will send just maxCheques number of cheques - maxCheques := 10 + maxCheques := 3 // the peer object used for sending debitorSvc.lock.Lock() diff --git a/swap/swap.go b/swap/swap.go index 6ec67c7d50..bc37bdb4f5 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -568,6 +568,43 @@ func (s *Swap) Cheques() (map[enode.ID]*PeerCheques, error) { return cheques, nil } +// placeholder: issue of locks +func (s *Swap) sentCheques() (map[enode.ID]*Cheque, error) { + sentCheques := make(map[enode.ID]*Cheque) + // get sent cheques from memory + s.peersLock.Lock() + for peer, swapPeer := range s.peers { + sentCheque := swapPeer.getLastSentCheque() + // don't add peer to result if there are no cheques + if sentCheque != nil { + sentCheques[peer] = sentCheque + } + } + s.peersLock.Unlock() + + // get sent cheques from store + chequesIterFunction := func(key []byte, value []byte) (stop bool, err error) { + peer := keyToID(string(key), sentChequePrefix) + // add cheque if not present yet + if sentCheque := sentCheques[peer]; sentCheque == nil { + // add cheque from store if not already in result + var sentCheque Cheque + err = json.Unmarshal(value, &sentCheque) + if err == nil { + sentCheques[peer] = &sentCheque + } + } + return stop, err + } + + err := s.store.Iterate(sentChequePrefix, chequesIterFunction) + if err != nil { + return nil, err + } + + return sentCheques, nil +} + // AvailableBalance returns the total balance of the chequebook against which new cheques can be written func (s *Swap) AvailableBalance() (uint64, error) { // get the LiquidBalance of the chequebook @@ -576,8 +613,8 @@ func (s *Swap) AvailableBalance() (uint64, error) { return 0, err } - // get all cheques - cheques, err := s.Cheques() + // get sent cheques + sentCheques, err := s.sentCheques() if err != nil { return 0, err } @@ -585,11 +622,7 @@ func (s *Swap) AvailableBalance() (uint64, error) { // Compute the total worth of cheques sent and how much of of this is cashed var sentChequesWorth uint64 var cashedChequesWorth uint64 - for _, peerCheques := range cheques { - sentCheque := peerCheques.LastSentCheque - if sentCheque == nil { - continue - } + for _, sentCheque := range sentCheques { sentChequesWorth += sentCheque.ChequeParams.CumulativePayout paidOut, err := s.contract.PaidOut(nil, sentCheque.ChequeParams.Beneficiary) if err != nil { diff --git a/swap/swap_test.go b/swap/swap_test.go index eb6fa00b07..656ae89b64 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -2068,7 +2068,6 @@ func TestAvailableBalance(t *testing.T) { if availableBalance != (netDeposit - uint64(chequeAmount)) { t.Fatalf("availableBalance not equal to deposited minus withdraw. availableBalance: %d, deposit minus withdrawn: %d", availableBalance, depositAmount.Uint64()-withdrawAmount.Uint64()) } - } // dummyMsgRW implements MessageReader and MessageWriter From e22d28e31c4644cf0c9f9ca0669bbd2f75cda5a8 Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Mon, 4 Nov 2019 11:24:40 +0100 Subject: [PATCH 12/26] swap: add pending cheques to sentcheques and availablebalance --- swap/swap.go | 31 ++++++++++++++++++++++++------- swap/swap_test.go | 34 +++++++++++++++++----------------- 2 files changed, 41 insertions(+), 24 deletions(-) diff --git a/swap/swap.go b/swap/swap.go index 6ec67c7d50..3afc6a89b3 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -545,18 +545,23 @@ func (s *Swap) Cheques() (map[enode.ID]*PeerCheques, error) { s.peersLock.Lock() for peer, swapPeer := range s.peers { swapPeer.lock.Lock() + pendingCheque := swapPeer.getPendingCheque() sentCheque := swapPeer.getLastSentCheque() receivedCheque := swapPeer.getLastReceivedCheque() // don't add peer to result if there are no cheques - if sentCheque != nil || receivedCheque != nil { - cheques[peer] = &PeerCheques{sentCheque, receivedCheque} + if sentCheque != nil || receivedCheque != nil || pendingCheque != nil { + cheques[peer] = &PeerCheques{pendingCheque, sentCheque, receivedCheque} } swapPeer.lock.Unlock() } s.peersLock.Unlock() // get peer cheques from store - err := s.addStoreCheques(sentChequePrefix, cheques) + err := s.addStoreCheques(pendingChequePrefix, cheques) + if err != nil { + return nil, err + } + err = s.addStoreCheques(sentChequePrefix, cheques) if err != nil { return nil, err } @@ -586,8 +591,12 @@ func (s *Swap) AvailableBalance() (uint64, error) { var sentChequesWorth uint64 var cashedChequesWorth uint64 for _, peerCheques := range cheques { - sentCheque := peerCheques.LastSentCheque - if sentCheque == nil { + var sentCheque *Cheque + if peerCheques.PendingCheque != nil { + sentCheque = peerCheques.PendingCheque + } else if peerCheques.LastSentCheque != nil { + sentCheque = peerCheques.LastSentCheque + } else { continue } sentChequesWorth += sentCheque.ChequeParams.CumulativePayout @@ -614,6 +623,8 @@ func (s *Swap) addStoreCheques(chequePrefix string, cheques map[enode.ID]*PeerCh err = json.Unmarshal(value, &peerCheque) if err == nil { switch chequePrefix { + case pendingChequePrefix: + cheques[peer].PendingCheque = &peerCheque case sentChequePrefix: cheques[peer].LastSentCheque = &peerCheque case receivedChequePrefix: @@ -629,19 +640,25 @@ func (s *Swap) addStoreCheques(chequePrefix string, cheques map[enode.ID]*PeerCh // PeerCheques contains the last cheque known to have been sent to a peer, as well as the last one received from the peer type PeerCheques struct { + PendingCheque *Cheque LastSentCheque *Cheque LastReceivedCheque *Cheque } // PeerCheques returns the last sent and received cheques for a given peer func (s *Swap) PeerCheques(peer enode.ID) (PeerCheques, error) { - var sentCheque, receivedCheque *Cheque + var pendingCheque, sentCheque, receivedCheque *Cheque swapPeer := s.getPeer(peer) if swapPeer != nil { + pendingCheque = swapPeer.getPendingCheque() sentCheque = swapPeer.getLastSentCheque() receivedCheque = swapPeer.getLastReceivedCheque() } else { + errPendingCheque := s.store.Get(pendingChequeKey(peer), &pendingCheque) + if errPendingCheque != nil && errPendingCheque != state.ErrNotFound { + return PeerCheques{}, errPendingCheque + } errSentCheque := s.store.Get(sentChequeKey(peer), &sentCheque) if errSentCheque != nil && errSentCheque != state.ErrNotFound { return PeerCheques{}, errSentCheque @@ -651,7 +668,7 @@ func (s *Swap) PeerCheques(peer enode.ID) (PeerCheques, error) { return PeerCheques{}, errReceivedCheque } } - return PeerCheques{sentCheque, receivedCheque}, nil + return PeerCheques{pendingCheque, sentCheque, receivedCheque}, nil } // loadLastReceivedCheque loads the last received cheque for the peer from the store diff --git a/swap/swap_test.go b/swap/swap_test.go index aa7c250410..46032e56e1 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -264,7 +264,7 @@ func TestCheques(t *testing.T) { storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer.ID(): {testPeerSentCheque, nil}, + testPeer.ID(): {nil, testPeerSentCheque, nil}, }, }, { @@ -275,7 +275,7 @@ func TestCheques(t *testing.T) { storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer.ID(): {testPeerSentCheque, testPeerReceivedCheque}, + testPeer.ID(): {nil, testPeerSentCheque, testPeerReceivedCheque}, }, }, { @@ -286,8 +286,8 @@ func TestCheques(t *testing.T) { storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer.ID(): {testPeerSentCheque, testPeerReceivedCheque}, - testPeer2.ID(): {testPeer2SentCheque, testPeer2ReceivedCheque}, + testPeer.ID(): {nil, testPeerSentCheque, testPeerReceivedCheque}, + testPeer2.ID(): {nil, testPeer2SentCheque, testPeer2ReceivedCheque}, }, }, { @@ -298,8 +298,8 @@ func TestCheques(t *testing.T) { storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer.ID(): {testPeerSentCheque2, testPeerReceivedCheque}, - testPeer2.ID(): {testPeer2SentCheque, testPeer2ReceivedCheque2}, + testPeer.ID(): {nil, testPeerSentCheque2, testPeerReceivedCheque}, + testPeer2.ID(): {nil, testPeer2SentCheque, testPeer2ReceivedCheque2}, }, }, { @@ -310,7 +310,7 @@ func TestCheques(t *testing.T) { storeSentCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3SentCheque}, storeReceivedCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3ReceivedCheque}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer3ID: {testPeer3SentCheque, testPeer3ReceivedCheque}, + testPeer3ID: {nil, testPeer3SentCheque, testPeer3ReceivedCheque}, }, }, { @@ -321,7 +321,7 @@ func TestCheques(t *testing.T) { storeSentCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3SentCheque, testPeer3ID: testPeer3SentCheque2}, storeReceivedCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3ReceivedCheque, testPeer3ID: testPeer3ReceivedCheque2}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer3ID: {testPeer3SentCheque2, testPeer3ReceivedCheque2}, + testPeer3ID: {nil, testPeer3SentCheque2, testPeer3ReceivedCheque2}, }, }, { @@ -332,9 +332,9 @@ func TestCheques(t *testing.T) { storeSentCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3SentCheque, testPeer3ID: testPeer3SentCheque2}, storeReceivedCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3ReceivedCheque, testPeer3ID: testPeer3ReceivedCheque2}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer.ID(): {testPeerSentCheque2, testPeerReceivedCheque}, - testPeer2.ID(): {testPeer2SentCheque, testPeer2ReceivedCheque2}, - testPeer3ID: {testPeer3SentCheque2, testPeer3ReceivedCheque2}, + testPeer.ID(): {nil, testPeerSentCheque2, testPeerReceivedCheque}, + testPeer2.ID(): {nil, testPeer2SentCheque, testPeer2ReceivedCheque2}, + testPeer3ID: {nil, testPeer3SentCheque2, testPeer3ReceivedCheque2}, }, }, } @@ -436,28 +436,28 @@ func TestPeerCheques(t *testing.T) { peer: testPeer, sentCheque: nil, receivedCheque: nil, - expectedCheques: PeerCheques{nil, nil}, + expectedCheques: PeerCheques{nil, nil, nil}, }, { name: "peer 1 with sent cheque", peer: testPeer, sentCheque: testPeerSentCheque, receivedCheque: nil, - expectedCheques: PeerCheques{testPeerSentCheque, nil}, + expectedCheques: PeerCheques{nil, testPeerSentCheque, nil}, }, { name: "peer 1 with sent and received cheque", peer: testPeer, sentCheque: testPeerSentCheque, receivedCheque: testPeerReceivedCheque, - expectedCheques: PeerCheques{testPeerSentCheque, testPeerReceivedCheque}, + expectedCheques: PeerCheques{nil, testPeerSentCheque, testPeerReceivedCheque}, }, { name: "peer 2 with received cheque", peer: testPeer2, sentCheque: nil, receivedCheque: testPeer2ReceivedCheque, - expectedCheques: PeerCheques{nil, testPeer2ReceivedCheque}, + expectedCheques: PeerCheques{nil, nil, testPeer2ReceivedCheque}, }, } // verify test cases @@ -467,7 +467,7 @@ func TestPeerCheques(t *testing.T) { testPeer3ID := newDummyPeer().Peer.ID() testPeer3SentCheque := newRandomTestCheque() testPeer3ReceivedCheque := newRandomTestCheque() - testPeer3ExpectedCheques := PeerCheques{testPeer3SentCheque, testPeer3ReceivedCheque} + testPeer3ExpectedCheques := PeerCheques{nil, testPeer3SentCheque, testPeer3ReceivedCheque} testPeerChequesDisconnected(t, testPeer3ID, testPeer3SentCheque, testPeer3ReceivedCheque, testPeer3ExpectedCheques) // verify cases for invalid peers @@ -539,7 +539,7 @@ func testPeerChequesInvalid(t *testing.T, invalidPeerIDs []enode.ID) { // verify results by calling PeerCheques function for _, invalidPeerID := range invalidPeerIDs { - verifyCheques(t, swap, invalidPeerID, PeerCheques{nil, nil}) + verifyCheques(t, swap, invalidPeerID, PeerCheques{nil, nil, nil}) } } From 355ba72a76cc5fb7ee7cbbe5dd733b974487decd Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Mon, 4 Nov 2019 15:29:51 +0100 Subject: [PATCH 13/26] swap: include pending cheques in tests --- swap/swap_test.go | 99 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 86 insertions(+), 13 deletions(-) diff --git a/swap/swap_test.go b/swap/swap_test.go index 46032e56e1..126840945d 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -209,8 +209,10 @@ func testBalances(t *testing.T, swap *Swap, expectedBalances map[enode.ID]int64) type chequesTestCase struct { name string protoPeers []*protocols.Peer + pendingCheques map[*protocols.Peer]*Cheque sentCheques map[*protocols.Peer]*Cheque receivedCheques map[*protocols.Peer]*Cheque + storePendingCheques map[enode.ID]*Cheque storeSentCheques map[enode.ID]*Cheque storeReceivedCheques map[enode.ID]*Cheque expectedCheques map[enode.ID]*PeerCheques @@ -221,16 +223,19 @@ func TestCheques(t *testing.T) { // generate peers and cheques // peer 1 testPeer := newDummyPeer().Peer + testPeerPendingCheque := newRandomTestCheque() testPeerSentCheque := newRandomTestCheque() testPeerReceivedCheque := newRandomTestCheque() testPeerSentCheque2 := newRandomTestCheque() // peer 2 testPeer2 := newDummyPeer().Peer + testPeer2PendingCheque := newRandomTestCheque() testPeer2SentCheque := newRandomTestCheque() testPeer2ReceivedCheque := newRandomTestCheque() testPeer2ReceivedCheque2 := newRandomTestCheque() // disconnected peer testPeer3ID := newDummyPeer().Peer.ID() + testPeer3PendingCheque := newRandomTestCheque() testPeer3SentCheque := newRandomTestCheque() testPeer3SentCheque2 := newRandomTestCheque() testPeer3ReceivedCheque := newRandomTestCheque() @@ -241,8 +246,10 @@ func TestCheques(t *testing.T) { { name: "no peers", protoPeers: []*protocols.Peer{}, + pendingCheques: map[*protocols.Peer]*Cheque{}, sentCheques: map[*protocols.Peer]*Cheque{}, receivedCheques: map[*protocols.Peer]*Cheque{}, + storePendingCheques: map[enode.ID]*Cheque{}, storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{}, @@ -250,8 +257,10 @@ func TestCheques(t *testing.T) { { name: "one peer", protoPeers: []*protocols.Peer{testPeer}, + pendingCheques: map[*protocols.Peer]*Cheque{}, sentCheques: map[*protocols.Peer]*Cheque{}, receivedCheques: map[*protocols.Peer]*Cheque{}, + storePendingCheques: map[enode.ID]*Cheque{}, storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{}, @@ -259,8 +268,10 @@ func TestCheques(t *testing.T) { { name: "one peer, one sent cheque", protoPeers: []*protocols.Peer{testPeer}, + pendingCheques: map[*protocols.Peer]*Cheque{}, sentCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerSentCheque}, receivedCheques: map[*protocols.Peer]*Cheque{}, + storePendingCheques: map[enode.ID]*Cheque{}, storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{ @@ -268,21 +279,25 @@ func TestCheques(t *testing.T) { }, }, { - name: "one peer, sent and received cheques", + name: "one peer, pending, sent and received cheques", protoPeers: []*protocols.Peer{testPeer}, + pendingCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerPendingCheque}, sentCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerSentCheque}, receivedCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerReceivedCheque}, + storePendingCheques: map[enode.ID]*Cheque{}, storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer.ID(): {nil, testPeerSentCheque, testPeerReceivedCheque}, + testPeer.ID(): {testPeerPendingCheque, testPeerSentCheque, testPeerReceivedCheque}, }, }, { name: "two peers, sent and received cheques", protoPeers: []*protocols.Peer{testPeer, testPeer2}, + pendingCheques: map[*protocols.Peer]*Cheque{}, sentCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerSentCheque, testPeer2: testPeer2SentCheque}, receivedCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerReceivedCheque, testPeer2: testPeer2ReceivedCheque}, + storePendingCheques: map[enode.ID]*Cheque{}, storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{ @@ -293,8 +308,10 @@ func TestCheques(t *testing.T) { { name: "two peers, successive sent and received cheques", protoPeers: []*protocols.Peer{testPeer, testPeer2}, + pendingCheques: map[*protocols.Peer]*Cheque{}, sentCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerSentCheque, testPeer2: testPeer2SentCheque, testPeer: testPeerSentCheque2}, receivedCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerReceivedCheque, testPeer2: testPeer2ReceivedCheque, testPeer2: testPeer2ReceivedCheque2}, + storePendingCheques: map[enode.ID]*Cheque{}, storeSentCheques: map[enode.ID]*Cheque{}, storeReceivedCheques: map[enode.ID]*Cheque{}, expectedCheques: map[enode.ID]*PeerCheques{ @@ -303,21 +320,25 @@ func TestCheques(t *testing.T) { }, }, { - name: "disconnected node, sent and received cheques", + name: "disconnected node, pending, sent and received cheques", protoPeers: []*protocols.Peer{}, + pendingCheques: map[*protocols.Peer]*Cheque{}, sentCheques: map[*protocols.Peer]*Cheque{}, receivedCheques: map[*protocols.Peer]*Cheque{}, + storePendingCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3PendingCheque}, storeSentCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3SentCheque}, storeReceivedCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3ReceivedCheque}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer3ID: {nil, testPeer3SentCheque, testPeer3ReceivedCheque}, + testPeer3ID: {testPeer3PendingCheque, testPeer3SentCheque, testPeer3ReceivedCheque}, }, }, { name: "disconnected node, successive sent and received cheques", protoPeers: []*protocols.Peer{}, + pendingCheques: map[*protocols.Peer]*Cheque{}, sentCheques: map[*protocols.Peer]*Cheque{}, receivedCheques: map[*protocols.Peer]*Cheque{}, + storePendingCheques: map[enode.ID]*Cheque{}, storeSentCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3SentCheque, testPeer3ID: testPeer3SentCheque2}, storeReceivedCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3ReceivedCheque, testPeer3ID: testPeer3ReceivedCheque2}, expectedCheques: map[enode.ID]*PeerCheques{ @@ -327,14 +348,16 @@ func TestCheques(t *testing.T) { { name: "full", protoPeers: []*protocols.Peer{testPeer, testPeer2}, + pendingCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerPendingCheque, testPeer2: testPeer2PendingCheque}, sentCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerSentCheque, testPeer2: testPeer2SentCheque, testPeer: testPeerSentCheque2}, receivedCheques: map[*protocols.Peer]*Cheque{testPeer: testPeerReceivedCheque, testPeer2: testPeer2ReceivedCheque, testPeer2: testPeer2ReceivedCheque2}, + storePendingCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3PendingCheque}, storeSentCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3SentCheque, testPeer3ID: testPeer3SentCheque2}, storeReceivedCheques: map[enode.ID]*Cheque{testPeer3ID: testPeer3ReceivedCheque, testPeer3ID: testPeer3ReceivedCheque2}, expectedCheques: map[enode.ID]*PeerCheques{ - testPeer.ID(): {nil, testPeerSentCheque2, testPeerReceivedCheque}, - testPeer2.ID(): {nil, testPeer2SentCheque, testPeer2ReceivedCheque2}, - testPeer3ID: {nil, testPeer3SentCheque2, testPeer3ReceivedCheque2}, + testPeer.ID(): {testPeerPendingCheque, testPeerSentCheque2, testPeerReceivedCheque}, + testPeer2.ID(): {testPeer2PendingCheque, testPeer2SentCheque, testPeer2ReceivedCheque2}, + testPeer3ID: {testPeer3PendingCheque, testPeer3SentCheque2, testPeer3ReceivedCheque2}, }, }, } @@ -360,6 +383,26 @@ func testCheques(t *testing.T, testCases []chequesTestCase) { peersMapping[pp] = peer } + // add test case peer pending cheques + for pp, sc := range tc.pendingCheques { + peer, ok := peersMapping[pp] + if !ok { + t.Fatalf("unexpected peer in test case sent cheques") + } + err := peer.setPendingCheque(sc) + if err != nil { + t.Fatal(err) + } + } + + // add test case store pending cheques + for p, sc := range tc.storePendingCheques { + err := swap.savePendingCheque(p, sc) + if err != nil { + t.Fatal(err) + } + } + // add test case peer sent cheques for pp, sc := range tc.sentCheques { peer, ok := peersMapping[pp] @@ -371,6 +414,7 @@ func testCheques(t *testing.T, testCases []chequesTestCase) { t.Fatal(err) } } + // add test case store sent cheques for p, sc := range tc.storeSentCheques { err := swap.saveLastSentCheque(p, sc) @@ -413,6 +457,7 @@ func testCheques(t *testing.T, testCases []chequesTestCase) { type peerChequesTestCase struct { name string peer *protocols.Peer + pendingCheque *Cheque sentCheque *Cheque receivedCheque *Cheque expectedCheques PeerCheques @@ -423,6 +468,7 @@ func TestPeerCheques(t *testing.T) { // generate peers and cheques // peer 1 testPeer := newDummyPeer().Peer + testPeerPendingCheque := newRandomTestCheque() testPeerSentCheque := newRandomTestCheque() testPeerReceivedCheque := newRandomTestCheque() // peer 2 @@ -434,6 +480,7 @@ func TestPeerCheques(t *testing.T) { { name: "peer 1 with no cheques", peer: testPeer, + pendingCheque: nil, sentCheque: nil, receivedCheque: nil, expectedCheques: PeerCheques{nil, nil, nil}, @@ -441,20 +488,31 @@ func TestPeerCheques(t *testing.T) { { name: "peer 1 with sent cheque", peer: testPeer, + pendingCheque: nil, sentCheque: testPeerSentCheque, receivedCheque: nil, expectedCheques: PeerCheques{nil, testPeerSentCheque, nil}, }, { - name: "peer 1 with sent and received cheque", + name: "peer 1 with pending cheque", + peer: testPeer, + pendingCheque: testPeerPendingCheque, + sentCheque: nil, + receivedCheque: nil, + expectedCheques: PeerCheques{testPeerPendingCheque, nil, nil}, + }, + { + name: "peer 1 with pending, sent and received cheque", peer: testPeer, + pendingCheque: testPeerPendingCheque, sentCheque: testPeerSentCheque, receivedCheque: testPeerReceivedCheque, - expectedCheques: PeerCheques{nil, testPeerSentCheque, testPeerReceivedCheque}, + expectedCheques: PeerCheques{testPeerPendingCheque, testPeerSentCheque, testPeerReceivedCheque}, }, { name: "peer 2 with received cheque", peer: testPeer2, + pendingCheque: nil, sentCheque: nil, receivedCheque: testPeer2ReceivedCheque, expectedCheques: PeerCheques{nil, nil, testPeer2ReceivedCheque}, @@ -465,10 +523,11 @@ func TestPeerCheques(t *testing.T) { // verify cases for disconnected peers testPeer3ID := newDummyPeer().Peer.ID() + testPeer3PendingCheque := newRandomTestCheque() testPeer3SentCheque := newRandomTestCheque() testPeer3ReceivedCheque := newRandomTestCheque() - testPeer3ExpectedCheques := PeerCheques{nil, testPeer3SentCheque, testPeer3ReceivedCheque} - testPeerChequesDisconnected(t, testPeer3ID, testPeer3SentCheque, testPeer3ReceivedCheque, testPeer3ExpectedCheques) + testPeer3ExpectedCheques := PeerCheques{testPeer3PendingCheque, testPeer3SentCheque, testPeer3ReceivedCheque} + testPeerChequesDisconnected(t, testPeer3ID, testPeer3PendingCheque, testPeer3SentCheque, testPeer3ReceivedCheque, testPeer3ExpectedCheques) // verify cases for invalid peers invalidPeers := []enode.ID{adapters.RandomNodeConfig().ID, {}} @@ -489,6 +548,14 @@ func testPeerCheques(t *testing.T, testCases []peerChequesTestCase) { t.Fatal(err) } + // add test case peer pending cheque + if tc.pendingCheque != nil { + err = peer.setPendingCheque(tc.pendingCheque) + if err != nil { + t.Fatal(err) + } + } + // add test case peer sent cheque if tc.sentCheque != nil { err = peer.setLastSentCheque(tc.sentCheque) @@ -511,14 +578,20 @@ func testPeerCheques(t *testing.T, testCases []peerChequesTestCase) { } } -func testPeerChequesDisconnected(t *testing.T, peerID enode.ID, sentCheque *Cheque, receivedCheque *Cheque, expectedCheques PeerCheques) { +func testPeerChequesDisconnected(t *testing.T, peerID enode.ID, pendingCheque *Cheque, sentCheque *Cheque, receivedCheque *Cheque, expectedCheques PeerCheques) { t.Helper() // create a test swap account swap, clean := newTestSwap(t, ownerKey, nil) defer clean() + // add store pending cheque + err := swap.savePendingCheque(peerID, pendingCheque) + if err != nil { + t.Fatal(err) + } + // add store sent cheque - err := swap.saveLastSentCheque(peerID, sentCheque) + err = swap.saveLastSentCheque(peerID, sentCheque) if err != nil { t.Fatal(err) } From a8b3cd1eeeebbcce0a539244a57c10d2910d0322 Mon Sep 17 00:00:00 2001 From: Ralph Pichler Date: Mon, 4 Nov 2019 15:40:03 +0100 Subject: [PATCH 14/26] swap: adjust balance before sending the cheque --- swap/peer.go | 5 +++++ swap/protocol_test.go | 8 ++------ swap/swap.go | 6 ------ 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/swap/peer.go b/swap/peer.go index 887480c5a8..97a339de33 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -202,6 +202,11 @@ func (p *Peer) sendCheque() error { return fmt.Errorf("error while saving pending cheque: %v", err) } + err = p.updateBalance(int64(cheque.Honey)) + if err != nil { + return fmt.Errorf("error while creating cheque: %v", err) + } + p.logger.Info("sending cheque to peer", "honey", cheque.Honey, "cumulativePayout", cheque.ChequeParams.CumulativePayout, "beneficiary", cheque.Beneficiary, "contract", cheque.Contract) return p.Send(context.Background(), &EmitChequeMsg{ Cheque: cheque, diff --git a/swap/protocol_test.go b/swap/protocol_test.go index 56eb15a228..a0aa2ff0b2 100644 --- a/swap/protocol_test.go +++ b/swap/protocol_test.go @@ -246,8 +246,8 @@ func TestTriggerPaymentThreshold(t *testing.T) { t.Fatal(err) } - // balance should only be adjusted by overDraft prior to confirm msg - if creditor.getBalance() != -int64(expectedAmount) { + // balance should be reset now + if creditor.getBalance() != 0 { t.Fatalf("Expected debitorSwap balance to be 0, but is %d", creditor.getBalance()) } @@ -286,10 +286,6 @@ func TestTriggerPaymentThreshold(t *testing.T) { t.Fatal(err) } - debitorSwap.handleConfirmChequeMsg(ctx, creditor, &ConfirmChequeMsg{ - Cheque: creditor.getPendingCheque(), - }) - if creditor.getBalance() != 0 { t.Fatalf("Expected debitorSwap balance to be 0, but is %d", creditor.getBalance()) } diff --git a/swap/swap.go b/swap/swap.go index 3afc6a89b3..49482c639c 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -433,12 +433,6 @@ func (s *Swap) handleConfirmChequeMsg(ctx context.Context, p *Peer, msg *Confirm return } - err = p.updateBalance(int64(cheque.Honey)) - if err != nil { - p.Drop(fmt.Sprintf("persistence error: %v", err)) - return - } - // since more swap traffic might have occurred since this cheque was already sent, we redo the payment threshold check err = s.checkPaymentThresholdAndSendCheque(p) if err != nil { From 521266096baaaff271b677f7ecffb03f13c51190 Mon Sep 17 00:00:00 2001 From: mortelli Date: Tue, 12 Nov 2019 14:08:16 -0300 Subject: [PATCH 15/26] swap: remove loadPendingCheque re-declaration --- swap/swap.go | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/swap/swap.go b/swap/swap.go index 21f3676f10..70b4cae9a0 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -729,16 +729,6 @@ func (s *Swap) loadPendingCheque(p enode.ID) (cheque *Cheque, err error) { return cheque, nil } -// loadPendingCheque loads the current pending cheque for the peer from the store -// and returns nil when there never was a pending cheque saved -func (s *Swap) loadPendingCheque(p enode.ID) (cheque *Cheque, err error) { - err = s.store.Get(pendingChequeKey(p), &cheque) - if err == state.ErrNotFound { - return nil, nil - } - return cheque, err -} - // loadBalance loads the current balance for the peer from the store // and returns 0 if there was no prior balance saved func (s *Swap) loadBalance(p enode.ID) (balance int64, err error) { From a3f9e18e66f53b44feee405fb3694cfb987ce5fa Mon Sep 17 00:00:00 2001 From: mortelli Date: Thu, 14 Nov 2019 15:15:16 -0300 Subject: [PATCH 16/26] swap: fix TestAvailableBalance based on cheque logic changes from master --- swap/swap_test.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/swap/swap_test.go b/swap/swap_test.go index 25ec7d20ce..0db939dc68 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -1636,10 +1636,6 @@ func TestAvailableBalance(t *testing.T) { t.Fatal(err) } - swap.handleConfirmChequeMsg(context.Background(), peer, &ConfirmChequeMsg{ - Cheque: peer.getPendingCheque(), - }) - availableBalance, err = swap.AvailableBalance() if err != nil { t.Fatal(err) From e48c165bf42759ce4cfde913533e62bebc7cbeac Mon Sep 17 00:00:00 2001 From: mortelli Date: Thu, 14 Nov 2019 15:15:55 -0300 Subject: [PATCH 17/26] swap/api: temporarily remove locks from Cheques function --- swap/api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/swap/api.go b/swap/api.go index d06b1e56dc..08d5ca410e 100644 --- a/swap/api.go +++ b/swap/api.go @@ -161,7 +161,7 @@ func (s *Swap) Cheques() (map[enode.ID]*PeerCheques, error) { // get peer cheques from memory s.peersLock.Lock() for peer, swapPeer := range s.peers { - swapPeer.lock.Lock() + //swapPeer.lock.Lock() //temporary pendingCheque := swapPeer.getPendingCheque() sentCheque := swapPeer.getLastSentCheque() receivedCheque := swapPeer.getLastReceivedCheque() @@ -169,7 +169,7 @@ func (s *Swap) Cheques() (map[enode.ID]*PeerCheques, error) { if sentCheque != nil || receivedCheque != nil || pendingCheque != nil { cheques[peer] = &PeerCheques{pendingCheque, sentCheque, receivedCheque} } - swapPeer.lock.Unlock() + //swapPeer.lock.Unlock() //temporary } s.peersLock.Unlock() From e97f0367d50bcd367132a61d02af27254b84d6e9 Mon Sep 17 00:00:00 2001 From: mortelli Date: Thu, 14 Nov 2019 15:37:03 -0300 Subject: [PATCH 18/26] swap: remove placeholder function --- swap/swap.go | 38 -------------------------------------- 1 file changed, 38 deletions(-) diff --git a/swap/swap.go b/swap/swap.go index 0f1bae1a7c..d89bc2cb23 100644 --- a/swap/swap.go +++ b/swap/swap.go @@ -20,7 +20,6 @@ import ( "context" "crypto/ecdsa" "encoding/hex" - "encoding/json" "errors" "fmt" "math/big" @@ -486,43 +485,6 @@ func (s *Swap) processAndVerifyCheque(cheque *Cheque, p *Peer) (uint64, error) { return actualAmount, nil } -// placeholder: issue of locks -func (s *Swap) sentCheques() (map[enode.ID]*Cheque, error) { - sentCheques := make(map[enode.ID]*Cheque) - // get sent cheques from memory - s.peersLock.Lock() - for peer, swapPeer := range s.peers { - sentCheque := swapPeer.getLastSentCheque() - // don't add peer to result if there are no cheques - if sentCheque != nil { - sentCheques[peer] = sentCheque - } - } - s.peersLock.Unlock() - - // get sent cheques from store - chequesIterFunction := func(key []byte, value []byte) (stop bool, err error) { - peer := keyToID(string(key), sentChequePrefix) - // add cheque if not present yet - if sentCheque := sentCheques[peer]; sentCheque == nil { - // add cheque from store if not already in result - var sentCheque Cheque - err = json.Unmarshal(value, &sentCheque) - if err == nil { - sentCheques[peer] = &sentCheque - } - } - return stop, err - } - - err := s.store.Iterate(sentChequePrefix, chequesIterFunction) - if err != nil { - return nil, err - } - - return sentCheques, nil -} - // loadLastReceivedCheque loads the last received cheque for the peer from the store // and returns nil when there never was a cheque saved func (s *Swap) loadLastReceivedCheque(p enode.ID) (cheque *Cheque, err error) { From 56b502d5461176abd2e536ca69c19c50f987e928 Mon Sep 17 00:00:00 2001 From: mortelli Date: Mon, 18 Nov 2019 09:12:36 -0300 Subject: [PATCH 19/26] swap: refactor sendCheque --- swap/peer.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/swap/peer.go b/swap/peer.go index 641416b4e8..d4a1bea474 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -197,14 +197,14 @@ func (p *Peer) sendCheque() error { return fmt.Errorf("error while creating cheque: %v", err) } - chequebookBalance, err := p.swap.AvailableBalance() + availableBalance, err := p.swap.AvailableBalance() if err != nil { return fmt.Errorf("error while getting available balance: %v", err) } chequeAmount := uint64(-p.getBalance()) - if chequebookBalance < chequeAmount { - return fmt.Errorf("cannot send created cheque, amount %d is greater than chequebook available balance %d", chequeAmount, chequebookBalance) + if availableBalance < chequeAmount { + return fmt.Errorf("cannot send created cheque, amount %d is greater than chequebook available balance %d", chequeAmount, availableBalance) } err = p.setPendingCheque(cheque) From b9385a5b5c54b2e883d463f690b5879266a0fc8c Mon Sep 17 00:00:00 2001 From: mortelli Date: Mon, 16 Dec 2019 10:48:56 -0300 Subject: [PATCH 20/26] swap: temporarily disable bad cheques check --- swap/peer.go | 2 +- swap/simulations_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/swap/peer.go b/swap/peer.go index 704f9796c6..d7e39d41fc 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -204,7 +204,7 @@ func (p *Peer) sendCheque() error { } chequeAmount := uint64(-p.getBalance()) - if availableBalance < chequeAmount { + if availableBalance < chequeAmount && false { return fmt.Errorf("cannot send created cheque, amount %d is greater than chequebook available balance %d", chequeAmount, availableBalance) } diff --git a/swap/simulations_test.go b/swap/simulations_test.go index 2b3a6e5aa2..f2cba87841 100644 --- a/swap/simulations_test.go +++ b/swap/simulations_test.go @@ -461,7 +461,7 @@ func TestMultiChequeSimulation(t *testing.T) { } // we will send just maxCheques number of cheques - maxCheques := 3 + maxCheques := 10 // the peer object used for sending debitorSvc.lock.Lock() From c60fe1e0ef8b6d18b174ed11460faf393ded1b7f Mon Sep 17 00:00:00 2001 From: mortelli Date: Mon, 16 Dec 2019 11:32:51 -0300 Subject: [PATCH 21/26] swap: minor refactor to TestMultiChequeSimulation function --- swap/simulations_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swap/simulations_test.go b/swap/simulations_test.go index f2cba87841..cf734f1c9b 100644 --- a/swap/simulations_test.go +++ b/swap/simulations_test.go @@ -478,7 +478,7 @@ func TestMultiChequeSimulation(t *testing.T) { if err := waitForChequeProcessed(t, params.backend, counter, lastCount, debitorSvc.swap.peers[creditor], expectedPayout); err != nil { t.Fatal(err) } - lastCount += 1 + lastCount++ expectedPayout += DefaultPaymentThreshold + 1 } From 472b40082d305090f604ed8b7d3a376a1ea38f4c Mon Sep 17 00:00:00 2001 From: mortelli Date: Mon, 23 Dec 2019 15:10:04 -0300 Subject: [PATCH 22/26] swap: re-enable bad cheques check --- swap/peer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/swap/peer.go b/swap/peer.go index d7e39d41fc..704f9796c6 100644 --- a/swap/peer.go +++ b/swap/peer.go @@ -204,7 +204,7 @@ func (p *Peer) sendCheque() error { } chequeAmount := uint64(-p.getBalance()) - if availableBalance < chequeAmount && false { + if availableBalance < chequeAmount { return fmt.Errorf("cannot send created cheque, amount %d is greater than chequebook available balance %d", chequeAmount, availableBalance) } From 171e7a28687643fde1777f2c222bdf043764c6e8 Mon Sep 17 00:00:00 2001 From: mortelli Date: Mon, 23 Dec 2019 15:27:45 -0300 Subject: [PATCH 23/26] swap: linter corrections --- swap/simulations_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/swap/simulations_test.go b/swap/simulations_test.go index 78366750ea..e914778602 100644 --- a/swap/simulations_test.go +++ b/swap/simulations_test.go @@ -344,7 +344,7 @@ func TestPingPongChequeSimulation(t *testing.T) { if err := waitForChequeProcessed(t, params.backend, counter, lastCount, ts1.swap.peers[p2], expectedPayout1); err != nil { t.Fatal(err) } - lastCount += 1 + lastCount++ expectedPayout1 += DefaultPaymentThreshold + 1 } else { if err := p1Peer.Send(context.Background(), &testMsgBigPrice{}); err != nil { @@ -353,7 +353,7 @@ func TestPingPongChequeSimulation(t *testing.T) { if err := waitForChequeProcessed(t, params.backend, counter, lastCount, ts2.swap.peers[p1], expectedPayout2); err != nil { t.Fatal(err) } - lastCount += 1 + lastCount++ expectedPayout2 += DefaultPaymentThreshold + 1 } } From ffc9bdce9cadf1752642d13fd5d3dbc73e30f6ea Mon Sep 17 00:00:00 2001 From: mortelli Date: Mon, 23 Dec 2019 15:28:14 -0300 Subject: [PATCH 24/26] swap: fix inverted chequebook deploy amounts in TestSwapLogToFile function --- swap/swap_test.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/swap/swap_test.go b/swap/swap_test.go index 3ebe167948..5f739c06bf 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -1554,11 +1554,11 @@ func TestSwapLogToFile(t *testing.T) { testAmount := int64(DefaultPaymentThreshold + 42) ctx := context.Background() - err = testDeploy(ctx, creditorSwap, big.NewInt(testAmount)) + err = testDeploy(ctx, creditorSwap, big.NewInt(0)) if err != nil { t.Fatal(err) } - err = testDeploy(ctx, debitorSwap, big.NewInt(0)) + err = testDeploy(ctx, debitorSwap, big.NewInt(testAmount)) if err != nil { t.Fatal(err) } @@ -1586,7 +1586,10 @@ func TestSwapLogToFile(t *testing.T) { defer cleanup() // now simulate sending the cheque to the creditor from the debitor - creditor.sendCheque() + err = creditor.sendCheque() + if err != nil { + t.Fatal(err) + } if logDirDebitor == "" { t.Fatal("Swap Log Dir is not defined") From 9cf629ea04df1ff6c63d96b682aeedb7618f97b5 Mon Sep 17 00:00:00 2001 From: mortelli Date: Mon, 23 Dec 2019 15:32:46 -0300 Subject: [PATCH 25/26] swap: re-add locks to Cheques function --- swap/api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/swap/api.go b/swap/api.go index 08d5ca410e..d06b1e56dc 100644 --- a/swap/api.go +++ b/swap/api.go @@ -161,7 +161,7 @@ func (s *Swap) Cheques() (map[enode.ID]*PeerCheques, error) { // get peer cheques from memory s.peersLock.Lock() for peer, swapPeer := range s.peers { - //swapPeer.lock.Lock() //temporary + swapPeer.lock.Lock() pendingCheque := swapPeer.getPendingCheque() sentCheque := swapPeer.getLastSentCheque() receivedCheque := swapPeer.getLastReceivedCheque() @@ -169,7 +169,7 @@ func (s *Swap) Cheques() (map[enode.ID]*PeerCheques, error) { if sentCheque != nil || receivedCheque != nil || pendingCheque != nil { cheques[peer] = &PeerCheques{pendingCheque, sentCheque, receivedCheque} } - //swapPeer.lock.Unlock() //temporary + swapPeer.lock.Unlock() } s.peersLock.Unlock() From 23cbc927a1e227afee4283d524f759c858c31932 Mon Sep 17 00:00:00 2001 From: mortelli Date: Tue, 14 Jan 2020 17:37:34 -0300 Subject: [PATCH 26/26] swap: remove redundant change in TestSwapLogToFile --- swap/swap_test.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/swap/swap_test.go b/swap/swap_test.go index b51456b026..d668110ba5 100644 --- a/swap/swap_test.go +++ b/swap/swap_test.go @@ -1211,10 +1211,7 @@ func TestSwapLogToFile(t *testing.T) { defer cleanup() // now simulate sending the cheque to the creditor from the debitor - err = creditor.sendCheque() - if err != nil { - t.Fatal(err) - } + creditor.sendCheque() if logDirDebitor == "" { t.Fatal("Swap Log Dir is not defined")