MOVEONLY: CWallet transaction code out of wallet.cpp/.h #21207
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
This is from a hacky, stolen script[1], but here are commands that can be used verify the change is move only. It just prints all the lines that were NOT moved (comment lines and #include lines) diff_lines=$(git diff HEAD^..HEAD | grep -v '^\(---\|+++\|@@ \)'| grep '^\([><] \)\|[+-]' | sed 's/^+/> /;s/^-/< /')
while IFS= read -r line || [ -n "$line" ]
do
contents="${line:2}"
count_removes="$(grep -cFxe "< $contents" <<< "$diff_lines" || true)"
count_adds="$(grep -cFxe "> $contents" <<< "$diff_lines" || true)"
test -z "$contents" || test "$count_removes" -eq "$count_adds" || echo "$line"
done <<< "$diff_lines"[1] script stolen from https://github.com/l0b0/diff-ignore-moved-lines/blob/master/diff-ignore-moved-lines.sh |
|
Rebased 2a855bf -> 01f4bb2 ( |
|
Requesting concept ACKs for this PR! No are no code changes, only moves, so the review should be trivial. Also, this is easy for me to rebase, so if anyone is concerned about conflicts and wants this delayed until another change is merged, that is no problem. I'm going to try to review PRs on the conflict list #21207 (comment), but it'd be good to know if any PRs in particular are a concern! |
|
I've ACK this change on the follow up PR. |
|
Concept ACK: less monolithic is more |
|
re-utACK 264b945 |
|
Rebased 264b945 -> ae93b95 ( |
|
re-utACK ae93b95 (checked that the range-diff is just changes inside the conflicting functions and that |
|
cr ACK ae93b95: dimmed zebra looks correct |
This commit just moves functions without making any changes. It can be reviewed with `git log -p -n1 --color-moved=dimmed_zebra` Motivation for this change is to make wallet.cpp/h less monolithic and start to make wallet transaction state tracking comprehensible so bugs in https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking can be fixed safely without introducing new problems. This commit moves wallet classes and methods that deal with transactions out of wallet.cpp/.h into better organized files: - transaction.cpp/.h - CWalletTx and CMerkleTx class definitions - receive.cpp/.h - functions checking received transactions and computing balances - spend.cpp/.h - functions creating transactions and finding spendable coins After bitcoin#20773, when loading is separated from syncing it will also be possible to move more wallet.cpp/.h functions to: - sync.cpp/.h - functions handling chain notifications and rescanning This commit arranges receive.cpp and spend.cpp functions in dependency order so it's possible to skim receive.cpp and get an idea of how computing balances works, and skim spend.cpp and get an idea of how transactions are created, without having to jump all over wallet.cpp where functions are not in order and there is a lot of unrelated code. Followup commit "refactor: Detach wallet transaction methods" in bitcoin#21206 follows up this PR and tweaks function names and arguments to reflect new locations. The two commits are split into separate PRs because this commit is more work to maintain and less work to review, while the other commit is less work to maintain and more work to review, so hopefully this commit can be merged earlier.
|
Rebased ae93b95 -> c7bd584 ( |
|
re-utACK c7bd584 In case we want more review / concept ACKs: cc @achow101, @instagibbs, @fjahr |
|
utACK c7bd584 I reviewed using |
Updated bitcoin/bitcoin#21207 (code moves out of wallet.cpp) for the Namecoin-specific changes made to receiving and spending name coins.
This commit just moves function without making any changes. It can be reviewed with
git log -p -n1 --color-moved=dimmed_zebraMotivation for this change is to make
wallet.cpp/hless monolithic and start to make wallet transaction state tracking comprehensible so bugs in https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking can be fixed safely without introducing new problems.This moves wallet classes and methods that deal with transactions out of
wallet.cpp/.hinto better organized files:transaction.cpp/.h- CWalletTx and CMerkleTx class definitionsreceive.cpp/.h- functions checking received transactions and computing balancesspend.cpp/.h- functions creating transactions and finding spendable coinsAfter #20773, when loading is separated from syncing it will also be possible to move more
wallet.cpp/.hfunctions to:sync.cpp/.h- functions handling chain notifications and rescanningThis commit arranges
receive.cppandspend.cppfunctions in dependency order so it's possible to skimreceive.cppand get an idea of how computing balances works, and skimspend.cppand get an idea of how transactions are created, without having to jump all overwallet.cppwhere functions are not in order and there is a lot of unrelated code.Followup commit "refactor: Detach wallet transaction methods" in #21206 follows up this PR and tweaks function names and arguments to reflect new locations. The two commits are split into separate PRs because this commit is more work to maintain and less work to review, while the other commit is less work to maintain and more work to review, so hopefully this commit can be merged earlier.