multi: remove function BalancesAtHeight

This commit deletes the function `BalancesAtHeight` since its only
usague is to find the push amount, which can be achieved by saving the
initial balances.
Another reason to remove it is to pave the way to incooperate our new
revocation log. If we ever need this function again, we can add it back
by visiting all the revocation logs to calculate the balances at a given
height.

channeldb/channel.go

@@ -183,11 +183,6 @@ 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")
-
-	// errHeightNotFound is returned when a query for channel balances at
-	// a height that we have not reached yet is made.
-	errHeightNotReached = fmt.Errorf("height requested greater than " +
-		"current commit height")
 )
 
 const (
@@ -1653,44 +1648,6 @@ func (c *OpenChannel) UpdateCommitment(newCommitment *ChannelCommitment,
 	return nil
 }
 
-// BalancesAtHeight returns the local and remote balances on our commitment
-// transactions as of a given height.
-//
-// NOTE: these are our balances *after* subtracting the commitment fee and
-// anchor outputs.
-func (c *OpenChannel) BalancesAtHeight(height uint64) (lnwire.MilliSatoshi,
-	lnwire.MilliSatoshi, error) {
-
-	if height > c.LocalCommitment.CommitHeight &&
-		height > c.RemoteCommitment.CommitHeight {
-
-		return 0, 0, errHeightNotReached
-	}
-
-	// If our current commit is as the desired height, we can return our
-	// current balances.
-	if c.LocalCommitment.CommitHeight == height {
-		return c.LocalCommitment.LocalBalance,
-			c.LocalCommitment.RemoteBalance, nil
-	}
-
-	// If our current remote commit is at the desired height, we can return
-	// the current balances.
-	if c.RemoteCommitment.CommitHeight == height {
-		return c.RemoteCommitment.LocalBalance,
-			c.RemoteCommitment.RemoteBalance, nil
-	}
-
-	// If we are not currently on the height requested, we need to look up
-	// the previous height to obtain our balances at the given height.
-	commit, err := c.FindPreviousState(height)
-	if err != nil {
-		return 0, 0, err
-	}
-
-	return commit.LocalBalance, commit.RemoteBalance, nil
-}
-
 // ActiveHtlcs returns a slice of HTLC's which are currently active on *both*
 // commitment transactions.
 func (c *OpenChannel) ActiveHtlcs() []HTLC {

channeldb/channel_test.go

@@ -78,25 +78,6 @@ type testChannelParams struct {
 // default channel that is creates for testing.
 type testChannelOption func(params *testChannelParams)
 
-// channelCommitmentOption is an option which allows overwriting of the default
-// commitment height and balances. The local boolean can be used to set these
-// balances on the local or remote commit.
-func channelCommitmentOption(height uint64, localBalance,
-	remoteBalance lnwire.MilliSatoshi, local bool) testChannelOption {
-
-	return func(params *testChannelParams) {
-		if local {
-			params.channel.LocalCommitment.CommitHeight = height
-			params.channel.LocalCommitment.LocalBalance = localBalance
-			params.channel.LocalCommitment.RemoteBalance = remoteBalance
-		} else {
-			params.channel.RemoteCommitment.CommitHeight = height
-			params.channel.RemoteCommitment.LocalBalance = localBalance
-			params.channel.RemoteCommitment.RemoteBalance = remoteBalance
-		}
-	}
-}
-
 // pendingHeightOption is an option which can be used to set the height the
 // channel is marked as pending at.
 func pendingHeightOption(height uint32) testChannelOption {
@@ -1415,174 +1396,6 @@ func TestCloseChannelStatus(t *testing.T) {
 	}
 }
 
-// TestBalanceAtHeight tests lookup of our local and remote balance at a given
-// height.
-func TestBalanceAtHeight(t *testing.T) {
-	const (
-		// Values that will be set on our current local commit in
-		// memory.
-		localHeight        = 2
-		localLocalBalance  = 1000
-		localRemoteBalance = 1500
-
-		// Values that will be set on our current remote commit in
-		// memory.
-		remoteHeight        = 3
-		remoteLocalBalance  = 2000
-		remoteRemoteBalance = 2500
-
-		// Values that will be written to disk in the revocation log.
-		oldHeight        = 0
-		oldLocalBalance  = 200
-		oldRemoteBalance = 300
-
-		// Heights to test error cases.
-		unknownHeight   = 1
-		unreachedHeight = 4
-	)
-
-	// putRevokedState is a helper function used to put commitments is
-	// the revocation log bucket to test lookup of balances at heights that
-	// are not our current height.
-	putRevokedState := func(c *OpenChannel, height uint64, local,
-		remote lnwire.MilliSatoshi) error {
-
-		err := kvdb.Update(c.Db.backend, func(tx kvdb.RwTx) error {
-			chanBucket, err := fetchChanBucketRw(
-				tx, c.IdentityPub, &c.FundingOutpoint,
-				c.ChainHash,
-			)
-			if err != nil {
-				return err
-			}
-
-			logKey := revocationLogBucket
-			logBucket, err := chanBucket.CreateBucketIfNotExists(
-				logKey,
-			)
-			if err != nil {
-				return err
-			}
-
-			// Make a copy of our current commitment so we do not
-			// need to re-fill all the required fields and copy in
-			// our new desired values.
-			commit := c.LocalCommitment
-			commit.CommitHeight = height
-			commit.LocalBalance = local
-			commit.RemoteBalance = remote
-
-			return appendChannelLogEntry(logBucket, &commit)
-		}, func() {})
-
-		return err
-	}
-
-	tests := []struct {
-		name                  string
-		targetHeight          uint64
-		expectedLocalBalance  lnwire.MilliSatoshi
-		expectedRemoteBalance lnwire.MilliSatoshi
-		expectedError         error
-	}{
-		{
-			name:                  "target is current local height",
-			targetHeight:          localHeight,
-			expectedLocalBalance:  localLocalBalance,
-			expectedRemoteBalance: localRemoteBalance,
-			expectedError:         nil,
-		},
-		{
-			name:                  "target is current remote height",
-			targetHeight:          remoteHeight,
-			expectedLocalBalance:  remoteLocalBalance,
-			expectedRemoteBalance: remoteRemoteBalance,
-			expectedError:         nil,
-		},
-		{
-			name:                  "need to lookup commit",
-			targetHeight:          oldHeight,
-			expectedLocalBalance:  oldLocalBalance,
-			expectedRemoteBalance: oldRemoteBalance,
-			expectedError:         nil,
-		},
-		{
-			name:                  "height not found",
-			targetHeight:          unknownHeight,
-			expectedLocalBalance:  0,
-			expectedRemoteBalance: 0,
-			expectedError:         ErrLogEntryNotFound,
-		},
-		{
-			name:                  "height not reached",
-			targetHeight:          unreachedHeight,
-			expectedLocalBalance:  0,
-			expectedRemoteBalance: 0,
-			expectedError:         errHeightNotReached,
-		},
-	}
-
-	for _, test := range tests {
-		test := test
-
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-
-			fullDB, cleanUp, err := MakeTestDB()
-			if err != nil {
-				t.Fatalf("unable to make test database: %v",
-					err)
-			}
-			defer cleanUp()
-
-			cdb := fullDB.ChannelStateDB()
-
-			// Create options to set the heights and balances of
-			// our local and remote commitments.
-			localCommitOpt := channelCommitmentOption(
-				localHeight, localLocalBalance,
-				localRemoteBalance, true,
-			)
-
-			remoteCommitOpt := channelCommitmentOption(
-				remoteHeight, remoteLocalBalance,
-				remoteRemoteBalance, false,
-			)
-
-			// Create an open channel.
-			channel := createTestChannel(
-				t, cdb, openChannelOption(),
-				localCommitOpt, remoteCommitOpt,
-			)
-
-			// Write an older commit to disk.
-			err = putRevokedState(channel, oldHeight,
-				oldLocalBalance, oldRemoteBalance)
-			if err != nil {
-				t.Fatalf("unexpected error: %v", err)
-			}
-
-			local, remote, err := channel.BalancesAtHeight(
-				test.targetHeight,
-			)
-			if err != test.expectedError {
-				t.Fatalf("expected: %v, got: %v",
-					test.expectedError, err)
-			}
-
-			if local != test.expectedLocalBalance {
-				t.Fatalf("expected local: %v, got: %v",
-					test.expectedLocalBalance, local)
-			}
-
-			if remote != test.expectedRemoteBalance {
-				t.Fatalf("expected remote: %v, got: %v",
-					test.expectedRemoteBalance, remote)
-			}
-		})
-	}
-}
-
 // TestHasChanStatus asserts the behavior of HasChanStatus by checking the
 // behavior of various status flags in addition to the special case of
 // ChanStatusDefault which is treated like a flag in the code base even though

lntest/itest/lnd_misc_test.go

@@ -340,8 +340,8 @@ func testSphinxReplayPersistence(net *lntest.NetworkHarness, t *harnessTest) {
 
 // testListChannels checks that the response from ListChannels is correct. It
 // tests the values in all ChannelConstraints are returned as expected. Once
-// ListChannels becomes mature, a test against all fields in ListChannels should
+// ListChannels becomes mature, a test against all fields in ListChannels
-// be performed.
+// should be performed.
 func testListChannels(net *lntest.NetworkHarness, t *harnessTest) {
 	ctxb := context.Background()
 
@@ -369,8 +369,8 @@ func testListChannels(net *lntest.NetworkHarness, t *harnessTest) {
 	net.SendCoins(t.t, btcutil.SatoshiPerBitcoin, alice)
 
 	// Open a channel with 100k satoshis between Alice and Bob with Alice
-	// being the sole funder of the channel. The minial HTLC amount is set to
+	// being the sole funder of the channel. The minial HTLC amount is set
-	// 4200 msats.
+	// to 4200 msats.
 	const customizedMinHtlc = 4200
 
 	chanAmt := btcutil.Amount(100000)
@@ -414,11 +414,19 @@ func testListChannels(net *lntest.NetworkHarness, t *harnessTest) {
 	// Check the returned response is correct.
 	aliceChannel := resp.Channels[0]
 
+	// Since Alice is the initiator, she pays the commit fee.
+	aliceBalance := int64(chanAmt) - aliceChannel.CommitFee
+
+	// Check the balance related fields are correct.
+	require.Equal(t.t, aliceBalance, aliceChannel.LocalBalance)
+	require.Zero(t.t, aliceChannel.RemoteBalance)
+	require.Zero(t.t, aliceChannel.PushAmountSat)
+
 	// Calculate the dust limit we'll use for the test.
 	dustLimit := lnwallet.DustLimitForSize(input.UnknownWitnessSize)
 
-	// defaultConstraints is a ChannelConstraints with default values. It is
+	// defaultConstraints is a ChannelConstraints with default values. It
-	// used to test against Alice's local channel constraints.
+	// is used to test against Alice's local channel constraints.
 	defaultConstraints := &lnrpc.ChannelConstraints{
 		CsvDelay:          4,
 		ChanReserveSat:    1000,
@@ -464,9 +472,14 @@ func testListChannels(net *lntest.NetworkHarness, t *harnessTest) {
 		)
 	}
 
-	// Check channel constraints match. Alice's local channel constraint should
+	// Check the balance related fields are correct.
-	// be equal to Bob's remote channel constraint, and her remote one should
+	require.Equal(t.t, aliceBalance, bobChannel.RemoteBalance)
-	// be equal to Bob's local one.
+	require.Zero(t.t, bobChannel.LocalBalance)
+	require.Zero(t.t, bobChannel.PushAmountSat)
+
+	// Check channel constraints match. Alice's local channel constraint
+	// should be equal to Bob's remote channel constraint, and her remote
+	// one should be equal to Bob's local one.
 	assertChannelConstraintsEqual(
 		t, aliceChannel.LocalConstraints, bobChannel.RemoteConstraints,
 	)

rpcserver.go

@@ -4080,21 +4080,13 @@ func createRPCOpenChannel(r *rpcServer, dbChannel *channeldb.OpenChannel,
 		channel.UnsettledBalance += channel.PendingHtlcs[i].Amount
 	}
 
-	// Lookup our balances at height 0, because they will reflect any
-	// push amounts that may have been present when this channel was
-	// created.
-	localBalance, remoteBalance, err := dbChannel.BalancesAtHeight(0)
-	if err != nil {
-		return nil, err
-	}
-
 	// If we initiated opening the channel, the zero height remote balance
 	// is the push amount. Otherwise, our starting balance is the push
 	// amount. If there is no push amount, these values will simply be zero.
 	if dbChannel.IsInitiator {
-		channel.PushAmountSat = uint64(remoteBalance.ToSatoshis())
+		channel.PushAmountSat = uint64(dbChannel.InitialRemoteBalance)
 	} else {
-		channel.PushAmountSat = uint64(localBalance.ToSatoshis())
+		channel.PushAmountSat = uint64(dbChannel.InitialLocalBalance)
 	}
 
 	if len(dbChannel.LocalShutdownScript) > 0 {