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

Backoff procedure erroneously resumed between two different send events #529

Merged
merged 2 commits into from
Sep 25, 2023

Conversation

ffranr
Copy link
Contributor

@ffranr ffranr commented Sep 22, 2023

Closes #508

This PR fixes the initial state of the proof courier's backoff procedure. Before this change, the initial state of the backoff procedure would erroneously include the previous send event(s). The fix involves using the outpoint to distinguish between send events.

Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

LGTM, just two minor comments 🎉

proof/archive.go Outdated Show resolved Hide resolved
proof/archive.go Outdated Show resolved Hide resolved
tapfreighter/chain_porter.go Show resolved Hide resolved
This change ensures that the locator contains different values between
two different send events which re-use the same Taproot Asset address.
@ffranr ffranr force-pushed the backoff-multi-send-bug branch from ee11ac8 to 30b333d Compare September 25, 2023 11:40
Copy link
Contributor

@jharveyb jharveyb left a comment

Choose a reason for hiding this comment

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

Good find! 💯

@@ -74,8 +75,16 @@ func (l *Locator) Hash() [32]byte {
}
buf.Write(l.ScriptKey.SerializeCompressed())

if l.OutPoint != nil {
err := lnwire.WriteOutPoint(&buf, *l.OutPoint)
Copy link
Member

Choose a reason for hiding this comment

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

Ah, forgot about the error return type. Since we ignore any errors returned by buf.Write() above (which AFAIK can never return an actual error on a bytes.Buffer but is part of the io.Writer interface anyway), we probably can do the same here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The was merged before I saw your comment! Let me know if you want me to open another PR to go back on the err handling (I'm guessing this is fine as is?).

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, no problem, let's leave it the way it is now.

@Roasbeef Roasbeef added this pull request to the merge queue Sep 25, 2023
Merged via the queue into main with commit 16b6c6f Sep 25, 2023
@guggero guggero deleted the backoff-multi-send-bug branch September 25, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Failed to find coin for address re-use in regtest
4 participants