NOTE: There is slight difference with original backport due to future changes
in bitcoin#19272, bitcoin#19763 - otherwise functional test p2p_addr_relay.py fails
fa1da3d4bfc0511a89f5b19d5a4d89e55ff7ccde test: Add basic addr relay test (MarcoFalke)
fa1793c1c44a3f75a09f9c636467b8274c541bdd net: Pass connman const when relaying address (MarcoFalke)
fa47a0b003f53708b6d5df1ed4e7f8a7c68aa3ac net: Make addr relay mockable (MarcoFalke)
Pull request description:
As usual:
* Switch to std::chrono time to be type-safe and mockable
* Add basic test that relies on mocktime to add code coverage
ACKs for top commit:
naumenkogs:
utACK fa1da3d
promag:
ACK fa1da3d4bfc0511a89f5b19d5a4d89e55ff7ccde (fabe56e44b6f683e24e37246a7a8851190947cb3 before https://github.com/bitcoin/bitcoin/pull/18454#issuecomment-607866453), fa5bf23d527a450e72c2bf13d013e5393b664ca3 was dropped since last review.
Tree-SHA512: 0552bf8fcbe375baa3cab62acd8c23b2994efa47daff818ad1116d0ffaa0b9e520dc1bca2bbc68369b25584e85e54861fe6fd0968de4f503b95439c099df9bd7
fixup - see #19272, #19763
## Issue being fixed or feature implemented
This is an implementation of DIP0027 "Credit Asset Locks".
It's a mechanism to fluidly exchange between Dash and credits.
## What was done?
This pull request includes:
- Asset Lock transaction
- Asset Unlock transaction (withdrawal)
- Credit Pool in coinbase
- Unit tests for Asset Lock/Unlock tx
- New functional test `feature_asset_locks.py`
RPC: currently locked amount (credit pool) is available through rpc call
`getblock`.
## How Has This Been Tested?
There added new unit tests for basic checks of transaction validity
(asset lock/unlock).
Also added new functional test "feature_asset_locks.py" that cover
typical cases, but not all corner cases yet.
## Breaking Changes
This feature should be activated as hard-fork because:
- It adds 2 new special transaction and one of them [asset unlock tx]
requires update consensus rulels
- It adds new data in coinbase tx (credit pool)
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**To release DIP 0027**
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Allow generating 12, 18, 24 words mnemonics. Default to 12 words as it's
the most popular option/de-facto a standard now imo.
## What was done?
Add `-mnemonicbits` option, add tests
## How Has This Been Tested?
run tests, play with wallets on regtest
## Breaking Changes
n/a, old wallets should not be affected
## Checklist:
- [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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
## Issue being fixed or feature implemented
Given the hard fork that happened on testnet, there is now lots of the
transactions that were made on the fork that is no longer valid. Some
transactions could be relayed and mined again but some like coinjoin
mixing won't be relayed because of 0 fee and transactions spending
coinbases from the forked branch are no longer valid at all.
## What was done?
Introduce `wipewallettxes` RPC and `wipetxes` command for `dash-wallet`
tool to be able to get rid of some/all txes in the wallet.
## How Has This Been Tested?
run tests, use rpc/command on testnet wallet
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] 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
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
451b96f7d2796d00eabaec56d831f9e9b1a569cc test: kill process group to avoid dangling processes (S3RK)
Pull request description:
This is an alternative to #19281
This PR fixes a problem when after test failure with `--failfast` option there could be dangling nodes. The nodes will continue to occupy rpc/p2p ports on the machine and will cause further test failures.
If there are any dangling nodes left at the end of the test run we kill the whole process group.
Pros: the operations is immediate and won't lead to CI timeout
Cons: the test_runner process is also killed and exit code is 137
Example output:
```
...
Early exiting after test failure
TEST | STATUS | DURATION
rpc_decodescript.py | ✓ Passed | 2 s
rpc_deprecated.py | ✓ Passed | 2 s
rpc_deriveaddresses.py | ✓ Passed | 2 s
rpc_dumptxoutset.py | ✖ Failed | 2 s
ALL | ✖ Failed | 8 s (accumulated)
Runtime: 4 s
Killed: 9
> echo $?
137
```
ACKs for top commit:
MarcoFalke:
review ACK 451b96f7d2796d00eabaec56d831f9e9b1a569cc
aitorjs:
ACK 451b96f7d2796d00eabaec56d831f9e9b1a569cc. Manual testing with and without **--failfast**.
Tree-SHA512: 87e510a1411b9e7571e63cf7ffc8b9a8935daf9112ffc0f069d6c406ba87743ec439808181f7e13cb97bb200fad528589786c47f0b43cf3a2ef0d06a23cb86dd
fa92af5af39a08982f785542df5419d6d5a4706d ci: Run feature_block and feature_abortnode in valgrind (MarcoFalke)
fa01febeaf801bade77a613e64f18b556ae16d86 test: Remove ci timeout restriction in test_runner (MarcoFalke)
Pull request description:
Also revert commit 0a4912e46a, because some tests take too long for this to be useful anymore.
Top commit has no ACKs.
Tree-SHA512: 363f14766e1f4a5860ab668a516b41acebc6fbdf11d8defb3a95a772dbf82304ca1f5f14b1dbad97f2029503e03d92e8c69df0466a8872409c20665838f617ed
d3bc18408146e91b3836f72360ff6fa2420b6887 doc: update release notes with getaddressinfo label deprecation (Jon Atack)
72af93f36479dc12d795f1d05fa3d8fbd9b293bd test: getaddressinfo label deprecation test (Jon Atack)
d48875fa20d0b71b978cb3d1f85dd9ec14e664cc rpc: deprecate getaddressinfo label field (Jon Atack)
dc0cabeda49a7edbfa71df22846721b6f6224aea test: remove getaddressinfo label tests (Jon Atack)
c7654af6f830577a54df12b5d65df93532db0dc2 doc: address pr17578 review feedback (Jon Atack)
Pull request description:
This PR builds on #17578 (now merged) and deprecates the rpc getaddressinfo `label` field. The deprecated behavior can be re-enabled by starting bitcoind with `-deprecatedrpc=label`.
See http://www.erisian.com.au/bitcoin-core-dev/log-2019-11-22.html#l-622 and https://github.com/bitcoin/bitcoin/pull/17283#issuecomment-554458001 for more context.
Reviewers: This PR may be tested manually by building, then running bitcoind with and without the `-deprecatedrpc=label` flag while verifying the rpc getaddressinfo output and help text.
Next step: add support for multiple labels.
ACKs for top commit:
jnewbery:
ACK d3bc18408146e91b3836f72360ff6fa2420b6887
laanwj:
ACK d3bc18408146e91b3836f72360ff6fa2420b6887
meshcollider:
utACK d3bc18408146e91b3836f72360ff6fa2420b6887
Tree-SHA512: f954402884ec54977def332c8160fd892f289b0d2aee1e91fed9ac3220f7e5b1f7fc6421b84cc7a5c824a0582eca4e6fc194e4e33ddd378c733c8941ac45f56d
## Issue being fixed or feature implemented
Speed thing up: 8075fc0c61
Unify things: ff1a390224 (and _probably_
fix issues like https://gitlab.com/dashpay/dash/-/jobs/4304343867),
876f5c3a9f,
ed58cdda13
Let tsan tests finish on smaller/slower machines:
ba1e3360f9
## What was done?
pls see individual commits
## How Has This Been Tested?
run tests locally and my in gitlab ci
https://gitlab.com/UdjinM6/dash/-/jobs/4319419014
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
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
## Issue being fixed or feature implemented
the problem with retries implemented in #4793 is that they don't really
do anything besides fetching results of a failed job multiple times 🙈
## What was done?
partially reverted changes done in #4793, implemented actual job
restart. dropped `--sleep` and `--retries` and added `--attempts`
instead.
## How Has This Been Tested?
Pick any test, add some randomly failing expression somewhere and run it
with some high number of retries.
For example:
```diff
diff --git a/test/functional/feature_dip0020_activation.py b/test/functional/feature_dip0020_activation.py
index 471e4fdc66..b56a954b78 100755
--- a/test/functional/feature_dip0020_activation.py
+++ b/test/functional/feature_dip0020_activation.py
@@ -69,6 +69,7 @@ class DIP0020ActivationTest(BitcoinTestFramework):
# Should be spendable now
tx0id = self.node.sendrawtransaction(tx0_hex)
assert tx0id in set(self.node.getrawmempool())
+ assert int(tx0id[0], 16) < 4
if __name__ == '__main__':
```
On develop:
```
./test/functional/test_runner.py feature_dip0020_activation.py --retries=100 --sleep=0
```
if this fails on the first run, it keeps "failing" (simply fetching the
same results actually) till the end.
With this patch:
```
./test/functional/test_runner.py feature_dip0020_activation.py --attempts=100
```
if this fails on the first run, it can actually succeed after a few
attempts now, unless you are extremely unlucky ofc 😄
Also, check [ci results in my repo
](https://cdn.artifacts.gitlab-static.net/93/b4/93b4f8b17e5dcccab1afee165b4d74d90f05800caf65d6c48a83a1a78c979587/2023_04_08/4081291268/4478867166/job.log?response-content-type=text%2Fplain%3B%20charset%3Dutf-8&response-content-disposition=inline&Expires=1680945516&KeyName=gprd-artifacts-cdn&Signature=2d4mHCJBbgRaTDiSQ6kKIy1PdIM=).
Note:
```
...
feature_dip3_v19.py failed at attempt 1/3, Duration: 159s
...
4/179 - [1mfeature_dip3_v19.py[0m passed, Duration: 244 s
...
feature_llmq_hpmn.py failed at attempt 1/3, Duration: 284s
...
feature_llmq_hpmn.py failed at attempt 2/3, Duration: 296s
...
11/179 - [1mfeature_llmq_hpmn.py[0m failed, Duration: 233 s
...
```
An example with 2 tests failing initially and then passing:
https://gitlab.com/dashpay/dash/-/jobs/4089689970
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] 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
## Motivation
Dash Core has a series of functional tests that do not behave in a
deterministic fashion and have a higher chance of failure, especially on
resource limited systems. This results in a lot of false-negatives
during CI runs, prompting the need to re-run them, which is annoying at
best and generates apathy towards CI failure at worst.
## History
The first approach was to isolate non-deterministic tests into their own
distinct GItLab runner, making it such that if a test failed, only that
one runner had to be restarted, instead of the multiple runners that
failed due to these tests.
One problem with this was that this approach effectively omitted these
tests from TSan and UBSan coverage as attempting to combine TSan and
UBSan would cause significant resource exhaustion.
## Description
An alternative approach is to introduce a new flag, `--retries`,
applicable only on non-deterministic tests, that allow a failed test to
be repeated up to a set number of times (default: 3), only reporting
failure once the limit is exhausted.
A limitation of this is that only the log dump from the last attempt
will be available.
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
f40b3b82df [tests] functional test for createmultisig RPC (Anthony Towns)
b9024fdda3 segwit support for createmultisig RPC (Anthony Towns)
d58055d25f Move AddAndGetDestinationForScript from wallet to outputype module (Anthony Towns)
9a44db2e46 Add outputtype module (Anthony Towns)
Pull request description:
Adds an "address_type" parameter that accepts "legacy", "p2sh-segwit", and "bech32" to choose the type of address created. Defaults to "legacy" rather than the value of the `-address-type` option for backwards compatibility.
As part of implementing this, OutputType is moved from wallet into its own module, and `AddAndGetDestinationForScript` is changed to apply to a `CKeyStore` rather than a wallet, and to invoke `keystore.AddCScript(script)` itself rather than expecting the caller to have done that.
Fixes#12502
Tree-SHA512: a08c1cfa89976e4fd7d29caa90919ebd34a446354d17abb862e99f2ee60ed9bc19d8a21a18547c51dc3812cb9fbed86af0bef2f1e971f62bf95cade4a7d86237
/**
* Dash specific comment
*
* Seems as event on_getdata works in our P2PInterface.
* But somehow it's never called for test 'test_in_flight_max',
* it may be due to bug in net_processing.
* Due to that, part of functional tests is disabled
*/
fab365835639a3da03f8ad9a58a0db6c6c4c2314 [qa] Test that getdata requests work as expected (Suhas Daftuar)
fa883ab35ad2d4328e35b1e855d0833740a6b910 net: Use mockable time for tx download (MarcoFalke)
Pull request description:
Two commits:
* First commit changes to mockable time for tx download (refactoring, should only have an effect on regtest)
* Second commit adds a test that uses mocktime to test tx download
ACKs for top commit:
laanwj:
code review ACK 16197/commits/fab365835639a3da03f8ad9a58a0db6c6c4c2314
jamesob:
ACK fab3658356
Tree-SHA512: 3a64a3e283ec4bab1f6e506404b11f0a564a5b61d2a7508ae738a61f035e57220484c66e0ae47d847fe9f7e3ff5cc834909d7b34a9bbcea6abe01f8742806908
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
cbf2d75d8f49b7b1e32acb5373b312b484f3fa6a qa: Add getdescriptorinfo functional test (João Barbosa)
Pull request description:
The `getdescriptorinfo` RPC was added in #15368, this PR adds some tests.
Top commit has no ACKs.
Tree-SHA512: 5bf3fb5842b975089821c7ac52202ecb23df255f655862646eb532e38e335ff963f8973bcf5b8bba386183281dc9bfe7279ba1cf25fd518c9a45fb45a9243e4d
37f2784952cb6f598f82922f9ce71d40c9d74e26 tests: Use colors and dots in test_runner.py output only if standard output is a terminal -- allows for using the test runner output as input to other programs (practicalswift)
Pull request description:
Use colors and dots in `test_runner.py` output only if standard output is a terminal -- allows for using the test runner output as input to other programs.
I found the need for this when parsing `test_runner.py` output while investigating intermittent functional test failures.
Before:
```
$ test/functional/test_runner.py wallet_hd.py > output 2>&1
$ less output
Temporary test directory at /tmp/test_runner_₿_🏃_20190807_074115
ESC[1mWARNING!ESC[0m There is already a bitcoind process running on this system. Tests may fail unexpectedly due to resource contention!
Remaining jobs: [wallet_hd.py]
.......................................^M ^M1/1 - ESC[1mwallet_hd.pyESC[0m passed, Duration: 20 s
ESC[1mTEST | STATUS | DURATION
ESC[0mESC[0;32mwallet_hd.py | ✓ Passed | 20 s
ESC[0mESC[1m
ALL | ✓ Passed | 20 s (accumulated)
ESC[0mRuntime: 20 s
```
After:
```
$ test/functional/test_runner.py wallet_hd.py > output 2>&1
$ less output
Temporary test directory at /tmp/test_runner_₿_🏃_20190807_074244
1/1 - wallet_hd.py passed, Duration: 20 s
WARNING! There is already a bitcoind process running on this system. Tests may fail unexpectedly due to resource contention!
Remaining jobs: [wallet_hd.py]
TEST | STATUS | DURATION
wallet_hd.py | ✓ Passed | 20 s
ALL | ✓ Passed | 20 s (accumulated)
Runtime: 20 s
```
ACKs for top commit:
laanwj:
ACK 37f2784952cb6f598f82922f9ce71d40c9d74e26
Tree-SHA512: f15d95f9e07de2954c326d63d7a4bcd2971faeaa00386600dec2fb915ec89475aeef1dbc968b2c12aa5e988d4b3ed1974d6da0b6a3f1e1a105cfd90e8cb97cf6
fa76285fddac613c518e73b35a7486ad2ab4b992 test: Explain why -whitelist is used in feature_fee_estimation (MarcoFalke)
faff85a69a5eb0fdfd8d9a24bc27d1812e49a152 test: Format feature_fee_estimation with pep8 (MarcoFalke)
Pull request description:
ACKs for top commit:
practicalswift:
ACK fa76285fddac613c518e73b35a7486ad2ab4b992 -- diff looks correct
Sjors:
ACK fa76285, every bit of clarification helps. It's clear that without `-whitelist` the test becomes extremely slow (it does pass).
Tree-SHA512: 13ec7e4cd0409e7bb76cbcd344e31c0f612c8ce4a1f1ec6ceaedf345f634bc09786ed38d38920c3469b2862c856ee3e5e42534ef90f531bd8dc83c3db3c06417
## Issue being fixed or feature implemented
`CDeterministicMNList` stores internally a map containing the hashes of
all properties that needed to be unique.
`pubKeyOperator` don't differ between the two schemes (legacy and
basic(v19)) but their serialisation do: hence their hash.
Because this internal map stores only hashes, then we need to
re-calculate hashes and repopulate.
So when we tried to revoke a masternode after the fork, the `ProUpRevTx`
couldn't be mined because the hash of the `pubKeyOperator` differed.
## What was done?
When retrieving a `CDeterministicMNList` for a given block, if v19 is
active for that block, then we repopulate the internal map.
## How Has This Been Tested?
Without this fix, `feature_dip3_v19.py` is failing with
`failed-calc-cb-mnmerkleroot` (Error encountered on Testnet)
## Breaking Changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] 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: pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
fa156999695ddaeb016d8320bee62f8d96679d55 test: Add basic test for BIP 37 (MarcoFalke)
Pull request description:
This does not add full coverage, but should be a good start and can be extended in the future. Currently, none of the BIP 37 p2p code has test coverage.
ACKs for top commit:
practicalswift:
Code review ACK fa156999695ddaeb016d8320bee62f8d96679d55 -- more testing coverage is better than less testing coverage
Tree-SHA512: d52e8be79240dffb769105c087ae0ae9305d599282546e4ca7379c4c7add2dbcd668265b46670aa07c357638044cf0f61a6fab7dba8971dd0f80c8f99768686e
45eff751c6d07007dabc365dc4c0e6c63e3fe5cf Add functional test for P2P eviction logic of inbound peers (Martin Zumsande)
Pull request description:
This adds a functional test for the eviction logic for inbound peers, which is triggered when the number of maximum connections is exceeded.
The functional test covers eviction protection for peers that have sent us blocks or txns recently, or that have faster pings. I couldn't find a way to test the logic of `CConnman::AttemptToEvictConnection` that is based on netgroup (see #14210 for related discussion)
Fixes#16660 (at least partially).
[Edit: Earlier, this PR also contained a unit test, which was removed after the discussion]
ACKs for top commit:
jonatack:
ACK 45eff751c6d07007dabc365dc4c0e6c63e3fe5cf
naumenkogs:
Tested ACK 45eff75
fjahr:
re-ACK 45eff751c6d07007dabc365dc4c0e6c63e3fe5cf
andrewtoth:
re-ACK 45eff751c6d07007dabc365dc4c0e6c63e3fe5cf
Tree-SHA512: 177208ab6f30dc62da1cc5f51e654f7c9770d8c6b42aca6ae7ecb30e29d3096e04d75739578e7d149a0f29dd92652b4a707e93c0f1be8aa7ed315e6ec3ab07a4
7daffc6a90a797ce7c365a893a31a31b0206985c [test] CScriptNum Decode Check as Unit Tests (Gillian Chu)
Pull request description:
The CScriptNum test (#14816) is a roundtrip test of the test framework. Thus, it would be better suited as a unit test. This is now possible with the introduction of the unit test module for the functional tests. See #18576.
This PR:
1. Refactors the CScriptNum tests into 2 unit tests, one in script.py and one in blocktools.py.
2. Extends the script.py CScriptNum test to trial larger numbers.
ACKs for top commit:
laanwj:
ACK 7daffc6a90a797ce7c365a893a31a31b0206985c
Tree-SHA512: 17a04a4bfff1b1817bfc167824c679455d9e06e6e0164c00a7e44f8aa5041c5f5080adcc1452fd80ba1a6d8409f976c982bc481d686c434edf97a5893a32a436
111880aaf7e12a12f0797f1b19673e3d96328edd [test] Add coverage to estimaterawfee and estimatesmartfee (Ben Woosley)
Pull request description:
This adds light functional coverage to estimaterawfee - a subset of
the testing applied to estimatesmartfee, and argument validation
testing to both estimaterawfee and estimatesmartfee.
One valid estimatesmartfee signature test is commented out because it
fails currently.
Extracted from #12940
Top commit has no ACKs.
Tree-SHA512: 361a883457b28b2dc75081666e49d6dc6b5d76eed40d858abe2dd4f35ece152cf1f99c94480a91f42a896aa2a73cf55f57921316fe66970b2d7ba691a3b17e2d
## Issue being fixed or feature implemented
## What was done?
Implementation of 4k collateral HPMN.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] 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: thephez <thephez@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
Co-authored-by: PastaPastaPasta <6443210+pastapastapasta@users.noreply.github.com>
Co-authored-by: UdjinM6 <1935069+Udjinm6@users.noreply.github.com>
Co-authored-by: Konstantin Akimov <545784+knst@users.noreply.github.com>
9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7 [docs] Improve commenting in ProcessGetData() (John Newbery)
2f032556e08a04807c71eb02104ca9589eaadf1b [test] test that an invalid GETDATA doesn't prevent processing of future messages (Amiti Uttarwar)
e257cf71c851e25e1a533bf1d4296f6b55c81332 [net processing] ignore unknown INV types in GETDATA messages (Amiti Uttarwar)
047ceac142246b5d51056a51dbf4645b31802be4 [net processing] ignore tx GETDATA from blocks-only peers (Amiti Uttarwar)
Pull request description:
Currently we'll stall peers that send us an unknown INV type in a GETDATA message. Be a bit more friendly and just drop the invalid request.
Ditto for blocks-relay-only peers that send us a GETDATA for a transaction.
There's a test for the first part. The second is difficult to test in the functional test framework since we aren't able to make blocks-relay-only connections.
ACKs for top commit:
sipa:
utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7
brakmic:
ACK 9847e205bf
luke-jr:
utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7
naumenkogs:
utACK 9847e20
ajtowns:
utACK 9847e205bf7edcac4c30ce4b6d62f482aa7bc1b7
Tree-SHA512: 6007f2fd839ffe737727f6fb8e8f083b2d9e05a510748f1d40b8f9be8fdf7b5419a36d8f1039923eec1ba2983e8f6f0436ec5fc196d9f6dcb0657f2ff8ff8e4c
8098dea06944f9de8b285f44958eb98761f133ee test: Add mempool_updatefromblock.py (Hennadii Stepanov)
Pull request description:
This PR adds a new test for mempool update of transaction descendants/ancestors information (count, size) when transactions have been re-added from a disconnected block to the mempool.
It could be helpful for working on PRs like #17925, #18191.
ACKs for top commit:
ariard:
ACK 8098dea
Tree-SHA512: 7e808fa8df8d7d7a7dbdc3f79361049b49c7bce9b58fd5539b28c9636bedac747695537e500d7ed68dc8bdb80167ad3f1c01086f7551691405d2ba2e38ef1d06
66fe7b1a98c03f690dcf60d359baac124658aeae test: added test for upgradewallet RPC (Harris)
Pull request description:
This PR adds tests for the newly merged *upgradewallet* RPC.
Additionally, it expands `test_framework/util.py` by adding the function `adjust_bitcoin_conf_for_pre_17` to support nodes that don't parse configuration sections.
This test uses two older node versions, v0.15.2 and v0.16.3, to create older wallet versions to be used by `upgradewallet`.
Fixes https://github.com/bitcoin/bitcoin/issues/18767
Top commit has no ACKs.
Tree-SHA512: bb72ff1e829e2c3954386cc308842820ef0828a4fbb754202b225a8748f92d4dcc5ec77fb146bfd5484a5c2f29ce95adf9f3fb4483437088ff3ea4a8d2c442c1
1c23ea5fe67b88fd72a1ff640dd1bbb21a34fbf4 test: fix bitcoind already running warnings on macOS (fanquake)
Pull request description:
On macOS, `pidof` installed via brew returns b'' rather than None.
Account for this, to remove spurious warnings from the test_runner.
ACKs for top commit:
laanwj:
ACK 1c23ea5fe67b88fd72a1ff640dd1bbb21a34fbf4
Tree-SHA512: 640f4323d4105eac5c7abb52daf80486d5d3b4a074720490ceeb97c3dd8d73a3de9a988d2550f1e2076c620bb10d452b2959d8b723d2ee64f499878909824e31
72eaab073bc747425fe551777154b13a6c4c37c9 tests: functional watch-only wallet tests (William Casarin)
72ffbdc5799c1707ecad674d701b43fb80b031d0 doc: add release note for include_watchonly default changes (William Casarin)
003a3c73c0450aa18ac2ab2ca47def2b8c53a7df rpcwallet: document include_watchonly default for watchonly wallets (William Casarin)
a50d9e6c0b8e8144d3deec58ec2e3449ba081151 rpcwallet: default include_watchonly to true for watchonly wallets (William Casarin)
Pull request description:
Right now it's a bit annoying to deal with watchonly wallets, many rpc commands have an `include_watchonly` argument that needs to be explicitly set.
Wallets created with `createwallet` can have a `disable_private_keys` parameter, for those wallets we already know that they are watchonly, so there's no reason to have to explicitly ask for it for every command. Instead we check this wallet flag when the `include_watchonly` parameter isn't set.
ACKs for top commit:
achow101:
Code review ACK 72eaab073bc747425fe551777154b13a6c4c37c9
Sjors:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9
promag:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9, code review only, didn't look closely to the test.
kallewoof:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9
fanquake:
ACK 72eaab073bc747425fe551777154b13a6c4c37c9 - I've looked over the changes, they make sense to me. Compiled and ran the tests etc.
Tree-SHA512: d3646b55e97f386594d7efc994f0712f3888475c6a5dc7f131ac9f8c49bf5d4677182b88f42b34152abe1ad101ecadd152b4c20e9d3c1267190db36f77ab8bd7
5ebc6b0eb267e0552c66fffc5e5afe7df8becf80 bitcoind: update -avoidpartialspends description to account for auto-enable for avoid_reuse wallets (Karl-Johan Alm)
ada258f8c8f92d44d893cf9f22d15acdeca40b1a doc: release notes for avoid_reuse (Karl-Johan Alm)
27669551da52099e4a6a401acd7aa32b32832423 wallet: enable avoid_partial_spends by default if avoid_reuse is set (Karl-Johan Alm)
8f2e208f7c0468f9ba92bc789a698281b1c81284 test: add test for avoidreuse feature (Karl-Johan Alm)
0bdfbd34cf4015de87741ff549db35e5064f4e16 wallet/rpc: add 'avoid_reuse' option to RPC commands (Karl-Johan Alm)
f904723e0d5883309cb0dd14b826bc45c5e776fb wallet/rpc: add setwalletflag RPC and MUTABLE_WALLET_FLAGS (Karl-Johan Alm)
8247a0da3a46d7c38943ee0304343ab7465305bd wallet: enable avoid_reuse feature (Karl-Johan Alm)
eec15662fad917b169f5e3b8baaf4301dcf00a7b wallet: avoid reuse flags (Karl-Johan Alm)
58928098c299efdc7c5ddf2dc20716ca5272f21b wallet: make IsWalletFlagSet() const (Karl-Johan Alm)
129a5bafd9a3efa2fa16d780885048a06566d262 wallet: rename g_known_wallet_flags constant to KNOWN_WALLET_FLAGS (Karl-Johan Alm)
Pull request description:
Add a new wallet flag called `avoid_reuse` which, when enabled, will keep track of when a specific destination has been spent from, and will actively "blacklist" any new UTXOs which send to an already-spent-from destination.
This improves privacy, as a payer could otherwise begin tracking a payee's wallet by regularly peppering a known UTXO with dust outputs, which would then be scooped up and used in payments by the payee, allowing the payer to map out (1) the inputs owned by the payee and (2) the destinations to which the payee is making payments.
This replaces #10386 and together with the (now merged) #12257 it addresses #10065 in full. The concerns raised in https://github.com/bitcoin/bitcoin/pull/10386#issuecomment-302361381 are also addressed due to #12257.
~~Note: this builds on top of #15780.~~ (merged)
ACKs for commit 5ebc6b:
jnewbery:
ACK 5ebc6b0eb
laanwj:
Concept and code-review ACK 5ebc6b0eb267e0552c66fffc5e5afe7df8becf80
meshcollider:
Code review ACK 5ebc6b0eb2
achow101:
ACK 5ebc6b0eb267e0552c66fffc5e5afe7df8becf80 modulo above nits
Tree-SHA512: fdef45826af544cbbb45634ac367852cc467ec87081d86d08b53ca849e588617e9a0a255b7e7bb28692d15332de58d6c3d274ac003355220e4213d7d9070742e
2081442c421cc4376e5d7839f68fbe7630e89103 test: Add test for rpc_whitelist (Emil Engler)
7414d3820c833566b4f48c6c120a18bf53978c55 Add RPC Whitelist Feature from #12248 (Jeremy Rubin)
Pull request description:
Summary
====
This patch adds the RPC whitelisting feature requested in #12248. RPC Whitelists help enforce application policies for services being built on top of Bitcoin Core (e.g., your Lightning Node maybe shouldn't be adding new peers). The aim of this PR is not to make it advisable to connect your Bitcoin node to arbitrary services, but to reduce risk and prevent unintended access.
Using RPC Whitelists
====
The way it works is you specify (in your bitcoin.conf) configurations such as
```
rpcauth=user1:4cc74397d6e9972e5ee7671fd241$11849357f26a5be7809c68a032bc2b16ab5dcf6348ef3ed1cf30dae47b8bcc71
rpcauth=user2:181b4a25317bff60f3749adee7d6bca0$d9c331474f1322975fa170a2ffbcb176ba11644211746b27c1d317f265dd4ada
rpcauth=user3:a6c8a511b53b1edcf69c36984985e$13cfba0e626db19061c9d61fa58e712d0319c11db97ad845fa84517f454f6675
rpcwhitelist=user1:getnetworkinfo
rpcwhitelist=user2:getnetworkinfo,getwalletinfo, getbestblockhash
rpcwhitelistdefault=0
```
Now user1 can only call getnetworkinfo, user2 can only call getnetworkinfo or getwalletinfo, while user3 can still call all RPCs.
If any rpcwhitelist is set, act as if all users are subject to whitelists unless rpcwhitelistdefault is set to 0. If rpcwhitelistdefault is set to 1 and no rpcwhitelist is set, act as if all users are subject to whitelists.
Review Request
=====
In addition to normal review, would love specific review from someone working on LN (e.g., @ roasbeef) and someone working on an infrastructure team at an exchange (e.g., @ jimpo) to check that this works well with their system.
Notes
=====
The rpc list is spelling sensitive -- whitespace is stripped though. Spelling errors fail towards the RPC call being blocked, which is safer.
It was unclear to me if HTTPReq_JSONRPC is the best function to patch this functionality into, or if it would be better to place it in exec or somewhere else.
It was also unclear to me if it would be preferred to cache the whitelists on startup or parse them on every RPC as is done with multiUserAuthorized. I opted for the cached approach as I thought it was a bit cleaner.
Future Work
=====
In a future PR, I would like to add an inheritance scheme. This seemed more controversial so I didn't want to include that here. Inheritance semantics are tricky, but it would also make these whitelists easier to read.
It also might be good to add a `getrpcwhitelist` command to facilitate permission discovery.
Tests
=====
Thanks to @ emilengler for adding tests for this feature. The tests cover all cases except for where `rpcwhitelistdefault=1` is used, given difficulties around testing with the current test framework.
ACKs for top commit:
laanwj:
ACK 2081442c421cc4376e5d7839f68fbe7630e89103
Tree-SHA512: 0dc1ac6a6f2f4b0be9c9054d495dd17752fe7b3589aeab2c6ac4e1f91cf4e7e355deedcb5d76d707cbb5a949c2f989c871b74d6bf129351f429569a701adbcbf
f3f6dde56e Test coinbase category in wallet rpcs (andrewtoth)
e982f0b682 Add all category options to wallet rpc help (andrewtoth)
Pull request description:
The current helptext for `listtransactions`, `listsinceblock` and `gettransaction` only list two of the five possible options for `category`. This incorrectly implies that these are the only two options, and can cause problems if the other three options aren't accounted for. Also, some of the documentation is incorrect when specifying which options are returned for which categories.
This PR updates the helptext for these RPCs and adds a functional regression test for the cases when the other three categories are returned.
Tree-SHA512: 67dd7ff6269a3b0f17f5d1a61b0ae1fb1f3778f05e1c440bfbb9b3a005c9c6d740abcace20f3d597cf2bd6779c494448690f13fab0bd2340f206213bc7890b51
89339d14607434b33cfa343dc75877b62b1dfe0e tests: Add test for loadblock option (Fabian Jahr)
Pull request description:
Fixes#17019
Was initially part of #17044 but as the test got larger it made sense to split it into its own commit as suggested in #17019 .
This is testing the `-loadblock` option by using the scripts in `contrib/linearize` to generate a `bootstrap.dat` file and starting a disconnected node with it. So it is also testing the linearize scripts which were untested before and needed to be made available for the CI environment, hence they are added to `DIST_CONTRIB` in `Makefile.am`.
ACKs for top commit:
laanwj:
ACK 89339d14607434b33cfa343dc75877b62b1dfe0e
Tree-SHA512: aede0cd6e8b21194973f3633bc07fa2672d66a6f85dfe6a57cee2bb269a65d19ea49d5f9ed7914a173b3847c76e70257aa865f44bde170c1999d9655b4862d1c
