Remove accounts rpcs #14023

pull jnewbery wants to merge 4 commits into bitcoin:master from jnewbery:remove_accounts_rpcs changing 10 files +112 −1021
  1. jnewbery commented at 1:02 pm on August 22, 2018: member

    This is the first part of #13825. It simply removes the RPC methods and tests.

    #13825 touches lots of files and will require frequent rebasing.

    Breaking it down for easier reviewing and fewer rebases.

  2. fanquake added the label Refactoring on Aug 22, 2018
  3. fanquake added the label Wallet on Aug 22, 2018
  4. DrahtBot commented at 2:56 pm on August 22, 2018: member
    • #14075 (Import watch only pubkeys to the keypool if private keys are disabled by achow101)
    • #13945 (Refactoring CRPCCommand with enum category by isghe)
    • #13249 (Make objects in range declarations immutable by default. Avoid unnecessary copying of objects in range declarations. by practicalswift)
    • #12490 ([Wallet] [RPC] Remove deprecated wallet rpc features from bitcoin_server by jnewbery)
    • #11652 (Add missing locks: validation.cpp + related by practicalswift)
    • #11413 ([wallet] [rpc] sendtoaddress/sendmany: Add explicit feerate option by kallewoof)
    • #9381 (Remove CWalletTx merging logic from AddToWallet by ryanofsky)

    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.

  5. laanwj added the label RPC/REST/ZMQ on Aug 23, 2018
  6. promag commented at 9:58 pm on August 23, 2018: member
    LGTM. Should update contrib/bitcoin-cli.bash-completion?
  7. gmaxwell commented at 7:22 am on August 24, 2018: contributor
    Is it still possible for a wallet to be in a state where it cannot spend funds unless they’re first ‘move’ed into the default account?
  8. jnewbery commented at 2:14 pm on August 24, 2018: member

    Is it still possible for a wallet to be in a state where it cannot spend funds unless they’re first ‘move’ed into the default account?

    I’m not aware of that issue. If you can point to a github issue or some other reference describing what you’re alluding to, that would be helpful.

    In any case, the sendmany and sendtoaddress RPCs will not consider accounts if an account argument is not passed, so I don’t see how the wallet could ever be in that state, either before or after this PR.

  9. MarcoFalke commented at 2:32 pm on August 24, 2018: member
    utACK 01137c17ac600177167060aac4b6d599dc52b953. This only removes test and rpc code, not touching the wallet code.
  10. DrahtBot added the label Needs rebase on Aug 25, 2018
  11. PierreRochard commented at 11:28 pm on August 26, 2018: contributor
    Tested ACK 01137c17ac600177167060aac4b6d599dc52b953, will update after the rebase
  12. [tests] Remove wallet accounts test 
    The accounts API will be removed in the next commit. Remove all
functional tests for the accounts API.
    c410f41575
  13. jnewbery force-pushed on Aug 27, 2018
  14. jnewbery commented at 1:56 pm on August 27, 2018: member
    Rebased and release note added.
  15. laanwj added this to the "Blockers" column in a project
  16. in src/wallet/rpcwallet.cpp:1457 in 288bccc472 outdated 
    1470-                    else
1471-                        entry.pushKV("category", "generate");
1472-                }
1473+                if (wtx.GetDepthInMainChain() < 1)
1474+                    entry.pushKV("category", "orphan");
1475+                else if (wtx.GetBlocksToMaturity() > 0)
    MarcoFalke commented at 2:04 pm on August 27, 2018:
    Any reason for this change?
    jnewbery commented at 2:45 pm on August 27, 2018:

    No reason, except a bad rebase. Thanks for catching this!

    Fixed in https://github.com/bitcoin/bitcoin/pull/14023/commits/bb08423d5ca866d4a139a3b57ff110d818d08b32

    PierreRochard commented at 6:54 pm on August 27, 2018:

    Good catch Marco. Since this slipped past me, I did some research and found that the CLion IDE effectively highlights the change, while github and diff2html do not. Adding it to my review process.

  17. MarcoFalke commented at 2:05 pm on August 27, 2018: member
    Almost utACK 288bccc472fe5a91acc0f4029a1daff689aead2f
  18. [wallet] Remove wallet account RPCs 
    Also remove the RPC deprecation tests for accounts, and make one small
change to another wallet test that relies on account behaviour.
    f0dc850bf6
  19. [wallet] Re-sort wallet RPC commands 
    This wasn't done in previous commit to make diff more reviewable.
    1f4b865e57
  20. [doc] Add release notes for 'account' API removal bb08423d5c
  21. jnewbery force-pushed on Aug 27, 2018
  22. DrahtBot removed the label Needs rebase on Aug 27, 2018
  23. laanwj commented at 3:52 pm on August 27, 2018: member
    utACK bb08423d5ca866d4a139a3b57ff110d818d08b32
  24. laanwj merged this on Aug 27, 2018
  25. laanwj closed this on Aug 27, 2018
  26. laanwj referenced this in commit f180e81d57 on Aug 27, 2018
  27. in doc/release-notes-14023.md:1 in bb08423d5c 
    0@@ -0,0 +1,8 @@
1+Accout API removed
    promag commented at 10:33 pm on August 27, 2018:
    Typo.
    practicalswift commented at 1:43 pm on August 30, 2018:

    FWIW, codespell catched this typo. Fixed in the codespell PR #13954.

    Please review :-)

  28. promag commented at 10:40 pm on August 27, 2018: member
    utACK bb08423, maybe fix typo in the next PR.
  29. laanwj removed this from the "Blockers" column in a project
  30. PierreRochard commented at 8:47 pm on August 28, 2018: contributor
    Post-merge tested ACK bb08423d5ca866d4a139a3b57ff110d818d08b32
  31. ryanofsky referenced this in commit 8fcb765084 on Oct 6, 2018
  32. jnewbery referenced this in commit c8d1f1513f on Oct 9, 2018
  33. ryanofsky referenced this in commit fa7fae5c36 on Oct 10, 2018
  34. ryanofsky referenced this in commit 4deba4ced9 on Oct 10, 2018
  35. jnewbery referenced this in commit 89306ab0df on Oct 10, 2018
  36. ryanofsky referenced this in commit 7cbe74f152 on Oct 15, 2018
  37. laanwj referenced this in commit 5150accdd2 on Nov 10, 2018
  38. ryanofsky referenced this in commit b6ccb77270 on Nov 12, 2018
  39. ryanofsky referenced this in commit 3f05bcbf5d on Nov 13, 2018
  40. ryanofsky referenced this in commit 65b740f92b on Nov 13, 2018
  41. MarcoFalke referenced this in commit e74649e951 on Nov 14, 2018
  42. JeremyRubin referenced this in commit 19581b0f3f on Nov 24, 2018
  43. HashUnlimited referenced this in commit 2d9f1046e7 on Nov 26, 2018
  44. deadalnix referenced this in commit 1b1bc9407a on Feb 6, 2020
  45. deadalnix referenced this in commit 5f7aec629b on Mar 5, 2020
  46. ftrader referenced this in commit e2d0c384e2 on May 19, 2020
  47. xdustinface referenced this in commit de7c6514a3 on Dec 22, 2020
  48. xdustinface referenced this in commit 96d306324f on Dec 22, 2020
  49. pravblockc referenced this in commit bdd3dc1d5f on Jul 25, 2021
  50. pravblockc referenced this in commit 109b0d1924 on Jul 27, 2021
  51. pravblockc referenced this in commit f96621071c on Aug 4, 2021
  52. UdjinM6 referenced this in commit a15815caf4 on Aug 7, 2021
  53. UdjinM6 referenced this in commit b25d608471 on Aug 7, 2021
  54. zkbot referenced this in commit 5b194067ea on Aug 12, 2021
  55. DrahtBot locked this on Sep 8, 2021


jnewbery DrahtBot promag gmaxwell MarcoFalke PierreRochard laanwj practicalswift

Labels
Refactoring Wallet RPC/REST/ZMQ

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: 2024-07-03 10:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me