highperfocused/lnd
1
0
Fork 0
mirror of https://github.com/lightningnetwork/lnd.git synced 2025-03-25 17:22:59 +01:00
Code Issues Packages Projects Releases Wiki Activity

lnwallet: make spendTx param in NewBreachRetribution optional

Browse Source 
In this commit, the NewBreachRetribution function is adjusted so that a
caller can optionally set the spendTx parameter to nil. In this case,
the function will check the revocation log to see if the local and
remote amount fields are available there and use them if they are.
If the fields are not present, which they might not be given a previous
migration that removed the fields, then an error is returned.
This commit is contained in:
Elle Mouton 2023-02-02 10:13:44 +02:00
parent 2c786ec66f
commit 91a2cd45d8
No known key found for this signature in database
GPG Key ID: D7D916376026F177
2 changed files with 136 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

92
lnwallet/channel.go
View File

@ -108,6 +108,10 @@ var (
// ErrOutputIndexOutOfRange is returned when an output index is greater
// than or equal to the length of a given transaction's outputs.
ErrOutputIndexOutOfRange = errors.New("output index is out of range")
// ErrRevLogDataMissing is returned when a certain wanted optional field
// in a revocation log entry is missing.
ErrRevLogDataMissing = errors.New("revocation log data missing")
)
// ErrCommitSyncLocalDataLoss is returned in the case that we receive a valid
@ -2287,8 +2291,12 @@ type BreachRetribution struct {
}
// NewBreachRetribution creates a new fully populated BreachRetribution for the
// passed channel, at a particular revoked state number, and one which targets
// the passed commitment transaction.
// passed channel, at a particular revoked state number. If the spend
// transaction that the breach retribution should target is known, then it can
// be provided via the spendTx parameter. Otherwise, if the spendTx parameter is
// nil, then the revocation log will be checked to see if it contains the info
// required to construct the BreachRetribution. If the revocation log is missing
// the required fields then ErrRevLogDataMissing will be returned.
func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64,
breachHeight uint32, spendTx *wire.MsgTx) (*BreachRetribution, error) {
@ -2493,7 +2501,11 @@ func createHtlcRetribution(chanState *channeldb.OpenChannel,
// createBreachRetribution creates a partially initiated BreachRetribution
// using a RevocationLog. Returns the constructed retribution, our amount,
// their amount, and a possible non-nil error.
// their amount, and a possible non-nil error. If the spendTx parameter is
// non-nil, then it will be used to glean the breach transaction's to-local and
// to-remote output amounts. Otherwise, the RevocationLog will be checked to
// see if these fields are present there. If they are not, then
// ErrRevLogDataMissing is returned.
func createBreachRetribution(revokedLog *channeldb.RevocationLog,
spendTx *wire.MsgTx, chanState *channeldb.OpenChannel,
keyRing *CommitmentKeyRing, commitmentSecret *btcec.PrivateKey,
@ -2523,18 +2535,33 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog,
if revokedLog.OurOutputIndex != channeldb.OutputIndexEmpty {
ourOutpoint.Index = uint32(revokedLog.OurOutputIndex)
// Sanity check that OurOutputIndex is within range.
if int(ourOutpoint.Index) >= len(spendTx.TxOut) {
return nil, 0, 0, fmt.Errorf("%w: ours=%v, "+
"len(TxOut)=%v", ErrOutputIndexOutOfRange,
ourOutpoint.Index, len(spendTx.TxOut),
)
// If the spend transaction is provided, then we use it to get
// the value of our output.
if spendTx != nil {
// Sanity check that OurOutputIndex is within range.
if int(ourOutpoint.Index) >= len(spendTx.TxOut) {
return nil, 0, 0, fmt.Errorf("%w: ours=%v, "+
"len(TxOut)=%v",
ErrOutputIndexOutOfRange,
ourOutpoint.Index, len(spendTx.TxOut),
)
}
// Read the amounts from the breach transaction.
//
// NOTE: ourAmt here includes commit fee and anchor
// amount (if enabled).
ourAmt = spendTx.TxOut[ourOutpoint.Index].Value
} else {
// Otherwise, we check to see if the revocation log
// contains our output amount. Due to a previous
// migration, this field may be empty in which case an
// error will be returned.
if revokedLog.OurBalance == nil {
return nil, 0, 0, ErrRevLogDataMissing
}
ourAmt = int64(revokedLog.OurBalance.ToSatoshis())
}
// Read the amounts from the breach transaction.
//
// NOTE: ourAmt here includes commit fee and anchor amount(if
// enabled).
ourAmt = spendTx.TxOut[ourOutpoint.Index].Value
}
// Construct the their outpoint.
@ -2544,16 +2571,35 @@ func createBreachRetribution(revokedLog *channeldb.RevocationLog,
if revokedLog.TheirOutputIndex != channeldb.OutputIndexEmpty {
theirOutpoint.Index = uint32(revokedLog.TheirOutputIndex)
// Sanity check that TheirOutputIndex is within range.
if int(revokedLog.TheirOutputIndex) >= len(spendTx.TxOut) {
return nil, 0, 0, fmt.Errorf("%w: theirs=%v, "+
"len(TxOut)=%v", ErrOutputIndexOutOfRange,
revokedLog.TheirOutputIndex, len(spendTx.TxOut),
)
}
// If the spend transaction is provided, then we use it to get
// the value of the remote parties' output.
if spendTx != nil {
// Sanity check that TheirOutputIndex is within range.
if int(revokedLog.TheirOutputIndex) >=
len(spendTx.TxOut) {
// Read the amounts from the breach transaction.
theirAmt = spendTx.TxOut[theirOutpoint.Index].Value
return nil, 0, 0, fmt.Errorf("%w: theirs=%v, "+
"len(TxOut)=%v",
ErrOutputIndexOutOfRange,
revokedLog.TheirOutputIndex,
len(spendTx.TxOut),
)
}
// Read the amounts from the breach transaction.
theirAmt = spendTx.TxOut[theirOutpoint.Index].Value
} else {
// Otherwise, we check to see if the revocation log
// contains remote parties' output amount. Due to a
// previous migration, this field may be empty in which
// case an error will be returned.
if revokedLog.TheirBalance == nil {
return nil, 0, 0, ErrRevLogDataMissing
}
theirAmt = int64(revokedLog.TheirBalance.ToSatoshis())
}
}
return &BreachRetribution{

72
lnwallet/channel_test.go
View File

@ -9482,7 +9482,7 @@ func TestCreateHtlcRetribution(t *testing.T) {
}
// TestCreateBreachRetribution checks that `createBreachRetribution` behaves as
// epxected.
// expected.
func TestCreateBreachRetribution(t *testing.T) {
t.Parallel()
@ -9524,11 +9524,15 @@ func TestCreateBreachRetribution(t *testing.T) {
}
// Create a dummy revocation log.
ourAmtMsat := lnwire.MilliSatoshi(ourAmt * 1000)
theirAmtMsat := lnwire.MilliSatoshi(theirAmt * 1000)
revokedLog := channeldb.RevocationLog{
CommitTxHash: commitHash,
OurOutputIndex: uint16(localIndex),
TheirOutputIndex: uint16(remoteIndex),
HTLCEntries: []*channeldb.HTLCEntry{htlc},
TheirBalance: &theirAmtMsat,
OurBalance: &ourAmtMsat,
}
// Create a log with an empty local output index.
@ -9545,14 +9549,25 @@ func TestCreateBreachRetribution(t *testing.T) {
expectedErr error
expectedOurAmt int64
expectedTheirAmt int64
noSpendTx bool
}{
{
name: "create retribution successfully",
name: "create retribution successfully " +
"with spend tx",
revocationLog: &revokedLog,
expectedErr: nil,
expectedOurAmt: ourAmt,
expectedTheirAmt: theirAmt,
},
{
name: "create retribution successfully " +
"without spend tx",
revocationLog: &revokedLog,
expectedErr: nil,
expectedOurAmt: ourAmt,
expectedTheirAmt: theirAmt,
noSpendTx: true,
},
{
name: "fail due to our index too big",
revocationLog: &channeldb.RevocationLog{
@ -9568,19 +9583,39 @@ func TestCreateBreachRetribution(t *testing.T) {
expectedErr: ErrOutputIndexOutOfRange,
},
{
name: "empty local output index",
name: "empty local output index with spend " +
"tx",
revocationLog: &revokedLogNoLocal,
expectedErr: nil,
expectedOurAmt: 0,
expectedTheirAmt: theirAmt,
},
{
name: "empty remote output index",
name: "empty local output index without spend " +
"tx",
revocationLog: &revokedLogNoLocal,
expectedErr: nil,
expectedOurAmt: 0,
expectedTheirAmt: theirAmt,
noSpendTx: true,
},
{
name: "empty remote output index with spend " +
"tx",
revocationLog: &revokedLogNoRemote,
expectedErr: nil,
expectedOurAmt: ourAmt,
expectedTheirAmt: 0,
},
{
name: "empty remote output index without spend " +
"tx",
revocationLog: &revokedLogNoRemote,
expectedErr: nil,
expectedOurAmt: ourAmt,
expectedTheirAmt: 0,
noSpendTx: true,
},
}
// assertRetribution is a helper closure that checks a given breach
@ -9623,8 +9658,13 @@ func TestCreateBreachRetribution(t *testing.T) {
for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
tx := spendTx
if tc.noSpendTx {
tx = nil
}
br, our, their, err := createBreachRetribution(
tc.revocationLog, spendTx,
tc.revocationLog, tx,
aliceChannel.channelState, keyRing,
dummyPrivate, leaseExpiry,
)
@ -9748,6 +9788,13 @@ func testNewBreachRetribution(t *testing.T, chanType channeldb.ChannelType) {
)
require.ErrorIs(t, err, channeldb.ErrNoPastDeltas)
// We also check that the same error is returned if no breach tx is
// provided.
_, err = NewBreachRetribution(
aliceChannel.channelState, stateNum, breachHeight, nil,
)
require.ErrorIs(t, err, channeldb.ErrNoPastDeltas)
// We now force a state transition which will give us a revocation log
// at height 0.
txid := aliceChannel.channelState.RemoteCommitment.CommitTx.TxHash()
@ -9797,10 +9844,25 @@ func testNewBreachRetribution(t *testing.T, chanType channeldb.ChannelType) {
t.Log(spew.Sdump(breachTx))
assertRetribution(br, 1, 0)
// Repeat the check but with the breach tx set to nil. This should work
// since the necessary info should now be found in the revocation log.
br, err = NewBreachRetribution(
aliceChannel.channelState, stateNum, breachHeight, nil,
)
require.NoError(t, err)
assertRetribution(br, 1, 0)
// Create the retribution using a stateNum+1 and we should expect an
// error.
_, err = NewBreachRetribution(
aliceChannel.channelState, stateNum+1, breachHeight, breachTx,
)
require.ErrorIs(t, err, channeldb.ErrLogEntryNotFound)
// Once again, repeat the check for the case when no breach tx is
// provided.
_, err = NewBreachRetribution(
aliceChannel.channelState, stateNum+1, breachHeight, nil,
)
require.ErrorIs(t, err, channeldb.ErrLogEntryNotFound)
}