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

SegWit wallet support #11403

Merged
merged 12 commits into from Jan 11, 2018

Conversation

@sipa
Member

sipa commented Sep 26, 2017

This implements a minimum viable implementation of SegWit wallet support, based on top of #11389, and includes part of the functionality from #11089.

Two new configuration options are added:

  • -addresstype, with options legacy, p2sh, and bech32. It controls what kind of addresses are produced by getnewaddress, getaccountaddress, and createmultisigaddress.
  • -changetype, with the same options, and by default equal to -addresstype, that controls what kind of change is used.

All wallet private and public keys can be used for any type of address. Support for address types dependent on different derivation paths will need a major overhaul of how our internal detection of outputs work. I expect that that will happen for a next major version.

The above also applies to imported keys, as having a distinction there but not for normal operations is a disaster for testing, and probably for comprehension of users. This has some ugly effects, like needing to associate the provided label to importprivkey with each style address for the corresponding key.

To deal with witness outputs requiring a corresponding redeemscript in wallet, three approaches are used:

  • All SegWit addresses created through getnewaddress or multisig RPCs explicitly get their redeemscripts added to the wallet file. This means that downgrading after creating a witness address will work, as long as the wallet file is up to date.
  • All SegWit keys in the wallet get an implicit redeemscript added, without it being written to the file. This means recovery of an old backup will work, as long as you use new software.
  • All keypool keys that are seen used in transactions explicitly get their redeemscripts added to the wallet files. This means that downgrading after recovering from a backup that includes a witness address will work.

These approaches correspond to solutions 3a, 1a, and 5a respectively from https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2. As argued there, there is no full solution for dealing with the case where you both downgrade and restore a backup, so that's also not implemented.

dumpwallet, importwallet, importmulti, signmessage and verifymessage don't work with SegWit addresses yet. They're remaining TODOs, for this PR or a follow-up. Because of that, several tests unexpectedly run with -addresstype=legacy for now.

@fanquake fanquake added the Wallet label Sep 26, 2017

@promag

Light review, it would be nice to push base PR's.

Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/wallet/init.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
Show outdated Hide outdated src/wallet/init.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
Show outdated Hide outdated test/functional/segwit.py Outdated
Show outdated Hide outdated src/wallet/init.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
@petertodd

This comment has been minimized.

Show comment
Hide comment
@petertodd

petertodd Sep 28, 2017

Contributor

A potential problem with supporting -addressstyle=segwit w/ Bech32 on day zero is it may confuse people into thinking segwit needs the new address format. OTOH, if we don't, it probably makes the P2SH adoption hell more likely.

Contributor

petertodd commented Sep 28, 2017

A potential problem with supporting -addressstyle=segwit w/ Bech32 on day zero is it may confuse people into thinking segwit needs the new address format. OTOH, if we don't, it probably makes the P2SH adoption hell more likely.

@flack

This comment has been minimized.

Show comment
Hide comment
@flack

flack Sep 28, 2017

Contributor

maybe it should be renamed to -addressstyle=bech32?

Contributor

flack commented Sep 28, 2017

maybe it should be renamed to -addressstyle=bech32?

laanwj added a commit that referenced this pull request Sep 29, 2017

Merge #11167: Full BIP173 (Bech32) support
8213838 [Qt] tolerate BIP173/bech32 addresses during input validation (Jonas Schnelli)
06eaca6 [RPC] Wallet: test importing of native witness scripts (NicolasDorier)
fd0041a Use BIP173 addresses in segwit.py test (Pieter Wuille)
e278f12 Support BIP173 in addwitnessaddress (Pieter Wuille)
c091b99 Implement BIP173 addresses and tests (Pieter Wuille)
bd355b8 Add regtest testing to base58_tests (Pieter Wuille)
6565c55 Convert base58_tests from type/payload to scriptPubKey comparison (Pieter Wuille)
8fd2267 Import Bech32 C++ reference code & tests (Pieter Wuille)
1e46ebd Implement {Encode,Decode}Destination without CBitcoinAddress (Pieter Wuille)

Pull request description:

  Builds on top of #11117.

  This adds support for:
  * Creating BIP173 addresses for testing (through `addwitnessaddress`, though by default it still produces P2SH versions)
  * Sending to BIP173 addresses (including non-v0 ones)
  * Analysing BIP173 addresses (through `validateaddress`)

  It includes a reformatted version of the [C++ Bech32 reference code](https://github.com/sipa/bech32/tree/master/ref/c%2B%2B) and an independent implementation of the address encoding/decoding logic (integrated with CTxDestination). All BIP173 test vectors are included.

  Not included (and intended for other PRs):
  * Full wallet support for SegWit (which would include automatically adding witness scripts to the wallet during automatic keypool topup, SegWit change outputs, ...) [see #11403]
  * Splitting base58.cpp and tests/base58_tests.cpp up into base58-specific code, and "address encoding"-code [see #11372]
  * Error locating in UI for BIP173 addresses.

Tree-SHA512: 238031185fd07f3ac873c586043970cc2db91bf7735c3c168cb33a3db39a7bda81d4891b649685bb17ef90dc63af0328e7705d8cd3e8dafd6c4d3c08fb230341
@MeshCollider

This comment has been minimized.

Show comment
Hide comment
@MeshCollider

MeshCollider Sep 29, 2017

Member

Needs rebase after #11167 merge but that'll make it clearer what's new here to review 👍

Member

MeshCollider commented Sep 29, 2017

Needs rebase after #11167 merge but that'll make it clearer what's new here to review 👍

Show outdated Hide outdated src/rpc/misc.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.h Outdated
@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Sep 29, 2017

Member

Rebased after #11167 merge.

Member

sipa commented Sep 29, 2017

Rebased after #11167 merge.

Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/wallet/init.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.cpp Outdated
@gmaxwell

This comment has been minimized.

Show comment
Hide comment
@gmaxwell

gmaxwell Oct 11, 2017

Member

Regarding petertodd's concern, I think something like flack's suggestion would address it.

Member

gmaxwell commented Oct 11, 2017

Regarding petertodd's concern, I think something like flack's suggestion would address it.

@kallewoof

utACK 5632374

Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
Show outdated Hide outdated src/wallet/rpcwallet.cpp Outdated
Show outdated Hide outdated src/script/standard.h Outdated
@theuni

Added a few more comments.

I suspect I'm missing something about the generic-ish functions in CWallet. Do you maybe have plans to make those per-wallet as @instagibbs suggested?

Show outdated Hide outdated src/rpc/misc.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/keystore.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.cpp Outdated
Show outdated Hide outdated src/wallet/wallet.cpp Outdated
@Sjors

This comment has been minimized.

Show comment
Hide comment
@Sjors

Sjors Oct 20, 2017

Member

The wallet probably needs to be able to generate both p2sh and bech32 addresses, depending on who the user needs to communicate the address to. In fact, the UI might need to display both at the same time.

For example, I might send an email requesting payment to a bech32 address, and telling the recipient to use the p2sh address only if their wallet doesn't support the new format. A backwards compatible QR code could contain (admittedly ad hoc): bitcoin:3ba1...?amount=0.01&bech32=bc1qw5....

So rather than -addressstyle, isn't it better to make address style an argument for calls to getnewaddress and createmultisigaddress?

I think this was brought up elsewhere, but If bech32 is the default, wouldn't that confuse RPC consumers, especially if they do any input validation? p2sh seems like a safer default.
Alternatively, perhaps the default can be bech32, but with a configure flag command line option to use a different default (and a warning in the release notes). It would be nice if RPC calls were versioned.

Member

Sjors commented Oct 20, 2017

The wallet probably needs to be able to generate both p2sh and bech32 addresses, depending on who the user needs to communicate the address to. In fact, the UI might need to display both at the same time.

For example, I might send an email requesting payment to a bech32 address, and telling the recipient to use the p2sh address only if their wallet doesn't support the new format. A backwards compatible QR code could contain (admittedly ad hoc): bitcoin:3ba1...?amount=0.01&bech32=bc1qw5....

So rather than -addressstyle, isn't it better to make address style an argument for calls to getnewaddress and createmultisigaddress?

I think this was brought up elsewhere, but If bech32 is the default, wouldn't that confuse RPC consumers, especially if they do any input validation? p2sh seems like a safer default.
Alternatively, perhaps the default can be bech32, but with a configure flag command line option to use a different default (and a warning in the release notes). It would be nice if RPC calls were versioned.

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Oct 20, 2017

Member

@Sjors There is an RPC argument added to address-creating RPCs to override the default.

Member

sipa commented Oct 20, 2017

@Sjors There is an RPC argument added to address-creating RPCs to override the default.

@Sjors

This comment has been minimized.

Show comment
Hide comment
@Sjors

Sjors Oct 20, 2017

Member

@sipa I see, perhaps -addressstyle should be -addressstyledefault instead to make that more clear?

Member

Sjors commented Oct 20, 2017

@sipa I see, perhaps -addressstyle should be -addressstyledefault instead to make that more clear?

@fanquake fanquake added this to In progress in Segwit Nov 4, 2017

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Nov 7, 2017

Member

rebase_me_pls

Member

MarcoFalke commented Nov 7, 2017

rebase_me_pls

@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Nov 8, 2017

Member

rebased_it

Member

sipa commented Nov 8, 2017

rebased_it

@bitcoin bitcoin unlocked this conversation Jan 11, 2018

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Jan 11, 2018

Member

Post-merge utACK

Member

luke-jr commented Jan 11, 2018

Post-merge utACK

@bitcoin bitcoin deleted a comment from tonyalester Jan 11, 2018

practicalswift pushed a commit to practicalswift/bitcoin that referenced this pull request Jan 11, 2018

Fix ListCoins test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4199: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "ListCoins": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/ListCoins

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

practicalswift pushed a commit to practicalswift/bitcoin that referenced this pull request Jan 11, 2018

Merge bitcoin#12150: Fix ListCoins test failure due to unset g_addres…
…s_type, g_change_type

f765bb3 Fix ListCoins test failure due to unset g_address_type, g_change_type (Russell Yanofsky)

Pull request description:

  New global variables were introduced in bitcoin#11403 and not setting them causes:

  ```
  test_bitcoin: wallet/wallet.cpp:4199: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
  unknown location(0): fatal error in "ListCoins": signal: SIGABRT (application abort requested)
  ```

  It's possible to reproduce the failure reliably by running:

  ```
  src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/ListCoins
  ```

  Failures happen nondeterministically because boost test framework doesn't run tests in a specified order, and tests that run previously can set the global variables and mask the bug.

  Example travis failure: https://travis-ci.org/bitcoin/bitcoin/builds/327642495

Tree-SHA512: 3e0875716f66bc0304cf92a26457e6b54ecfe15ed962f4343577b05fc56bb577554422b7f53949ad6085ac5798ad7816b8176c5b01e050ddbfbb925d2732767a

classesjack pushed a commit to classesjack/bitcoin that referenced this pull request Jan 12, 2018

Fix ListCoins test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4199: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "ListCoins": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/ListCoins

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

@fanquake fanquake moved this from In progress to Done in Segwit Jan 13, 2018

jonasschnelli added a commit that referenced this pull request Jan 24, 2018

Merge #12194: Add change type option to fundrawtransaction
16f6f59 [qa] Test fundrawtransaction with change_type option (João Barbosa)
536ddeb [rpc] Add change_type option to fundrawtransaction (João Barbosa)
31dbd5a [wallet] Add change type to CCoinControl (João Barbosa)

Pull request description:

  Adds a new option `change_type` to `fundrawtransaction` RPC. This is useful to override the node `-changetype` argument.

  The new option is exclusive to `changeAddress` option, setting both raises a RPC error.

  See also #11403, #12119.

Tree-SHA512: 654686444f6125e37015a62f167064d54ec335701534988447be4687fa5ef9c7980a8a07cc0a03fff6ea6c4c1abf0f77a8843d535c4f3fe0bf93f968a4e676e6

Christewart added a commit to Christewart/bitcoin that referenced this pull request Feb 8, 2018

Fix ListCoins test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4199: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "ListCoins": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/ListCoins

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

hkjn added a commit to hkjn/bitcoin that referenced this pull request Feb 12, 2018

Fix ListCoins test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4199: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "ListCoins": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/ListCoins

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Feb 13, 2018

Fix rescan test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4259: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "importwallet_rescan": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/importwallet_rescan

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

sipa added a commit to sipa/bitcoin that referenced this pull request Feb 14, 2018

Fix rescan test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4259: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "importwallet_rescan": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/importwallet_rescan

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

sipa added a commit that referenced this pull request Feb 14, 2018

Merge #12424: Fix rescan test failure due to unset g_address_type, g_…
…change_type

b7f6002 Fix rescan test failure due to unset g_address_type, g_change_type (Russell Yanofsky)

Pull request description:

  New global variables were introduced in #11403 and not setting them causes:

  ```
  test_bitcoin: wallet/wallet.cpp:4259: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
  unknown location(0): fatal error in "importwallet_rescan": signal: SIGABRT (application abort requested)
  ```

  It's possible to reproduce the failure reliably by running:

  ```
  src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/importwallet_rescan
  ```

  Failures happen nondeterministically because boost test framework doesn't run tests in a specified order, and tests that run previously can set the global variables and mask the bug.

  This is similar to bug #12150. Example travis failure is https://travis-ci.org/bitcoin/bitcoin/jobs/340642010

Tree-SHA512: ab40662b3356892b726f1f552e22d58d86b5e982538741e52b37ee447a0c97c76c24ae543687edf2e25d9dd925722909d37abfae95d93bf09e23fa245a4c3351

laanwj added a commit that referenced this pull request Feb 14, 2018

Fix rescan test failure due to unset g_address_type, g_change_type
New global variables were introduced in #11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4259: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "importwallet_rescan": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/importwallet_rescan

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

Github-Pull: #12424
Rebased-From: b7f6002
Tree-SHA512: 1cc64db3b1d886d793e9d194b318dde3d5f628bde778a50513de4bf54dcfc77152885e72608927e3e490d253350ca0381847539a904cb31862f3a6fceac88dc1

esotericnonsense added a commit to esotericnonsense/bitcoin that referenced this pull request Feb 19, 2018

Fix rescan test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4259: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "importwallet_rescan": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/importwallet_rescan

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

Willtech added a commit to Willtech/bitcoin that referenced this pull request Feb 23, 2018

Fix rescan test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4259: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "importwallet_rescan": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/importwallet_rescan

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Mar 15, 2018

Split up key and script metadata for better type safety
Suggested by Matt Corallo <git@bluematt.me>
bitcoin#11403 (comment)

Combining the maps was probably never a good arrangement but is more
problematic now in presence of WitnessV0ScriptHash and WitnessV0KeyHash types.

Conflicts:
	src/wallet/wallet.h
	src/wallet/walletdb.cpp

HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Mar 16, 2018

Fix ListCoins test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4199: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "ListCoins": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/ListCoins

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

HashUnlimited pushed a commit to chaincoin/chaincoin that referenced this pull request Mar 16, 2018

Fix rescan test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4259: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "importwallet_rescan": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/importwallet_rescan

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

Github-Pull: bitcoin#12424
Rebased-From: b7f6002
Tree-SHA512: 1cc64db3b1d886d793e9d194b318dde3d5f628bde778a50513de4bf54dcfc77152885e72608927e3e490d253350ca0381847539a904cb31862f3a6fceac88dc1

virtload added a commit to bitcoin-atom/bitcoin-atom that referenced this pull request Apr 4, 2018

Split up key and script metadata for better type safety
Suggested by Matt Corallo <git@bluematt.me>
bitcoin/bitcoin#11403 (comment)

Combining the maps was probably never a good arrangement but is more
problematic now in presence of WitnessV0ScriptHash and WitnessV0KeyHash types.

@meyer9 meyer9 referenced this pull request Apr 22, 2018

Closed

Merge segwit wallet support #29

HashUnlimited pushed a commit to HashUnlimited/chaincoin that referenced this pull request Jul 31, 2018

Fix rescan test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4259: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "importwallet_rescan": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/importwallet_rescan

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

ccebrecos added a commit to BTCAssessors/bitcoin that referenced this pull request Sep 14, 2018

Fix rescan test failure due to unset g_address_type, g_change_type
New global variables were introduced in bitcoin#11403 and not setting them causes:

    test_bitcoin: wallet/wallet.cpp:4259: CTxDestination GetDestinationForKey(const CPubKey&, OutputType): Assertion `false' failed.
    unknown location(0): fatal error in "importwallet_rescan": signal: SIGABRT (application abort requested)

It's possible to reproduce the failure reliably by running:

    src/test/test_bitcoin --log_level=test_suite --run_test=wallet_tests/importwallet_rescan

Failures happen nondeterministically because boost test framework doesn't run
tests in a specified order, and tests that run previously can set the global
variables and mask the bug.

Github-Pull: bitcoin#12424
Rebased-From: b7f6002
Tree-SHA512: 1cc64db3b1d886d793e9d194b318dde3d5f628bde778a50513de4bf54dcfc77152885e72608927e3e490d253350ca0381847539a904cb31862f3a6fceac88dc1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment