New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wallet] Remove addwitnessaddress #14296

Merged
merged 8 commits into from Oct 24, 2018

Conversation

Projects
None yet
7 participants
@jnewbery
Member

jnewbery commented Sep 23, 2018

Fully removes the addwitnessaddress RPC method, which was deprecated in V0.17

@jnewbery jnewbery changed the title from Remove addwitnessaddress to [wallet] Remove addwitnessaddress Sep 23, 2018

@fanquake fanquake added the Wallet label Sep 23, 2018

@ken2812221

This comment has been minimized.

Member

ken2812221 commented Sep 23, 2018

Concept ACK

jnewbery added some commits Sep 23, 2018

[test] Fix flake8 warnings in tests
Fix all flake8 warnings in tests that are about to be updated
to remove addwitnessaddress
[tests] Remove deprecated addwitnessaddress call from feature_nulldummy
addwitnessaddress is deprecated. Replace the call to addwitnessaddress
with a call to getnewaddress(address_type='p2sh-segwit')
[tests] Remove deprecated addwitnessaddress call from wallet_dump.py
addwitnessaddress is deprecated. Remove the call to that RPC from
wallet_dump.py and improve testing of all types of address (legacy,
p2sh-segwit and bech32)
@jnewbery

This comment has been minimized.

Member

jnewbery commented Sep 23, 2018

Travis was failing because of new flake8 warnings. I've added a new commit removing all flake8 warnings from touched tests.

@DrahtBot

This comment has been minimized.

Contributor

DrahtBot commented Sep 23, 2018

Reviewers, this pull request conflicts with the following ones:
  • #14534 (Enable flake8 rule E225 which checks for missing whitespace around op… by jbampton)
  • #14502 (Rpc help helper class by karel-3d)

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.

@DrahtBot

This comment has been minimized.

Contributor

DrahtBot commented Sep 28, 2018

Coverage Change (pull 14296) Reference (master)
Lines -0.0326 % 87.0361 %
Functions +0.1317 % 84.1130 %
Branches -0.0485 % 51.5451 %
@promag

Tested ACK 850908a, some comments though.

nit, commit ebec90a prefix could be rpc — doesn't touch wallet code?

addwitnessaddress RPC method removed
------------------------------------
The `addwitnessaddress` RPC was added for segwit testing in version 0.13.1. It

This comment has been minimized.

@promag

promag Oct 2, 2018

Member

From doc/release-notes/release-notes-0.13.0.md

  • New RPC commands: generatetoaddress, importprunedfunds, removeprunedfunds, signmessagewithprivkey,
    getmempoolancestors, getmempooldescendants, getmempoolentry,
    createwitnessaddress, addwitnessaddress.

This comment has been minimized.

@jnewbery
------------------------------------
The `addwitnessaddress` RPC was added for segwit testing in version 0.13.1. It
was deprecated in version 0.17.0. This version fully removes the RPC method.

This comment has been minimized.

@promag

promag Oct 2, 2018

Member

From doc/release-notes/release-notes-0.16.0.md

  • The wallet RPC addwitnessaddress was deprecated and will be removed in version 0.17,

This comment has been minimized.

@jnewbery
@jnewbery

This comment has been minimized.

Member

jnewbery commented Oct 17, 2018

Thanks for the corrections @promag . I've fixed them both.

@sipa

This comment has been minimized.

Member

sipa commented Oct 17, 2018

utACK 2b91e42

@promag

This comment has been minimized.

Member

promag commented Oct 18, 2018

Tested ACK 2b91e42, only change was fixing release notes.

@MarcoFalke MarcoFalke merged commit 2b91e42 into bitcoin:master Oct 24, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request Oct 24, 2018

Merge #14296: [wallet] Remove addwitnessaddress
2b91e42 [docs] Add release note for removing getwitnessaddress (John Newbery)
ebec90a [wallet] Remove deprecated addwitnessaddress RPC method (John Newbery)
07e3f58 [test] Remove deprecated addwitnessaddress from feature_segwit.py (John Newbery)
82f2fa0 [test] Remove deprecated addwitnessaddress from wallet_bumpfee.py (John Newbery)
9d7ee18 [test] Remove deprecated addwitnessaddress from p2p_compactblocks.py (John Newbery)
3cf77f0 [tests] Remove deprecated addwitnessaddress call from wallet_dump.py (John Newbery)
bdefc97 [tests] Remove deprecated addwitnessaddress call from feature_nulldummy (John Newbery)
67d7d67 [test] Fix flake8 warnings in tests (John Newbery)

Pull request description:

  Fully removes the `addwitnessaddress` RPC method, which was deprecated in V0.17

Tree-SHA512: 8fa8a2a721a81262fbdedbe1cef031e6a07aa6abbc9760dbc62738fc4f688b44bd737d0f3cdb1aec046866a6395befbfecde0f34e76a99e11d3cf566cad1d0de

@jnewbery jnewbery deleted the jnewbery:remove_addwitnessaddress branch Oct 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment