Remove unused ResendWalletTransactions notification #10584

pull ryanofsky wants to merge 1 commits into bitcoin:master from ryanofsky:pr/noresend changing 4 files +0 −27

ryanofsky commented at 10:40 PM on June 13, 2017: member

No description provided.
Remove unused ResendWalletTransactions notification b3643d9bdf
fanquake added the label Refactoring on Jun 14, 2017
fanquake added the label Wallet on Jun 14, 2017
sipa commented at 12:36 AM on June 14, 2017: member

We're not rebroadcasting unconfirmed transactions anymore? :open_mouth:
gmaxwell commented at 4:06 AM on June 14, 2017: contributor

wtf and why are tests passing?
instagibbs commented at 4:24 AM on June 14, 2017: member

There's no test for automatic rebroadcasting, AFAICT, only manual?
jonasschnelli commented at 6:17 AM on June 14, 2017: contributor

IMO we still do re-broadcast old transactions: https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L2907

This PR does only remove the connection from the main signals to the validation interface (the wallet) while the PR does not remove the signal (and the emit of the signal) itself.

Seems NACKish to me.

I think we do rebroadcast fine. But we have no tests for it (except of the intentional rebroadcast via RPC call).
laanwj commented at 6:45 AM on June 14, 2017: member
NACK.

This code is not unused.
- ResendWalletTransactions is connected to g_signals.Broadcast
- Broadcast is invoked from here: https://github.com/bitcoin/bitcoin/blob/master/src/net_processing.cpp#L2907
Better tests would be obviously welcome.
laanwj closed this on Jun 14, 2017
ryanofsky commented at 12:01 PM on June 14, 2017: member

Oops, dumb mistake. The PR just didn't delete the signal it was connecting to, which is why it compiled.
DrahtBot locked this on Sep 8, 2021

Contributors

Labels