ChannelManager+Router++ Logger Arc --> Deref #622
Conversation
|
Oop.. I'll rebase tmrw |
valentinewallace
force-pushed
the
chanmgr-logger-arc-to-deref
branch
from
920f09b to
e7eb342
Compare
|
Rebased! |
Codecov Report
@@ Coverage Diff @@
## master #622 +/- ##
==========================================
- Coverage 91.26% 91.22% -0.05%
==========================================
Files 35 35
Lines 20787 20675 -112
==========================================
- Hits 18972 18861 -111
+ Misses 1815 1814 -1
Continue to review full report at Codecov.
|
| @@ -1144,7 +1137,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> { | |||
| /// | |||
| /// Note that it is still possible to hit these assertions in case we find a preimage on-chain | |||
| /// but then have a reorg which settles on an HTLC-failure on chain. | |||
| fn get_update_fulfill_htlc(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> { | |||
| fn get_update_fulfill_htlc<L: Deref>(&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage, logger: &L) -> Result<(Option<msgs::UpdateFulfillHTLC>, Option<ChannelMonitorUpdate>), ChannelError> where L::Target: Logger { | |||
There was a problem hiding this comment.
This (and a thousand other places) probably doesn't need to be a Deref - we pass in a reference to the thing, so might as well just dereference it at the callsite and pass in L: Logger.
There was a problem hiding this comment.
Hm, it looks like that causes this error: https://i.imgur.com/2wdqfaX.png And requiring Sized would seemingly be an issue for e.g. the fuzz crate's TestLogger, since it contains a pointer to a trait object?
There was a problem hiding this comment.
Ah, alright. Funny that rust can't figure out how to do it without the "&*", but, totally fine to leave it as-is if it doesnt work.
|
Oh, can you do the same for PeerManager's Arc as well? |
valentinewallace
force-pushed
the
chanmgr-logger-arc-to-deref
branch
3 times, most recently
from
4eca9ba to
fc897de
Compare
This caused a bunch of cascading changes, including passing loggers down to Channels in function calls rather than having each Channel have a pointer to the ChannelManager's Logger (which was a circular reference). Other structs that the Channel had passed its Logger to also had their loggers removed. Other newly unused Loggers were also removed, especially when keeping them would've caused a bunch of extra test changes to be necessary, e.g. with the ChainWatchInterfaceUtil's Logger.
valentinewallace
force-pushed
the
chanmgr-logger-arc-to-deref
branch
from
fc897de to
2f2ef85
Compare
valentinewallace
force-pushed
the
chanmgr-logger-arc-to-deref
branch
from
2f2ef85 to
222f0cb
Compare
Done! (build failing because the code coverage upload website seems to be down) |
This caused a bunch of cascading changes, including
passing loggers down to Channels in function calls
rather than having each Channel have a pointer to the
ChannelManager's Logger (which was a circular reference).
Other structs that the Channel had passed its Logger to also
had their loggers removed. Other newly unused Loggers were
also removed, especially when keeping them would've caused
a bunch of extra test changes to be necessary, e.g. with
the ChainWatchInterfaceUtil's Logger.
Sorry, as always these PRs are massive and hard to break up. But, the changes are almost all really mechanical.