Remove resendwallettransactions RPC method #15680

pull jnewbery wants to merge 2 commits into bitcoin:master from jnewbery:2019_03_remove_resendwallettransactions changing 4 files +22 −100
  1. jnewbery commented at 9:28 PM on March 27, 2019: member

    Remove resendwallettransactions RPC method

    This RPC was added for testing wallet rebroadcasts. Since we now have a real test for wallet rebroadcasts, it's no longer needed.

    The call in wallet_basic.py can be removed because wallet_resendwallettransactions.py tests wallet rebroadcast.

  2. in src/wallet/wallet.cpp:2134 in f81c814bc5 outdated
    2136 | +
    2137 | +    { // cs_wallet scope
    2138 | +    LOCK(cs_wallet);
    2139 | +
    2140 | +    // Relay transactions
    2141 | +    for (std::pair<const uint256, CWalletTx>& item : mapWallet)
    


    MarcoFalke commented at 9:38 PM on March 27, 2019:
        for (auto& item : mapWallet)
    

    Style-nit: Since this isn't move only of the above method anyway, you are welcome to do style fixups (and clang-format, etc.)


    jnewbery commented at 4:16 PM on March 28, 2019:

    ok, fixed this and some other style nits.

  3. MarcoFalke commented at 9:39 PM on March 27, 2019: member

    Concept ACK

    I wasn't aware that this was a hidden RPC. Good to finally remove this privacy-footgun

  4. promag commented at 10:29 PM on March 27, 2019: member

    Should deprecate first? Or that doesn't apply to hidden RPC?

  5. DrahtBot commented at 10:43 PM on March 27, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #15632 (Remove ResendWalletTransactions from the Validation Interface by jnewbery)
    • #13756 (wallet: "avoid_reuse" wallet flag for improved privacy by kallewoof)
    • #11413 ([wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option by kallewoof)

    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.

  6. fanquake added the label Wallet on Mar 27, 2019
  7. fanquake added the label RPC/REST/ZMQ on Mar 27, 2019
  8. jnewbery renamed this:
    Remove resendwallettransactions RPC method
    [WIP] Remove resendwallettransactions RPC method
    on Mar 28, 2019
  9. jnewbery force-pushed on Mar 28, 2019
  10. jnewbery commented at 4:14 PM on March 28, 2019: member

    Should deprecate first? Or that doesn't apply to hidden RPC?

    This RPC isn't part of the public API so I don't think it needs deprecation. We could raise a JSON RPC exception telling users that transactions are rebroadcast automatically and that they just need to wait.

    The feature_pruning.py test was failing with this PR. I got bored of debugging a failing test case that took 30 minutes to run, so I resurrected #10591 here: #15686. That commit removes the resendwallettransactions call from feature_pruning.py.

  11. jnewbery force-pushed on Mar 28, 2019
  12. jnewbery commented at 4:17 PM on March 28, 2019: member

    This now depends on #15686. Please review that first.

  13. promag commented at 12:06 AM on March 29, 2019: member

    Should deprecate first? Or that doesn't apply to hidden RPC?

    This RPC isn't part of the public API so I don't think it needs deprecation. We could raise a JSON RPC exception telling users that transactions are rebroadcast automatically and that they just need to wait.

    Then I think it's fine to just remove it.

  14. jnewbery force-pushed on Mar 29, 2019
  15. jnewbery renamed this:
    [WIP] Remove resendwallettransactions RPC method
    Remove resendwallettransactions RPC method
    on Mar 29, 2019
  16. jnewbery commented at 5:47 PM on March 29, 2019: member

    Rebased on master and removed the [WIP] tag.

  17. MarcoFalke commented at 6:47 PM on March 29, 2019: member
    ./test/functional/wallet_basic.py:10:1: F401 'test_framework.util.sync_mempools' imported but unused
    
  18. [rpc] remove resendwallettransactions RPC
    This RPC was added for testing wallet rebroadcasts. Since we now have a
    real test for wallet rebroadcasts, it's no longer needed.
    
    The call in wallet_basic.py can be removed because
    wallet_resendwallettransactions.py tests wallet rebroadcast.
    f5162458cd
  19. jnewbery force-pushed on Mar 29, 2019
  20. jnewbery force-pushed on Mar 29, 2019
  21. in src/wallet/wallet.cpp:2115 in a0767201a0 outdated
    2111 | @@ -2112,53 +2112,38 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const
    2112 |          return CTransaction(tx1) == CTransaction(tx2);
    2113 |  }
    2114 |  
    2115 | -std::vector<uint256> CWallet::ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime)
    


    jonatack commented at 4:16 PM on March 31, 2019:

    I'm likely missing something, but should ResendWalletTransactionsBefore be removed from /src/wallet/wallet.h:950-951 as well?

        // ResendWalletTransactionsBefore may only be called if fBroadcastTransactions!
        std::vector<uint256> ResendWalletTransactionsBefore(interfaces::Chain::Lock& locked_chain, int64_t nTime);
    

    jnewbery commented at 1:30 PM on April 1, 2019:

    You're not missing anything. Now fixed. Thanks!

  22. jnewbery force-pushed on Apr 1, 2019
  23. in src/wallet/wallet.cpp:2135 in f9b34f7cc2 outdated
    2167 | +    { // cs_wallet scope
    2168 | +    LOCK(cs_wallet);
    2169 | +
    2170 | +    // Relay transactions
    2171 | +    for (std::pair<const uint256, CWalletTx>& item : mapWallet)
    2172 | +    {
    


    MarcoFalke commented at 2:50 PM on April 1, 2019:

    Untitled png


    MarcoFalke commented at 2:51 PM on April 1, 2019:

    jnewbery commented at 3:20 PM on April 1, 2019:

    Fixed!

  24. MarcoFalke approved
  25. MarcoFalke commented at 2:52 PM on April 1, 2019: member

    utACK f9b34f7cc2232dfd776885e20230ff3c660c2f7d

  26. [wallet] Remove ResendWalletTransactionsBefore
    This is only called from ResendWalletTransactions(), so bring it inline.
    ea1a2d8794
  27. promag commented at 3:14 PM on April 1, 2019: member

    utACK if you make @MarcoFalke happy.

  28. jnewbery force-pushed on Apr 1, 2019
  29. MarcoFalke commented at 4:30 PM on April 1, 2019: member

    re-utACK ea1a2d8794

    Only whitespace changes since my previous review

  30. promag commented at 12:32 PM on April 2, 2019: member

    utACK ea1a2d8.

  31. MarcoFalke merged this on Apr 2, 2019
  32. MarcoFalke closed this on Apr 2, 2019

  33. MarcoFalke referenced this in commit 8dbb2c5e67 on Apr 2, 2019
  34. deadalnix referenced this in commit 92426570b6 on May 5, 2020
  35. deadalnix referenced this in commit 662edf2e7a on May 6, 2020
  36. kittywhiskers referenced this in commit 9d440b2112 on Nov 6, 2021
  37. kittywhiskers referenced this in commit e274e56e0e on Nov 30, 2021
  38. kittywhiskers referenced this in commit b3b95b4edb on Dec 3, 2021
  39. kittywhiskers referenced this in commit 5bc0fc5fbe on Dec 4, 2021
  40. kittywhiskers referenced this in commit 0f8f88e095 on Dec 6, 2021
  41. kittywhiskers referenced this in commit f90d0e6a61 on Dec 8, 2021
  42. kittywhiskers referenced this in commit 866371aed4 on Dec 8, 2021
  43. kittywhiskers referenced this in commit cea7f807ba on Dec 8, 2021
  44. kittywhiskers referenced this in commit 4ca5f32881 on Dec 11, 2021
  45. kittywhiskers referenced this in commit af59184145 on Dec 12, 2021
  46. kittywhiskers referenced this in commit 0ae76271ec on Dec 12, 2021
  47. kittywhiskers referenced this in commit 8ce858e633 on Dec 12, 2021
  48. kittywhiskers referenced this in commit 980303a7ac on Dec 12, 2021
  49. kittywhiskers referenced this in commit c04f74b1d4 on Dec 12, 2021
  50. DrahtBot locked this on Dec 16, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 15:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me