wallet: Remove `CWalletTx::fTimeReceivedIsTxTime` #32768

CWalletTx::fTimeReceivedIsTxTime is no longer used and can be removed. This additionally allows the removal of the upgraded_txs loop in LoadWallet.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32768.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #32763 (wallet: Replace CWalletTx::mapValue and vOrderForm with explicit class members by achow101)
• #32685 (wallet: Allow read-only database access for info and dump commands by PeterWrighten)
• #27865 (wallet: Track no-longer-spendable TXOs separately by achow101)
• #27286 (wallet: Keep track of the wallet's own transaction outputs in memory by achow101)

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.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

lgtm ACK 9eb2c82e7c911a066781d67e6846cf6bbbaba6e9

ACK 9eb2c82e7c911a066781d67e6846cf6bbbaba6e9

ACK https://github.com/bitcoin/bitcoin/pull/32768/commits/9eb2c82e7c911a066781d67e6846cf6bbbaba6e9

fTimeReceivedIsTxTime has had no functional impact since it was removed in https://github.com/bitcoin/bitcoin/commit/471426fb3b2c2fa37640c03819c4f7be69ba8301#diff-286fedc8ff8f47bca92c7024d2c75272aa5bcfd733379eac1555e6036528d6b1

ACK https://github.com/bitcoin/bitcoin/commit/9eb2c82e7c911a066781d67e6846cf6bbbaba6e9

The fTimeReceivedIsTxTime once indicated whether a transactions "time" was the time the wallet heard about the transaction, or the time that the transaction was included in a block.

It was set to true for transactions created by the wallet, where presumably we care more about what time we made the transaction then what time we heard about it in a block, but for other transaction's the time that we "receive" the transaction may be much later (e.g. IBD) than the time it was included in a block, so we don't want the transaction's "time" to be nTimeReceived .:

https://github.com/bitcoin/bitcoin/blob/4405b78d6059e536c36974088a8ed4d9f0f29898/main.cpp#L263-L279 [^1]

This field was mistakenly an unsigned int rather than a char/bool:

https://github.com/bitcoin/bitcoin/blob/4405b78d6059e536c36974088a8ed4d9f0f29898/main.h#L678

A later commit (e4ff4e6898d378b1a3e83791034a7af455fde3ab), that AFAICT predates the practice of using dummy types for deprecated fields in serialization, attempted to "fix" this inefficiency of using 4 bytes to represent a bool. Since the commit was adding an nVersion field to the serialized transaction data anyways, and nVersion is 4 bytes, it reused the fTimeReceivedIsTxTime field as the nVersion field, and changed fTimeReceivedIsTxTime to a bool and moved it to the end of tx serialization:

diff --git a/main.h b/main.h
index 7d1145b5ff..dda5f730c7 100644

-        READWRITE(fTimeReceivedIsTxTime);
+        READWRITE(nVersion);
+        if (fRead && nVersion < 100)
+            const_cast<CWalletTx*>(this)->fTimeReceivedIsTxTime = nVersion;
         READWRITE(nTimeReceived);
         READWRITE(fFromMe);
         READWRITE(fSpent);
+        if (nVersion >= 31404)
+        {
+            READWRITE(fTimeReceivedIsTxTime);
+            READWRITE(fUnused);
+            READWRITE(strFromAccount);
+        }
     )

This broke transaction times for wallets that were upgraded and then loaded into an older bitcoin client, since these wallets would have giant version numbers like 31404 in the fTimeReceivedIsTxTime for every transaction, and fTimeReceivedIsTxTime would always be true.

A later commit (https://github.com/bitcoin/bitcoin/commit/865c3a23832e36d50cb873d38c976032b027b5d3) reverted the above change to fix this upgrade-then-downgrade bug, but it had to include a special case for fixing transactions that had very large fTimeReceivedIsTxTime that matched the version numbers that were affected by this bug

https://github.com/bitcoin/bitcoin/blob/482d2553764ef1fa3f9c06389b2e13a735d86eda/src/wallet/walletdb.cpp#L1033-L1053

And Bitcoin Core retains this special-case handling today, even though, for a long time, even before the introduction of smart time in #1393, GetTxTime() has not used fTimeReceivedIsTxTime (471426fb3b2c2fa37640c0381):

https://github.com/bitcoin/bitcoin/blob/471426fb3b2c2fa37640c03819c4f7be69ba8301/src/wallet.cpp#L354-L358

[^1]: This snippet is from the initial git commit: 4405b78d6059e536c36

ACK https://github.com/bitcoin/bitcoin/pull/32768/commits/9eb2c82e7c911a066781d67e6846cf6bbbaba6e9

ACK https://github.com/bitcoin/bitcoin/commit/9eb2c82e7c911a066781d67e6846cf6bbbaba6e9

Code Reviewed https://github.com/bitcoin/bitcoin/commit/9eb2c82e7c911a066781d67e6846cf6bbbaba6e9