Skip to content

Commit

Permalink
Merge c285c32 into 9f358db
Browse files Browse the repository at this point in the history
  • Loading branch information
@cfromknecht
cfromknecht committed May 28, 2018
2 parents 9f358db + c285c32 commit cd9f855
Show file tree
Hide file tree
Showing 2 changed files with 217 additions and 39 deletions.
26 changes: 13 additions & 13 deletions lnwallet/channel.go
View file
Expand Up @@ -4071,6 +4071,7 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) (
remoteChainTail := lc.remoteCommitChain.tail().height + 1
localChainTail := lc.localCommitChain.tail().height

source := lc.ShortChanID()
chanID := lnwire.NewChanIDFromOutPoint(&lc.channelState.FundingOutpoint)

// Determine the set of htlcs that can be forwarded as a result of
Expand All @@ -4093,19 +4094,22 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) (
continue
}

uncommitted := (pd.addCommitHeightRemote == 0 ||
pd.addCommitHeightLocal == 0)
if pd.EntryType == Add && uncommitted {
continue
}
// For each type of HTLC, we will only consider forwarding it if
// both of the remote and local heights are non-zero. If either
// of these values is zero, it has yet to be committed in both
// the local and remote chains.
committedAdd := pd.addCommitHeightRemote > 0 &&
pd.addCommitHeightLocal > 0
committedRmv := pd.removeCommitHeightRemote > 0 &&
pd.removeCommitHeightLocal > 0

// Using the height of the remote and local commitments,
// preemptively compute whether or not to forward this HTLC for
// the case in which this in an Add HTLC, or if this is a
// Settle, Fail, or MalformedFail.
shouldFwdAdd := remoteChainTail == pd.addCommitHeightRemote &&
localChainTail >= pd.addCommitHeightLocal
shouldFwdRmv := remoteChainTail >= pd.removeCommitHeightRemote &&
shouldFwdRmv := remoteChainTail == pd.removeCommitHeightRemote &&
localChainTail >= pd.removeCommitHeightLocal

// We'll only forward any new HTLC additions iff, it's "freshly
Expand All @@ -4114,8 +4118,7 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) (
// don't re-forward any already processed HTLC's after a
// restart.
switch {
case pd.EntryType == Add && shouldFwdAdd:

case pd.EntryType == Add && committedAdd && shouldFwdAdd:
// Construct a reference specifying the location that
// this forwarded Add will be written in the forwarding
// package constructed at this remote height.
Expand All @@ -4128,13 +4131,12 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) (
pd.isForwarded = true
addsToForward = append(addsToForward, pd)

case pd.EntryType != Add && shouldFwdRmv:

case pd.EntryType != Add && committedRmv && shouldFwdRmv:
// Construct a reference specifying the location that
// this forwarded Settle/Fail will be written in the
// forwarding package constructed at this remote height.
pd.DestRef = &channeldb.SettleFailRef{
Source: lc.ShortChanID(),
Source: source,
Height: remoteChainTail,
Index: settleFailIndex,
}
Expand Down Expand Up @@ -4199,8 +4201,6 @@ func (lc *LightningChannel) ReceiveRevocation(revMsg *lnwire.RevokeAndAck) (
}
}

source := lc.ShortChanID()

// Now that we have gathered the set of HTLCs to forward, separated by
// type, construct a forwarding package using the height that the remote
// commitment chain will be extended after persisting the revocation.
Expand Down
230 changes: 204 additions & 26 deletions lnwallet/channel_test.go
View file
Expand Up @@ -3943,8 +3943,8 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) {
}
defer cleanUp()

// We'll now add two HTLC's from Bob to Alice, then Bob will initiate a
// state transition.
// We'll now add two HTLC's from Alice to Bob, then Alice will initiate
// a state transition.
var htlcAmt lnwire.MilliSatoshi = 100000
htlc, _ := createHTLC(0, htlcAmt)
if _, err := aliceChannel.AddHTLC(htlc, nil); err != nil {
Expand Down Expand Up @@ -3975,21 +3975,26 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) {
if err != nil {
t.Fatal(err)
}
bobSig, bobHtlcSigs, err := bobChannel.SignNextCommitment()
if err != nil {
t.Fatal(err)
}

// Alice should detect that she doesn't need to forward any HTLC's.
_, aliceHtlcsToForward, _, err := aliceChannel.ReceiveRevocation(
bobRevocation,
)
fwdPkg, _, _, err := aliceChannel.ReceiveRevocation(bobRevocation)
if err != nil {
t.Fatal(err)
}
if len(aliceHtlcsToForward) != 0 {
if len(fwdPkg.Adds) != 0 {
t.Fatalf("alice shouldn't forward any HTLC's, instead wants to "+
"forward %v htlcs", len(fwdPkg.Adds))
}
if len(fwdPkg.SettleFails) != 0 {
t.Fatalf("alice shouldn't forward any HTLC's, instead wants to "+
"forward %v htlcs", len(aliceHtlcsToForward))
"forward %v htlcs", len(fwdPkg.SettleFails))
}

// Now, have Bob initiate a transition to lock in the Adds sent by
// Alice.
bobSig, bobHtlcSigs, err := bobChannel.SignNextCommitment()
if err != nil {
t.Fatal(err)
}

err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs)
Expand All @@ -4001,15 +4006,19 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) {
t.Fatal(err)
}

// Bob on the other hand, should detect that he now has 2 incoming
// HTLC's that he can forward along.
_, bobHtlcsToForward, _, err := bobChannel.ReceiveRevocation(aliceRevocation)
// Bob should now detect that he now has 2 incoming HTLC's that he can
// forward along.
fwdPkg, _, _, err = bobChannel.ReceiveRevocation(aliceRevocation)
if err != nil {
t.Fatal(err)
}
if len(bobHtlcsToForward) != 2 {
if len(fwdPkg.Adds) != 2 {
t.Fatalf("bob should forward 2 hltcs, instead has %v",
len(bobHtlcsToForward))
len(fwdPkg.Adds))
}
if len(fwdPkg.SettleFails) != 0 {
t.Fatalf("bob should forward 0 hltcs, instead has %v",
len(fwdPkg.SettleFails))
}

// We'll now restart both Alice and Bob. This emulates a reconnection
Expand All @@ -4018,18 +4027,20 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) {
if err != nil {
t.Fatalf("unable to restart alice: %v", err)
}
defer aliceChannel.Stop()
bobChannel, err = restartChannel(bobChannel)
if err != nil {
t.Fatalf("unable to restart bob: %v", err)
}
defer bobChannel.Stop()

// With both nodes restarted, Bob will now attempt to cancel one of
// Alice's HTLC's.
err = bobChannel.FailHTLC(htlc2.ID, []byte("failreason"), nil, nil, nil)
err = bobChannel.FailHTLC(htlc.ID, []byte("failreason"), nil, nil, nil)
if err != nil {
t.Fatalf("unable to cancel HTLC: %v", err)
}
err = aliceChannel.ReceiveFailHTLC(htlc2.ID, []byte("bad"))
err = aliceChannel.ReceiveFailHTLC(htlc.ID, []byte("bad"))
if err != nil {
t.Fatalf("unable to recv htlc cancel: %v", err)
}
Expand All @@ -4048,25 +4059,192 @@ func TestLockedInHtlcForwardingSkipAfterRestart(t *testing.T) {
if err != nil {
t.Fatal(err)
}
aliceSig, aliceHtlcSigs, err = aliceChannel.SignNextCommitment()
if err != nil {
t.Fatal(err)
}

// At this point, Bob receives the revocation from Alice, which is now
// his signal to examine all the HTLC's that have been locked in to
// process.
_, bobHtlcsToForward, _, err = bobChannel.ReceiveRevocation(aliceRevocation)
fwdPkg, _, _, err = bobChannel.ReceiveRevocation(aliceRevocation)
if err != nil {
t.Fatal(err)
}

// Bob should detect that he doesn't need to forward *any* HTLC's, as
// he was the one that initiated extending the commitment chain of
// Alice.
if len(bobHtlcsToForward) != 0 {
t.Fatalf("bob shouldn't forward any htlcs, but has: %v",
spew.Sdump(bobHtlcsToForward))
if len(fwdPkg.Adds) != 0 {
t.Fatalf("alice shouldn't forward any HTLC's, instead wants to "+
"forward %v htlcs", len(fwdPkg.Adds))
}
if len(fwdPkg.SettleFails) != 0 {
t.Fatalf("alice shouldn't forward any HTLC's, instead wants to "+
"forward %v htlcs", len(fwdPkg.SettleFails))
}

// Now, begin another state transition led by Alice, and fail the second
// HTLC part-way through the dance.
aliceSig, aliceHtlcSigs, err = aliceChannel.SignNextCommitment()
if err != nil {
t.Fatal(err)
}
err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs)
if err != nil {
t.Fatal(err)
}

// Failing the HTLC here will cause the update to be included in Alice's
// remote log, but it should not be committed by this transition.
err = bobChannel.FailHTLC(htlc2.ID, []byte("failreason"), nil, nil, nil)
if err != nil {
t.Fatalf("unable to cancel HTLC: %v", err)
}
err = aliceChannel.ReceiveFailHTLC(htlc2.ID, []byte("bad"))
if err != nil {
t.Fatalf("unable to recv htlc cancel: %v", err)
}

bobRevocation, _, err = bobChannel.RevokeCurrentCommitment()
if err != nil {
t.Fatal(err)
}

// Alice should detect that she doesn't need to forward any Adds's, but
// that the Fail has been locked in an can be forwarded.
_, adds, settleFails, err := aliceChannel.ReceiveRevocation(bobRevocation)
if err != nil {
t.Fatal(err)
}
if len(adds) != 0 {
t.Fatalf("alice shouldn't forward any HTLC's, instead wants to "+
"forward %v htlcs", len(adds))
}
if len(settleFails) != 1 {
t.Fatalf("alice should only forward %d HTLC's, instead wants to "+
"forward %v htlcs", 1, len(settleFails))
}
if settleFails[0].ParentIndex != htlc.ID {
t.Fatalf("alice should forward fail for htlcid=%d, instead "+
"forwarding id=%d", htlc.ID,
settleFails[0].ParentIndex)
}

// We'll now restart both Alice and Bob. This emulates a reconnection
// between the two peers.
aliceChannel, err = restartChannel(aliceChannel)
if err != nil {
t.Fatalf("unable to restart alice: %v", err)
}
defer aliceChannel.Stop()
bobChannel, err = restartChannel(bobChannel)
if err != nil {
t.Fatalf("unable to restart bob: %v", err)
}
defer bobChannel.Stop()

// Readd the Fail to both Alice and Bob's channels, as the non-committed
// update will not have survived the restart.
err = bobChannel.FailHTLC(htlc2.ID, []byte("failreason"), nil, nil, nil)
if err != nil {
t.Fatalf("unable to cancel HTLC: %v", err)
}
err = aliceChannel.ReceiveFailHTLC(htlc2.ID, []byte("bad"))
if err != nil {
t.Fatalf("unable to recv htlc cancel: %v", err)
}

// Have Alice initiate a state transition, which does not include the
// HTLCs just readded to the channel state.
aliceSig, aliceHtlcSigs, err = aliceChannel.SignNextCommitment()
if err != nil {
t.Fatal(err)
}
err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs)
if err != nil {
t.Fatal(err)
}
bobRevocation, _, err = bobChannel.RevokeCurrentCommitment()
if err != nil {
t.Fatal(err)
}

// Alice should detect that she doesn't need to forward any HTLC's, as
// the updates haven't been committed by Bob yet.
fwdPkg, _, _, err = aliceChannel.ReceiveRevocation(bobRevocation)
if err != nil {
t.Fatal(err)
}
if len(fwdPkg.Adds) != 0 {
t.Fatalf("alice shouldn't forward any HTLC's, instead wants to "+
"forward %v htlcs", len(fwdPkg.Adds))
}
if len(fwdPkg.SettleFails) != 0 {
t.Fatalf("alice shouldn't forward any HTLC's, instead wants to "+
"forward %v htlcs", len(fwdPkg.SettleFails))
}

// Now initiate a final update from Bob to lock in the final Fail.
bobSig, bobHtlcSigs, err = bobChannel.SignNextCommitment()
if err != nil {
t.Fatal(err)
}

err = aliceChannel.ReceiveNewCommitment(bobSig, bobHtlcSigs)
if err != nil {
t.Fatal(err)
}

aliceRevocation, _, err = aliceChannel.RevokeCurrentCommitment()
if err != nil {
t.Fatal(err)
}

// Bob should detect that he has nothing to forward, as he hasn't
// received any HTLCs.
fwdPkg, _, _, err = bobChannel.ReceiveRevocation(aliceRevocation)
if err != nil {
t.Fatal(err)
}
if len(fwdPkg.Adds) != 0 {
t.Fatalf("bob should forward 4 hltcs, instead has %v",
len(fwdPkg.Adds))
}
if len(fwdPkg.SettleFails) != 0 {
t.Fatalf("bob should forward 0 hltcs, instead has %v",
len(fwdPkg.SettleFails))
}

// Finally, have Bob initiate a state transition that locks in the Fail
// added after the restart.
aliceSig, aliceHtlcSigs, err = aliceChannel.SignNextCommitment()
if err != nil {
t.Fatal(err)
}
err = bobChannel.ReceiveNewCommitment(aliceSig, aliceHtlcSigs)
if err != nil {
t.Fatal(err)
}
bobRevocation, _, err = bobChannel.RevokeCurrentCommitment()
if err != nil {
t.Fatal(err)
}

// When Alice receives the revocation, she should detect that she
// can now forward the freshly locked-in Fail.
_, adds, settleFails, err = aliceChannel.ReceiveRevocation(bobRevocation)
if err != nil {
t.Fatal(err)
}
if len(adds) != 0 {
t.Fatalf("alice shouldn't forward any HTLC's, instead wants to "+
"forward %v htlcs", len(adds))
}
if len(settleFails) != 1 {
t.Fatalf("alice should only forward one HTLC, instead wants to "+
"forward %v htlcs", len(settleFails))
}
if settleFails[0].ParentIndex != htlc2.ID {
t.Fatalf("alice should forward fail for htlcid=%d, instead "+
"forwarding id=%d", htlc2.ID,
settleFails[0].ParentIndex)
}
}

Expand Down

0 comments on commit cd9f855

Please sign in to comment.