Skip to content

Expose confirmation count for pending 'channel open' transactions #9677

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Expose confirmation count for pending 'channel open' transactions #9677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6e2bdd0
chainntnfs: notify BlockHeight in Updates event
NishantBansal2003 May 19, 2025
eda86d1
channeldb: add MarkConfirmationHeight to OpenChannel
NishantBansal2003 May 19, 2025
a72e4fe
funding: persist ConfirmationHeight upon first funding confirmation
NishantBansal2003 May 19, 2025
d5a6f55
lnrpc+rpcserver: add ConfirmationsUntilActive and ConfirmationHeight
NishantBansal2003 May 19, 2025
88c2b30
itest: add test for ConfirmationsUntilActive and ConfirmationHeight
NishantBansal2003 May 19, 2025
a45ae64
docs: add release notes
NishantBansal2003 May 19, 2025
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
33 changes: 30 additions & 3 deletions chainntnfs/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,32 @@ type TxConfirmation struct {
Block *wire.MsgBlock
}

// TxUpdateInfo contains information about a transaction before it has reached
// its required number of confirmations. Transactions are registered for
// notification for a specific number of "required" confirmations, this struct
// will update the caller incrementally after each new block is found as long as
// the transaction is not yet fully regarded as confirmed.
type TxUpdateInfo struct {
// BlockHeight is the height of the block that contains the transaction.
BlockHeight uint32

// NumConfsLeft is the number of confirmations left for the transaction
// to be regarded as fully confirmed.
NumConfsLeft uint32
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I recommend adding some convenience methods to this struct which can then be used by tests:

func (t TxConfirmation) String() string {
    return fmt.Sprintf("TxConfirmation{NumConfsLeft: %d, BlockHeight: %d}",
        t.NumConfsLeft, t.BlockHeight)
}

func (t TxConfirmation) Equal(other *TxConfirmation) bool {
    return t.NumConfsLeft == other.NumConfsLeft &&
           t.BlockHeight == other.BlockHeight
}

this make sure we always compare all values rather then having to compare them separately as seen in the tests.


// String returns a string representation of TxUpdateInfo.
func (t TxUpdateInfo) String() string {
return fmt.Sprintf("TxUpdateInfo{NumConfsLeft: %d, BlockHeight: %d}",
t.NumConfsLeft, t.BlockHeight)
}

// Equal returns true if the TxUpdateInfo is equal to other TxUpdateInfo.
func (t TxUpdateInfo) Equal(other *TxUpdateInfo) bool {
return t.NumConfsLeft == other.NumConfsLeft &&
t.BlockHeight == other.BlockHeight
}

// ConfirmationEvent encapsulates a confirmation notification. With this struct,
// callers can be notified of: the instance the target txid reaches the targeted
// number of confirmations, how many confirmations are left for the target txid
Expand All @@ -229,11 +255,12 @@ type ConfirmationEvent struct {

// Updates is a channel that will sent upon, at every incremental
// confirmation, how many confirmations are left to declare the
// transaction as fully confirmed.
// transaction as fully confirmed, along with the height of the block
// that contains the transaction.
//
// NOTE: This channel must be buffered with the number of required
// confirmations.
Updates chan uint32
Updates chan TxUpdateInfo

// NegativeConf is a channel that will be sent upon if the transaction
// confirms, but is later reorged out of the chain. The integer sent
Expand Down Expand Up @@ -262,7 +289,7 @@ func NewConfirmationEvent(numConfs uint32, cancel func()) *ConfirmationEvent {
// the channel so we need to create a larger buffer to avoid
// blocking the notifier.
Confirmed: make(chan *TxConfirmation, 1),
Updates: make(chan uint32, numConfs),
Updates: make(chan TxUpdateInfo, numConfs),
NegativeConf: make(chan int32, 1),
Done: make(chan struct{}, 1),
Cancel: cancel,
Expand Down
40 changes: 28 additions & 12 deletions chainntnfs/txnotifier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -947,8 +947,12 @@ func (n *TxNotifier) dispatchConfDetails(

// We'll send a 0 value to the Updates channel,
// indicating that the transaction/output script has already
// been confirmed.
err := n.notifyNumConfsLeft(ntfn, 0)
// been confirmed, and include the block height at which the
// transaction was included.
err := n.notifyNumConfsLeft(ntfn, TxUpdateInfo{
NumConfsLeft: 0,
BlockHeight: details.BlockHeight,
})
if err != nil {
return err
}
Expand Down Expand Up @@ -977,7 +981,10 @@ func (n *TxNotifier) dispatchConfDetails(
// confirmations are left for the transaction/output script to
// be confirmed.
numConfsLeft := confHeight - n.currentHeight
err := n.notifyNumConfsLeft(ntfn, numConfsLeft)
err := n.notifyNumConfsLeft(ntfn, TxUpdateInfo{
NumConfsLeft: numConfsLeft,
BlockHeight: details.BlockHeight,
})
if err != nil {
return err
}
Expand Down Expand Up @@ -1731,7 +1738,10 @@ func (n *TxNotifier) NotifyHeight(height uint32) error {
for confRequest := range confRequests {
confSet := n.confNotifications[confRequest]
for _, ntfn := range confSet.ntfns {
txConfHeight := confSet.details.BlockHeight +
// blockHeight is the height of the block which
// contains the transaction.
blockHeight := confSet.details.BlockHeight
txConfHeight := blockHeight +
ntfn.NumConfirmations - 1
numConfsLeft := txConfHeight - height

Expand All @@ -1744,7 +1754,10 @@ func (n *TxNotifier) NotifyHeight(height uint32) error {
continue
}

err := n.notifyNumConfsLeft(ntfn, numConfsLeft)
err := n.notifyNumConfsLeft(ntfn, TxUpdateInfo{
NumConfsLeft: numConfsLeft,
BlockHeight: blockHeight,
})
if err != nil {
return err
}
Expand Down Expand Up @@ -2099,25 +2112,28 @@ func (n *TxNotifier) TearDown() {
}

// notifyNumConfsLeft sends the number of confirmations left to the
// notification subscriber through the Event.Updates channel.
// notification subscriber through the Event.Updates channel, along with the
// block height in which the transaction was included.
//
// NOTE: must be used with the TxNotifier's lock held.
func (n *TxNotifier) notifyNumConfsLeft(ntfn *ConfNtfn, num uint32) error {
func (n *TxNotifier) notifyNumConfsLeft(ntfn *ConfNtfn,
info TxUpdateInfo) error {

// If the number left is no less than the recorded value, we can skip
// sending it as it means this same value has already been sent before.
if num >= ntfn.numConfsLeft {
if info.NumConfsLeft >= ntfn.numConfsLeft {
Log.Debugf("Skipped dispatched update (numConfsLeft=%v) for "+
"request %v conf_id=%v", num, ntfn.ConfRequest,
ntfn.ConfID)
"request %v conf_id=%v", info.NumConfsLeft,
ntfn.ConfRequest, ntfn.ConfID)

return nil
}

// Update the number of confirmations left to the notification.
ntfn.numConfsLeft = num
ntfn.numConfsLeft = info.NumConfsLeft

select {
case ntfn.Event.Updates <- num:
case ntfn.Event.Updates <- info:
case <-n.quit:
return ErrTxNotifierExiting
}
Expand Down
101 changes: 59 additions & 42 deletions chainntnfs/txnotifier_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,12 +271,14 @@ func TestTxNotifierFutureConfDispatch(t *testing.T) {
// We should only receive one update for tx1 since it only requires
// one confirmation and it already met it.
select {
case numConfsLeft := <-ntfn1.Event.Updates:
const expected = 0
if numConfsLeft != expected {
case updDetails := <-ntfn1.Event.Updates:
expected := chainntnfs.TxUpdateInfo{
NumConfsLeft: 0,
BlockHeight: 11,
}
if !updDetails.Equal(&expected) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for the different signal here, but we should just update tests to use require instead if we touch it, with require.Equal(t, expected, updDetails) this becomes a one-liner and the context is clear from the line number. Do you agree @ziggie1984?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Should I also remove the convenience methods from TxUpdateInfo?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cool, if the convenience methods are not used after this change yeah we can remove it.

t.Fatalf("Received incorrect confirmation update: tx1 "+
"expected %d confirmations left, got %d",
expected, numConfsLeft)
"expected %s, got %s", expected, updDetails)
}
default:
t.Fatal("Expected confirmation update for tx1")
Expand All @@ -300,12 +302,14 @@ func TestTxNotifierFutureConfDispatch(t *testing.T) {
// We should only receive one update for tx2 since it only has one
// confirmation so far and it requires two.
select {
case numConfsLeft := <-ntfn2.Event.Updates:
const expected = 1
if numConfsLeft != expected {
case updDetails := <-ntfn2.Event.Updates:
expected := chainntnfs.TxUpdateInfo{
NumConfsLeft: 1,
BlockHeight: 11,
}
if !updDetails.Equal(&expected) {
t.Fatalf("Received incorrect confirmation update: tx2 "+
"expected %d confirmations left, got %d",
expected, numConfsLeft)
"expected %s, got %s", expected, updDetails)
}
default:
t.Fatal("Expected confirmation update for tx2")
Expand Down Expand Up @@ -341,12 +345,14 @@ func TestTxNotifierFutureConfDispatch(t *testing.T) {
// We should only receive one update since the last at the new height,
// indicating how many confirmations are still left.
select {
case numConfsLeft := <-ntfn2.Event.Updates:
const expected = 0
if numConfsLeft != expected {
case updDetails := <-ntfn2.Event.Updates:
expected := chainntnfs.TxUpdateInfo{
NumConfsLeft: 0,
BlockHeight: 11,
}
if !updDetails.Equal(&expected) {
t.Fatalf("Received incorrect confirmation update: tx2 "+
"expected %d confirmations left, got %d",
expected, numConfsLeft)
"expected %s, got %s", expected, updDetails)
}
default:
t.Fatal("Expected confirmation update for tx2")
Expand Down Expand Up @@ -411,12 +417,14 @@ func TestTxNotifierHistoricalConfDispatch(t *testing.T) {
err = n.UpdateConfDetails(ntfn1.HistoricalDispatch.ConfRequest, &txConf1)
require.NoError(t, err, "unable to update conf details")
select {
case numConfsLeft := <-ntfn1.Event.Updates:
const expected = 0
if numConfsLeft != expected {
case updDetails := <-ntfn1.Event.Updates:
expected := chainntnfs.TxUpdateInfo{
NumConfsLeft: 0,
BlockHeight: 9,
}
if !updDetails.Equal(&expected) {
t.Fatalf("Received incorrect confirmation update: tx1 "+
"expected %d confirmations left, got %d",
expected, numConfsLeft)
"expected %s, got %s", expected, updDetails)
}
default:
t.Fatal("Expected confirmation update for tx1")
Expand All @@ -443,12 +451,14 @@ func TestTxNotifierHistoricalConfDispatch(t *testing.T) {
err = n.UpdateConfDetails(ntfn2.HistoricalDispatch.ConfRequest, &txConf2)
require.NoError(t, err, "unable to update conf details")
select {
case numConfsLeft := <-ntfn2.Event.Updates:
const expected = 1
if numConfsLeft != expected {
case updDetails := <-ntfn2.Event.Updates:
expected := chainntnfs.TxUpdateInfo{
NumConfsLeft: 1,
BlockHeight: 9,
}
if !updDetails.Equal(&expected) {
t.Fatalf("Received incorrect confirmation update: tx2 "+
"expected %d confirmations left, got %d",
expected, numConfsLeft)
"expected %s, got %s", expected, updDetails)
}
default:
t.Fatal("Expected confirmation update for tx2")
Expand Down Expand Up @@ -485,12 +495,14 @@ func TestTxNotifierHistoricalConfDispatch(t *testing.T) {
// We should only receive one update for tx2 since the last one,
// indicating how many confirmations are still left.
select {
case numConfsLeft := <-ntfn2.Event.Updates:
const expected = 0
if numConfsLeft != expected {
case updDetails := <-ntfn2.Event.Updates:
expected := chainntnfs.TxUpdateInfo{
NumConfsLeft: 0,
BlockHeight: 9,
}
if !updDetails.Equal(&expected) {
t.Fatalf("Received incorrect confirmation update: tx2 "+
"expected %d confirmations left, got %d",
expected, numConfsLeft)
"expected %s, got %s", expected, updDetails)
}
default:
t.Fatal("Expected confirmation update for tx2")
Expand Down Expand Up @@ -1490,12 +1502,14 @@ func TestTxNotifierConfReorg(t *testing.T) {
// We should only receive one update for tx2 since it only requires
// one confirmation and it already met it.
select {
case numConfsLeft := <-ntfn2.Event.Updates:
const expected = 0
if numConfsLeft != expected {
case updDetails := <-ntfn2.Event.Updates:
expected := chainntnfs.TxUpdateInfo{
NumConfsLeft: 0,
BlockHeight: 12,
}
if !updDetails.Equal(&expected) {
t.Fatalf("Received incorrect confirmation update: tx2 "+
"expected %d confirmations left, got %d",
expected, numConfsLeft)
"expected %s, got %s", expected, updDetails)
}
default:
t.Fatal("Expected confirmation update for tx2")
Expand All @@ -1520,15 +1534,18 @@ func TestTxNotifierConfReorg(t *testing.T) {
// confirmations and it has already met them.
for i := uint32(1); i <= 2; i++ {
select {
case numConfsLeft := <-ntfn3.Event.Updates:
expected := tx3NumConfs - i
if numConfsLeft != expected {
t.Fatalf("Received incorrect confirmation update: tx3 "+
"expected %d confirmations left, got %d",
expected, numConfsLeft)
case updDetails := <-ntfn3.Event.Updates:
expected := chainntnfs.TxUpdateInfo{
NumConfsLeft: tx3NumConfs - i,
BlockHeight: 12,
}
if !updDetails.Equal(&expected) {
t.Fatalf("Received incorrect confirmation "+
"update: tx3 expected %s, got %s",
expected, updDetails)
}
default:
t.Fatal("Expected confirmation update for tx2")
t.Fatal("Expected confirmation update for tx3")
}
}

Expand Down
Loading
Loading