fa6af312277bb1b7e57d9b764d411c5b0873829f test: Document why syncwithvalidationinterfacequeue is needed in tests (MarcoFalke)
fa135a13b8ddaa117bd090ec43a3eab3a95755c1 Revert "test: Add missing sync_all to wallet_balance test" (MarcoFalke)
Pull request description:
syncwithvalidationinterfacequeue is a hidden test-only RPC, so it should not be used when it is not needed. Thus, either remove it or explain why it is needed.
ACKs for top commit:
fjahr:
Code review ACK fa6af312277bb1b7e57d9b764d411c5b0873829f
Tree-SHA512: de30db4ab521184091ee5beeab02989138cf7cf05088f766a2fb106151b239310b63d5380cb79e2a072f72c5ae9513aecae8eb9c1c7be713771585c3cb04d63a
9e1cb1adf1800efe429e348650931f2669b0d2c0 [trivial/doc] Fix comment type (Amiti Uttarwar)
8f30260a67166a6ab7c0f33f7ec1990d3c31761e [doc] Update unbroadcast description in RPC results (Amiti Uttarwar)
750456d6f29c63d57af05bfbdd6035bb9c965de2 [trivial] Remove misleading 'const' (Amiti Uttarwar)
fa32e676e5833a5c5fc735ef00c0a80f5fab7a2c [test] Manage node connections better in mempool persist test (Amiti Uttarwar)
1f94bb0c744a103b633c1051e8fbc01e612097dc [doc] Provide rationale for randomization in scheduling. (Amiti Uttarwar)
9c8a55d9cb0ec73f10b196e79b637aa601c0a6b7 [mempool] Don't throw expected error message when upgrading (Amiti Uttarwar)
ba5498318233ab81decbc585e9619d8ffe2df1b0 [test] Test that wallet transactions aren't rebroadcast before 12 hours (Amiti Uttarwar)
00d44a534b4e5ae249b8011360c6b0f7dc731581 [test] P2P connection behavior should meet expectations (Amiti Uttarwar)
bd093ca15de762fdaf0937a0877d17b0c2bce16e [test] updates to unbroadcast test (Amiti Uttarwar)
dab298d9ab5a5a41685f437db9081fa7b395fa73 [docs] add release notes (Amiti Uttarwar)
Pull request description:
This PR is a follow up to #18038 which introduced the idea of an unbroadcast set & focuses mostly on documentation updates and test fixes. One small functionality update to not throw an expected error in `LoadMempool` when you upgrade software versions.
#18895 is another follow up to that addresses other functionality updates.
Background context:
The unbroadcast set is a mechanism for the mempool to track locally submitted transactions (via wallet or RPC). The node does a best-effort of delivering the transactions to the network via retries every 10-15 minutes until either a `GETDATA` is received or the transaction is removed from the mempool.
ACKs for top commit:
MarcoFalke:
ACK 9e1cb1adf1 👁
gzhao408:
ACK [`9e1cb1a`](9e1cb1adf1)
Tree-SHA512: 0cd51c4ca368b9dce92d50d73ec6e9df278a259e609eef2858f24cb8595ad07acc3db781d9eb0c351715f18fca5a2b4526838981fdb34a522427e9dc868bdaa6
## Issue being fixed or feature implemented
autoresending was really slow
## What was done?
reduced the time range to from 1-3 hours from now
## How Has This Been Tested?
hasn't
## Breaking Changes
Shouldn't be
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
fab0c820fa4c0c3227eec85c64310a3bf938a149 rpc: Clarify that block count means height excl genesis (MarcoFalke)
Pull request description:
There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in https://github.com/bitcoin/bitcoin/pull/16292#issuecomment-506303256.
However, it really returns the height, which is `0` for the genesis block.
So clarify that and also remove the misleading "longest blockchain" comment.
Finally, fix the wallet test that incorrectly used this rpc.
ACKs for top commit:
instagibbs:
utACK fab0c820fa
promag:
ACK fab0c82, sorry for the misconception.
Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
faf6f156ffd1a8ed1aed047428d791a8c13c162b test: Add missing syncwithvalidationinterfacequeue (MarcoFalke)
Pull request description:
The wallet rebroadcast functionality learns about new blocks via the validation interface queue. To avoid test failures such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/31119387#L466 , we can sync with the queue before advancing the test.
ACKs for top commit:
jonatack:
ACK faf6f156 this makes sense; the fix was previously added to mempool_persist.py and wallet_zapwallettxes.py in #12217 and to wallet_balance.py in #16302. It is also used in src/test/validation_block_tests.cpp (processnewblock_signals_ordering) and src/bench/wallet_balance.cpp.
Tree-SHA512: d72fd4b597b669d8111007902b523e946712913cd6eea6f9a695b0f04ecbe2321d05019873af999a95b9e0aa0f5c140a17109b37503723e40c9eab24ec358eb7
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
* 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>
