Commit Graph

37 Commits

Author SHA1 Message Date
Samuel Dobson
4fcd7850b0 Merge #17578: rpc: simplify getaddressinfo labels, deprecate previous behavior
8925df86c4df16b1070343fef8e4d238f3cc3bd1 doc: update release notes (Jon Atack)
8bb405bbadf11391ccba7b334b4cfe66dc85b390 test: getaddressinfo labels purpose deprecation test (Jon Atack)
60aba1f2f11529add115d963d05599130288ae28 rpc: simplify getaddressinfo labels, deprecate previous behavior (Jon Atack)
7851f14ccf2bcd1e9b2ad48e5e08881be06d9d21 rpc: incorporate review feedback from PR 17283 (Jon Atack)

Pull request description:

  This PR builds on #17283 (now merged) and is followed by #17585.

  It modifies the value returned by rpc getaddressinfo `labels` to an array of label name strings and deprecates the previous behavior of returning an array of JSON hash structures containing label `name` and address `purpose` key/value pairs.

  before
  ```
    "labels": [
      {
        "name": "DOUBLE SPEND",
        "purpose": "receive"
      }
  ```
  after
  ```
    "labels": [
      "DOUBLE SPEND"
    ]
  ```

  The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=labelspurpose`.

  For context, see:
  - https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001
  - http://www.erisian.com.au/bitcoin-core-dev/log-2019-12-13.html#l-425 (lines 425-427)
  - http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622

  Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=labelspurpose` flag while verifying the rpc getaddressinfo help text and `labels` output.

  Next steps: deprecate the rpc getaddressinfo `label` field (EDIT: done in #17585) and add support for multiple labels per address. This PR will unblock those.

ACKs for top commit:
  jnewbery:
    reACK 8925df8
  promag:
    Code review ACK 8925df86c4df16b1070343fef8e4d238f3cc3bd1.
  meshcollider:
    Code review ACK 8925df86c4df16b1070343fef8e4d238f3cc3bd1

Tree-SHA512: c2b717209996da32b6484de7bb8800e7048410f9ce6afdb3e02a6866bd4a8f2c730f905fca27b10b877b91cf407f546e69e8c4feb9cd934325a6c71c166bd438
2023-04-25 23:14:25 +03:00
Wladimir J. van der Laan
00b3dbd877 Merge #17283: rpc: improve getaddressinfo test coverage, help, code docs
33f5fc32e5bfbe1e89c4d20ce455bcc6dc194151 test: add rpc getaddressinfo labels test coverage (Jon Atack)
0f3539ac6d772fc646b5f184fa1efe77bf632f6a test: add listlabels test in wallet_labels.py (Jon Atack)
1388de83900eaced906d369fe9e8887ae74b2dcf rpc: add getaddressinfo code documentation (Jon Atack)
2ee0cb3330ccf70f0540cb42370796e32eff1569 rpc: update getaddressinfo RPCExamples to bech32 (Jon Atack)
8d1ed0c263f8cdff7189f02040b5d02238d93da0 rpc: clarify label vs labels in getaddressinfo RPCHelpman (Jon Atack)
5a0ed850700dfb19167d40b38f80313bd5e427ca rpc: improve getaddressinfo RPCHelpman content (Jon Atack)
70cda342cd20d0e0cd9f28405457544036968f2d rpc: improve getaddressinfo RPCHelpman formatting (Jon Atack)

Pull request description:

  This PR is a continuation of the work in https://github.com/bitcoin/bitcoin/pull/12892.

  Main motivations:
  - There is currently no test coverage for the getaddressinfo `labels` response. Coverage here is a prerequisite before deprecating the `label` response or adding multiple labels per address.
  - `bitcoin-cli help getaddressinfo` returns a few content errors, difficult-to-read formatting, and no explanation why it returns both `label` and `labels` and how they relate, which can be confusing for application developers.

  Changes by order of commits:
  - [x] improve/fix getaddressinfo RPCHelpman layout formatting
  - [x] improve/fix getaddressinfo RPCHelpman content
  - [x] clarify the `label` and `labels` fields in getaddressinfo RPCHelpman
  - [x] update getaddressinfo RPCExamples addresses to bech32
  - [x] add getaddressinfo code docs
  - [x] add a `listlabels` test assertion in wallet_labels.py
  - [x] add missing getaddressinfo `labels` test coverage and improve the existing `label` tests

  Here are gists of the CLI help output:
  [`bitcoin-cli help getaddressinfo` before this PR](https://gist.github.com/jonatack/022af5221a85c069780359a22643c810)
  [`bitcoin-cli help getaddressinfo` after this PR](https://gist.github.com/jonatack/4ee5f6abc62a3d99269570206a5f90ba)

  It seems we ought to begin a deprecation process for the getaddressinfo `label` field? If yes, I have a follow-up ready. _--> EDIT: Deprecation follow-ups #17578 and #17585 now build on this PR._

ACKs for top commit:
  fjahr:
    Re-ACK 33f5fc32e5bfbe1e89c4d20ce455bcc6dc194151
  jnewbery:
    ACK 33f5fc32e5bfbe1e89c4d20ce455bcc6dc194151.

Tree-SHA512: a001aa863090ec2566a31059477945b1c303ebeb430b33472f8b150e420fa5742fc33bca9d95571746395b607f43f6078dd5b53e238ac1f3fc648b51c8f79a07
2023-04-17 19:34:02 +03:00
Samuel Dobson
e4fcb170cf Merge #16873: rpc: fix regression in gettransaction
1b41c2c8a126ef4be183e1d800a17d85cab8837b test: improve gettransaction test coverage (Jon Atack)
0f34f54888f680bfbe7a29ac278636d7178a99bb rpc: fix regression in gettransaction (Jon Atack)

Pull request description:

  Closes #16872.

  PR #16866 renamed the `decode` argument in gettransaction to `verbose` to make it more consistent with other RPC calls like getrawtransaction. However, it inadvertently overloaded the "details" field when `verbose` is passed. The result is that the original "details" field is no longer returned correctly, which seems to be a breaking API change.

  This PR:

  - takes the simplest path to restoring the "details" field by renaming the decoded one back to "decoded" while leaving the `verbose` argument for API consistency, which was the main intent of #16866,

  - addresses [this comment](https://github.com/bitcoin/bitcoin/pull/16185#discussion_r320740413) by mentioning in the RPC help that the new decoded field is equivalent to decoderawtransaction, and

  - updates the help, functional test, and release note.

  Reviewers, to test this manually, build and run `bitcoin-cli help gettransaction` and `bitcoin-cli gettransaction <wallet txid> false true`, and verify that the command returns both `details` and `decoded` fields.

ACKs for top commit:
  jnewbery:
    tACK 1b41c2c8a126ef4be183e1d800a17d85cab8837b

Tree-SHA512: 287edd5db7ed58fe8b548975aba58628bd45ed708b28f40174f10a35a455d89f796fbf27430aa881fc376f47aabda8803f74d4d100683bd86577a02279091cf3
2023-04-06 20:14:58 +03:00
Samuel Dobson
234d472944 Merge #16866: wallet: Rename 'decode' argument in gettransaction method to 'verbose'
7dee8f48088c75ab0e51be60679505f8ce570919 [wallet] Rename 'decode' argument in gettransaction method to 'verbose' (John Newbery)

Pull request description:

  This makes the RPC method consistent with other RPC methods that have a
  'verbose' option.

  Change the name of the return object from 'decoded' to details.

  Update help text.

ACKs for top commit:
  promag:
    ACK 7dee8f48088c75ab0e51be60679505f8ce570919.
  meshcollider:
    Code review ACK 7dee8f48088c75ab0e51be60679505f8ce570919
  0xB10C:
    ACK 7dee8f48088c75ab0e51be60679505f8ce570919: reviewed code

Tree-SHA512: a3a62265c8e6e914591f3b3b9f9dd4f42240dc8dab9cbac6ed8d8b8319b6cc847db2ad1689d5440c162e0698f31e39fc6b868ed918b2f62879d61b9865cae66b
2023-04-06 20:14:58 +03:00
MeshCollider
4c72e6966d Merge #16185: gettransaction: add an argument to decode the transaction
9965940e35c445ccded55510348af228ff22f0e9 doc: Add release note for the new gettransaction argument (darosior)
b8b3f0435a2837d3897e9e232ef6ca839ce74eb8 tests: Add a new functional test for gettransaction (darosior)
7f3bb247a811582d1aa4805d8e601c19808dc7ba gettransaction: add an argument to decode the transaction (darosior)

Pull request description:

  This PR adds a new parameter to the `gettransaction` call : `decode`. If set to `true`, it will add a new `decoded` field to the response. This mimics the behavior of `getrawtransaction`'s `verbose` argument to avoid using 2 calls if we want to decode a wallet transaction (`gettransaction` then `decoderawtransaction`).

  Fix #16181 .

ACKs for top commit:
  meshcollider:
    re-utACK 9965940e35c445ccded55510348af228ff22f0e9

Tree-SHA512: bcb6b4bd252b3488d6afc77659c499c2ad99fd58661eb24b6a2e17014c74f22e47fde70e00fedb4f4754915786622ad02483b2cf2c4dea0ab0eb4ac8276dbeee
2023-04-06 20:14:58 +03:00
MarcoFalke
2cae37806b Merge #13424: Consistently validate txid / blockhash length and encoding in rpc calls
5eb20f81d9 Consistently use ParseHashV to validate hash inputs in rpc (Ben Woosley)

Pull request description:

  ParseHashV validates the length and encoding of the string and throws
  an informative RPC error on failure, which is as good or better than
  these alternative calls.

  Note I switched ParseHashV to check string length first, because
  IsHex tests that the length is even, and an error like:
  "must be of length 64 (not 63, for X)" is much more informative than
  "must be hexadecimal string (not X)" in that case.

  Split from #13420

Tree-SHA512: f0786b41c0d7793ff76e4b2bb35547873070bbf7561d510029e8edb93f59176277efcd4d183b3185532ea69fc0bbbf3dbe9e19362e8017007ae9d51266cd78ae
2023-04-06 20:14:58 +03:00
Wladimir J. van der Laan
d90a46175b Merge #19350: test: Refactor tests using restart_node
20b6e959449d0c07639599b99ba917d2cac62493 test: refactor functional tests to use restart_node (Christopher Coverdale)

Pull request description:

  fixes #19345

  This PR replaces consecutive calls to `stop_node()` and `start_node()` with `restart_node()` where appropriate in the functional tests.

  The commit messages are repetitive but focused on each file changed with the intention of squashing if applicable.

ACKs for top commit:
  laanwj:
    ACK 20b6e959449d0c07639599b99ba917d2cac62493

Tree-SHA512: 1cfa1fb8c5f01a7b00fe44e80dbef072147f21e3891098817acd4275b0c5d91dc1c787594209e117edd418f2fa3a7b2dfcbafdf87efc07f740040938d641f3a9
2023-02-27 23:12:41 -06:00
Wladimir J. van der Laan
0528026442 Merge #16929: test: follow-up to rpc: default maxfeerate value as BTC/kB
6659810e2f38994813aa9d7644d570ae0152fa2c test: use named args for sendrawtransaction calls (Jon Atack)
5c1cd78b7e582660a78d9d9dec673967a6b78936 doc: improve rawtransaction code/test docs (Jon Atack)
acc14c50932c7353f94d3d4367d05021606e0ca9 test: fix incorrect value in rpc_rawtransaction.py (Jon Atack)

Pull request description:

  Follow-up to PR #16521.

  - Fix incorrect value in rpc_rawtransaction test as per https://github.com/bitcoin/bitcoin/pull/16521/files#r325842308
  - Improve the code docs
  - Use named arguments as per https://github.com/bitcoin/bitcoin/pull/16521/files#r310715127

  Happy to squash or keep only the first commit if the others are too fixup-y.

ACKs for top commit:
  laanwj:
    ACK 6659810e2f38994813aa9d7644d570ae0152fa2c

Tree-SHA512: bf5258f23802ab3ba3defb8791097e08e63f3e2af21023f832cd270dc88d1fa04349e921d69f9f5fedac5dce5cd3c1cc46b48febbede4bc18dccb8be994565b2
2023-01-01 20:16:57 -06:00
Wladimir J. van der Laan
2660c9722b Merge #16521: rpc: Use the default maxfeerate value as BTC/kB
2dfd6834ef8737e16e4b96df0c459f30a0721d6c test: Add test for default maxfeerate in sendrawtransaction (Joonmo Yang)
261843e4bef96ab296a9775819a99bfa60cad743 wallet/rpc: Use the default maxfeerate value as BTC/kB (Joonmo Yang)

Pull request description:

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

  This patch tries to treat `maxfeerate` in sendrawtransaction/testmempoolaccept RPC as a rate(BTC/kB) instead of an absolute value(BTC).
  The included test case checks if the new behavior works correctly, by using the transaction with an absolute fee of ~0.02BTC, where the fee rate is ~0.2BTC/kB.
  This test should be failing if the default `maxfeerate` is 0.1BTC, but pass if the default value is 0.1BTC/kB

ACKs for top commit:
  laanwj:
    ACK 2dfd6834ef8737e16e4b96df0c459f30a0721d6c (ACKs by Sjors and MarcoFalke above for trivially different code)

Tree-SHA512: a1795bffe8a182acef8844797955db1f60bb0c0ded97148f3572dc265234d5219271a3a7aa0b6418a43f73b2b2720ef7412ba169c99bb1cdcac52051f537d6af
2023-01-01 20:16:57 -06:00
Kittywhiskers Van Gogh
e8bf39f2dc merge bitcoin#19967: Replace (dis)?connect_nodes globals with TestFramework methods
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2022-10-17 08:03:12 +05:30
Wladimir J. van der Laan
86a11a83a2
Merge #18873: test: Fix intermittent sync_blocks failures
fa3f9a05660687bf4146e089050e944a1d6cbe3c test: Fix intermittent sync_blocks failures (MarcoFalke)

Pull request description:

  Fixes #18872
  Fixes #18737
  Fixes #18801

  See docstring for motivation and description

ACKs for top commit:
  laanwj:
    Code review ACK fa3f9a05660687bf4146e089050e944a1d6cbe3c

Tree-SHA512: acd52d386a6849f7ff1cb1a51a439dc3c76e0e3a4dd8d22030df0ebb09b44497c61c56331ff65724b695d82d86b0ebb24608f9e637008e5dacb7676b0c448889
2022-09-07 22:25:40 +03:00
MarcoFalke
6dbc9aba0d Merge #17675: tests: Enable tests which are incorrectly skipped when running test_runner.py --usecli
5ac804a9eb0cdbdcff8b50ecfb736f8793cab805 tests: Use a default of supports_cli=True (instead of supports_cli=False) (practicalswift)
993e38a4e2fa66093314b988dfbe459f46aa5864 tests: Mark functional tests not supporting bitcoin-cli (--usecli) as such (practicalswift)

Pull request description:

  Annotate functional tests supporting `bitcoin-cli` (`--usecli`) as such.

  Prior to this commit 74 tests were unnecessarily skipped when running `test_runner.py --usecli`.

  Before [bitcoin original commit stats]:

  ```
  $ test/functional/test_runner.py --usecli > /dev/null 2>&1
  $ echo $?
  0
  $ test/functional/test_runner.py --usecli 2>&1 | cut -f2 -d'|' | \
      grep -E ' (Passed|Skipped) *$' | sort | uniq -c
        9  ✓ Passed
      126  ○ Skipped
  ```

  After [dash numbers]:

  ```
  $ test/functional/test_runner.py --usecli > /dev/null 2>&1
  $ echo $?
  0
  $ test/functional/test_runner.py --usecli 2>&1 | cut -f2 -d'|' | \
      grep -E ' (Passed|Skipped) *$' | sort | uniq -c
       110  ✓ Passed
       51  ○ Skipped
  ```

  Context: `--usecli` was introduced in f6ade9ce1a

ACKs for top commit:
  laanwj:
    Code review ACK 5ac804a9eb0cdbdcff8b50ecfb736f8793cab805

Tree-SHA512: 249c0b691a74cf201c729df86c3db2b3faefa53b94703941e566943d252c6d14924e935a8da4f592951574235923fbb7cd22612a5e7e02ff6c762c55a2320ca3
2022-06-08 12:35:12 +07:00
Kittywhiskers Van Gogh
01d00f3227 merge bitcoin#19671: Remove -zapwallettxes 2022-05-13 18:06:06 +05:30
MarcoFalke
0bbd8bfab6
Merge #14631: [tests] Move deterministic address import to setup_nodes
3fd7e76f6d [tests] Move deterministic address import to setup_nodes (John Newbery)

Pull request description:

  This requires a small changes to a few tests, but means that
  deterministic addresses will always be imported (unless setup_nodes
  behaviour is explicitly overridden).

  Tidies up the way we import deterministic addresses, requested in review comment here: https://github.com/bitcoin/bitcoin/pull/14468#discussion_r225594586.

Tree-SHA512: 2b32edf500e286c463398487ab1153116a1dc90f64a53614716373311abdc83d8a251fdd8f42d1146b56e308664deaf62952113f66e98bc37f23968096d1a961
2022-05-01 22:52:30 +03:00
Kittywhiskers Van Gogh
07a4d48249 merge bitcoin#18918: Move salvagewallet into wallettool 2022-03-06 12:30:20 +05:30
MarcoFalke
8ca90f3f99 Merge #15891: test: Require standard txs in regtest by default
fa89badf887dcc01e5bdece248b5e7d234fee227 test: Require standard txs in regtest (MarcoFalke)
fa9b4191609c3ef75e69d391eb91e4d5c1e0bcf5 test: Add test that mainnet requires standard txs (MarcoFalke)
fa613ca0a8f99c4771859de9e571878530d3ecb5 chainparams: Remove unused fMineBlocksOnDemand (MarcoFalke)

Pull request description:

  I don't see a reason why regtest should allow non-standard txs, as it makes testing mainnet behaviour such as #15846 unnecessarily hard and unintuitive.

  Of course, testnet policy remains unchanged to allow propagation of non-standard txs.

ACKs for top commit:
  ajtowns:
    ACK fa89badf887dcc01e5bdece248b5e7d234fee227

Tree-SHA512: c4c675affb054868850bd2683aa07f4c741a448cbacb2ea8334191e105f426b0790fe6a468be61e9c5880d24154f7bf1c7075051697172dce92180c1bc3a1c90
2022-01-30 18:31:00 -05:00
MarcoFalke
d03d75fba8
Merge #16898: test: Remove connect_nodes_bi
fadfd844de8c53034a97dfa6f771ffe9f523fba2 test: Remove unused connect_nodes_bi (MarcoFalke)
fa3b9ee8b2280af4bcbcfffff275aaf8dd125929 scripted-diff: test: Replace connect_nodes_bi with connect_nodes (MarcoFalke)
faaee1e39a91b3f603881655d3980c29af09852b test: Use connect_nodes when connecting nodes in the test_framework (MarcoFalke)
1111bb91f517838e5b9f778bf6b5a9c8d561e857 test: Reformat python imports to aid scripted diff (MarcoFalke)

Pull request description:

  By default all test nodes are connected in a chain. However, instead of just a single connection between each pair of nodes, we end up with up to four connections for a "middle" node (two outbound, two inbound, from each side).

  This is generally redundant (tx and block relay should succeed with just a single connection) and confusing. For example, test timeouts after a call to `sync_` may be racy and hard to reproduce. On top of that, the test `debug.log`s are hard to read because txs and block invs may be relayed on the same connection multiple times.

  Fix this by inlining `connect_nodes_bi` in the two tests that need it, and then replace it with a single `connect_nodes` in all other tests.

  Historic background:

  `connect_nodes_bi` has been introduced as a (temporary?) workaround for bug #5113 and #5138, which has long been fixed in #5157 and #5662.

ACKs for top commit:
  laanwj:
    ACK fadfd844de8c53034a97dfa6f771ffe9f523fba2
  jonasschnelli:
    utACK fadfd844de8c53034a97dfa6f771ffe9f523fba2 - more of less a cleanup PR.
  promag:
    Tested ACK fadfd844de8c53034a97dfa6f771ffe9f523fba2, ran extended tests.

Tree-SHA512: 2d027a8fd150749c071b64438a0a78ec922178628a7dbb89fd1212b0fa34febd451798c940101155d3617c0426c2c4865174147709894f1f1bb6cfa336aa7e24
2021-12-15 20:09:59 +05:30
Kittywhiskers Van Gogh
c04f74b1d4 merge bitcoin#15680: Remove resendwallettransactions RPC method 2021-12-12 21:07:49 +05:30
Kittywhiskers Van Gogh
269501259c merge bitcoin#14845: Add wallet_balance.py
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2021-12-12 19:46:11 +05:30
Kittywhiskers Van Gogh
c6c307b3f7 merge bitcoin#13541: sendrawtransaction maxfeerate 2021-12-12 11:57:44 +05:30
Wladimir J. van der Laan
71b4cf307b Merge #14180: qa: Run all tests even if wallet is not compiled
fac95398366f644911b58f1605e6bc37fb76782d qa: Run all tests even if wallet is not compiled (MarcoFalke)
faa669cbcd1fc799517b523b0f850e01b11bf40a qa: Premine to deterministic address with -disablewallet (MarcoFalke)

Pull request description:

  Currently the test_runner would exit if the wallet was not compiled into the Bitcoin Core executable. However, a lot of the tests run without the wallet just fine and there is no need to globally require the wallet to run the tests.

Tree-SHA512: 63177260aa29126fd20f0be217a82b10b62288ab846f96f1cbcc3bd2c52702437703475d91eae3f8d821a3149fc62b725a4c5b2a7b3657b67ffcbc81532a03bb
2021-09-21 17:24:56 -04:00
Wladimir J. van der Laan
7006a38ba5 Merge #15404: [test] Remove -txindex to start nodes
8e4b4f683a0b342cec24cd51b1e98433034ea2ea Address test todos by removing -txindex to nodes. Originally added when updating getrawtransaction to stop searching unspent utxos. (Amiti Uttarwar)

Pull request description:

  Original todos added when removing getrawtransaction default behavior of searching unspent utxos.

Tree-SHA512: d080953c3b0d2e5dca2265a15966dc25985a614c9cc86271ecd6276178ce428c85e262c24df92501695c32fed7beec0339b989f03cce91b57fb2efba201b7809
2021-09-13 16:04:36 -04:00
MarcoFalke
8226f22efb Merge #15247: qa: Use wallet to retrieve raw transactions
fa5278a419 qa: Use wallet to retrieve raw transactions (MarcoFalke)
fa2198328e qa: Style-only fixes in touched files (MarcoFalke)

Pull request description:

  Instead of asking the coin database and block storage about a transaction, pull it directly from the wallet in wallet related tests.

  This refactoring only makes sense in light of #15159.

  <sub>This product may contain minor stylistic cleanups

Tree-SHA512: ec34c7150d873da9f19fead3f7e3f758baba5ef10061942384c470a47a6f320690109be9c5160f0c8bc228272a729653d44c78471455337318f657d6c164ba23
2021-08-16 23:56:24 -03:00
Kittywhiskers Van Gogh
e8e48b33bb merge #15159: Remove lookup to UTXO set from GetTransaction
No need for extra `-txindex` in Dash-specific tests, it's `true` by default
2021-08-09 12:38:04 +05:30
MarcoFalke
2f59f766d6 Merge #14410: rpcwallet: 'ischange' field for 'getaddressinfo' RPC
14a06525b2 tests: add test for 'getaddressinfo' RPC result 'ischange' field (whythat)
93d1aa9abc rpcwallet: add 'ischange' field to 'getaddressinfo' response (whythat)

Pull request description:

  Implementation of proposal in #14396.

  This introduces `CWallet::IsChange(CScript&)` method and replaces original `CWallet::IsChange(CTxOut&)` method with overloaded version that delegates to the new method with *txout*'s `scriptPubKey`. In this way `TODO` note from the original method can still be addressed in a single place.

Tree-SHA512: ef5dbc82d76b4b9b2fa6a70abc3385a677c55021f79e187ee2f392ee32bc6b406191f4129acae5c17b0206e72b6712e7e0cad574a4bbd966871c2e656c45e041

# Conflicts:
#	doc/release-notes-14282.md
#	src/wallet/rpcwallet.cpp
2021-07-28 12:08:07 -04:00
MarcoFalke
76eb5e3a03 Merge #19887: test: Fix flaky wallet_basic test
56b018ca7f37d25041b74f1bec305bdf54a55b9b test: Fix flaky wallet_basic test (Fabian Jahr)

Pull request description:

  Fixes #19853

  I investigated the issue in #19876 and I still intend to fix the underlying issue of a race when using wallet RPCs right after starting a node in that PR. However, since that is a bit more complicated than I initially thought it makes sense to merge the fix of the test so the intermittent test failures stop. This fix in the test is going to be needed, either way, #19876 will only provide an error where before it was reporting a false balance.

Top commit has no ACKs.

Tree-SHA512: 52bb2388a3e77aa20d26ab0fd45796bc1781483b1cffe49cbb44e2488a72e76998edfb1198495373f9c6fd2ec26064d4176bd1a64dd59806622d5e50a4f4e870
2021-07-15 19:30:07 -05:00
PastaPastaPasta
0f648122cd
Update test/functional/wallet_basic.py
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2021-06-30 18:12:37 -05:00
MarcoFalke
ca9247fac4
Merge #15378: tests: Added missing tests for RPC wallet errors
dc3b2ccb5f tests: Added missing tests for RPC wallet errors (Ben Carman)

Pull request description:

Tree-SHA512: b18dcd4f7547c974c93ae67dcd92a168bdb55951b164cf174cb1e59e0daa463187068aec43108309a75d65721a5c0bcdf10a16a9869620f160121e2287559926
2021-06-28 19:05:21 -05:00
Wladimir J. van der Laan
cc38451955
Merge #12755: [tests] Better stderr testing
beee49b [tests] Allow stderr to be tested against specified string (John Newbery)
e503671 [Tests] Use LIBC_FATAL_STDERR_=1 in tests (John Newbery)
c22ce8a [Tests] Write stdout/stderr to datadir instead of temp file. (John Newbery)

Pull request description:

  **Due to a merge conflict, this is now based on #10267. Please review that PR first!**

  Subset of #12379 now that parts of that PR have been merged.

  #12362 was only observed when running the functional tests locally because:

  - by defatul libc logs to `/dev/tty` instead of stderr
  - the functional tests only check for substring inclusion in stderr when we're expecting bitcoind to fail.

  This PR tightens our checking of stderr and will cause tests to fail if there is any unexpected message in stderr:

  - commit *Write stdout/stderr to datadir instead of temp file* writes stderr to a file in the datadir instead of a temporary file. This helps with debugging in the case of failure.
  - commit *Use LIBC_FATAL_STDERR=1 in tests* ensures that libc failures are logged to stderr instead of the terminal.
  commit *Assert that bitcoind stdout is empty on shutdown* asserts that stderr is empty on bitcoind shutdown.

Tree-SHA512: 21111030e667b3b686f2a7625c2b625ebcfb6998e1cccb4f3932e8b5d21fb514b19a73ac971595d049343430e9a63155986a7f5648cad55b8f36f3c58b1c7048
2021-06-28 02:00:44 +03:00
MarcoFalke
f1ec546b35
Merge #13136: [tests] Fix flake8 warnings in several wallet functional tests
a533834d50 [tests] Fix flake8 warnings in several wallet functional tests (John Newbery)

Pull request description:

  Fixes flake8 warnings in several wallet functional tests.

  Several wallet functional tests need rewrite to remove the accounts API (#13075). To prepare for that, I fixed all the flake8 warnings in those tests.

  #13075 is blocked on a bitcoind bug. This PR is just the flake8 fixes so we're not completely blocked.

Tree-SHA512: 2dc1d589b2f8f4318083a681e487532d0f8f3d57e8bc8f37b660b728ffc33329b88e9251eb223104aea89f293c3f4074ca700fe690e645617326b859da3e93c3
2021-05-25 14:09:35 +03:00
Pieter Wuille
341b5a9714 Merge #13566: Fix get balance
702ae1e21a [RPC] [wallet] allow getbalance to use min_conf and watch_only without accounts. (John Newbery)
cf15761f6d [wallet] GetBalance can take a min_depth argument. (John Newbery)
0f3d6e9ab7 [wallet] factor out GetAvailableWatchOnlyBalance() (John Newbery)
7110c830f8 [wallet] deduplicate GetAvailableCredit logic (John Newbery)
ef7bc8893c [wallet] Factor out GetWatchOnlyBalance() (John Newbery)
4279da4785 [wallet] GetBalance can take an isminefilter filter. (John Newbery)

Pull request description:

  #12953 inadvertently removed the functionality to call `getbalance "*" <int> <bool>` to get the wallet's balance with either minconfs or include_watchonly.

  This restores that functionality (when `-deprecatedrpc=accounts`), and also makes it possible to call ``getbalance minconf=<int> include_watchonly=<bool>` when accounts are not being used.

Tree-SHA512: 67e84de9291ed6d34b23c626f4dc5988ba0ae6c99708d02b87dd3aaad3f4b6baa6202a66cc2dadd30dd993a39de8036ee920fcaa8cbb1c5dfe606e6fac183344
2020-12-22 17:58:12 +01:00
MarcoFalke
42a98b0bc8 Merge bitcoin#13075: [tests] Remove 'account' API from wallet functional tests
5d536619ab [tests] Remove 'account' API from wallet functional tests (John Newbery)

Pull request description:

  Next step in #12952. Removes all usage of the 'account' API from the wallet functional tests, except:

  - rpc_deprecated.py (which specifically tests the `-deprecatedrpc=accounts` command line argument is working properly).
  - `wallet_labels.py` (which tests that both the 'label' and 'account' APIs work in V0.17).

  'account' API usage for both of those tests can be removed once V0.17 has been branched.

  Also excluded is:

  - `wallet_importprunedfunds.py` (which fails due to a bitcoind OOM error)

Tree-SHA512: 6701b32f83d2d47597ba093ded665d7aa630f7a9c759ff15e3e33a3e3bc7600e8d29cf4e72aed5f8f9f6769cc9b614c681951720eab1ed2473f5f8dec57e7a6f
2020-12-22 17:58:11 +01:00
Wladimir J. van der Laan
95cfaeea17 Merge bitcoin#12953: Deprecate accounts
cead28b [docs] Add release notes for deprecated 'account' API (John Newbery)
72c9575 [wallet] [tests] Add tests for accounts/labels APIs (John Newbery)
109e05d [wallet] [rpc] Deprecate wallet 'account' API (John Newbery)
3576ab1 [wallet] [rpc] Deprecate account RPC methods (John Newbery)
3db1ba0 [tests] Set -deprecatedrpc=accounts in tests (John Newbery)
4e671f0 [tests] Rename rpc_listtransactions.py to wallet_listtransactions.py (John Newbery)
a28b907 [wallet] [rpc] Remove duplicate entries in rpcwallet.cpp's CRPCCommand table (John Newbery)

Pull request description:

  Deprecate all accounts functionality and make it only accessible by using `-deprecatedrpc=accounts`.

  Accounts specific RPCs, account arguments, and account related results all require the `-deprecatedrpc=accunts` startup option now in order to see account things.

  Several wallet functional tests use the accounts system. Those tests are unchanged, except to start the nodes with `-deprecatedrpc=accounts`. We can slowly migrate those tests to use the 'label' API instead of the 'account' API before accounts are fully removed.

Tree-SHA512: 89f4ae2fe6de4a1422f1817b0997ae22d63ab5a1a558362ce923a3871f3e42963405d6573c69c27f1764679cdee5b51bf52202cc407f1361bfd8066d652f3f37
2020-12-22 17:53:03 +01:00
dustinface
122078b9ec
backport: bitcoin#10583 - [RPC] Split part of validateaddress into getaddressinfo (#3880)
* [rpc] split wallet and non-wallet parts of DescribeAddressVisitor

* [rpc] Move DescribeAddressVisitor to rpc/util

* Create getaddressinfo RPC and deprecate parts of validateaddress

Moves the parts of validateaddress which require the wallet into getaddressinfo
which is part of the wallet RPCs. Mark those parts of validateaddress which
require the wallet as deprecated.

Validateaddress will  call getaddressinfo
for the data that both share for right now.

Moves IsMine functions to libbitcoin_common and then links libbitcoin_wallet
before libbitcoin_common in order to prevent linker errors since IsMine is no
longer used in libbitcoin_server.

* scripted-diff: validateaddress to getaddressinfo in tests

Change all instances of validateaddress to getaddressinfo since it seems that
no test actually uses validateaddress for actually validating addresses.

-BEGIN VERIFY SCRIPT-
find ./test/functional -path '*py' -not -path ./test/functional/wallet_disable.py -not -path ./test/functional/rpc_deprecated.py -not -path ./test/functional/wallet_address_types.py -exec sed -i'' -e 's/validateaddress/getaddressinfo/g' {} \;
-END VERIFY SCRIPT-

* wallet: Add missing description of "hdchainid"

* Update src/wallet/rpcwallet.cpp

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>

Co-authored-by: John Newbery <john@johnnewbery.com>
Co-authored-by: Andrew Chow <achow101-github@achow101.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2020-12-17 13:46:20 +01:00
pasta
2bda82bdbe
Fix #10579
Signed-off-by: pasta <pasta@dashboost.org>
2020-12-15 11:06:25 -06:00
Pieter Wuille
a2f107424b
Merge #10579: [RPC] Split signrawtransaction into wallet and non-wallet RPC command
d60234885b Add test for signrawtransaction (Andrew Chow)
eefff65a4b scripted-diff: change signrawtransaction to signrawtransactionwithwallet in tests (Andrew Chow)
1e79c055cd Split signrawtransaction into wallet and non-wallet (Andrew Chow)

Pull request description:

  This PR is part of #10570. It also builds on top of #10571.

  This PR splits `signrawtransaction` into two commands, `signrawtransactionwithkey` and `signrawtransactionwithwallet`. `signrawtransactionwithkey` requires private keys to be passed in and does not use the wallet for any signing. `signrawtransactionwithwallet` uses the wallet to sign a raw transaction and does not have any parameters to take private keys.

  The `signrawtransaction` RPC has been marked as deprecated and will call the appropriate RPC command based upon the parameters given. A test was added to check this behavior is still consistent with the original behavior.

  All tests that used `signrawtransaction` have been updated to use one of the two new RPCs. Most uses were changed to `signrawtransactionwithwallet`. These were changed via a scripted diff.

Tree-SHA512: d0adf5b4cd7077639c504ec07bee262a3b94658d34db0a5c86a263b6393f7aa62f45129eafe29a7c861aa58440dd19348ee0c8b685e8a62d6f4adae8ec8f8cb3
2020-12-15 11:06:25 -06:00
PastaPastaPasta
b07a7b810c
Backport 11796 + 11774 (#3612)
* Merge #11796: [tests] Functional test naming convention

5fecd84 [tests] Remove redundant import in blocktools.py test (Anthony Towns)
9b20bb4 [tests] Check tests conform to naming convention (Anthony Towns)
7250b4e [tests] README.md nit fixes (Anthony Towns)
82b2712 [tests] move witness util functions to blocktools.py (John Newbery)
1e10854 [tests] [docs] update README for new test naming scheme (John Newbery)

Pull request description:

  Splitting #11774 into two parts -- this part updates the README with the proposed naming convention, and adds some checks to test_runner.py that the number of tests violating the naming convention doesn't increase too much. Idea is this part of the change should not introduce merge conflicts or require much rebasing, so reviews of the complicated bits won't become invalidated too often; while the second part will just be file renames, which will require regular rebasing and will introduce merge conflicts with pending PRs, but can be merged later, and should also be much easier to review, since it will only include relatively trivial changes.

Tree-SHA512: b96557d41714addbbfe2aed62fb5a48639eaeb1eb3aba30ac1b3a86bb3cb8d796c6247f9c414c4695c4bf54c0ec9968ac88e2f88fb62483bc1a2f89368f7fc80

* update violation count

Signed-off-by: pasta <pasta@dashboost.org>

* Merge #11774: [tests] Rename functional tests

6f881cc880 [tests] Remove EXPECTED_VIOLATION_COUNT (Anthony Towns)
3150b3fea7 [tests] Rename misc functional tests. (Anthony Towns)
81b79f2c39 [tests] Rename rpc_* functional tests. (Anthony Towns)
61b8f7f273 [tests] Rename p2p_* functional tests. (Anthony Towns)
90600bc7db [tests] Rename wallet_* functional tests. (Anthony Towns)
ca6523d0c8 [tests] Rename feature_* functional tests. (Anthony Towns)

Pull request description:

  This PR changes the functional tests to have a consistent naming scheme:

      tests for individual RPC methods are named rpc_...
      tests for interfaces (REST, ZMQ, RPC features) are named interface_...
      tests that explicitly test the p2p interface are named p2p_...
      tests for wallet features are named wallet_...
      tests for mining features are named mining_...
      tests for mempool behaviour are named mempool_...
      tests for full features that aren't wallet/mining/mempool are named feature_...

  Rationale: it's sometimes difficult for new contributors to know what's already covered by existing tests and where new tests should be added. Naming in a consistent fashion makes it easier to see what's already covered at a glance.

Tree-SHA512: 4246790552d42bbd95f6d5bdf67702b81b3b2c583ce7eaf1fe6d8e254721279b47315973c6e9ae82dad6e4c747f12188160764bf2624c0f8f3b4d39330ec8b16

* rename tests and edit associated strings to align test-suite with test name standards

Signed-off-by: pasta <pasta@dashboost.org>

* fix grammar in test/functional/test_runner.py

Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>

* ci: Fix excluded test names

* rename feature_privatesend.py to rpc_privatesend.py

Signed-off-by: pasta <pasta@dashboost.org>

Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
Co-authored-by: dustinface <35775977+xdustinface@users.noreply.github.com>
Co-authored-by: xdustinface <xdustinfacex@gmail.com>
2020-07-17 02:44:20 +03:00