Commit graph

26316 commits

Author SHA1 Message Date
Wladimir J. van der Laan
663fd92b28
Merge #20266: wallet: fix change detection of imported internal descriptors
bd93fc9945 Fix change detection of imported internal descriptors (Andrew Chow)

Pull request description:

  Import internal descriptors were having address book entries added which meant they would be detected as non-change. Fix this and add a test for it.

ACKs for top commit:
  laanwj:
    Code review ACK bd93fc9945
  meshcollider:
    utACK bd93fc9945
  promag:
    Code review ACK bd93fc9945.

Tree-SHA512: 8fa9e364be317627ec171eedffdb505976c0e7f1e55bc7e8cfdffa3aeea5db24d231f55166602cd0e97a5ba621acc871de0a765c75d0c65678f83e93c3b657c5
2020-11-09 15:14:45 +01:00
Wladimir J. van der Laan
f70eb51b05
Merge #20318: build: Ensure source tarball has leading directory name
faa2f06f5e scripted-diff: [build] Ensure source tarball has leading directory name (MarcoFalke)

Pull request description:

  This has been fixed in 0.20, so it needs to be fixed on master as well to avoid a regression

  #18945

ACKs for top commit:
  laanwj:
    ACK faa2f06f5e
  hebasto:
    ACK faa2f06f5e, tested gitian builds only.
  promag:
    ACK faa2f06f5e.

Tree-SHA512: e3b025c29c45b025002abc35262bb5d771f6cbd807f1c256c477c243685e93cd43ad9f642b38e3cf218590912abe6ea0ddfec3bfbef36f99080aad74ed6cc0af
2020-11-09 15:06:20 +01:00
MarcoFalke
7e373294a5
Merge #20315: travis: Remove s390x build
fa2c3c0d96 ci: Set LC_ALL=C to allow running the s390x tests in qemu (MarcoFalke)
fac0517836 travis: Remove s390x build (MarcoFalke)

Pull request description:

  This has been discussed in the last meeting.

  Refer to the commit body for more details.

Top commit has no ACKs.

Tree-SHA512: 8e0455286ce41c95ed2e5eb624ac534251bb4a321f13d26d14356497e0c39f841372e166373ffd4a0a9fa379636c2cfb535bd92534fff427cdcb827354e66b6c
2020-11-06 18:08:32 +01:00
MarcoFalke
4727c1ca24
Merge #20328: cirrus: Skip tasks on the gui repo main branch
66667acc53 cirrus: Skip tasks on the gui repo main branch (MarcoFalke)

Pull request description:

  No need to run every build twice, once in the main repo and then in the read-only gui mirror repo

ACKs for top commit:
  decryp2kanon:
    ACK 66667ac
  hebasto:
    ACK 66667acc53, though still preferring `only_if` as showing skipped tasks as successful ones seems a bit confused.

Tree-SHA512: 0d35bd115152e06ba4dc5f364130ba5496167d960c44eac2c76192ff9bf7c51f46ab72e2d054dcc6a91818a18dffbbc262f8a4c4483857158c0af4f55dfe9b28
2020-11-06 18:03:49 +01:00
MarcoFalke
66667acc53
cirrus: Skip tasks on the gui repo main branch 2020-11-06 10:05:30 +01:00
fanquake
a0c00ff7c0
Merge #20298: macOS deploy: use the new plistlib API
04a69c200e macOS deploy: use the new plistlib API (Jonas Schnelli)

Pull request description:

  See https://docs.python.org/3/library/plistlib.html.
  The old API was deprecated in 3.4 and removed in 3.9.

  ~~AFAIK the macdeployplus scripts is only used when calling `make deploy` locally (on macOS). The linux cross compile build (like gitian) are not affected by this PR.~~

ACKs for top commit:
  fanquake:
    ACK 04a69c200e - I checked that `make deploy` on macOS currently fails when building master and using Python 3.9. This PR fixes that, and it's fine to use (and backport) these changes as they only require Python 3.4. Related note: I think we could just about drop our native_biplist dependency entirely given some changes upstream.
  practicalswift:
    ACK 04a69c200e: patch looks correct

Tree-SHA512: c5bb60c5157b371d680c82e0978470a488f3edc58cd09e1be635fed59420f227dd113e901c28e15a463da6fe81dc64d08a701b1fdfeb4502f418785707dbebbc
2020-11-06 15:34:55 +08:00
MarcoFalke
c51c2753a4
Merge #20326: tests: Fix ecdsa_verify in test framework
568a1d7261 fix ecdsa verify in test framework (Stepan Snigirev)

Pull request description:

  This PR fixes a small bug in the test framework in `verify_ecdsa` function.
  `r` in ecdsa signature is modulo curve order, so if the point `R` calculated during verification has x-coordinate that is larger than the curve order, the verification will fail in the test framework but pass in libsecp256k1.

  Example (all in hex):
  public key: `0289d889551598a0263746c01e5882ccf9b7dc4ca5a37108482c9d80de40e0a8cf`
  der signature: `3006020104020104` (r = 4, s = 4)
  message: `3232323232323232323232323232323232323232323232323232323232323232`

  libsecp256k1 returns `true`, test framework returns `false`.

ACKs for top commit:
  sipa:
    utACK 568a1d7261

Tree-SHA512: 9e9c58498f10085d2ad85e95caff6c92793799d2a40696ef43febcd7d313c8c3d5ecec715ca903cbb8432a8a96bd0065d86d060966d4ee651c3871ce16c252bf
2020-11-06 08:08:27 +01:00
MarcoFalke
65460c207c
Merge #20324: wallet: Set DatabaseStatus::SUCCESS in MakeSQLiteDatabase
faf5fa7413 wallet: Set DatabaseStatus::SUCCESS in MakeSQLiteDatabase (MarcoFalke)

Pull request description:

  This is a refactor to set the status to `SUCCESS` (like it is done in `MakeBerkeleyDatabase`, too). It also happens to fix a false positive valgrind warning (tested with bionic-gcc and focal-clang):

  ```
   node1 stderr ==28149== Conditional jump or move depends on uninitialised value(s)
  ==28149==    at 0x464471: LoadWallets(interfaces::Chain&) (load.cpp:105)
  ==28149==    by 0x44BFBA: interfaces::(anonymous namespace)::WalletClientImpl::load() (wallet.cpp:510)
  ==28149==    by 0x1640F9: AppInitMain(util::Ref const&, NodeContext&, interfaces::BlockAndHeaderTipInfo*) (init.cpp:1815)
  ==28149==    by 0x144F3F: AppInit (bitcoind.cpp:142)
  ==28149==    by 0x144F3F: main (bitcoind.cpp:172)
  ==28149==
  {
     <insert_a_suppression_name_here>
     Memcheck:Cond
     fun:_Z11LoadWalletsRN10interfaces5ChainE
     fun:_ZN10interfaces12_GLOBAL__N_116WalletClientImpl4loadEv
     fun:_Z11AppInitMainRKN4util3RefER11NodeContextPN10interfaces21BlockAndHeaderTipInfoE
     fun:AppInit
     fun:main
  }

  TEST                                             | STATUS    | DURATION

  wallet_hd.py --descriptors                       | ✖ Failed  | 69 s
  ```

ACKs for top commit:
  achow101:
    ACK faf5fa7413

Tree-SHA512: e8cbac195d05518467f89725d413bdf226d74671eba1c1eb80b3a61d65724af75a1fe93bcb5c608eaa0d54eddce992738bd923e7d83e493f54c3f4c67b66408c
2020-11-06 08:02:00 +01:00
Stepan Snigirev
568a1d7261 fix ecdsa verify in test framework 2020-11-05 23:16:55 +01:00
MarcoFalke
faf5fa7413
wallet: Set DatabaseStatus::SUCCESS in MakeSQLiteDatabase 2020-11-05 20:48:41 +01:00
MarcoFalke
f5cdc290d5
Merge #20316: test: Fix wallet_multiwallet test issue on Windows
fa00ff0399 test: Fix wallet_multiwallet test issue on Windows (MarcoFalke)

Pull request description:

  Fixes:

  ```
  Traceback (most recent call last):
    File "test\functional\test_framework\test_framework.py", line 126, in main
      self.run_test()
    File "test/functional/wallet_multiwallet.py", line 120, in run_test
      assert_equal(sorted(map(lambda w: w['name'], self.nodes[0].listwalletdir()['wallets'])), sorted(in_wallet_dir))
    File "test\functional\test_framework\util.py", line 49, in assert_equal
      raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
  AssertionError: not(['', 'sub\\w5', 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink', 'w8'] == ['', 'sub/w5', 'w', 'w1', 'w2', 'w3', 'w7', 'w7_symlink', 'w8'])

ACKs for top commit:
  promag:
    ACK fa00ff0399.

Tree-SHA512: 7a809a352677a216465cef59e866e4881272e302e897cebf7d9645bf87aebeaf54435bb0692bb5c1381c2dd680e8a34e640ea18ca6e2a4087e3233cd9c24ed04
2020-11-05 18:45:05 +01:00
MarcoFalke
faa2f06f5e
scripted-diff: [build] Ensure source tarball has leading directory name
-BEGIN VERIFY SCRIPT-
sed -i 's|git archive --|git archive --prefix="${DISTNAME}/" --|g'                          $(git grep -l 'git archive' ./contrib)
sed -i 's|tar -xf "\?${\?GIT_ARCHIVE}\?"\?|tar --strip-components=1 -xf "${GIT_ARCHIVE}"|g' $(git grep -l 'tar -xf'     ./contrib)
-END VERIFY SCRIPT-
2020-11-05 17:31:24 +01:00
MarcoFalke
9bb078351b
Merge #20308: wallet: Set bilingual error completely
090b8385af Set bilingual error completely (Hennadii Stepanov)

Pull request description:

  Fix https://github.com/bitcoin-core/gui/issues/128

ACKs for top commit:
  MarcoFalke:
    review ACK 090b8385af
  practicalswift:
    ACK 090b8385af: patch looks correct!

Tree-SHA512: ef400291a866c3116377a4439a23de89a1c5e3ef4597d682138f88d90612846aabb31228b98a8722e7f58b4b499a58adc732bc40ac28fae6d18fce1d4953c96a
2020-11-05 14:51:37 +01:00
MarcoFalke
fa00ff0399
test: Fix wallet_multiwallet test issue on Windows 2020-11-05 13:50:15 +01:00
MarcoFalke
d94777bd52
Merge #20302: net: Make it easier to reason about node eviction by removing unused NodeEvictionCandidate::addr (CAddress)
f1f433e8ca Make it easier to reason about node eviction by removing unused NodeEvictionCandidate::addr (CAddress) (practicalswift)

Pull request description:

  Make it easier to reason about node eviction by removing unused `NodeEvictionCandidate::addr` (`CAddress`).

ACKs for top commit:
  jnewbery:
    utACK f1f433e8ca

Tree-SHA512: fef91d7b412b8a4f172370cff6c37eb8c3db0ba618f5daf2dcc8737c8fcef7b9b820d7ee99cd0a9eae7dd653a096cf83d5113776b0d1d9a324147581674e9ede
2020-11-05 13:00:13 +01:00
MarcoFalke
fa2c3c0d96
ci: Set LC_ALL=C to allow running the s390x tests in qemu
Without the fix, the tests immediately abort:

terminate called after throwing an instance of 'std::runtime_error'
  what():  locale::facet::_S_create_c_locale name not valid
./qt/test/test_bitcoin-qt: line 2: 116150 Aborted
2020-11-05 12:54:51 +01:00
MarcoFalke
fac0517836
travis: Remove s390x build
The build didn't find too many issues, and they are generally only
endianness related, so easy to fix if they do make it into the main
developement branch.

With travis migrating its service, it fails too often intermittently. It
might be more appropriate to run it manually once before every release.
2020-11-05 12:00:18 +01:00
Hennadii Stepanov
090b8385af
Set bilingual error completely 2020-11-05 11:28:37 +02:00
MarcoFalke
f33e332541
Merge #20303: fuzz: Assert expected DecodeHexTx behaviour when using legacy decoding
d7901ab8d2 fuzz: Assert expected DecodeHexTx behaviour when using legacy decoding (practicalswift)

Pull request description:

  Assert expected `DecodeHexTx` behaviour when using legacy decoding.

  As suggested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/20290#issuecomment-720989597.

ACKs for top commit:
  MarcoFalke:
    review ACK d7901ab8d2

Tree-SHA512: 3285680059e6fa73b0fb2c52b775f6319de1ac616f731206662b742764dc888cdfd1ac1f1fcfdfd5418d2006475a852d1c1a56a7035f772f0a6b2a84f5de93bc
2020-11-05 07:57:28 +01:00
fanquake
6954e4d16c
Merge #20283: test: Only try witness deser when checking for witness deser failure
fae45c34d1 test: Only try witness deserialize when checking for witness deserialize failure (MarcoFalke)

Pull request description:

  Witness deserialize will fail always. (This is what the test is checking for)

  Consequently, non-witness deserialize is also tried, and it might succeed accidentally. Avoid that by not trying non-witness deserialize.

  Fixes #20249

ACKs for top commit:
  jnewbery:
    utACK fae45c34d1

Tree-SHA512: 45e65b31603e3ca839776a7ed30e363b32eba20dfb67b7b55bff06715876850d4f6ba22f8ea4911a62e1f8ffff395bf187b23c46ddc766516b97057df000deb3
2020-11-05 14:56:02 +08:00
MarcoFalke
83650e4df5
Merge #20199: wallet: ignore (but warn) on duplicate -wallet parameters
58cfbc38e0 Ignoring (but warn) on duplicate -wallet parameters (Jonas Schnelli)

Pull request description:

  I expect that there are many users with load on startup wallet definitions in `bitcoin.conf` or via startup CLI argument.
  With the new `settings.json` r/w configuration file, users unloading and loading a wallet through the GUI or via the RPC calls might end up with a duplicate `-wallet` entry (one that still remains in bitcoin.conf or CLI) plus the new duplication in `settings.json` due to the unload/load.

  Steps to reproduce
  * create wallet (if via RPC set `load_on_startup` or unloadwallet/loadwallet then set `load_on_startup`).
  * stop bitcoin
  * start bitcoind again with same `--wallet=mywallet`

  I guess it is acceptable to skip duplicates.

ACKs for top commit:
  achow101:
    Tested ACK 58cfbc38e0
  meshcollider:
    Code review ACK 58cfbc38e0
  ryanofsky:
    Code review ACK 58cfbc38e0. Changes since previous review: rebased, tweaked warning message, squashed/fixed test

Tree-SHA512: f94e5a999bdd7dc291f0bc142911b0a8033929350d6f6a35b58c4a06a3c8f83147be0f0c402d4e946dedbbcc85b7e023b672c731b6d7a8984d4780017c961cfb
2020-11-05 07:51:07 +01:00
practicalswift
d7901ab8d2 fuzz: Assert expected DecodeHexTx behaviour when using legacy decoding 2020-11-04 23:11:50 +00:00
MarcoFalke
6760088015
Merge #20300: fuzz: Add missing ECC_Start to descriptor_parse test
5cafe2b25c fuzz: Add missing ECC_Start to descriptor_parse test (Ivan Metlushko)

Pull request description:

  Fixes fuzzing harness.

  I also observed that the corpus for this test consists only of `xprv...` keys while we are using regtest parameters. So for proper fuzzing we need either A) to update the corpus and replace `xprv...` with `tprv...` B) switch to main net in the test

ACKs for top commit:
  MarcoFalke:
    review ACK 5cafe2b25c
  practicalswift:
    Tested ACK 5cafe2b25c

Tree-SHA512: 7415a98a445ce0f96219637d2362fecfc1191ad104f55d79ca92b0c92cde165e00646be5bf3fda956385e3cb22540eca457e575048493367cdf0e00a27d7cdb8
2020-11-04 20:38:18 +01:00
MarcoFalke
deb2b27c0d
Merge #20294: ci: Run more ci configs on cirrus
fa8e494554 ci: Run ci configs on cirrus (MarcoFalke)

Pull request description:

  Now that cirrus ci runs more stable than travis ci, we can try to move more configs over there to see if any issues arise.

ACKs for top commit:
  practicalswift:
    ACK fa8e494554: patch looks correct
  decryp2kanon:
    reACK fa8e494554

Tree-SHA512: e2d1838050b6199d11fa06d1cc9d804883ec5df7d65386c950e8124c0067dc1aaa62ec84c9842c8263e2cf5b17fc819ce85689338113f8d69edb1954f06e76e2
2020-11-04 20:13:08 +01:00
MarcoFalke
ed9f547750
Merge #20299: test: Fix intermittent rpc_net issue
fa2ecadd0d test: Fix intermittent rpc_net issue (MarcoFalke)

Pull request description:

  The test fails because getpeerinfo and getnettotals are not synchronised, so a `wait_until` is needed for each RPC (separately).

  Fixes https://cirrus-ci.com/task/4663366629195776?command=ci#L5034

ACKs for top commit:
  jnewbery:
    utACK fa2ecadd0d

Tree-SHA512: 5ea7128801aab8dbe3d9e6737545ff4ee770e4a9c5a2096ba2339a688424f1879ccba6bf8bcb219983acf86eb28af06fc629586613e7fe28aeffadd2c98633e8
2020-11-04 17:16:41 +01:00
Ivan Metlushko
5cafe2b25c fuzz: Add missing ECC_Start to descriptor_parse test 2020-11-04 22:55:03 +07:00
MarcoFalke
1209b6c692
Merge #20212: net: fix output of peer address in version message
af3b0dfc54 net: fix output of peer address in version message (Vasil Dimov)

Pull request description:

  If `-logips -debug=net` is specified then we print the contents of the
  version message we send to the peer, including his address. Because the
  addresses in the version message use pre-BIP155 encoding they cannot
  represent a Tor v3 address and we would actually send 16 `0`s instead (a
  dummy IPv6 address). However we would print the full address in the log
  message. Before this fix:

  ```
  2020-10-21T12:24:17Z send version message: version 70016, blocks=653500, us=[::]:0, them=xwjtp3mj427zdp4tljiiivg2l5ijfvmt5lcsfaygtpp6cw254kykvpyd.onion:8333, peer=0
  ```

  This is confusing because we pretend to send one thing while we actually
  send another. Adjust the printout to reflect what we are sending. After
  this fix:

  ```
  2020-10-21T12:26:54Z send version message: version 70016, blocks=653500, us=[::]:0, them=[::]:0, peer=0
  ```

ACKs for top commit:
  MarcoFalke:
    review ACK af3b0dfc54
  jnewbery:
    utACK af3b0dfc54

Tree-SHA512: f169d7b4f07c219e541f7c37ea23b82c77e50085fc72ec62f1dd46970389916e177268d07d45c7be94dd209d1903f8f23eaff62b7fa782f6057dd36bb96bba82
2020-11-04 13:41:52 +01:00
practicalswift
f1f433e8ca Make it easier to reason about node eviction by removing unused NodeEvictionCandidate::addr (CAddress) 2020-11-04 12:22:06 +00:00
MarcoFalke
fa2ecadd0d
test: Fix intermittent rpc_net issue 2020-11-04 13:21:54 +01:00
Jonas Schnelli
04a69c200e macOS deploy: use the new plistlib API
See https://docs.python.org/3/library/plistlib.html.
The new API was added in 3.4 and old removed in 3.9.
2020-11-04 10:28:02 +01:00
MarcoFalke
88776c2926
Merge #20245: test: Run script_assets_test even if built --with-libs=no
fa3967efdb test: Replace ARRAYLEN with C++11 ranged for loop (MarcoFalke)
fafc529053 test: Run AssetTest even if built --with-libs=no (MarcoFalke)
faf58ab139 ci: Add --with-libs=no to one ci config (MarcoFalke)

Pull request description:

  `script_assets_test` doesn't call libbitcoinconsensus, so it seems confusing to require it

ACKs for top commit:
  fanquake:
    ACK fa3967efdb - looks ok to me.

Tree-SHA512: 8744fef64c5d7dc19a0ef4ae9b3df5b3f356253bf000f12723064794c5e50df071824d436059985f1112d94c1b530b65cbeb6b8435114a91b195480620eafc59
2020-11-04 08:51:14 +01:00
Samuel Dobson
5d32009f1a
Merge #20220: wallet, rpc: explicit fee rate follow-ups/fixes for 0.21
0be29000c0 rpc: update conf_target helps for correctness/consistency (Jon Atack)
778b9be406 wallet, rpc: fix send subtract_fee_from_outputs help (Jon Atack)
603c005083 wallet: add rpc send explicit fee rate coverage (Jon Atack)
dd341e602d wallet: add sendtoaddress/sendmany explicit fee rate coverage (Jon Atack)
44e7bfa603 wallet: add walletcreatefundedpsbt explicit fee rate coverage (Jon Atack)
6e1ea4273e test: refactor for walletcreatefundedpsbt fee rate coverage (Jon Atack)
3ac7b0c6f1 wallet: fundrawtx fee rate coverage, fixup ParseConfirmTarget() (Jon Atack)
2d8eba8f84 wallet: combine redundant bumpfee invalid params and args tests (Jon Atack)
1697a40b6f wallet: improve bumpfee error/help, add explicit fee rate coverage (Jon Atack)
fc5721723d wallet: fix SetFeeEstimateMode() error message (Jon Atack)
052427eef1 wallet, bugfix: fix bumpfee with explicit fee rate modes (Jon Atack)

Pull request description:

  Follow-up to #11413 providing a base to build on for #19543:

  - bugfix for `bumpfee` raising a JSON error with explicit feerates, fixes issue #20219
  - adds explicit feerate test coverage for `bumpfee`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send`, `sendtoaddress`, and `sendmany`
  - improves a few related RPC error messages and `ParseConfirmTarget()` / error message
  - fixes/improves the explicit fee rate information in the 6 RPC helps, of which 2 were also missing `conf_target` sat/B units

  This provides a spec and regression coverage for the potential next step of a universal `sat/vB` feerate argument (see #19543), as well as immediate coverage and minimum fixes for 0.21.

ACKs for top commit:
  kallewoof:
    Concept/Tested ACK 0be29000c0
  meshcollider:
    Code review + functional test run ACK 0be29000c0

Tree-SHA512: efd965003e991cba51d4504e2940f06ab3d742e34022e96a673606b44fad85596aa03a8c1809f06df7ebcf21a38e18a891e54392fe3d6fb4d120bbe4ea0cf5e0
2020-11-04 16:35:23 +13:00
Samuel Dobson
17c6fb176a
Merge #20282: wallet: change upgradewallet return type to be an object
2ead31fb1b [wallet] Return object from upgradewallet RPC (Sishir Giri)

Pull request description:

  Change the return type of upgradewallet to be an object for future extensibility.

  Also return any error string returned from the `UpgradeWallet()` function.

ACKs for top commit:
  MarcoFalke:
    ACK 2ead31fb1b
  meshcollider:
    Tested ACK 2ead31fb1b

Tree-SHA512: bcc7432d2f35093ec2463ea19e894fa885b698c0e8d8e4bd2f979bd4d722cbfed53ec589d6280968917893c64649dc9e40800b8d854273b0f9a1380f51afbdb1
2020-11-04 14:51:42 +13:00
MarcoFalke
fa8e494554
ci: Run ci configs on cirrus 2020-11-03 20:26:58 +01:00
Wladimir J. van der Laan
95bde34a71
Merge #20237: net: Hardcoded seeds update for 0.21
6866259fab net: Hardcoded seeds update for 0.21 (Wladimir J. van der Laan)
36e875b4c5 contrib: Add new versions to makeseeds.py and update gitignore (RandyMcMillan)

Pull request description:

  Stats:

  ```
    IPv4   IPv6  Onion Pass
  426728  59523   7900 Initial
  426728  59523   7900 Skip entries with invalid address
  426728  59523   7900 After removing duplicates
  426727  59523   7900 Skip entries from suspicious hosts
  123226  51785   7787 Enforce minimal number of blocks
  121710  51322   7586 Require service bit 1
    4706   1427   3749 Require minimum uptime
    4124   1098   3681 Require a known and recent user agent
    4033   1075   3681 Filter out hosts with multiple bitcoin ports
     512    140    512 Look up ASNs and limit results per ASN and per net
  ```
  I've credited RandyMcMillan for the first commit because of #20190.

  There are at least enough onions this time! Number of IPv6 nodes that pass all the requirements seems similar to last time in #18506.

  For the next major release we'll want TORv3 hardcoded peers as well. This makes no sense now as there are hardly any. But it'd make sense to think about how to collect them because they cannot come from the DNS seeds.

  ### Reviewing
  ```
  2020-10-28 12:04:45     jnewbery  wumpus: Do you have any suggestions for how to review #20237 ?
  2020-10-28 12:28:37     wumpus  jnewbery: previous PRs like it might be a guide there (#18506, #16999), e.g. people could try to repeat the last step in https://github.com/bitcoin/bitcoin/tree/master/contrib/seeds#seeds and see if it ends up with the same .h file, you could also repeat the entire process but as the list of peers from the seeder will be different every time that will give a (slightly, hopefully)
  2020-10-28 12:28:37     wumpus  different output
  2020-10-28 12:49:40     wumpus  testing what part of the peers are connectable is also useful
  2020-10-28 12:51:05     wumpus  or to go deeper, whether most part of the nodes are 'good nodes' and not say spy nodes, but i don't know what means of testing
  ```

ACKs for top commit:
  jonatack:
    ACK 6866259fab

Tree-SHA512: 6b913ec92932de03304301a0cbf7b4a912ed09d890b019deeb449b8fa787c4994222368c6bf08b3c6e2bfa474442612e1c9de9327ec46ba59c37a5f38af50c75
2020-11-03 15:04:07 +01:00
Jonas Schnelli
58cfbc38e0 Ignoring (but warn) on duplicate -wallet parameters 2020-11-03 12:06:32 +01:00
MarcoFalke
218fe60d91
Merge #20290: fuzz: Fix DecodeHexTx fuzzing harness issue
28f8cb13d4 fuzz: Fix DecodeHexTx fuzzing harness issue (practicalswift)

Pull request description:

  Fix `DecodeHexTx` fuzzing harness issue.

  Before this patch:

  ```
  $ src/test/fuzz/decode_tx
  decode_tx: test/fuzz/decode_tx.cpp:29:
      void test_one_input(const std::vector<uint8_t> &):
      Assertion `result_try_witness_and_maybe_no_witness' failed.
  …
  ```

  After this patch:

  ```
  $ src/test/fuzz/decode_tx
  …
  ```

ACKs for top commit:
  MarcoFalke:
    review ACK 28f8cb13d4

Tree-SHA512: 2ed11b2f00a4c6fa3e8eea76a2a37d89a4b8d52815264676fe3de0a26ad7906cfafda9b843ceede2fd428815472e01fd1f87afb851282a8c7839bd4c87dc382b
2020-11-03 09:48:15 +01:00
MarcoFalke
5174b534da
Merge #20289: fuzz: Check for addrv1 compatibility before using addrv1 serializer/deserializer on CService
c2cf8a18c2 fuzz: Check for addrv1 compatibility before using addrv1 serializer on CService (practicalswift)

Pull request description:

  Check for addrv1 compatibility before using addrv1 serializer/deserializer on `CService`:

  Before this patch:

  ```
  $ src/test/fuzz/service_deserialize
  service_deserialize: test/fuzz/deserialize.cpp:85:
      void (anonymous namespace)::AssertEqualAfterSerializeDeserialize(const T &, const int) [T = CService]:
      Assertion `Deserialize<T>(Serialize(obj, version)) == obj' failed.
  ```

  After this patch:

  ```
  $ src/test/fuzz/service_deserialize
  …
  ```

  Related change: #20247

ACKs for top commit:
  MarcoFalke:
    review ACK c2cf8a18c2

Tree-SHA512: dba6ddc60e8ef621011d844281461f1741de08c4af1a2b7156c810af44306cef7ec582de5974752db02ca085cfd23da0296d70b694e59ee262589d829fa0626e
2020-11-03 09:14:22 +01:00
fanquake
8387f832d6
Merge #20187: Addrman: test-before-evict bugfix and improvements for block-relay-only peers
16d9bfc417 Avoid test-before-evict evictions of current peers (Suhas Daftuar)
e8b215a086 Refactor test for existing peer connection into own function (Suhas Daftuar)
4fe338ab3e Call CAddrMan::Good() on block-relay-only peer addresses (Suhas Daftuar)
daf5553126 Avoid calling CAddrMan::Connected() on block-relay-only peer addresses (Suhas Daftuar)

Pull request description:

  This PR does two things:

  * Block-relay-only interaction with addrman.
    * Calling `CAddrMan::Connected()` on an address that was a block-relay-only peer causes the time we report in `addr` messages containing that peer to be updated; particularly now that we use anchor connections with a our block-relay-only peers, this risks leaking information about those peers.  So, stop this.
    * Avoiding calling `CAddrMan::Good()` on block-relay-only peer addresses causes the addrman logic around maintaining the new and tried table to be less good, and in particular makes it so that block-relay-only peer addresses are more likely to be evicted from the addrman (for no good reason I can think of).  So, mark those addresses as good when we connect.

  * Fix test-before-evict bug. There's a bug where if we get a collision in the tried table with an existing address that is one of our current peers, and the connection is long-lived enough, then `SelectTriedCollisions()` might return that existing peer address to us as a test-before-evict connection candidate. However, our logic for new outbound connections would later prevent us from actually making a connection; the result would be that when we get a collision with a long-lived current peer, that peer's address is likely to get evicted from the tried table.  Fix this by checking to see if a test-before-evict candidate is a peer we're currently connected to, and if so, mark it as `Good()`.

ACKs for top commit:
  sipa:
    utACK 16d9bfc417
  amitiuttarwar:
    code review ACK 16d9bfc417
  mzumsande:
    Code-Review ACK 16d9bfc417.
  jnewbery:
    utACK 16d9bfc417
  ariard:
    Code Review ACK 16d9bfc.
  jonatack:
    Tested ACK 16d9bfc417

Tree-SHA512: 188ccb814e436937cbb91d29d73c316ce83f4b9c22f1cda56747f0949a093e10161ae724e87e4a2d85ac40f85f5f6b4e87e97d350a1ac44f80c57783f4423324
2020-11-03 09:38:35 +08:00
practicalswift
28f8cb13d4 fuzz: Fix DecodeHexTx fuzzing harness issue 2020-11-02 22:21:03 +00:00
practicalswift
c2cf8a18c2 fuzz: Check for addrv1 compatibility before using addrv1 serializer on CService 2020-11-02 21:45:40 +00:00
Wladimir J. van der Laan
ca18860563
Merge #20263: Update assumed chain params
fa90ba36d3 Update assumed chain params (MarcoFalke)

Pull request description:

  > Oh, by the way, the same procedure as last year, Miss Sophie?
  > Same procedure as every year, James.

  See https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off

ACKs for top commit:
  jonatack:
    ACK fa90ba36d3 per `git diff fa0c1b5 fa90ba3` and re-running getblockheader/getblockhash/getchaintxstats on the signet chain
  dergoegge:
    ACK fa90ba36d3 - mainnet and testnet data matches my node.
  theStack:
    re-ACK fa90ba36d3 ✔️
  darosior:
    re-ACK fa90ba36d3 for mainnet and testnet.

Tree-SHA512: 83044cc59d9fc873cb29f13008d00b54cb4bb4646c1ca53ab4f429e7333a32402becb888d3be117d390c1297c2f7d083f77ae12ac8633265edceb3cfefac087f
2020-11-02 18:53:23 +01:00
Wladimir J. van der Laan
ef4c7c4e0b
Merge #18788: tests: Update more tests to work with descriptor wallets
c7b7e0a692 tests: Make only desc wallets for wallet_multwallet.py --descriptors (Andrew Chow)
d4b67ad214 Avoid creating legacy wallets in wallet_importdescriptors.py (Andrew Chow)
6c9c12bf87 Update feature_backwards_compatibility for descriptor wallets (Andrew Chow)
9a4c631e1c Update wallet_labels.py to not require descriptors=False (Andrew Chow)
242aed7cc1 tests: Add a --legacy-wallet that is mutually exclusive with --descriptors (Andrew Chow)
388053e172 Disable some tests for tool_wallet when descriptors (Andrew Chow)
47d3243160 Make raw multisig tests legacy wallet only in rpc_rawtransaction.py (Andrew Chow)
59d3da5bce Do addmultisigaddress tests in legacy wallet mode in wallet_address_types.py (Andrew Chow)
25bc5dccbf Use importdescriptors when in descriptor wallet mode in wallet_createwallet.py (Andrew Chow)
0bd1860300 Avoid dumpprivkey and watchonly behavior in rpc_signrawtransaction.py (Andrew Chow)
08067aebfd Add script equivalent of functions in address.py (Andrew Chow)
86968882a8 Add descriptor wallet output to tool_wallet.py (Andrew Chow)
3457679870 Use separate watchonly wallet for multisig in feature_nulldummy.py (Andrew Chow)
a42652ec10 Move import and watchonly tests to be legacy wallet only in wallet_balance.py (Andrew Chow)
4b871909d6 Use importdescriptors for descriptor wallets in wallet_bumpfee.py (Andrew Chow)
c2711e4230 Avoid dumpprivkey in wallet_listsinceblock.py (Andrew Chow)
553dbf9af4 Make import tests in wallet_listtransactions.py legacy wallet only (Andrew Chow)
dc81418fd0 Use a separate watchonly wallet in rpc_fundrawtransaction.py (Andrew Chow)
a357111047 Update wallet_importprunedfunds to avoid dumpprivkey (Andrew Chow)

Pull request description:

  I went through all the tests and checked whether they passed with descriptor wallets. This partially informed some changes in #16528. Some tests needed changes to work with descriptor wallets. These were primarily due to import and watchonly behavior. There are some tests and test cases that only test legacy wallet behavior so those tests won't be run with descriptor wallets.

  This PR updates more tests to have to the `--descriptors` switch in `test_runner.py`. Additionally a mutually exclusive `--legacy-wallet` option has been added to force legacy wallets. This does nothing currently but will be useful in the future when descriptor wallets are the default. For the tests that rely on legacy wallet behavior, this option is being set so that we don't forget in the future. Those tests are `feature_segwit.py`, `wallet_watchonly.py`, `wallet_implicitsegwit.py`, `wallet_import_with_label.py`, and `wallet_import_with_label.py`.

  If you invert the `--descriptors`/`--legacy-wallet` default so that descriptor wallets are the default, all tests (besides the legacy wallet specific ones) will pass.

ACKs for top commit:
  MarcoFalke:
    review ACK c7b7e0a692 🎿
  laanwj:
    ACK c7b7e0a692

Tree-SHA512: 2f4e87815005d1d0a2543ea7947f7cd7593d8cf5312228ef85f8e096f19739b225769961943049cb44f6f07a35b8de988e2246ab9aca5bb5a0b2e62694d5637d
2020-11-02 18:50:37 +01:00
MarcoFalke
b6a00e76ab
Merge #20279: doc: release process updates/fixups
e5f3e95a8e doc: fix getchaintxstats fields in release-process.md (Jon Atack)

Pull request description:

  ISTM the getchaintxstats fields should be `window_final_block_hash` rather than `window_last_block_hash`. While here, replace getblockchaininfo with getblockheader (and getblockhash) instead of getblockchaininfo for updating the nMinimumChainWork and defaultAssumeValid consensus params, update the example PR, and improve a link with a named anchor tag.

  Markdown rendering here: https://github.com/jonatack/bitcoin/blob/release-process-getchaintxstats-fix/doc/release-process.md

ACKs for top commit:
  theStack:
    re-ACK e5f3e95a8e

Tree-SHA512: 48c9c65f10d65e461da8d4935af56b6c67e6faca94e4593237f754d8c48f03bef2b9b4a71e5d1009b215a415ba7c4c4218aca6dce97238101ca1c81f5d098bdb
2020-11-02 15:58:25 +01:00
MarcoFalke
fae45c34d1
test: Only try witness deserialize when checking for witness deserialize failure 2020-11-02 14:19:44 +01:00
MarcoFalke
c5ec0367d7
Merge #20165: Only relay Taproot spends if next block has it active
3d0556d410 Increase feature_taproot inactive test coverage (Pieter Wuille)
525cbd425e Only relay Taproot spends if next block has it active (Pieter Wuille)

Pull request description:

  There should be no change to mempool transaction behavior for witness v1 transactions as long as no activation is defined. Until that point, we should treat the consensus rules as under debate, and for soft-fork safety, that means spends should be treated as non-standard.

  It's possible to go further: don't relay them unless the consensus rules are actually active for the next block. This extends non-relay to the period where a deployment is defined, started, locked in, or failed. I see no downsides to this, and the code change is very simple.

ACKs for top commit:
  Sjors:
    utACK 3d0556d410
  MarcoFalke:
    review ACK 3d0556d410 🏓
  jnewbery:
    utACK 3d0556d410

Tree-SHA512: ca625a2981716b4b44e8f3722718fd25fd04e25bf3ca1684924b8974fca49f7c1d438fdd9dcdfbc091a442002e20d441d42c41a0e2096e74a61068da6c60267a
2020-11-02 10:12:06 +01:00
Sishir Giri
2ead31fb1b [wallet] Return object from upgradewallet RPC 2020-11-02 08:38:38 +00:00
MarcoFalke
fa90ba36d3
Update assumed chain params 2020-11-02 09:22:08 +01:00
MarcoFalke
867dbeba5f
Merge #20281: docs: Correct getblockstats documentation for (sw)total_weight
5d9917464a docs: Correct getblockstats documentation for (sw)total_weight (Nadav Ivgi)

Pull request description:

ACKs for top commit:
  MarcoFalke:
    ACK 5d9917464a

Tree-SHA512: eb9e8f05c61d5363cf698f0b9b51ff42557e783be64c6a814cd8f4073499ff0cbfe00528b12cc0f5e4de245458a62cbd30eb817a7d1e020ee3cbfa83a95eb550
2020-11-02 08:44:59 +01:00
Samuel Dobson
26d7941224
Merge #20230: wallet: Fix bug when just created encrypted wallet cannot get address
bf6855a909 wallet: Fix bug when just created encrypted wallet cannot get address (Hennadii Stepanov)

Pull request description:

  Fix https://github.com/bitcoin-core/gui/issues/105

ACKs for top commit:
  achow101:
    Tested ACK bf6855a909
  kristapsk:
    ACK bf6855a909
  meshcollider:
    Tested ACK bf6855a909

Tree-SHA512: eca0ab306d7206f2e5db568e83217bd854caac104379f4d8fb261db832d4d6310cbb1eab44ce9b05a5ac2eb5879a623b729752a88810f8370c24518a8d81292d
2020-11-02 11:58:19 +13:00