Refactor transaction/accounting time #1393

1. 
   luke-jr commented at 6:55 pm on May 28, 2012: member

   Status: Tested and seems to work

   1. Guarantee listtransactions order consistency by storing a position for each entry
   2. Add “blocktime” and (for wallet transactions) “timereceived” to transaction Objects
   3. Implement “smart” times according to the following logic:
      • If sending a transaction, assign its timestamp to the current time.
      • If receiving a transaction outside a block, assign its timestamp to the current time.
      • If receiving a block with a future timestamp, assign all its (not already known) transactions’ timestamps to the current time.
      • If receiving a block with a past timestamp, before the most recent known transaction (that we care about), assign all its (not already known) transactions’ timestamps to the same timestamp as that most-recent-known transaction.
      • If receiving a block with a past timestamp, but after the most recent known transaction, assign all its (not already known) transactions’ timestamps to the block time.

   This supercedes #1159. Discussion: https://bitcointalk.org/?topic=54527

2. 
   gmaxwell commented at 1:58 pm on May 29, 2012: contributor

   In spite of the non-determinism of the ‘smart time’ I like these criteria a lot.
3. 
   luke-jr commented at 4:18 pm on June 19, 2012: member

   So got around to testing… all my transactions are showing a smart time of 1 (01/01/1970 00:00). Guess I have some fixing to do.
4. 
   luke-jr commented at 9:06 pm on June 21, 2012: member

   Reminder: A||B is boolean in C++, not the first value that casts to true. :(

   Fixed.

5. 
   luke-jr commented at 1:33 am on June 30, 2012: member

   Fixed a bug and tested some more.

   Two open issues:

   • Would be nice if sorting transactions by time in Bitcoin-Qt respected the fixed order; I think Qt does it right now, though :/
   • Probably during -rescan and initial blockchain catchup, the block time should be trusted even if the user has transactions dated after them? Opinions on this case (easy to test by sending a transaction before catching up)
6. 
   laanwj commented at 9:43 am on July 17, 2012: member

   ACK

   Re:Qt, sorting transactions by time sorts the transactions by time, nothing else. Anything else would be confusing IMO. The only thing that can be sensibly changed is the ‘default’ order, when not explicitly sorting by anything. In the transaction details we can show all the different times.

7. 
   luke-jr commented at 3:29 pm on July 17, 2012: member

   @laanwj Except for extreme differences in the local clock, the assigned order from this patch is always chronological; but right now, sorting by time in Bitcoin-Qt has no logic behind the order of transactions with the same time.
8. 
   BitcoinPullTester commented at 5:36 am on August 10, 2012: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f45fa25bb9efb6f83409e71b972f07cce0eb4520 for binaries and test log.
9. 
   gmaxwell commented at 12:28 pm on August 23, 2012: contributor

   Where does this stand?
10. 
    luke-jr commented at 5:43 pm on August 23, 2012: member

    Been running fine for me since next-test 2012-07
11. Store a fixed order of transactions (and accounting) in the wallet

    For backward compatibility, new accounting data is stored after a \0 in the comment string.
    This way, old versions and third-party software should load and store them, but all actual use (listtransactions, for example) ignores it.

    9c7722b7c5
12. JSON-RPC: Add "blocktime" and (for wallet transactions) "timereceived" to transaction Object outputs bdbfd2329a
13. Choose reasonable "smart" times to display for transactions

    Logic:
    - If sending a transaction, assign its timestamp to the current time.
    - If receiving a transaction outside a block, assign its timestamp to the current time.
    - If receiving a block with a future timestamp, assign all its (not already known) transactions' timestamps to the current time.
    - If receiving a block with a past timestamp, before the most recent known transaction (that we care about), assign all its (not already known) transactions' timestamps to the same timestamp as that most-recent-known transaction.
    - If receiving a block with a past timestamp, but after the most recent known transaction, assign all its (not already known) transactions' timestamps to the block time.

    c3f95ef13f
14. 
    gmaxwell commented at 7:38 pm on August 23, 2012: contributor

    ACK, works for me, apparently works for other people.
15. gmaxwell referenced this in commit 47753fa369 on Aug 23, 2012
16. gmaxwell merged this on Aug 23, 2012
17. gmaxwell closed this on Aug 23, 2012

    ---

18. 
    gavinandresen commented at 6:26 pm on September 8, 2012: contributor

    Hindsight is always 20/20…. … but this pull didn’t get sufficient testing/code review. I think a test plan should have been written and followed to try to catch the bugs earlier.
19. 
    luke-jr commented at 8:39 pm on September 8, 2012: member

    I agree on lack of testing. Unfortunately, the tests it included didn’t cover enough (and I haven’t thought of any way to actually test the problems found either).
20. suprnurd referenced this in commit 6aaec3ae2a on Dec 5, 2017
21. lateminer referenced this in commit 816c2e6f97 on Jan 22, 2019
22. lateminer referenced this in commit 541a688bee on May 6, 2020
23. DrahtBot locked this on Sep 8, 2021

Contributors
luke-jr gmaxwell laanwj BitcoinPullTester gavinandresen

Milestone
0.7.0