[wallet] Refactor relay transactions#15728
Conversation
|
Base PR has been merged. This is ready for review. |
promag
left a comment
There was a problem hiding this comment.
utACK afb608b, since the refactor improves code comments and avoids nesting code.
src/wallet/wallet.cpp
Outdated
There was a problem hiding this comment.
This comment is replaced by the Don't relay conflicted or already confirmed transactions comment above the call to GetDepthInMainChain().
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
ryanofsky
left a comment
There was a problem hiding this comment.
utACK afb608b1e92773aba2698b698bcf62011ff7b783. This isn't a pure refactoring so you may want to point out changes in behavior in the commit description (no longer asserting false if GetBroadcastTransactions if false, no longer printing relay message if p2pEnabled is false).
afb608b to
57516eb
Compare
ryanofsky
left a comment
There was a problem hiding this comment.
utACK 57516ebdfcd288cb6c255fb0fbc94b7fb1e50af7 (just last commits, not base commits). Only change since last review is updating commit description.
This refactors the CWalletTx::RelayWalletTransaction() function to be clearer and adds comments. It also makes two minor behavior changes: - no longer assert if fBroadcastTransactions is false. Just return false from the function. - no longer print the relay message if p2pEnabled is set to false (since the transaction is not actually relayed).
57516eb to
7a9046e
Compare
|
rebased |
|
utACK 7a9046e. |
|
utACK 7a9046e |
Summary: This refactors the CWalletTx::RelayWalletTransaction() function to be clearer and adds comments. It also makes two minor behavior changes: - no longer assert if fBroadcastTransactions is false. Just return false from the function. - no longer print the relay message if p2pEnabled is set to false (since the transaction is not actually relayed). --- Backport of Core [[bitcoin/bitcoin#15728 | PR15728]] Test Plan: ninja check-all Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6406
Refactor
CWalletTx::RelayWalletTransaction()function.This was a suggestion from the wallet-node separation PR: #15288 (comment), which we deferred until after the main PR was merged.
There are also makes two minor behavior changes: