Skip to content

[0.16] Further Backports#13253

Merged
laanwj merged 5 commits intobitcoin:0.16from
fanquake:0-16-further-backports
May 24, 2018
Merged

[0.16] Further Backports#13253
laanwj merged 5 commits intobitcoin:0.16from
fanquake:0-16-further-backports

Conversation

@fanquake
Copy link
Member

@fanquake fanquake commented May 17, 2018 •
edited
Loading

Backports:

to the 0.16 branch.

@fanquake fanquake added this to the 0.16.1 milestone May 17, 2018
@fanquake fanquake requested review from jnewbery and sdaftuar May 17, 2018 02:28
sdaftuar and others added 3 commits May 17, 2018 10:50
@sdaftuar @fanquake
[qa] Handle disconnect_node race
b8aacd6 
Several tests call disconnect_nodes() on each node-pair in rapid
succession, resulting in a race condition if a node disconnects a peer
in-between the calculation of the nodeid's to disconnect and the
invocation of the disconnectnode rpc call.  Handle this.

GitHub-Pull: bitcoin#13201
Rebased-From: 09c6699
@harding @fanquake
RPC Docs: gettxout*: clarify bestblock and unspent counts
4087dd0 
GitHub-Pull: bitcoin#13184
Rebased-From: f30e9be
@jnewbery @fanquake
[wallet] Fix zapwallettxes/multiwallet interaction.
4c14e7b 
-zapwallettxes should be disallowed when starting bitcoin in multiwallet
mode.

GitHub-Pull: bitcoin#13030
Rebased-From: 3476e3c
@fanquake fanquake force-pushed the 0-16-further-backports branch from ac6c2c6 to e56783e Compare May 17, 2018 02:50
@fanquake
Copy link
Member Author

fanquake commented May 17, 2018

Removed #13199 for now, that may be slightly more complicated to backport.

@laanwj
Copy link
Member

laanwj commented May 17, 2018

There seems to be a problem with two of the tests:

wallet_multiwallet.py                 | ✖ Failed  | 3 s
wallet_multiwallet.py --usecli        | ✖ Failed  | 4 s

@jnewbery
Copy link
Contributor

jnewbery commented May 17, 2018

Fails because commit [wallet] [tests] Test disallowed multiwallet params uses the new assert_start_raises_init_error() syntax introduced in #12718.

Can be fixed by backporting #12718 (although I don't know how much that would conflict) or by applying this diff to the Test disallowed multiwallet params commit:

diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py
index 0c4b97a0c..3a34c43b7 100755
--- a/test/functional/wallet_multiwallet.py
+++ b/test/functional/wallet_multiwallet.py
@@ -57,17 +57,17 @@ class MultiWalletTest(BitcoinTestFramework):
         self.assert_start_raises_init_error(0, ['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory')
 
         self.log.info("Do not allow -zapwallettxes with multiwallet")
-        self.nodes[0].assert_start_raises_init_error(['-zapwallettxes', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file")
-        self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=1', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file")
-        self.nodes[0].assert_start_raises_init_error(['-zapwallettxes=2', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file")
+        self.assert_start_raises_init_error(0, ['-zapwallettxes', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file")
+        self.assert_start_raises_init_error(0, ['-zapwallettxes=1', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file")
+        self.assert_start_raises_init_error(0, ['-zapwallettxes=2', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file")
 
         self.log.info("Do not allow -salvagewallet with multiwallet")
-        self.nodes[0].assert_start_raises_init_error(['-salvagewallet', '-wallet=w1', '-wallet=w2'], "Error: -salvagewallet is only allowed with a single wallet file")
-        self.nodes[0].assert_start_raises_init_error(['-salvagewallet=1', '-wallet=w1', '-wallet=w2'], "Error: -salvagewallet is only allowed with a single wallet file")
+        self.assert_start_raises_init_error(0, ['-salvagewallet', '-wallet=w1', '-wallet=w2'], "Error: -salvagewallet is only allowed with a single wallet file")
+        self.assert_start_raises_init_error(0, ['-salvagewallet=1', '-wallet=w1', '-wallet=w2'], "Error: -salvagewallet is only allowed with a single wallet file")
 
         self.log.info("Do not allow -upgradewallet with multiwallet")
-        self.nodes[0].assert_start_raises_init_error(['-upgradewallet', '-wallet=w1', '-wallet=w2'], "Error: -upgradewallet is only allowed with a single wallet file")
-        self.nodes[0].assert_start_raises_init_error(['-upgradewallet=1', '-wallet=w1', '-wallet=w2'], "Error: -upgradewallet is only allowed with a single wallet file")
+        self.assert_start_raises_init_error(0, ['-upgradewallet', '-wallet=w1', '-wallet=w2'], "Error: -upgradewallet is only allowed with a single wallet file")
+        self.assert_start_raises_init_error(0, ['-upgradewallet=1', '-wallet=w1', '-wallet=w2'], "Error: -upgradewallet is only allowed with a single wallet file")
 
         # if wallets/ doesn't exist, datadir should be the default wallet dir
         wallet_dir2 = data_dir('walletdir')

jnewbery and others added 2 commits May 18, 2018 14:09
@jnewbery @fanquake
[wallet] [tests] Test disallowed multiwallet params
5ff571e 
Add a test to check that bitcoind fails to start when specifying
-zapwallettxes, -salvagewallet and -upgradewallet when running in
multiwallet mode.

GitHub-Pull: bitcoin#13030
Rebased-From: 1f83839
@skeees @fanquake
Hold cs_main while calling UpdatedBlockTip() and ui.NotifyBlockTip
acdf433 
Ensures that callbacks are invoked in the order in which the chain is updated
Resolves bitcoin#12978

GitHub-Pull: bitcoin#12988
Rebased-From: d86edd3
@fanquake fanquake force-pushed the 0-16-further-backports branch from e56783e to acdf433 Compare May 18, 2018 06:10
@fanquake
Copy link
Member Author

fanquake commented May 18, 2018

Thanks @jnewbery, I've updated the Test disallowed multiwallet params commit with your suggested changes.

@fanquake fanquake mentioned this pull request May 18, 2018
Merged
@laanwj
Copy link
Member

laanwj commented May 18, 2018

utACK 6c4a1f777ed934f313389a56e5f68d34d5804c7f

@maflcko
Copy link
Member

maflcko commented May 18, 2018

The last commit is already on the 0.16 branch, no need to include it twice.

@maflcko
Copy link
Member

maflcko commented May 18, 2018

utACK acdf433 (without the last commit they are clean cherry-picks except for the test change. Didn't review anything else)

@fanquake fanquake force-pushed the 0-16-further-backports branch from 6c4a1f7 to acdf433 Compare May 20, 2018 05:55
@laanwj laanwj merged commit acdf433 into bitcoin:0.16 May 24, 2018
laanwj added a commit that referenced this pull request May 24, 2018
@laanwj
Merge #13253: [0.16] Further Backports
50b2c9e 
acdf433 Hold cs_main while calling UpdatedBlockTip() and ui.NotifyBlockTip (Jesse Cohen)
5ff571e [wallet] [tests] Test disallowed multiwallet params (John Newbery)
4c14e7b [wallet] Fix zapwallettxes/multiwallet interaction. (John Newbery)
4087dd0 RPC Docs: gettxout*: clarify bestblock and unspent counts (David A. Harding)
b8aacd6 [qa] Handle disconnect_node race (Suhas Daftuar)

Pull request description:

  Backports:
  - #13201 [qa] Handle disconnect_node race
  - #13184 RPC Docs: gettxout*: clarify bestblock and unspent counts
  - #13030 [bugfix] [wallet] Fix zapwallettxes/multiwallet interaction.
  - #12988 Hold cs_main while calling UpdatedBlockTip() signal

  to the 0.16 branch.

Tree-SHA512: 8f65002bbafaf9c436f89051b2d79bf6a668fbd07bd317c64af238ed4a7c8efe776864b739a7f2869f1e3daa16f2f4366a85f41b188f9c454879d2c7b309be50
@Bushstar Bushstar mentioned this pull request Jun 16, 2018
Merged
@fanquake fanquake deleted the 0-16-further-backports branch June 17, 2018 05:48
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jnewbery jnewbery Awaiting requested review from jnewbery

@sdaftuar sdaftuar Awaiting requested review from sdaftuar

Assignees

No one assigned

Labels

Backport

Projects

None yet

Milestone

0.16.1

Development

Successfully merging this pull request may close these issues.

7 participants

@fanquake @laanwj @jnewbery @maflcko @sdaftuar @harding @skeees