Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
joostjager
left a comment
There was a problem hiding this comment.
Interesting direct implementation of Deref. Will carry this forward for async kv store.
|
Perhaps you want to do this one too #3752 (comment) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3848 +/- ##
==========================================
- Coverage 89.93% 89.91% -0.02%
==========================================
Files 163 163
Lines 131276 131278 +2
Branches 131276 131278 +2
==========================================
- Hits 118062 118043 -19
- Misses 10529 10545 +16
- Partials 2685 2690 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Did it in #3851 |
5f7a934 to
2fc13d1
Compare
|
Oops, squash-pushed a rustfmt fix. |
Pretty thorough work |
tnull
left a comment
There was a problem hiding this comment.
Ah, actually, rustfmt is unhappy here, too.
Because LDK generally relies on traits passed via `Deref`, its tempting to just wrap implementations in `Arc` even when we don't otherwise need access to the implementation later. This does result in an unnecessary allocation, which is one-time and not a huge deal but irritates my OCD substantially. Here we drop the unnecessary `Arc` indirections in favor of simply implementing `Deref` directly on the sync wrappers used in `bump_transaction`.
|
Oops, fixed. |
2fc13d1 to
9e0b7b2
Compare
Just remove some unnecessary clones