-
Notifications
You must be signed in to change notification settings - Fork 411
Avoid unnecessarily cloning unsigned Transaction when broadcasting #2582
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
Avoid unnecessarily cloning unsigned Transaction when broadcasting #2582
Conversation
Our `Trusted*` wrappers in `chan_utils` expose additional inner fields by reference. However, because they were not explicitly marked as returning a reference with the wrapped struct's lifetimes, rustc was considering them to return a reference with the wrapper struct's lifetime. This is unnecessarily restrictive, and resulted in the addition of a clone in 9850c58 which we remove here.
@@ -1668,17 +1668,17 @@ impl<'a> TrustedCommitmentTransaction<'a> { | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't this apply to Txid?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its not returned by reference, its returned by copy, so there's no need to add the lifetime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I meant its equivalent to clone right ? 😅
But yeah since its already like that, nbd i guess.
Otherwise, LGTM. |
Codecov ReportPatch coverage:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2582 +/- ##
==========================================
- Coverage 90.63% 90.63% -0.01%
==========================================
Files 113 113
Lines 59054 59057 +3
Branches 59054 59057 +3
==========================================
+ Hits 53522 53524 +2
- Misses 5532 5533 +1
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@@ -1668,17 +1668,17 @@ impl<'a> TrustedCommitmentTransaction<'a> { | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I meant its equivalent to clone right ? 😅
But yeah since its already like that, nbd i guess.
Our
Trusted*
wrappers inchan_utils
expose additional innerfields by reference. However, because they were not explicitly
marked as returning a reference with the wrapped struct's
lifetimes, rustc was considering them to return a reference with
the wrapper struct's lifetime.
This is unnecessarily restrictive, and resulted in the addition of
a clone in 9850c58 which we remove
here.