diff options
author | fanquake <fanquake@gmail.com> | 2022-10-13 11:57:35 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-10-13 12:09:44 +0800 |
commit | aa6fb37accc6445855c81b6b236e574586df9c28 (patch) | |
tree | 6858904d4c1c832e76cc217e95372c77ba625fd8 /depends | |
parent | 7e5fe034618e4d7dfc1c203cb2e29220389fb1a6 (diff) | |
parent | b01682a812f0841170657708ef0e896b904fcd77 (diff) |
Merge bitcoin/bitcoin#26205: wallet: #25768 follow ups
b01682a812f0841170657708ef0e896b904fcd77 refactor: revert m_next_resend to not be std::atomic (stickies-v)
9245f456705b285e2d9afcc01a6155e1b3f92fad wallet: only update m_next_resend when actually resending (stickies-v)
7fbde8af5c06694eecd4ce601109bd826a54bd6f refactor: carve out tx resend timer logic into ShouldResend (stickies-v)
01f3534632d18c772901fb6ce22f6394eae96799 refactor: remove unused locks for ResubmitWalletTransactions (stickies-v)
c6e8e11fb030ef406752761530421a9e2f0f5d4f wallet: fix capitalization in docstring (stickies-v)
Pull request description:
This PR addresses the outstanding comments/issues from #25768:
- capitalization [typo](https://github.com/bitcoin/bitcoin/pull/25768#discussion_r958572522) in docstring
- remove [unused locks](https://github.com/bitcoin/bitcoin/commit/01f3534632d18c772901fb6ce22f6394eae96799) that we previously needed for `ReacceptWalletTransactions()`
- before #25768, only `ResendWalletTransactions()` would reset `m_next_resend` (formerly called `nNextResend`). By unifying it with `ReacceptWalletTransactions()` into `ResubmitWalletTransactions()`, the number of callsites that would reset the `m_next_resend` timer increased
- since `m_next_resend` is only used in case of `relay=true` (formerly `ResendWalletTransactions()`), this is unintuitive
- it leads to [unexpected behaviour](https://github.com/bitcoin/bitcoin/pull/25768#issuecomment-1252619427) such as transactions potentially never being rebroadcasted.
- it makes the ResubmitWalletTransactions()` logic [more complicated than strictly necessary](https://github.com/bitcoin/bitcoin/pull/25768#discussion_r962828563)
- since #25768, we relied on an earlier call of `ResubmitWalletTransactions(relay=false, force=true)` to initialize `m_next_resend()`, I think we can more elegantly do that by just providing `m_next_resend` with a default value
- just to highlight: this commit introduces behaviour change
Note: the `if (!fBroadcastTransactions)` in `CWallet:ShouldResend()` is duplicated on purpose, since it potentially avoids the slightly more expensive `if (!chain().isReadyToBroadcast())` check afterwards. I don't have a strong view on it, so happy to remove that additional check to reduce the diff, too.
ACKs for top commit:
aureleoules:
ACK b01682a812f0841170657708ef0e896b904fcd77
achow101:
ACK b01682a812f0841170657708ef0e896b904fcd77
Tree-SHA512: ac5f1d8858f8dd736dd1480f385984d660c1916b62a42562317020e8f9fd6a30bd8f23d973d47e4c9480d744c5ba39fdbefd69568a5eb0589a8422d7e5971c1c
Diffstat (limited to 'depends')
0 files changed, 0 insertions, 0 deletions