From bc8d674958f3ad0ec66b66b3bf1d26bf3149df0d Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Mon, 23 Oct 2017 23:18:26 -0700 Subject: [PATCH] htlcswitch: Remove constructor functions for htlcPacket. The constructor functions have no additional logic other than passing function parameters into struct fields. Given the large function signatures, it is more clear to directly construct the htlcPacket in client code than call a function with lots of positional arguments. --- htlcswitch/link.go | 47 +++++++++++------- htlcswitch/packet.go | 55 --------------------- htlcswitch/switch.go | 39 ++++++++------- htlcswitch/switch_test.go | 100 +++++++++++++++++++++----------------- 4 files changed, 106 insertions(+), 135 deletions(-) diff --git a/htlcswitch/link.go b/htlcswitch/link.go index d038292e..762ef1f7 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -698,14 +698,15 @@ func (l *channelLink) handleDownStreamPkt(pkt *htlcPacket, isReProcess bool) { isObfuscated = true } - upddateFail := &lnwire.UpdateFailHTLC{ - Reason: reason, + failPkt := &htlcPacket{ + src: l.ShortChanID(), + payHash: htlc.PaymentHash, + amount: htlc.Amount, + isObfuscated: isObfuscated, + htlc: &lnwire.UpdateFailHTLC{ + Reason: reason, + }, } - failPkt := newFailPacket( - l.ShortChanID(), upddateFail, - htlc.PaymentHash, htlc.Amount, - isObfuscated, - ) // TODO(roasbeef): need to identify if sent // from switch so don't need to obfuscate @@ -1177,11 +1178,14 @@ func (l *channelLink) processLockedInHtlcs( // received. So we'll forward the HTLC to the switch which // will handle propagating the settle to the prior hop. case lnwallet.Settle: - settleUpdate := &lnwire.UpdateFufillHTLC{ - PaymentPreimage: pd.RPreimage, + settlePacket := &htlcPacket{ + src: l.ShortChanID(), + payHash: pd.RHash, + amount: pd.Amount, + htlc: &lnwire.UpdateFufillHTLC{ + PaymentPreimage: pd.RPreimage, + }, } - settlePacket := newSettlePacket(l.ShortChanID(), - settleUpdate, pd.RHash, pd.Amount) // Add the packet to the batch to be forwarded, and // notify the overflow queue that a spare spot has been @@ -1196,12 +1200,15 @@ func (l *channelLink) processLockedInHtlcs( case lnwallet.Fail: // Fetch the reason the HTLC was cancelled so we can // continue to propagate it. - failUpdate := &lnwire.UpdateFailHTLC{ - Reason: lnwire.OpaqueReason(pd.FailReason), - ChanID: l.ChanID(), + failPacket := &htlcPacket{ + src: l.ShortChanID(), + payHash: pd.RHash, + amount: pd.Amount, + isObfuscated: false, + htlc: &lnwire.UpdateFailHTLC{ + Reason: lnwire.OpaqueReason(pd.FailReason), + }, } - failPacket := newFailPacket(l.ShortChanID(), failUpdate, - pd.RHash, pd.Amount, false) // Add the packet to the batch to be forwarded, and // notify the overflow queue that a spare spot has been @@ -1564,8 +1571,12 @@ func (l *channelLink) processLockedInHtlcs( continue } - updatePacket := newAddPacket(l.ShortChanID(), - fwdInfo.NextHop, addMsg, obfuscator) + updatePacket := &htlcPacket{ + src: l.ShortChanID(), + dest: fwdInfo.NextHop, + htlc: addMsg, + obfuscator: obfuscator, + } packetsToForward = append(packetsToForward, updatePacket) } } diff --git a/htlcswitch/packet.go b/htlcswitch/packet.go index 540335fe..1f68e0f9 100644 --- a/htlcswitch/packet.go +++ b/htlcswitch/packet.go @@ -45,58 +45,3 @@ type htlcPacket struct { // errors inside the htlcswitch packet. isObfuscated bool } - -// newInitPacket creates htlc switch add packet which encapsulates the add htlc -// request and additional information for proper forwarding over htlc switch. -func newInitPacket(destNode [33]byte, htlc *lnwire.UpdateAddHTLC) *htlcPacket { - return &htlcPacket{ - destNode: destNode, - amount: htlc.Amount, - htlc: htlc, - } -} - -// newAddPacket creates htlc switch add packet which encapsulates the add htlc -// request and additional information for proper forwarding over htlc switch. -func newAddPacket(src, dest lnwire.ShortChannelID, - htlc *lnwire.UpdateAddHTLC, e ErrorEncrypter) *htlcPacket { - - return &htlcPacket{ - amount: htlc.Amount, - dest: dest, - src: src, - htlc: htlc, - obfuscator: e, - } -} - -// newSettlePacket creates htlc switch ack/settle packet which encapsulates the -// settle htlc request which should be created and sent back by last hope in -// htlc path. -func newSettlePacket(src lnwire.ShortChannelID, htlc *lnwire.UpdateFufillHTLC, - payHash [sha256.Size]byte, amount lnwire.MilliSatoshi) *htlcPacket { - - return &htlcPacket{ - src: src, - payHash: payHash, - htlc: htlc, - amount: amount, - } -} - -// newFailPacket creates htlc switch fail packet which encapsulates the fail -// htlc request which propagated back to the original hope who sent the htlc -// add request if something wrong happened on the path to the final -// destination. -func newFailPacket(src lnwire.ShortChannelID, htlc *lnwire.UpdateFailHTLC, - payHash [sha256.Size]byte, amount lnwire.MilliSatoshi, - isObfuscated bool) *htlcPacket { - - return &htlcPacket{ - src: src, - payHash: payHash, - htlc: htlc, - amount: amount, - isObfuscated: isObfuscated, - } -} diff --git a/htlcswitch/switch.go b/htlcswitch/switch.go index b375dd38..73791e78 100644 --- a/htlcswitch/switch.go +++ b/htlcswitch/switch.go @@ -202,7 +202,10 @@ func (s *Switch) SendHTLC(nextNode [33]byte, htlc *lnwire.UpdateAddHTLC, // Generate and send new update packet, if error will be received on // this stage it means that packet haven't left boundaries of our // system and something wrong happened. - packet := newInitPacket(nextNode, htlc) + packet := &htlcPacket{ + destNode: nextNode, + htlc: htlc, + } if err := s.forward(packet); err != nil { s.removePendingPayment(payment.amount, payment.paymentHash) return zeroPreimage, err @@ -477,13 +480,14 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { return err } - source.HandleSwitchPacket(newFailPacket( - packet.src, - &lnwire.UpdateFailHTLC{ + source.HandleSwitchPacket(&htlcPacket{ + src: packet.src, + payHash: htlc.PaymentHash, + isObfuscated: true, + htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, - htlc.PaymentHash, 0, true, - )) + }) err = errors.Errorf("unable to find link with "+ "destination %v", packet.dest) log.Error(err) @@ -524,14 +528,14 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { return err } - source.HandleSwitchPacket(newFailPacket( - packet.src, - &lnwire.UpdateFailHTLC{ + source.HandleSwitchPacket(&htlcPacket{ + src: packet.src, + payHash: htlc.PaymentHash, + isObfuscated: true, + htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, - htlc.PaymentHash, - 0, true, - )) + }) err = errors.Errorf("unable to find appropriate "+ "channel link insufficient capacity, need "+ @@ -558,13 +562,14 @@ func (s *Switch) handlePacketForward(packet *htlcPacket) error { return err } - source.HandleSwitchPacket(newFailPacket( - packet.src, - &lnwire.UpdateFailHTLC{ + source.HandleSwitchPacket(&htlcPacket{ + src: packet.src, + payHash: htlc.PaymentHash, + isObfuscated: true, + htlc: &lnwire.UpdateFailHTLC{ Reason: reason, }, - htlc.PaymentHash, 0, true, - )) + }) err = errors.Errorf("unable to add circuit: "+ "%v", err) log.Error(err) diff --git a/htlcswitch/switch_test.go b/htlcswitch/switch_test.go index 05cd6031..3a06846b 100644 --- a/htlcswitch/switch_test.go +++ b/htlcswitch/switch_test.go @@ -32,8 +32,6 @@ var ( func TestSwitchForward(t *testing.T) { t.Parallel() - var packet *htlcPacket - alicePeer := newMockServer(t, "alice") bobPeer := newMockServer(t, "bob") @@ -57,14 +55,15 @@ func TestSwitchForward(t *testing.T) { // bob channel link. preimage := [sha256.Size]byte{1} rhash := fastsha256.Sum256(preimage[:]) - packet = newAddPacket( - aliceChannelLink.ShortChanID(), - bobChannelLink.ShortChanID(), - &lnwire.UpdateAddHTLC{ + packet := &htlcPacket{ + src: aliceChannelLink.ShortChanID(), + dest: bobChannelLink.ShortChanID(), + obfuscator: newMockObfuscator(), + htlc: &lnwire.UpdateAddHTLC{ PaymentHash: rhash, Amount: 1, - }, newMockObfuscator(), - ) + }, + } // Handle the request and checks that bob channel link received it. if err := s.forward(packet); err != nil { @@ -85,12 +84,14 @@ func TestSwitchForward(t *testing.T) { // Create settle request pretending that bob link handled the add htlc // request and sent the htlc settle request back. This request should // be forwarder back to Alice link. - packet = newSettlePacket( - bobChannelLink.ShortChanID(), - &lnwire.UpdateFufillHTLC{ + packet = &htlcPacket{ + src: bobChannelLink.ShortChanID(), + payHash: rhash, + amount: 1, + htlc: &lnwire.UpdateFufillHTLC{ PaymentPreimage: preimage, }, - rhash, 1) + } // Handle the request and checks that payment circuit works properly. if err := s.forward(packet); err != nil { @@ -143,14 +144,15 @@ func TestSkipIneligibleLinksMultiHopForward(t *testing.T) { // Alice. preimage := [sha256.Size]byte{1} rhash := fastsha256.Sum256(preimage[:]) - packet = newAddPacket( - aliceChannelLink.ShortChanID(), - bobChannelLink.ShortChanID(), - &lnwire.UpdateAddHTLC{ + packet = &htlcPacket{ + src: aliceChannelLink.ShortChanID(), + dest: bobChannelLink.ShortChanID(), + htlc: &lnwire.UpdateAddHTLC{ PaymentHash: rhash, Amount: 1, - }, newMockObfuscator(), - ) + }, + obfuscator: newMockObfuscator(), + } // The request to forward should fail as err := s.forward(packet) @@ -207,8 +209,6 @@ func TestSkipIneligibleLinksLocalForward(t *testing.T) { func TestSwitchCancel(t *testing.T) { t.Parallel() - var request *htlcPacket - alicePeer := newMockServer(t, "alice") bobPeer := newMockServer(t, "bob") @@ -232,14 +232,15 @@ func TestSwitchCancel(t *testing.T) { // to bob channel link. preimage := [sha256.Size]byte{1} rhash := fastsha256.Sum256(preimage[:]) - request = newAddPacket( - aliceChannelLink.ShortChanID(), - bobChannelLink.ShortChanID(), - &lnwire.UpdateAddHTLC{ + request := &htlcPacket{ + src: aliceChannelLink.ShortChanID(), + dest: bobChannelLink.ShortChanID(), + obfuscator: newMockObfuscator(), + htlc: &lnwire.UpdateAddHTLC{ PaymentHash: rhash, Amount: 1, - }, newMockObfuscator(), - ) + }, + } // Handle the request and checks that bob channel link received it. if err := s.forward(request); err != nil { @@ -260,10 +261,13 @@ func TestSwitchCancel(t *testing.T) { // Create settle request pretending that bob channel link handled // the add htlc request and sent the htlc settle request back. This // request should be forwarder back to alice channel link. - request = newFailPacket( - bobChannelLink.ShortChanID(), - &lnwire.UpdateFailHTLC{}, - rhash, 1, true) + request = &htlcPacket{ + src: bobChannelLink.ShortChanID(), + payHash: rhash, + amount: 1, + isObfuscated: true, + htlc: &lnwire.UpdateFailHTLC{}, + } // Handle the request and checks that payment circuit works properly. if err := s.forward(request); err != nil { @@ -287,8 +291,6 @@ func TestSwitchCancel(t *testing.T) { func TestSwitchAddSamePayment(t *testing.T) { t.Parallel() - var request *htlcPacket - alicePeer := newMockServer(t, "alice") bobPeer := newMockServer(t, "bob") @@ -312,14 +314,15 @@ func TestSwitchAddSamePayment(t *testing.T) { // to bob channel link. preimage := [sha256.Size]byte{1} rhash := fastsha256.Sum256(preimage[:]) - request = newAddPacket( - aliceChannelLink.ShortChanID(), - bobChannelLink.ShortChanID(), - &lnwire.UpdateAddHTLC{ + request := &htlcPacket{ + src: aliceChannelLink.ShortChanID(), + dest: bobChannelLink.ShortChanID(), + obfuscator: newMockObfuscator(), + htlc: &lnwire.UpdateAddHTLC{ PaymentHash: rhash, Amount: 1, - }, newMockObfuscator(), - ) + }, + } // Handle the request and checks that bob channel link received it. if err := s.forward(request); err != nil { @@ -349,10 +352,13 @@ func TestSwitchAddSamePayment(t *testing.T) { // Create settle request pretending that bob channel link handled // the add htlc request and sent the htlc settle request back. This // request should be forwarder back to alice channel link. - request = newFailPacket( - bobChannelLink.ShortChanID(), - &lnwire.UpdateFailHTLC{}, - rhash, 1, true) + request = &htlcPacket{ + src: bobChannelLink.ShortChanID(), + payHash: rhash, + amount: 1, + isObfuscated: true, + htlc: &lnwire.UpdateFailHTLC{}, + } // Handle the request and checks that payment circuit works properly. if err := s.forward(request); err != nil { @@ -464,12 +470,16 @@ func TestSwitchSendPayment(t *testing.T) { t.Fatalf("unable obfuscate failure: %v", err) } - packet := newFailPacket(aliceChannelLink.ShortChanID(), - &lnwire.UpdateFailHTLC{ + packet := &htlcPacket{ + src: aliceChannelLink.ShortChanID(), + payHash: rhash, + amount: 1, + isObfuscated: true, + htlc: &lnwire.UpdateFailHTLC{ Reason: reason, ID: 1, }, - rhash, 1, true) + } if err := s.forward(packet); err != nil { t.Fatalf("can't forward htlc packet: %v", err)