Use block time for wallet transactions found in rescan #1159

1. 
   sipa commented at 4:26 pm on April 28, 2012: member

   Instead of rescan time.
2. Use block time for wallet transactions found in rescan 30e2bfdbe9
3. 
   luke-jr commented at 5:49 pm on April 28, 2012: member

   During normal 24/7 operation, the first-seen time is still used, right?
4. 
   sipa commented at 8:59 pm on April 28, 2012: member

   the behaviour with this patch would be: transactions first seen in a “tx” message or self-generated get clock time, transactions first seen in a block or found by rescanning: block time.
5. 
   luke-jr commented at 9:22 pm on April 28, 2012: member

   It’s problematic if transactions can be “timed” older than the most recent wallet transaction, or in the future. Doing so would make “listtransactions” out of order (or worse, reordered).
6. 
   gmaxwell commented at 9:58 pm on April 28, 2012: contributor

   In general I prefer this behavior to the current one… though txn appearing ‘out of order’ in the transaction history is unfortunate. … but traditional banking sites do that all the time and people seem to survive.
7. 
   luke-jr commented at 10:55 pm on April 28, 2012: member

   I’d mind less if they actually appeared out of order, but I’m pretty sure they’re sorted by time, so they’ll reorder. :/

   I care less, if listtransactions shows all 3 times (seen, block, and “best guess”) and sorts by seen time…

8. 
   laanwj commented at 9:18 am on April 29, 2012: member

   I think it’s fine for the JSON call to show all the different times, as people may need them for different purposes.

   On the other hand, @Sipa’s change is great for the GUI.

9. 
   ghost commented at 5:26 am on May 9, 2012: none

   Why not do it this way, without changing function args list:

    0diff --git a/src/wallet.cpp b/src/wallet.cpp
    1index 9989098..defab00 100644
    2--- a/src/wallet.cpp
    3+++ b/src/wallet.cpp
    4@@ -322,7 +322,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn)
    5         CWalletTx& wtx = (*ret.first).second;
    6         wtx.BindWallet(this);
    7         bool fInsertedNew = ret.second;
    8-        if (fInsertedNew)
    9+        if (fInsertedNew && wtx.nTimeReceived == 0)
   10             wtx.nTimeReceived = GetAdjustedTime();
   11
   12         bool fUpdated = false;
   13@@ -397,6 +397,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl
   14         if (fExisted || IsMine(tx) || IsFromMe(tx))
   15         {
   16             CWalletTx wtx(this,tx);
   17+            wtx.nTimeReceived = pblock->GetBlockTime();
   18             // Get merkle branch if transaction was found in a block
   19             if (pblock)
   20                 wtx.SetMerkleBranch(pblock);
   

10. 
    gmaxwell commented at 2:43 am on May 18, 2012: contributor

    Can we get some more discussion here? Agreement on this would be nice.
11. 
    luke-jr commented at 2:50 am on May 18, 2012: member

    We discussed this a bit back in December. I think having all 3 times (received, block, and “smart”) in JSON-RPC and just the smart times in Bitcoin-Qt is the best solution.

    0"time": smart timestamp,
    1"receivetime": timestamp,
    2"blocktime": timestamp,
    

12. 
    gmaxwell commented at 4:29 am on May 18, 2012: contributor

    Thats sounds good to me— perhaps a later commit can make a tooltip show the three times in the gui? I assume that recievetime would be a sent time for txn you sent (being that you received them the same instant)?
13. 
    luke-jr commented at 5:15 am on May 18, 2012: member

    Yes, the smart time would also logically be the sent time as well.
14. 
    laanwj commented at 6:30 am on May 18, 2012: member

    Right, we can put all the times in the “transaction details” window.
15. 
    jgarzik commented at 3:41 pm on August 1, 2012: contributor

    I’m officially -ENOCARE. Code appears correct to my minimal scan. @gavinandresen ? @gmaxwell based on your comments, it sounds like you ACK this code?
16. 
    gavinandresen commented at 3:53 pm on August 1, 2012: contributor

    ACK. Looks obviously better than the current behavior.
17. 
    luke-jr commented at 3:55 pm on August 1, 2012: member

    This is NACK since it breaks listtransactions. #1393
18. 
    sipa commented at 3:57 pm on August 1, 2012: member

    I’m very unsure myself whether I want this merged. Luke’s version is more complex but probably behaves as expected in more use cases.

    I’m not sure either should be merged without some testing…

19. gmaxwell closed this on Aug 23, 2012

    ---

20. 
    gmaxwell commented at 8:14 pm on August 23, 2012: contributor

    (Obsoleted by Luke’s version.)
21. lateminer referenced this in commit fcdf663b23 on Jan 22, 2019
22. lateminer referenced this in commit 1ae53d0a2a on Dec 25, 2019
23. dexX7 referenced this in commit f473a31a7d on Aug 20, 2020
24. DrahtBot locked this on Sep 8, 2021

Contributors
sipa luke-jr gmaxwell laanwj ghost jgarzik gavinandresen

Milestone
0.7.0