5117 Commits

Author SHA1 Message Date
MacroFake
1c5c951713
Merge bitcoin/bitcoin#26380: Revert "test: check importing wallets when blocks are pruned throw an error"
e1eadaa72d6831d1d0a53ba97c215dc4cdb64436 Revert "test: check importing wallets when blocks are pruned throw an error" (Aurèle Oulès)

Pull request description:

  The test doesn't pass (not detected by the normal CI, because it is an extended test):

  ```
  Traceback (most recent call last):
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/test_framework/test_framework.py", line 133, in main
      self.run_test()
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/feature_pruning.py", line 480, in run_test
      self.wallet_test()
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/feature_pruning.py", line 361, in wallet_test
      assert_raises_rpc_error(-4, "Importing wallets is disabled when blocks are pruned", self.nodes[2].importwallet, "abc")
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/test_framework/util.py", line 130, in assert_raises_rpc_error
      assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/test_framework/util.py", line 145, in try_rpc
      raise AssertionError(
  AssertionError: Expected substring not found in error message:
  substring: 'Importing wallets is disabled when blocks are pruned'
  error message: 'Only legacy wallets are supported by this command'.
  ```

  So revert it for now, which will be done anyway in https://github.com/bitcoin/bitcoin/pull/24865/commits. (This commit is taken from there)

ACKs for top commit:
  andrewtoth:
    ACK e1eadaa72d6831d1d0a53ba97c215dc4cdb64436

Tree-SHA512: 10f556ea1aa97dc9cb64f91055977eecb9534f658170aabb4909c3e85ca6c20588c7a021219356fab678e0e2bec999d347facd00054f07a9445ad393e6353b4c
2022-10-24 16:51:52 +02:00
MacroFake
fa590cfaae
test: Fix intermittent issue in p2p_sendtxrcncl.py 2022-10-24 16:07:23 +02:00
Fabian Jahr
d885bb2f6e
test: Test exclusion of OP_RETURN from getblockstats 2022-10-23 01:33:41 +02:00
Fabian Jahr
ba9d288b24
test: Fix getblockstats test data generator
The getblockstats RPC functional test is using previously generated test data that is part of the repository. That test data can be regenerated by running the test file with `--gen-test-data` which invokes the `generate_test_data()` function. That function still relied on the old wallet behavior of having a default wallet to work. Because of this the function was broken and this change fixes this. The fact that this was broken did was not noticed previously because the function is not used by the automated test suite by default.
2022-10-23 01:33:41 +02:00
Fabian Jahr
2ca5a496c2
rpc: Improve getblockstats
- Fix getblockstats for block height 0 which previously returned an error.
- Introduce alternative utxo_*_actual statistics which exclude unspendables: Genesis block, BIP30, unspendable outputs
- Update test data
- Explicitly test Genesis block results
2022-10-23 01:33:41 +02:00
MacroFake
8c5c98db47
Merge bitcoin/bitcoin#26248: net: Set relay in version msg to peers with relay permission in -blocksonly mode
dddd1acf58cb7bf328ce3e74d1dc0e8cbd503247 net: Set relay in version msg to peers with relay permission (MacroFake)

Pull request description:

  Seems odd to set the `relay` permission in -blocksonly mode and also ask the peer not to relay transactions.

ACKs for top commit:
  dergoegge:
    ACK dddd1acf58cb7bf328ce3e74d1dc0e8cbd503247
  naumenkogs:
    ACK dddd1acf58
  mzumsande:
    ACK dddd1acf58cb7bf328ce3e74d1dc0e8cbd503247

Tree-SHA512: 7bb0e964993ea4982747ae2801fe963ff88586e2ded03015b60ab83172b5b61f2d50e9cde9d7711b7ab207f8639467ecafc4d011ea151ec6c82c722f510f4df7
2022-10-21 11:18:48 +02:00
fanquake
f2859c3aa8
Merge bitcoin/bitcoin#25727: util, config: error on startup if conf or reindex are set in config file
deba6fe3158cd0b2283e0901a072e434ba5b594e test: update feature_config_args.py (josibake)
2e3826cbcd675dcd1d03970233ba5e143e09eb75 util: warn if reindex is used in conf (josibake)
5e744f423838fe7d45453541271bc1a07cd62eac util: disallow setting conf in bitcoin.conf (josibake)

Pull request description:

  In help from `bitcoind -h` it specifes that `conf` can only be used from the commandline. However, if `conf` is set in a `bitcoin.conf` file, there is no error and from reading the logs it seems as if the `conf=<other file>` is being used, despite it being ignored. To recreate, you can setup a `bitcoin.conf` file in the default directory, add `conf=<some other file>.conf` and in the separate config file set whichever config value you want and verify that it is being ignored. alternatively, if you set `includeconf=<some other file>.conf` , your config in `<some other file>` will be picked up.

  This PR fixes this by having the node error when reading the config file if `conf=` is set.

  Additionally, it was mentioned in a recent [PR review club](https://bitcoincore.reviews/24858) that if `reindex=1` is set in the config file, the node will reindex on every startup, which is undesirable:
   ```irc
  17:14 <larryruane> michaelfolkson: Reindex is requested by the user (node operator) as a configuration option (command line or in the config file, tho you probably would never put it in the file, or else it would reindex on every startup!)
  ```

  This PR also has a commit to warn if `reindex=1` is set in the config file.

ACKs for top commit:
  hebasto:
    ACK deba6fe3158cd0b2283e0901a072e434ba5b594e, tested on Ubuntu 22.04.
  aureleoules:
    tACK deba6fe3158cd0b2283e0901a072e434ba5b594e
  ryanofsky:
    Code review ACK deba6fe3158cd0b2283e0901a072e434ba5b594e.

Tree-SHA512: 619fd0aa14e98af1166d6beb92651f5ba3f10d38b8ee132957f094f19c3a37313d9f4d7be2e4019f3fc9a2ca5fa42d03eb539ad820e27efec7ee58a26eb520b1
2022-10-21 16:39:44 +08:00
fanquake
6da45649c2
Merge bitcoin/bitcoin#26259: test: Test year 2106 block timestamps
fafc96aaf4c92feec50074e34a3fc1edc13cab4c test: Test year 2106 block timestamps (MacroFake)

Pull request description:

  Alternative to https://github.com/bitcoin/bitcoin/pull/21362 that closes https://github.com/bitcoin/bitcoin/issues/21356

ACKs for top commit:
  Sjors:
    utACK fafc96a

Tree-SHA512: 196d98f42d6f7f0222312b7bd1c68b3bd30cb6f0cbaccb900cfc5fcc689494adb2a7d7d6023c1ff1e8cf871047ec37eeca41386e31029d99cabf9343b4fd2a03
2022-10-21 16:29:52 +08:00
fanquake
085f83940d
Merge bitcoin/bitcoin#26344: wallet: Fix sendall with watchonly wallets and specified inputs
315fd4dbabb6b631b755811742a3bdf93e1241bf test: Test for out of bounds vout in sendall (Andrew Chow)
b132c85650afb2182f2e58e903f3d6f86fd3fb22 wallet: Check utxo prevout index out of bounds in sendall (Andrew Chow)
708b72b7151c855cb5dac2fb6a81e8f35153c46f test: Test that sendall works with watchonly spending specific utxos (Andrew Chow)
6bcd7e2a3b52f855db84cd23b5ee70d27be3434f wallet: Correctly check ismine for sendall (Andrew Chow)

Pull request description:

  The `sendall` RPC would previously fail when used with a watchonly wallet and specified inputs. This failure was caused by checking isminetype equality with ISMINE_ALL rather than a bitwise AND as IsMine can never return ISMINE_ALL.

  Also added a test.

ACKs for top commit:
  w0xlt:
    ACK 315fd4dbab
  furszy:
    ACK 315fd4db

Tree-SHA512: fb55cf6524e789964770b803f401027319f0351433ea084ffa7c5e6f1797567a608c956b7f7c5bd542aa172c4b7b38b07d0976f5ec587569efead27266e8664c
2022-10-21 16:24:15 +08:00
Andrew Chow
315fd4dbab test: Test for out of bounds vout in sendall 2022-10-20 13:25:13 -04:00
Andrew Chow
708b72b715 test: Test that sendall works with watchonly spending specific utxos 2022-10-20 13:21:03 -04:00
MacroFake
fafc96aaf4
test: Test year 2106 block timestamps
* Use maximum timestamp in getblocktemplate test
* Mine block with maximum timestamp and MTP in blockchain test
2022-10-20 14:45:50 +02:00
fanquake
2ac71d20b2
Merge bitcoin/bitcoin#25595: Verify PSBT inputs rather than check for fields being empty
e133264c5b1f72e94dcb9cebd85cdb523fcf8070 Add test for PSBT input verification (Greg Sanders)
d25699280af1ea45bebc884f63a10da7ea275ef9 Verify PSBT inputs rather than check for fields being empty (Greg Sanders)

Pull request description:

  In a few keys spots, PSBT finality is checked by looking for non-empty witness data.

  This complicates a couple things:
  1) Empty data can be valid in certain cases
  2) User may be passed bogus final data by a counterparty during PSBT work happening, and end up with incorrect signatures that they may not be able to check in other contexts if the UTXO doesn't exist yet in chain/mempool, timelocks, etc.

  On the whole I think these heavier checks are worth it in case someone is actually assuming the signatures are correct if our API is saying so.

ACKs for top commit:
  achow101:
    ACK e133264c5b1f72e94dcb9cebd85cdb523fcf8070

Tree-SHA512: 9de4fbb0be1257b081781f5df908fd55666e3acd5c4e36beb3b3f2f5a6aed69ff77068c44cde6127e159e773293fd9ced4c0bb47e693969f337e74dc8af030da
2022-10-20 08:13:14 +08:00
Sebastian Falbesoner
fa54d3011e test: check for false-positives in rpc_scanblocks.py 2022-10-20 01:33:17 +02:00
Sebastian Falbesoner
3bca6cd61a test: add compact block filter (BIP158) helper routines
By now, we add one helper for calculating ranged hashes and another one
for finding relevant scriptPubKeys given a block.
2022-10-20 01:33:17 +02:00
Sebastian Falbesoner
25ee74dd11 test: add SipHash implementation for generic data in Python
We will need this in the next commit to calculate ranged hashes
of scriptPubKeys as defined in BIP158.
2022-10-20 01:32:48 +02:00
Aurèle Oulès
e1eadaa72d
Revert "test: check importing wallets when blocks are pruned throw an error"
This reverts commit 4aff7a48a4e0f1075306f181a276b8a74c857022.
2022-10-19 16:51:47 +02:00
MacroFake
0aa641f5f4
Merge bitcoin/bitcoin#26206: test: check importing wallets when blocks are pruned throw an error
4aff7a48a4e0f1075306f181a276b8a74c857022 test: check importing wallets when blocks are pruned throw an error (brunoerg)

Pull request description:

  This PR adds test coverage for the following error:
  437b608df2/src/wallet/rpc/backup.cpp (L513-L518)

ACKs for top commit:
  andrewtoth:
    ACK 4aff7a48a4e0f1075306f181a276b8a74c857022

Tree-SHA512: fbbf6056cb3759f726b8a5ff25fca51bf47e973e5d655ec164e2bec88e2dbd3b243677869d2cf33af268ea635ca0f2e9f737c4734077fc5a936ac3a24ad4b88b
2022-10-19 09:11:47 +02:00
MacroFake
faf44876db
Move ::nMaxTipAge into ChainstateManager 2022-10-18 14:07:59 +02:00
Greg Sanders
e133264c5b Add test for PSBT input verification 2022-10-17 11:13:30 -04:00
Gleb Naumenko
e56d1d2afd test: Add functional tests for sendtxrcncl message from outbound 2022-10-17 12:36:14 +03:00
Gleb Naumenko
cfcef60779 test: Add functional tests for sendtxrcncl from inbound 2022-10-17 12:36:13 +03:00
Sebastian Falbesoner
ae3626ea52 test: use MiniWallet for rpc_scanblocks.py 2022-10-13 22:07:59 +02:00
Andrew Chow
92be831847
Merge bitcoin/bitcoin#25412: rest: add /deploymentinfo endpoint
a8250e30f16f2919ea5aa122b2880b076bd398a3 doc: add release note about `/rest/deploymentinfo` (brunoerg)
5c960200242d237f2cf74309b8fd29e8162682ed doc: add `/deploymentinfo` in REST-interface (brunoerg)
3e44bee08eb93e086179b92007649d47652aa439 test: add coverage for `/rest/deploymentinfo` (brunoerg)
91497031cbd74a0665b7fc31eb6b73bfb7bd0d40 rest: add `/deploymentinfo` (brunoerg)

Pull request description:

  #23508 added a new RPC named `getdeploymentinfo`, it moved the softfork section from `getblockchaininfo` into this new one. In the REST interface, we have an endpoint named`/rest/chaininfo.json` (which refers to `getblockchaininfo`), so, this PR adds a new REST endpoint named `/deploymentinfo` which refers to `getdeploymentinfo`.

  You can use it by passing a block hash, e.g: '/rest/deploymentinfo/<BLOCKHASH>.json' or you can use it without passing a block hash to get the 'deploymentinfo' for the last block.

ACKs for top commit:
  jonatack:
    re-ACK a8250e30f16f2919ea5aa122b2880b076bd398a3 rebase-only since my last review at c65f82bb
  achow101:
    ACK a8250e30f16f2919ea5aa122b2880b076bd398a3
  stickies-v:
    re-ACK a8250e30f1

Tree-SHA512: 0735183b6828d51a72ed0e2be5a09b314ac4693f548982c6e9adaa0ef07a55aa428d3b2d1b1de70b83169811a663a8624b686166e5797f624dcc00178b9796e6
2022-10-13 13:30:55 -04:00
Andrew Chow
5ff3d1e5ce
Merge bitcoin/bitcoin#24269: test: add functional test for -discover
bff05bd7456d3634b0c83539293a753db6d76376 test: add functional test for -discover (brunoerg)

Pull request description:

  This PR adds a functional test for `-discover`. It tests different scenarios where `localaddresses` should be empty or may contain the addresses. Obs: `localaddresses` is not always accurate, so it's not possible to ensure (100%) it will contain any addresses.

  515200298b/src/init.cpp (L449)

  Obs: See #24258  - It adds test coverage for this field but for nodes with proxy.

ACKs for top commit:
  mzumsande:
    Code review ACK bff05bd7456d3634b0c83539293a753db6d76376
  achow101:
    ACK bff05bd7456d3634b0c83539293a753db6d76376
  rajarshimaitra:
    tACK bff05bd7456d3634b0c83539293a753db6d76376

Tree-SHA512: 8782497c146bce1ba86fda6146f3847465d7069f2cb6b84f2afc8f3b43efa813442bffe7447e9ce02adee304100b60365409bf0e5d875dfb880038442feec2a6
2022-10-13 11:47:42 -04:00
Andrew Chow
bc2b1f0fe2
Merge bitcoin/bitcoin#23549: Add scanblocks RPC call (attempt 2)
626b7c8493ea1063f30ae4f62e1b36eb87adf685 fuzz: add scanblocks as safe for fuzzing (James O'Beirne)
94fe5453c7f8a86136dc9a6e0b370afd32564209 test: rpc: add scanblocks functional test (Jonas Schnelli)
6ef2566b68cb8570220c13df11c5cb5a5f4f7a4d rpc: add scanblocks - scan for relevant blocks with descriptors (Jonas Schnelli)
a4258f6e81a476ce1687e2d58f7d2bf16162a172 rpc: move-only: consolidate blockchain scan args (James O'Beirne)

Pull request description:

  Revives #20664. All feedback from the previous PR has either been responded to inline or incorporated here.

  ---

  Major changes from Jonas' PR:
  - consolidated arguments for scantxoutset/scanblocks
  - substantial cleanup of the functional test

  Here's the range-diff (`git range-diff master jonasschnelli/2020/12/filterblocks_rpc jamesob/2021-11-scanblocks`): https://gist.github.com/jamesob/aa4a975344209f0316444b8de2ec1d18

  ### Original PR description

  > The `scanblocks` RPC call allows one to get relevant blockhashes from a set of descriptors by scanning all blockfilters in a given range.
  >
  > **Example:**
  >
  > `scanblocks start '["addr(<bitcoin_address>)"]' 661000` (returns relevant blockhashes for `<bitcoin_address>` from blockrange 661000->tip)
  >
  > ## Why is this useful?
  > **Fast wallet rescans**: get the relevant blocks and only rescan those via `rescanblockchain getblockheader(<hash>)[height] getblockheader(<hash>)[height])`. A future PR may add an option to allow to provide an array of blockhashes to `rescanblockchain`.
  >
  > **prune wallet rescans**: (_needs additional changes_): together with a call to fetch blocks from the p2p network if they have been pruned, it would allow to rescan wallets back to the genesis block in pruned mode (relevant #15946).
  >
  > **SPV mode** (_needs additional changes_): it would be possible to build the blockfilterindex from the p2p network (rather then deriving them from the blocks) and thus allow some sort of hybrid-SPV mode with moderate bandwidth consumption (related #9483)

ACKs for top commit:
  furszy:
    diff re-ACK 626b7c8

Tree-SHA512: f84e4dcb851b122b39e9700c58fbc31e899cdcf9b587df9505eaf1f45578cc4253e89ce2a45d1ff21bd213e31ddeedbbcad2c80810f46755b30acc17b07e2873
2022-10-13 10:48:16 -04:00
Andrew Chow
6912a28f08
Merge bitcoin/bitcoin#25667: assumeutxo: snapshot initialization
bf9597606166323158bbf631137b82d41f39334f doc: add note about snapshot chainstate init (James O'Beirne)
e4d799528696c5ede38c257afaffd367917e0de8 test: add testcases for snapshot initialization (James O'Beirne)
cced4e7336d93a2dc88e4a61c49941887766bd72 test: move-only-ish: factor out LoadVerifyActivateChainstate() (James O'Beirne)
51fc9241c08a00f1f407f1534853a5cddbbc0a23 test: allow on-disk coins and block tree dbs in tests (James O'Beirne)
3c361391b8f5971eb3c7b620aa7ad9b437cc515e test: add reset_chainstate parameter for snapshot unittests (James O'Beirne)
00b357c215ed900145bd770525a341ba0ed9c027 validation: add ResetChainstates() (James O'Beirne)
3a29dfbfb2c16a50d854f6f81428a68aa9180509 move-only: test: make snapshot chainstate setup reusable (James O'Beirne)
8153bd9247dad3982d54488bcdb3960470315290 blockmanager: avoid undefined behavior during FlushBlockFile (James O'Beirne)
ad67ff377c2b271cb4683da2fb25fd295557f731 validation: remove snapshot datadirs upon validation failure (James O'Beirne)
34d159033106cc595cfa852695610bfe419c989c add utilities for deleting on-disk leveldb data (James O'Beirne)
252abd1e8bc5cdf4368ad55e827a873240535b28 init: add utxo snapshot detection (James O'Beirne)
f9f1735f139b6a1f1c7fea50717ff90dc4ba2bce validation: rename snapshot chainstate dir (James O'Beirne)
d14bebf100aaaa25c7558eeed8b5c536da99885f db: add StoragePath to CDBWrapper/CCoinsViewDB (James O'Beirne)

Pull request description:

  This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606)

  ---

  Half of the replacement for #24232. The original PR grew larger than expected throughout the review process.

  This change adds the ability to initialize a snapshot-based chainstate during init if one is detected on disk. This is of course unused as of now (aside from in unittests) given that we haven't yet enabled actually loading snapshots.

  Don't be scared! There are some big move-only commits in here.

  Accompanying changes include:

  - moving the snapshot coinsdb directory from being called `chainstate_[base blockhash]` to `chainstate_snapshot`, since we only support one snapshot in use at a time. This simplifies some logic, but it necessitates writing that base blockhash out to a file within the coinsdb dir. See [discussion here](https://github.com/bitcoin/bitcoin/pull/24232#discussion_r832762880).
  - adding a simple fix in `FlushBlockFile()` that avoids a crash when attemping to flush to disk before `LoadBlockIndexDB()` is called, which happens when calling `MaybeRebalanceCaches()` during multiple chainstate init.
  - improving the unittest to allow testing with on-disk chainstates - necessary to test a simulated restart and re-initialization.

ACKs for top commit:
  naumenkogs:
    utACK bf9597606166323158bbf631137b82d41f39334f
  ariard:
    Code Review ACK bf9597606
  ryanofsky:
    Code review ACK bf9597606166323158bbf631137b82d41f39334f. Changes since last review: rebasing, switching from CAutoFile to AutoFile, adding comments, switching from BOOST_CHECK to Assert in test util, using chainman.GetMutex() in tests, destroying one ChainstateManager before creating a new one in tests
  fjahr:
    utACK bf9597606166323158bbf631137b82d41f39334f
  aureleoules:
    ACK bf9597606166323158bbf631137b82d41f39334f

Tree-SHA512: 15ae75caf19f8d12a12d2647c52897904d27b265a7af6b4ae7b858592eeadb8f9da6c2394b6baebec90adc28742c053e3eb506119577dae7c1e722ebb3b7bcc0
2022-10-13 10:19:27 -04:00
glozow
147d64dbdf
Merge bitcoin/bitcoin#25858: psbt: Only include PSBT_OUT_TAP_TREE when the output has a script path
9e386afb67bf8fa71b72f730da1695eeb11828cd tests: Test that PSBT_OUT_TAP_TREE is included correctly (Andrew Chow)
30ff25cf37eec4b09ab40424eb5d6a4a80410955 psbt: Only include m_tap_tree if it has scripts (Andrew Chow)
0577d423adda8e719d7611d03355680c8fbacab8 psbt: Change m_tap_tree to store just the tuples (Andrew Chow)
22c051ca70bae73e0430b05fb9d879591df27699 tests: Test that PSBT_OUT_TAP_TREE is combined correctly (Andrew Chow)
7df6e1bb77a96eac4fbcba424bbe780636b86650 psbt: Fix merging of m_tap_tree (Andrew Chow)
0652dc53b291bd295caff4093ec2854fd4b34645 [BugFix]: Do not allow deserializing PSBT with empty PSBT_OUT_TAP_TREE (Jeremy Rubin)

Pull request description:

  PSBT_OUT_TAP_TREE should not be included for outputs that do not have such a tree. This should be disallowed during parsing, as well as prior to serialization when the field is populated during updating.

  Also added some test cases.

  Alternative to #25856

ACKs for top commit:
  instagibbs:
    ACK 9e386afb67
  darosior:
    ACK 9e386afb67bf8fa71b72f730da1695eeb11828cd

Tree-SHA512: ce5c02a69752d176dbd967c1e8d30129b1905c8f186aeeef034576c1de82059271a1ee846bd040f5be4e66bb77ba711dcf14ac1e597c5707d7e7e2293f6cfefb
2022-10-13 09:40:27 -04:00
fanquake
1d277f4223
Merge bitcoin/bitcoin#26280: rpc: Return coinbase flag in scantxoutset
fa08663344eb4cea335804d149b46ff4ff081b1c rpc: Return coinbase flag in scantxoutset (MacroFake)

Pull request description:

  I guess it can't hurt to return this for someone that wants to know it

ACKs for top commit:
  aureleoules:
    ACK fa08663344eb4cea335804d149b46ff4ff081b1c
  shaavan:
    ACK fa08663344eb4cea335804d149b46ff4ff081b1c

Tree-SHA512: 04c554b3ed9877bab93ffcf0c1a4430cd41b30c5f4f3bf462a518fc8b3d68832dd85a29e81bd805eaa16e987856933d7a888a8c126f670bb2844bbd5ca1bf902
2022-10-12 10:28:32 +08:00
fanquake
5fc3939850
Merge bitcoin/bitcoin#22087: Validate port-options
04526787b5f6613d1f1ad78434e1dd24ab88dd76 Validate `port` options (amadeuszpawlik)
f8387c42343867779170a0f96ef64e6acff5c481 Validate port value in `SplitHostPort` (amadeuszpawlik)

Pull request description:

  Validate `port`-options, so that invalid values are rejected early in the startup.
  Ports are `uint16_t`s, which effectively limits a port's value to <=65535. As discussed in https://github.com/bitcoin/bitcoin/pull/24116 and https://github.com/bitcoin/bitcoin/pull/24344, port "0" is considered invalid too.
  Proposed in https://github.com/bitcoin/bitcoin/issues/21893#issuecomment-835784223

  The `SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut)` now returns a bool that indicates whether the port value was set and within the allowed range. This is an improvement that can be used not only for port validation of options at startup, but also in rpc calls, etc,

ACKs for top commit:
  luke-jr:
    utACK 04526787b5f6613d1f1ad78434e1dd24ab88dd76
  ryanofsky:
    Code review ACK 04526787b5f6613d1f1ad78434e1dd24ab88dd76. Just suggested changes since last review: reverting some SplitHostPort changes, adding release notes, avoiding 'GetArgs[0]` problem.

Tree-SHA512: f1ac80bf98520b287a6413ceadb41bc3a93c491955de9b9319ee1298ac0ab982751905762a287e748997ead6198a8bb7a3bc8817ac9e3d2468e11ab4a0f8496d
2022-10-12 08:59:18 +08:00
Leonardo Araujo
2dede9f675 Adjust RPCTypeCheckObj error string 2022-10-10 18:08:00 -03:00
Ben Woosley
6630a1e844
Add warning on first startup if free disk space is less than necessary
To accommodate the expected blocks data.

Co-authored-by: Antoine Poinsot <darosior@protonmail.com>
Co-authored-by: benthecarman <benthecarman@live.com>
Co-authored-by: Justin Litchfield <litch@me.com>
Co-authored-by: Liran Cohen <c.liran.c@gmail.com>
Co-authored-by: Ryan Loomba <ryan.loomba@gmail.com>
Co-authored-by: Buck Perley <bucko.perley@gmail.com>
Co-authored-by: bajjer <bajjer@bajjer.xyz>
Co-authored-by: Suhail Saqan <suhail.saqan@gmail.com>
Co-authored-by: Christopher Sweeney <sweeney.chris@gmail.com>
Co-authored-by: Alyssa <orbitalturtle@protonmail.com>
Co-authored-by: Ben Schroth <ben@styng.social>
Co-authored-by: Jason Hester <mail@jason-hester.me>
Co-authored-by: Matt Clough <Matt.clough@pm.me>
Co-authored-by: Elise Schedler <eliseschedler@gmail.com>
Co-authored-by: ghander <cen254@gmail.com>
Co-authored-by: PopeLaz <btclz@fastmail.com>
Co-authored-by: Aurèle Oulès <hello@aureleoules.com>
2022-10-10 16:58:14 -04:00
MacroFake
fa08663344
rpc: Return coinbase flag in scantxoutset 2022-10-07 15:04:28 +02:00
MacroFake
fa8a305ddd
test: Remove confusing DUMMY_P2WPKH_SCRIPT 2022-10-07 13:11:05 +02:00
josibake
deba6fe315
test: update feature_config_args.py
add two new test cases for conf and reindex
2022-10-06 18:20:30 -04:00
Andrew Chow
9e386afb67 tests: Test that PSBT_OUT_TAP_TREE is included correctly 2022-10-06 15:32:51 -04:00
Andrew Chow
22c051ca70 tests: Test that PSBT_OUT_TAP_TREE is combined correctly 2022-10-06 15:32:45 -04:00
muxator
9153ff3e27 rpc: add non-regression test about deriveaddresses crash when index is 2147483647
This test would cause a crash in bitcoind (see #26274) if the fix given in the
previous commit was not applied.
2022-10-06 12:03:36 +02:00
amadeuszpawlik
04526787b5 Validate port options
Check `port` options for invalid values (ports are parsed as uint16, so
in practice values >65535 are invalid; port 0 is undefined and therefore
considered invalid too). This allows for an early rejection of faulty
values and an supplying an informative message to the user.

Splits tests in `feature_proxy.py` to cover both invalid `hostname`
and `port` values.

Adds a release-note as previously valid `-port` and `-rpcport` values
can now result in errors.
2022-10-05 19:30:15 +02:00
Jonas Schnelli
94fe5453c7 test: rpc: add scanblocks functional test
Co-authored-by: James O'Beirne <james.obeirne@gmail.com>
2022-10-04 13:51:35 -04:00
MacroFake
dddd1acf58
net: Set relay in version msg to peers with relay permission 2022-10-04 16:07:00 +02:00
w0xlt
17cad44851 test: refactor RPCPackagesTest to use MiniWallet 2022-10-03 22:37:40 -03:00
brunoerg
bff05bd745 test: add functional test for -discover 2022-09-30 08:26:45 -03:00
brunoerg
4aff7a48a4 test: check importing wallets when blocks are pruned throw an error 2022-09-29 14:43:43 -03:00
glozow
b2da6dd943
Merge bitcoin/bitcoin#26138: test: Avoid race in disconnect_nodes helper
faeea28753a94c45618c1b0ba83bb8700c53009a test: Avoid race in disconnect_nodes helper (MacroFake)

Pull request description:

  Also wait for the other node to notice the closed socket. Otherwise, the other node is not able to use the connect helper.

  Fixes https://github.com/bitcoin/bitcoin/issues/26014

ACKs for top commit:
  stickies-v:
    ACK faeea2875
  glozow:
    ACK faeea28753a94c45618c1b0ba83bb8700c53009a

Tree-SHA512: 2f0fa6812c0519aba3eaf21f0c70073b768fcd4dad23989d57e138ee9057a7da1a6b281645e9bff4051259cdca51568700e066491ac6b6daae99f30e395159ca
2022-09-28 16:21:19 +01:00
Sebastian Falbesoner
d99af861d0 test: check that listdescriptors descriptor strings are sorted
Tests the change introduced in PR #25931 ("rpc: sort listdescriptors
result", commit 50996241f2b0eefeaab4fcd11b9730fa2dc107ae).
2022-09-22 19:29:43 +02:00
fanquake
b1f44ecdcd
Merge bitcoin/bitcoin#25737: rpc: treat univalue type check error as RPC_TYPE_ERROR, not RPC_MISC_ERROR
e68d380797918e655decb76fc11725197d6d5323 rpc: remove unneeded RPCTypeCheckArgument checks (furszy)
55566630c60d23993a52ed54c95e7891f4588d57 rpc: treat univalue type check error as RPC_TYPE_ERROR, not RPC_MISC_ERROR (furszy)

Pull request description:

  Same rationale as #26039, tackling another angle of the problem.

  #### Context
  We have the same univalue type error checking code spread/duplicated few times:
  `RPCTypeCheckObj`, `RPCTypeCheckArgument`, `UniValue::checkType`.

  In the first two functions, we are properly returning an `RPC_TYPE_ERROR` while in `UniValue::checkType`
  we are throwing an `std::runtime_error` which is caught by the RPC server request handler, who invalidly
  treats it as `RPC_MISC_ERROR` (which is a generic error return code that provides no information to the user).

  #### Proposed Changes

  Throw a custom exception from `Univalue::checkType` (instead of a plain
  `std::runtime_error`) and catch it on the RPC server request handler.

  So we properly return `RPC_TYPE_ERROR` (-3) on every arg type error and
  not the general `RPC_MISC_ERROR` (-1).

  This will allow us to remove all the `RPCTypeCheckArgument` calls. As them are redundant since #25629.

Top commit has no ACKs.

Tree-SHA512: 4e4c41851fd4e2b01a2d8b94e71513f9831f810768ebd89684caca4901e87d3677980003949bcce441f9ca607a1b38a5894839b6c492f5947b8bab8cd9423ba6
2022-09-21 11:19:44 +01:00
fanquake
97f865bb76
Merge bitcoin/bitcoin#25989: init: abort if i2p/cjdns are chosen via -onlynet but are unreachable
68209a7b5c0326e14508d9cf749771605bd6ffe7 rpc: make addpeeraddress work with cjdns addresses (Martin Zumsande)
a8a9ed67cc447d204304ccfd844c45fd76486c6a init: Abort if i2p/cjdns are chosen via -onlynet but unreachable (Martin Zumsande)

Pull request description:

  If the networks i2p / cjdns are chosen via `-onlynet` but the user forgot to provide `-i2psam` / `-cjdnsreachable`, no outbound connections will be made - it would be nice to inform the user about that.
  The solution proposed here mimics existing behavior for `-onlynet=onion` and non-specified `-onion`/`-proxy` where we already abort with an InitError - if reviewers would prefer to just print a warning, please say so.

  The second commit adds CJDNS support to the debug-only `addpeeraddress` RPC allowing to add CJDNS addresses to addrman for testing and debug purposes. (if `-cjdnsreachable=1`)

  This is the result of an [IRC discussion](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2022-09-01#848066;) with vasild.

ACKs for top commit:
  vasild:
    ACK 68209a7b5c0326e14508d9cf749771605bd6ffe7
  dergoegge:
    ACK 68209a7b5c0326e14508d9cf749771605bd6ffe7

Tree-SHA512: 6db9787f01820190f14f90a0b39e4206603421eb7521f792879094d8bbf4d4d0bfd70665eadcc40994ac7941a15ab5a8d65c4779fba5634c0e6fa66eb0972b8d
2022-09-21 11:00:47 +01:00
MacroFake
7184fb866f
Merge bitcoin/bitcoin#26143: test: wait for the expected basic block filter index in interface_rest
aee29c5d9ebd8b68fc7e6607d58284f9b91078d1 test: wait for the expected basic block filter index in `interface_rest` (brunoerg)

Pull request description:

  Fixes #26098

  Wait for the expected 'basic block filter index' to not cause issues when calling `/blockfilterheaders/basic/`, like:
  9bd842a592/src/rest.cpp (L423-L424)

Top commit has no ACKs.

Tree-SHA512: 36abe28a76cd01ce7ac1ae9258ce1a9a5473d985c498d915c1130256bc800d0d1207708a195b78bbcb00478ec9c373a2fbfeb26c1fddeb35abe8b253e0308058
2022-09-21 09:55:07 +02:00
brunoerg
aee29c5d9e test: wait for the expected basic block filter index in interface_rest 2022-09-20 14:10:34 -03:00