Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Channeldb: Store HTLC Extra TLVs in Onion Blob Varbytes #7710

Merged
merged 3 commits into from
Jun 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 83 additions & 5 deletions channeldb/channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,10 @@ var (
// ErrMissingIndexEntry is returned when a caller attempts to close a
// channel and the outpoint is missing from the index.
ErrMissingIndexEntry = fmt.Errorf("missing outpoint from index")

// ErrOnionBlobLength is returned is an onion blob with incorrect
// length is read from disk.
ErrOnionBlobLength = errors.New("onion blob < 1366 bytes")
carlaKC marked this conversation as resolved.
Show resolved Hide resolved
)

const (
Expand Down Expand Up @@ -2001,15 +2005,15 @@ func (c *OpenChannel) ActiveHtlcs() []HTLC {
// which ones are present on their commitment.
remoteHtlcs := make(map[[32]byte]struct{})
for _, htlc := range c.RemoteCommitment.Htlcs {
onionHash := sha256.Sum256(htlc.OnionBlob)
onionHash := sha256.Sum256(htlc.OnionBlob[:])
remoteHtlcs[onionHash] = struct{}{}
}

// Now that we know which HTLC's they have, we'll only mark the HTLC's
// as active if *we* know them as well.
activeHtlcs := make([]HTLC, 0, len(remoteHtlcs))
for _, htlc := range c.LocalCommitment.Htlcs {
onionHash := sha256.Sum256(htlc.OnionBlob)
onionHash := sha256.Sum256(htlc.OnionBlob[:])
if _, ok := remoteHtlcs[onionHash]; !ok {
continue
}
Expand Down Expand Up @@ -2056,7 +2060,7 @@ type HTLC struct {

// OnionBlob is an opaque blob which is used to complete multi-hop
// routing.
OnionBlob []byte
OnionBlob [lnwire.OnionPacketSize]byte
carlaKC marked this conversation as resolved.
Show resolved Hide resolved

// HtlcIndex is the HTLC counter index of this active, outstanding
// HTLC. This differs from the LogIndex, as the HtlcIndex is only
Expand All @@ -2068,11 +2072,39 @@ type HTLC struct {
// from the HtlcIndex as this will be incremented for each new log
// update added.
LogIndex uint64

// ExtraData contains any additional information that was transmitted
// with the HTLC via TLVs. This data *must* already be encoded as a
// TLV stream, and may be empty. The length of this data is naturally
// limited by the space available to TLVs in update_add_htlc:
// = 65535 bytes (bolt 8 maximum message size):
// - 2 bytes (bolt 1 message_type)
// - 32 bytes (channel_id)
// - 8 bytes (id)
// - 8 bytes (amount_msat)
// - 32 bytes (payment_hash)
// - 4 bytes (cltv_expiry
// - 1366 bytes (onion_routing_packet)
// = 64083 bytes maximum possible TLV stream
//
// Note that this extra data is stored inline with the OnionBlob for
// legacy reasons, see serialization/deserialization functions for
// detail.
ExtraData []byte
carlaKC marked this conversation as resolved.
Show resolved Hide resolved
carlaKC marked this conversation as resolved.
Show resolved Hide resolved
}

// SerializeHtlcs writes out the passed set of HTLC's into the passed writer
// using the current default on-disk serialization format.
//
// This inline serialization has been extended to allow storage of extra data
// associated with a HTLC in the following way:
// - The known-length onion blob (1366 bytes) is serialized as var bytes in
// WriteElements (ie, the length 1366 was written, followed by the 1366
// onion bytes).
// - To include extra data, we append any extra data present to this one
carlaKC marked this conversation as resolved.
Show resolved Hide resolved
// variable length of data. Since we know that the onion is strictly 1366
// bytes, any length after that should be considered to be extra data.
//
// NOTE: This API is NOT stable, the on-disk format will likely change in the
// future.
func SerializeHtlcs(b io.Writer, htlcs ...HTLC) error {
Expand All @@ -2082,9 +2114,17 @@ func SerializeHtlcs(b io.Writer, htlcs ...HTLC) error {
}

for _, htlc := range htlcs {
// The onion blob and hltc data are stored as a single var
// bytes blob.
onionAndExtraData := make(
[]byte, lnwire.OnionPacketSize+len(htlc.ExtraData),
)
copy(onionAndExtraData, htlc.OnionBlob[:])
copy(onionAndExtraData[lnwire.OnionPacketSize:], htlc.ExtraData)

if err := WriteElements(b,
htlc.Signature, htlc.RHash, htlc.Amt, htlc.RefundTimeout,
htlc.OutputIndex, htlc.Incoming, htlc.OnionBlob[:],
htlc.OutputIndex, htlc.Incoming, onionAndExtraData,
yyforyongyu marked this conversation as resolved.
Show resolved Hide resolved
htlc.HtlcIndex, htlc.LogIndex,
); err != nil {
return err
Expand All @@ -2098,6 +2138,17 @@ func SerializeHtlcs(b io.Writer, htlcs ...HTLC) error {
// io.Reader. The bytes within the passed reader MUST have been previously
// written to using the SerializeHtlcs function.
//
// This inline deserialization has been extended to allow storage of extra data
// associated with a HTLC in the following way:
// - The known-length onion blob (1366 bytes) and any additional data present
// are read out as a single blob of variable byte data.
// - They are stored like this to take advantage of the variable space
// available for extension without migration (see SerializeHtlcs).
// - The first 1366 bytes are interpreted as the onion blob, and any remaining
// bytes as extra HTLC data.
// - This extra HTLC data is expected to be serialized as a TLV stream, and
// its parsing is left to higher layers.
//
// NOTE: This API is NOT stable, the on-disk format will likely change in the
// future.
func DeserializeHtlcs(r io.Reader) ([]HTLC, error) {
Expand All @@ -2113,14 +2164,41 @@ func DeserializeHtlcs(r io.Reader) ([]HTLC, error) {

htlcs = make([]HTLC, numHtlcs)
for i := uint16(0); i < numHtlcs; i++ {
var onionAndExtraData []byte
if err := ReadElements(r,
&htlcs[i].Signature, &htlcs[i].RHash, &htlcs[i].Amt,
&htlcs[i].RefundTimeout, &htlcs[i].OutputIndex,
&htlcs[i].Incoming, &htlcs[i].OnionBlob,
&htlcs[i].Incoming, &onionAndExtraData,
&htlcs[i].HtlcIndex, &htlcs[i].LogIndex,
); err != nil {
return htlcs, err
}

// Sanity check that we have at least the onion blob size we
// expect.
if len(onionAndExtraData) < lnwire.OnionPacketSize {
return nil, ErrOnionBlobLength
}

// First OnionPacketSize bytes are our fixed length onion
// packet.
copy(
htlcs[i].OnionBlob[:],
onionAndExtraData[0:lnwire.OnionPacketSize],
)

// Any additional bytes belong to extra data. ExtraDataLen
// will be >= 0, because we know that we always have a fixed
carlaKC marked this conversation as resolved.
Show resolved Hide resolved
// length onion packet.
extraDataLen := len(onionAndExtraData) - lnwire.OnionPacketSize
carlaKC marked this conversation as resolved.
Show resolved Hide resolved
if extraDataLen > 0 {
htlcs[i].ExtraData = make([]byte, extraDataLen)

copy(
htlcs[i].ExtraData,
onionAndExtraData[lnwire.OnionPacketSize:],
)
}
}

return htlcs, nil
Expand Down
116 changes: 111 additions & 5 deletions channeldb/channel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/lightningnetwork/lnd/clock"
"github.com/lightningnetwork/lnd/keychain"
"github.com/lightningnetwork/lnd/kvdb"
"github.com/lightningnetwork/lnd/lnmock"
"github.com/lightningnetwork/lnd/lntest/channels"
"github.com/lightningnetwork/lnd/lnwire"
"github.com/lightningnetwork/lnd/shachain"
Expand Down Expand Up @@ -366,12 +367,13 @@ func TestOpenChannelPutGetDelete(t *testing.T) {
// Create the test channel state, with additional htlcs on the local
// and remote commitment.
localHtlcs := []HTLC{
{Signature: testSig.Serialize(),
{
Signature: testSig.Serialize(),
Incoming: true,
Amt: 10,
RHash: key,
RefundTimeout: 1,
OnionBlob: []byte("onionblob"),
OnionBlob: lnmock.MockOnion(),
},
}

Expand All @@ -382,7 +384,7 @@ func TestOpenChannelPutGetDelete(t *testing.T) {
Amt: 10,
RHash: key,
RefundTimeout: 1,
OnionBlob: []byte("onionblob"),
OnionBlob: lnmock.MockOnion(),
},
}

Expand Down Expand Up @@ -612,8 +614,10 @@ func TestChannelStateTransition(t *testing.T) {
LogIndex: uint64(i * 2),
HtlcIndex: uint64(i),
}
htlc.OnionBlob = make([]byte, 10)
copy(htlc.OnionBlob[:], bytes.Repeat([]byte{2}, 10))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Wondering where this testcase came from, I think onion-blobs were always 1366 in size also before this change ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well previously a OnionBlob was just a byte slice, so they could be less than 1366. They never would be, as we wouldn't be able to read them, but unit tests could get away with junk data.

TL;DR: past selves be short-cuttin :')

copy(
htlc.OnionBlob[:],
bytes.Repeat([]byte{2}, lnwire.OnionPacketSize),
)
htlcs = append(htlcs, htlc)
htlcAmt += htlc.Amt
}
Expand Down Expand Up @@ -1527,3 +1531,105 @@ func TestFinalHtlcs(t *testing.T) {
_, err = cdb.LookupFinalHtlc(chanID, unknownHtlcID)
require.ErrorIs(t, err, ErrHtlcUnknown)
}

// TestHTLCsExtraData tests serialization and deserialization of HTLCs
// combined with extra data.
func TestHTLCsExtraData(t *testing.T) {
t.Parallel()

mockHtlc := HTLC{
Signature: testSig.Serialize(),
Incoming: false,
Amt: 10,
RHash: key,
RefundTimeout: 1,
OnionBlob: lnmock.MockOnion(),
}

testCases := []struct {
name string
htlcs []HTLC
}{
{
// Serialize multiple HLTCs with no extra data to
// assert that there is no regression for HTLCs with
// no extra data.
name: "no extra data",
carlaKC marked this conversation as resolved.
Show resolved Hide resolved
htlcs: []HTLC{
mockHtlc, mockHtlc,
},
},
{
name: "mixed extra data",
yyforyongyu marked this conversation as resolved.
Show resolved Hide resolved
htlcs: []HTLC{
mockHtlc,
{
Signature: testSig.Serialize(),
Incoming: false,
Amt: 10,
RHash: key,
RefundTimeout: 1,
OnionBlob: lnmock.MockOnion(),
ExtraData: []byte{1, 2, 3},
},
mockHtlc,
{
Signature: testSig.Serialize(),
Incoming: false,
Amt: 10,
RHash: key,
RefundTimeout: 1,
OnionBlob: lnmock.MockOnion(),
ExtraData: bytes.Repeat(
[]byte{9}, 999,
),
},
},
},
}

for _, testCase := range testCases {
testCase := testCase

t.Run(testCase.name, func(t *testing.T) {
t.Parallel()

var b bytes.Buffer
err := SerializeHtlcs(&b, testCase.htlcs...)
require.NoError(t, err)

r := bytes.NewReader(b.Bytes())
htlcs, err := DeserializeHtlcs(r)
require.NoError(t, err)
require.Equal(t, testCase.htlcs, htlcs)
})
}
}

// TestOnionBlobIncorrectLength tests HTLC deserialization in the case where
// the OnionBlob saved on disk is of an unexpected length. This error case is
// only expected in the case of database corruption (or some severe protocol
// breakdown/bug). A HTLC is manually serialized because we cannot force a
// case where we write an onion blob of incorrect length.
func TestOnionBlobIncorrectLength(t *testing.T) {
Roasbeef marked this conversation as resolved.
Show resolved Hide resolved
t.Parallel()

var b bytes.Buffer

var numHtlcs uint16 = 1
require.NoError(t, WriteElement(&b, numHtlcs))

require.NoError(t, WriteElements(
&b,
// Number of HTLCs.
numHtlcs,
// Signature, incoming, amount, Rhash, Timeout.
testSig.Serialize(), false, lnwire.MilliSatoshi(10), key,
uint32(1),
// Write an onion blob that is half of our expected size.
bytes.Repeat([]byte{1}, lnwire.OnionPacketSize/2),
))

_, err := DeserializeHtlcs(&b)
require.ErrorIs(t, err, ErrOnionBlobLength)
}
3 changes: 2 additions & 1 deletion contractcourt/briefcase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"github.com/lightningnetwork/lnd/channeldb"
"github.com/lightningnetwork/lnd/input"
"github.com/lightningnetwork/lnd/kvdb"
"github.com/lightningnetwork/lnd/lnmock"
"github.com/lightningnetwork/lnd/lntest/channels"
"github.com/lightningnetwork/lnd/lnwallet"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -745,7 +746,7 @@ func TestCommitSetStorage(t *testing.T) {
activeHTLCs := []channeldb.HTLC{
{
Amt: 1000,
OnionBlob: make([]byte, 0),
OnionBlob: lnmock.MockOnion(),
Signature: make([]byte, 0),
},
}
Expand Down
2 changes: 1 addition & 1 deletion contractcourt/htlc_incoming_contest_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ func (h *htlcIncomingContestResolver) Supplement(htlc channeldb.HTLC) {
func (h *htlcIncomingContestResolver) decodePayload() (*hop.Payload,
[]byte, error) {

onionReader := bytes.NewReader(h.htlc.OnionBlob)
onionReader := bytes.NewReader(h.htlc.OnionBlob[:])
iterator, err := h.OnionProcessor.ReconstructHopIterator(
onionReader, h.htlc.RHash[:],
)
Expand Down
7 changes: 4 additions & 3 deletions contractcourt/htlc_incoming_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/lightningnetwork/lnd/htlcswitch/hop"
"github.com/lightningnetwork/lnd/invoices"
"github.com/lightningnetwork/lnd/kvdb"
"github.com/lightningnetwork/lnd/lnmock"
"github.com/lightningnetwork/lnd/lntest/mock"
"github.com/lightningnetwork/lnd/lntypes"
"github.com/lightningnetwork/lnd/lnwallet"
Expand All @@ -29,7 +30,6 @@ var (
testResPreimage = lntypes.Preimage{1, 2, 3}
testResHash = testResPreimage.Hash()
testResCircuitKey = models.CircuitKey{}
testOnionBlob = []byte{4, 5, 6}
testAcceptHeight int32 = 1234
testHtlcAmount = 2300
)
Expand Down Expand Up @@ -139,8 +139,9 @@ func TestHtlcIncomingResolverExitSettle(t *testing.T) {

ctx.waitForResult(true)

expetedOnion := lnmock.MockOnion()
if !bytes.Equal(
ctx.onionProcessor.offeredOnionBlob, testOnionBlob,
ctx.onionProcessor.offeredOnionBlob, expetedOnion[:],
) {

t.Fatal("unexpected onion blob")
Expand Down Expand Up @@ -375,7 +376,7 @@ func newIncomingResolverTestContext(t *testing.T, isExit bool) *incomingResolver
htlc: channeldb.HTLC{
Amt: lnwire.MilliSatoshi(testHtlcAmount),
RHash: testResHash,
OnionBlob: testOnionBlob,
OnionBlob: lnmock.MockOnion(),
},
},
htlcExpiry: testHtlcExpiry,
Expand Down
3 changes: 2 additions & 1 deletion contractcourt/htlc_outgoing_contest_resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/lightningnetwork/lnd/channeldb"
"github.com/lightningnetwork/lnd/input"
"github.com/lightningnetwork/lnd/kvdb"
"github.com/lightningnetwork/lnd/lnmock"
"github.com/lightningnetwork/lnd/lntest/mock"
"github.com/lightningnetwork/lnd/lntypes"
"github.com/lightningnetwork/lnd/lnwallet"
Expand Down Expand Up @@ -183,7 +184,7 @@ func newOutgoingResolverTestContext(t *testing.T) *outgoingResolverTestContext {
htlc: channeldb.HTLC{
Amt: lnwire.MilliSatoshi(testHtlcAmount),
RHash: testResHash,
OnionBlob: testOnionBlob,
OnionBlob: lnmock.MockOnion(),
},
},
}
Expand Down
Loading