Regression test for ResendWalletTransactions #5940

1. 
   gavinandresen commented at 7:01 pm on March 23, 2015: contributor

   Adds a regression test for the wallet’s ResendWalletTransactions function, which uses a new, hidden RPC command “resendwallettransactions.”

   I refactored main’s Broadcast signal so it is passed the best-block time, which let me remove a global variable shared between main.cpp and the wallet (nTimeBestReceived).

   I also manually tested the “rebroadcast unconfirmed every half hour or so” functionionality by:

   1. Running bitcoind -connect=0.0.0.0:8333
   2. Creating a couple of send-to-self transactions
   3. Connect to a peer using -addnode
   4. Waited a while, monitoring debug.log, until I see: 2015-03-23 18:48:10 ResendWalletTransactions: rebroadcast 2 unconfirmed transactions

   One last change with this pull request: don’t bother putting ResendWalletTransactions messages in debug.log unless unconfirmed transactions were actually rebroadcast.

2. 
   jonasschnelli commented at 7:13 pm on March 23, 2015: contributor

   utACK.

   Two points:

   Would it make sense to somehow remove this command when running in a non test/dev. mode? I don’t have a good idea here (detect –enable-debug, only during --regtest & --testnet seems not to be the best approach).

   How about returning a array of rebroadcasted wtx ids instead of a single count int?

3. 
   gavinandresen commented at 7:20 pm on March 23, 2015: contributor

   @jonasschnelli : consensus on IRC was just making it hidden is the right thing to do.

   RE: returning array of txid: good idea, I’ll do that.

   And I’ll fix the -DDEBUG_LOCKORDER travis error (RPC method needs to lock cs_main).

4. gavinandresen force-pushed on Mar 23, 2015
5. 
   gavinandresen commented at 7:37 pm on March 23, 2015: contributor

   Updated.

   Note: I decided on std::vector<uint256> CWallet::ResendWalletTransactionsBefore(int64_t nTime) instead of passing in a reference to a vector that is filled in because the vector will be small (if you have lots of unconfirmed transactions in your wallet you are doing something wrong).

6. 
   jonasschnelli commented at 7:52 pm on March 23, 2015: contributor

   Tested ACK.
7. 
   luke-jr commented at 10:14 pm on March 23, 2015: member

   This test appears that it would fail to catch an error, if mempool syncing is ever merged. It would be better if the nodes were all restarted concurrently to clear their mempools, but failing that perhaps just a comment on this limitation in the test code would be good to have.
8. 
   laanwj commented at 7:41 am on March 24, 2015: member

   utACK

   This test appears that it would fail to catch an error, if mempool syncing is ever merged.

   We’ll worry about that then, and if. Could add a comment, I guess.

9. laanwj added the label Tests on Mar 24, 2015
10. laanwj added the label RPC on Mar 24, 2015
11. laanwj added the label Wallet on Mar 24, 2015
12. 
    gavinandresen commented at 1:30 pm on March 24, 2015: contributor

    @luke-jr : if mempool syncing between peers is implemented, then ResendWalletTransactions will probably go away entirely, so this unit test will be obsolete.

    I suppose I could add a comment, but I think it is usually a mistake to clutter up code with speculative comments about code that may or may not get written in the future.

13. Regression test for ResendWalletTransactions

    Adds a regression test for the wallet's ResendWalletTransactions function, which uses a new, hidden RPC command "resendwallettransactions."
    
    I refactored main's Broadcast signal so it is passed the best-block time, which let me remove a global variable shared between main.cpp and the wallet (nTimeBestReceived).
    
    I also manually tested the "rebroadcast unconfirmed every half hour or so" functionality by:
    
    1. Running bitcoind -connect=0.0.0.0:8333
    2. Creating a couple of send-to-self transactions
    3. Connect to a peer using -addnode
    4. Waited a while, monitoring debug.log, until I see:
    ```2015-03-23 18:48:10 ResendWalletTransactions: rebroadcast 2 unconfirmed transactions```
    
    One last change: don't bother putting ResendWalletTransactions messages in debug.log unless unconfirmed transactions were actually rebroadcast.

    0f5954c434
14. gavinandresen force-pushed on Mar 24, 2015
15. laanwj merged this on Mar 30, 2015
16. laanwj closed this on Mar 30, 2015

    ---

17. laanwj referenced this in commit 446bb70fcd on Mar 30, 2015
18. gavinandresen deleted the branch on Apr 24, 2015
19. MarcoFalke locked this on Sep 8, 2021

Contributors
gavinandresen jonasschnelli luke-jr laanwj

Labels
Wallet Tests RPC/REST/ZMQ