htlcswitch: implement strict forwarding for locally dispatched payments

In this commit, we address an issue that could arise when using the
SendToRoute RPC. In this RPC, we specify the exact hops that a payment
should take. However, within the switch, we would set a constraint for
the first hop to be any hop as long as the first peer was at the end of
it. This would cause discrepancies when attempting to use the RPC as the
payment would actually go through another hop with the same peer. We fix
this by explicitly specifying the channel ID of the first hop.

Fixes #1500.
Fixes #1515.
This commit is contained in:
Wilmer Paulino 2018-07-09 18:11:25 -07:00 committed by Olaoluwa Osuntokun
parent 8baf172614
commit 7535371238
4 changed files with 154 additions and 127 deletions

@ -215,9 +215,11 @@ func TestChannelLinkSingleHopPayment(t *testing.T) {
// * alice<->bob commitment state to be updated.
// * user notification to be sent.
receiver := n.bobServer
rhash, err := n.makePayment(n.aliceServer, receiver,
n.bobServer.PubKey(), hops, amount, htlcAmt,
totalTimelock).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
rhash, err := n.makePayment(
n.aliceServer, receiver, firstHop, hops, amount, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
if err != nil {
t.Fatalf("unable to make the payment: %v", err)
}
@ -309,9 +311,11 @@ func TestChannelLinkBidirectionalOneHopPayments(t *testing.T) {
sender: "alice",
}
_, r.err = n.makePayment(n.aliceServer, n.bobServer,
n.bobServer.PubKey(), hopsForwards, amt, htlcAmt,
totalTimelock).Wait(5 * time.Minute)
firstHop := n.firstBobChannelLink.ShortChanID()
_, r.err = n.makePayment(
n.aliceServer, n.bobServer, firstHop,
hopsForwards, amt, htlcAmt, totalTimelock,
).Wait(5 * time.Minute)
resultChan <- r
}(i)
}
@ -324,9 +328,11 @@ func TestChannelLinkBidirectionalOneHopPayments(t *testing.T) {
sender: "bob",
}
_, r.err = n.makePayment(n.bobServer, n.aliceServer,
n.aliceServer.PubKey(), hopsBackwards, amt, htlcAmt,
totalTimelock).Wait(5 * time.Minute)
firstHop := n.aliceChannelLink.ShortChanID()
_, r.err = n.makePayment(
n.bobServer, n.aliceServer, firstHop,
hopsBackwards, amt, htlcAmt, totalTimelock,
).Wait(5 * time.Minute)
resultChan <- r
}(i)
}
@ -442,9 +448,11 @@ func TestChannelLinkMultiHopPayment(t *testing.T) {
// * Alice<->Bob commitment states to be updated.
// * user notification to be sent.
receiver := n.carolServer
rhash, err := n.makePayment(n.aliceServer, n.carolServer,
n.bobServer.PubKey(), hops, amount, htlcAmt,
totalTimelock).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
rhash, err := n.makePayment(
n.aliceServer, n.carolServer, firstHop, hops, amount, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
if err != nil {
t.Fatalf("unable to send payment: %v", err)
}
@ -518,10 +526,11 @@ func TestExitNodeTimelockPayloadMismatch(t *testing.T) {
// The proper value of the outgoing CLTV should be the policy set by
// the receiving node, instead we set it to be a random value.
hops[0].OutgoingCTLV = 500
_, err = n.makePayment(n.aliceServer, n.bobServer,
n.bobServer.PubKey(), hops, amount, htlcAmt,
htlcExpiry).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, n.bobServer, firstHop, hops, amount, htlcAmt,
htlcExpiry,
).Wait(30 * time.Second)
if err == nil {
t.Fatalf("payment should have failed but didn't")
}
@ -570,10 +579,11 @@ func TestExitNodeAmountPayloadMismatch(t *testing.T) {
// value of the amount to forward should be the amount that the
// receiving node expects to receive.
hops[0].AmountToForward = 1
_, err = n.makePayment(n.aliceServer, n.bobServer,
n.bobServer.PubKey(), hops, amount, htlcAmt,
htlcExpiry).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, n.bobServer, firstHop, hops, amount, htlcAmt,
htlcExpiry,
).Wait(30 * time.Second)
if err == nil {
t.Fatalf("payment should have failed but didn't")
} else if err.Error() != lnwire.CodeIncorrectPaymentAmount.String() {
@ -617,9 +627,11 @@ func TestLinkForwardTimelockPolicyMismatch(t *testing.T) {
// Next, we'll make the payment which'll send an HTLC with our
// specified parameters to the first hop in the route.
_, err = n.makePayment(n.aliceServer, n.carolServer,
n.bobServer.PubKey(), hops, amount, htlcAmt,
htlcExpiry).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, n.carolServer, firstHop, hops, amount, htlcAmt,
htlcExpiry,
).Wait(30 * time.Second)
// We should get an error, and that error should indicate that the HTLC
// should be rejected due to a policy violation.
@ -673,9 +685,11 @@ func TestLinkForwardFeePolicyMismatch(t *testing.T) {
// Next, we'll make the payment which'll send an HTLC with our
// specified parameters to the first hop in the route.
_, err = n.makePayment(n.aliceServer, n.bobServer,
n.bobServer.PubKey(), hops, amountNoFee, amountNoFee,
htlcExpiry).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, n.bobServer, firstHop, hops, amountNoFee,
amountNoFee, htlcExpiry,
).Wait(30 * time.Second)
// We should get an error, and that error should indicate that the HTLC
// should be rejected due to a policy violation.
@ -729,9 +743,11 @@ func TestLinkForwardMinHTLCPolicyMismatch(t *testing.T) {
// Next, we'll make the payment which'll send an HTLC with our
// specified parameters to the first hop in the route.
_, err = n.makePayment(n.aliceServer, n.bobServer,
n.bobServer.PubKey(), hops, amountNoFee, htlcAmt,
htlcExpiry).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, n.bobServer, firstHop, hops, amountNoFee,
htlcAmt, htlcExpiry,
).Wait(30 * time.Second)
// We should get an error, and that error should indicate that the HTLC
// should be rejected due to a policy violation (below min HTLC).
@ -786,9 +802,11 @@ func TestUpdateForwardingPolicy(t *testing.T) {
// First, send this 10 mSAT payment over the three hops, the payment
// should succeed, and all balances should be updated accordingly.
payResp, err := n.makePayment(n.aliceServer, n.carolServer,
n.bobServer.PubKey(), hops, amountNoFee, htlcAmt,
htlcExpiry).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
payResp, err := n.makePayment(
n.aliceServer, n.carolServer, firstHop, hops, amountNoFee,
htlcAmt, htlcExpiry,
).Wait(30 * time.Second)
if err != nil {
t.Fatalf("unable to send payment: %v", err)
}
@ -836,9 +854,10 @@ func TestUpdateForwardingPolicy(t *testing.T) {
// Next, we'll send the payment again, using the exact same per-hop
// payload for each node. This payment should fail as it won't factor
// in Bob's new fee policy.
_, err = n.makePayment(n.aliceServer, n.carolServer,
n.bobServer.PubKey(), hops, amountNoFee, htlcAmt,
htlcExpiry).Wait(30 * time.Second)
_, err = n.makePayment(
n.aliceServer, n.carolServer, firstHop, hops, amountNoFee,
htlcAmt, htlcExpiry,
).Wait(30 * time.Second)
if err == nil {
t.Fatalf("payment should've been rejected")
}
@ -895,9 +914,11 @@ func TestChannelLinkMultiHopInsufficientPayment(t *testing.T) {
// * user notification to be sent.
receiver := n.carolServer
rhash, err := n.makePayment(n.aliceServer, n.carolServer,
n.bobServer.PubKey(), hops, amount, htlcAmt,
totalTimelock).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
rhash, err := n.makePayment(
n.aliceServer, n.carolServer, firstHop, hops, amount, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
if err == nil {
t.Fatal("error haven't been received")
} else if !strings.Contains(err.Error(), "insufficient capacity") {
@ -991,8 +1012,10 @@ func TestChannelLinkMultiHopUnknownPaymentHash(t *testing.T) {
}
// Send payment and expose err channel.
_, err = n.aliceServer.htlcSwitch.SendHTLC(n.bobServer.PubKey(), htlc,
newMockDeobfuscator())
_, err = n.aliceServer.htlcSwitch.SendHTLC(
n.firstBobChannelLink.ShortChanID(), htlc,
newMockDeobfuscator(),
)
if err.Error() != lnwire.CodeUnknownPaymentHash.String() {
t.Fatal("error haven't been received")
}
@ -1066,10 +1089,11 @@ func TestChannelLinkMultiHopUnknownNextHop(t *testing.T) {
)
n.bobServer.htlcSwitch.RemoveLink(bobChanID)
bobPub := n.bobServer.PubKey()
firstHop := n.firstBobChannelLink.ShortChanID()
receiver := n.carolServer
rhash, err := n.makePayment(n.aliceServer, receiver, bobPub, hops,
amount, htlcAmt, totalTimelock).Wait(30 * time.Second)
rhash, err := n.makePayment(
n.aliceServer, receiver, firstHop, hops, amount, htlcAmt,
totalTimelock).Wait(30 * time.Second)
if err == nil {
t.Fatal("error haven't been received")
} else if err.Error() != lnwire.CodeUnknownNextPeer.String() {
@ -1173,9 +1197,11 @@ func TestChannelLinkMultiHopDecodeError(t *testing.T) {
n.firstBobChannelLink, n.carolChannelLink)
receiver := n.carolServer
rhash, err := n.makePayment(n.aliceServer, n.carolServer,
n.bobServer.PubKey(), hops, amount, htlcAmt,
totalTimelock).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
rhash, err := n.makePayment(
n.aliceServer, n.carolServer, firstHop, hops, amount, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
if err == nil {
t.Fatal("error haven't been received")
}
@ -1257,9 +1283,11 @@ func TestChannelLinkExpiryTooSoonExitNode(t *testing.T) {
startingHeight-10, n.firstBobChannelLink)
// Now we'll send out the payment from Alice to Bob.
_, err = n.makePayment(n.aliceServer, n.bobServer,
n.bobServer.PubKey(), hops, amount, htlcAmt,
totalTimelock).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, n.bobServer, firstHop, hops, amount, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
// The payment should've failed as the time lock value was in the
// _past_.
@ -1315,9 +1343,11 @@ func TestChannelLinkExpiryTooSoonMidNode(t *testing.T) {
startingHeight-10, n.firstBobChannelLink, n.carolChannelLink)
// Now we'll send out the payment from Alice to Bob.
_, err = n.makePayment(n.aliceServer, n.bobServer,
n.bobServer.PubKey(), hops, amount, htlcAmt,
totalTimelock).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, n.bobServer, firstHop, hops, amount, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
// The payment should've failed as the time lock value was in the
// _past_.
@ -1412,9 +1442,11 @@ func TestChannelLinkSingleHopMessageOrdering(t *testing.T) {
// * settle request to be sent back from bob to alice.
// * alice<->bob commitment state to be updated.
// * user notification to be sent.
_, err = n.makePayment(n.aliceServer, n.bobServer,
n.bobServer.PubKey(), hops, amount, htlcAmt,
totalTimelock).Wait(30 * time.Second)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, n.bobServer, firstHop, hops, amount, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
if err != nil {
t.Fatalf("unable to make the payment: %v", err)
}
@ -3236,9 +3268,11 @@ func TestChannelRetransmission(t *testing.T) {
//
// TODO(roasbeef); increase timeout?
receiver := n.bobServer
rhash, err := n.makePayment(n.aliceServer, receiver,
n.bobServer.PubKey(), hops, amount, htlcAmt,
totalTimelock).Wait(time.Second * 5)
firstHop := n.firstBobChannelLink.ShortChanID()
rhash, err := n.makePayment(
n.aliceServer, receiver, firstHop, hops, amount,
htlcAmt, totalTimelock,
).Wait(time.Second * 5)
if err == nil {
ct.Fatalf("payment shouldn't haven been finished")
}
@ -3529,9 +3563,10 @@ func TestChannelLinkShutdownDuringForward(t *testing.T) {
n.firstBobChannelLink, n.carolChannelLink,
)
firstHop := n.firstBobChannelLink.ShortChanID()
n.makePayment(
n.aliceServer, n.carolServer, n.bobServer.PubKey(),
hops, amount, htlcAmt, totalTimelock,
n.aliceServer, n.carolServer, firstHop, hops, amount, htlcAmt,
totalTimelock,
)
// Strobe the batch ticker of Bob's incoming link, waiting for it to
@ -3705,16 +3740,20 @@ func TestChannelLinkAcceptDuplicatePayment(t *testing.T) {
// With the invoice now added to Carol's registry, we'll send the
// payment. It should succeed w/o any issues as it has been crafted
// properly.
_, err = n.aliceServer.htlcSwitch.SendHTLC(n.bobServer.PubKey(), htlc,
newMockDeobfuscator())
_, err = n.aliceServer.htlcSwitch.SendHTLC(
n.firstBobChannelLink.ShortChanID(), htlc,
newMockDeobfuscator(),
)
if err != nil {
t.Fatalf("unable to send payment to carol: %v", err)
}
// Now, if we attempt to send the payment *again* it should be rejected
// as it's a duplicate request.
_, err = n.aliceServer.htlcSwitch.SendHTLC(n.bobServer.PubKey(), htlc,
newMockDeobfuscator())
_, err = n.aliceServer.htlcSwitch.SendHTLC(
n.firstBobChannelLink.ShortChanID(), htlc,
newMockDeobfuscator(),
)
if err != nil {
t.Fatalf("error shouldn't have been received got: %v", err)
}
@ -3760,9 +3799,10 @@ func TestChannelLinkAcceptOverpay(t *testing.T) {
// When we actually go to send the payment, we'll actually create an
// invoice at Carol for only half of this amount.
receiver := n.carolServer
firstHop := n.firstBobChannelLink.ShortChanID()
rhash, err := n.makePayment(
n.aliceServer, n.carolServer, n.bobServer.PubKey(),
hops, amount/2, htlcAmt, totalTimelock,
n.aliceServer, n.carolServer, firstHop, hops, amount/2, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
if err != nil {
t.Fatalf("unable to send payment: %v", err)
@ -4732,9 +4772,10 @@ func TestForwardingAsymmetricTimeLockPolicies(t *testing.T) {
n.carolChannelLink,
)
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, n.carolServer, n.bobServer.PubKey(), hops,
amount, htlcAmt, totalTimelock,
n.aliceServer, n.carolServer, firstHop, hops, amount, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
if err != nil {
t.Fatalf("unable to send payment: %v", err)

@ -340,7 +340,8 @@ func (s *Switch) ProcessContractResolution(msg contractcourt.ResolutionMsg) erro
// SendHTLC is used by other subsystems which aren't belong to htlc switch
// package in order to send the htlc update.
func (s *Switch) SendHTLC(nextNode [33]byte, htlc *lnwire.UpdateAddHTLC,
func (s *Switch) SendHTLC(firstHop lnwire.ShortChannelID,
htlc *lnwire.UpdateAddHTLC,
deobfuscator ErrorDecrypter) ([sha256.Size]byte, error) {
// Create payment and add to the map of payment in order later to be
@ -369,6 +370,7 @@ func (s *Switch) SendHTLC(nextNode [33]byte, htlc *lnwire.UpdateAddHTLC,
packet := &htlcPacket{
incomingChanID: sourceHop,
incomingHTLCID: paymentID,
outgoingChanID: firstHop,
destNode: nextNode,
htlc: htlc,
}
@ -783,56 +785,24 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error {
// User have created the htlc update therefore we should find the
// appropriate channel link and send the payment over this link.
case *lnwire.UpdateAddHTLC:
// Try to find links by node destination.
s.indexMtx.RLock()
links, err := s.getLinks(pkt.destNode)
link, err := s.getLinkByShortID(pkt.outgoingChanID)
s.indexMtx.RUnlock()
if err != nil {
s.indexMtx.RUnlock()
log.Errorf("unable to find links by destination %v", err)
log.Errorf("Link %v not found", pkt.outgoingChanID)
return &ForwardingError{
ErrorSource: s.cfg.SelfKey,
FailureMessage: &lnwire.FailUnknownNextPeer{},
}
}
s.indexMtx.RUnlock()
// Try to find destination channel link with appropriate
// bandwidth.
var (
destination ChannelLink
largestBandwidth lnwire.MilliSatoshi
)
for _, link := range links {
// We'll skip any links that aren't yet eligible for
// forwarding.
if !link.EligibleToForward() {
continue
}
bandwidth := link.Bandwidth()
if bandwidth > largestBandwidth {
largestBandwidth = bandwidth
}
if bandwidth >= htlc.Amount {
destination = link
break
}
}
// If the channel link we're attempting to forward the update
// over has insufficient capacity, then we'll cancel the HTLC
// as the payment cannot succeed.
if destination == nil {
err := fmt.Errorf("insufficient capacity in available "+
"outgoing links: need %v, max available is %v",
htlc.Amount, largestBandwidth)
if !link.EligibleToForward() {
err := fmt.Errorf("Link %v is not available to forward",
pkt.outgoingChanID)
log.Error(err)
// Note that we don't need to populate an update here,
// as this will go directly back to the router.
// The update does not need to be populated as the error
// will be returned back to the router.
htlcErr := lnwire.NewTemporaryChannelFailure(nil)
return &ForwardingError{
ErrorSource: s.cfg.SelfKey,
@ -841,12 +811,23 @@ func (s *Switch) handleLocalDispatch(pkt *htlcPacket) error {
}
}
// Send the packet to the destination channel link which
// manages then channel.
//
// TODO(roasbeef): should return with an error
pkt.outgoingChanID = destination.ShortChanID()
return destination.HandleSwitchPacket(pkt)
if link.Bandwidth() < htlc.Amount {
err := fmt.Errorf("Link %v has insufficient capacity: "+
"need %v, has %v", pkt.outgoingChanID,
htlc.Amount, link.Bandwidth())
log.Error(err)
// The update does not need to be populated as the error
// will be returned back to the router.
htlcErr := lnwire.NewTemporaryChannelFailure(nil)
return &ForwardingError{
ErrorSource: s.cfg.SelfKey,
ExtraMsg: err.Error(),
FailureMessage: htlcErr,
}
}
return link.HandleSwitchPacket(pkt)
// We've just received a settle update which means we can finalize the
// user payment and return successful response.

@ -1330,8 +1330,7 @@ func TestSkipIneligibleLinksLocalForward(t *testing.T) {
// We'll attempt to send out a new HTLC that has Alice as the first
// outgoing link. This should fail as Alice isn't yet able to forward
// any active HTLC's.
alicePub := aliceChannelLink.Peer().PubKey()
_, err = s.SendHTLC(alicePub, addMsg, nil)
_, err = s.SendHTLC(aliceChannelLink.ShortChanID(), addMsg, nil)
if err == nil {
t.Fatalf("local forward should fail due to inactive link")
}
@ -1656,7 +1655,8 @@ func TestSwitchSendPayment(t *testing.T) {
// Handle the request and checks that bob channel link received it.
errChan := make(chan error)
go func() {
_, err := s.SendHTLC(aliceChannelLink.Peer().PubKey(), update,
_, err := s.SendHTLC(
aliceChannelLink.ShortChanID(), update,
newMockDeobfuscator())
errChan <- err
}()
@ -1664,8 +1664,10 @@ func TestSwitchSendPayment(t *testing.T) {
go func() {
// Send the payment with the same payment hash and same
// amount and check that it will be propagated successfully
_, err := s.SendHTLC(aliceChannelLink.Peer().PubKey(), update,
newMockDeobfuscator())
_, err := s.SendHTLC(
aliceChannelLink.ShortChanID(), update,
newMockDeobfuscator(),
)
errChan <- err
}()
@ -1793,9 +1795,10 @@ func TestLocalPaymentNoForwardingEvents(t *testing.T) {
// wait for Alice to receive the preimage for the payment before
// proceeding.
receiver := n.bobServer
firstHop := n.firstBobChannelLink.ShortChanID()
_, err = n.makePayment(
n.aliceServer, receiver, n.bobServer.PubKey(), hops, amount,
htlcAmt, totalTimelock,
n.aliceServer, receiver, firstHop, hops, amount, htlcAmt,
totalTimelock,
).Wait(30 * time.Second)
if err != nil {
t.Fatalf("unable to make the payment: %v", err)
@ -1852,10 +1855,11 @@ func TestMultiHopPaymentForwardingEvents(t *testing.T) {
finalAmt, testStartingHeight, n.firstBobChannelLink,
n.carolChannelLink,
)
firstHop := n.firstBobChannelLink.ShortChanID()
for i := 0; i < numPayments; i++ {
_, err := n.makePayment(
n.aliceServer, n.carolServer, n.bobServer.PubKey(),
hops, finalAmt, htlcAmt, totalTimelock,
n.aliceServer, n.carolServer, firstHop, hops, finalAmt,
htlcAmt, totalTimelock,
).Wait(30 * time.Second)
if err != nil {
t.Fatalf("unable to send payment: %v", err)

@ -664,7 +664,7 @@ func (r *paymentResponse) Wait(d time.Duration) (chainhash.Hash, error) {
// * from Alice to Carol through the Bob
// * from Alice to some another peer through the Bob
func (n *threeHopNetwork) makePayment(sendingPeer, receivingPeer lnpeer.Peer,
firstHopPub [33]byte, hops []ForwardingInfo,
firstHop lnwire.ShortChannelID, hops []ForwardingInfo,
invoiceAmt, htlcAmt lnwire.MilliSatoshi,
timelock uint32) *paymentResponse {
@ -708,8 +708,9 @@ func (n *threeHopNetwork) makePayment(sendingPeer, receivingPeer lnpeer.Peer,
// Send payment and expose err channel.
go func() {
_, err := sender.htlcSwitch.SendHTLC(firstHopPub, htlc,
newMockDeobfuscator())
_, err := sender.htlcSwitch.SendHTLC(
firstHop, htlc, newMockDeobfuscator(),
)
paymentErr <- err
}()