1e17b74207 test: no longer connect nodes in parallel in `start_masternodes` (UdjinM6)
be72ef5592 test: use `setmnthreadactive` to get controlable `connect_nodes` behaviour (UdjinM6)
e2ed82a7ae feat(rpc): introduce `setmnthreadactive` (regtest-only) (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
This adds a new rpc command to enable/disable automatic masternode connections creation. We need this for #6276. 1e17b74207 is extracted from ede1833ba4 to avoid multiple jobs calling `setmnthreadactive` on the same node in parallel.
## What was done?
Add `setmnthreadactive` rpc and use it
## How Has This Been Tested?
run tests
## 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
ACKs for top commit:
kwvg:
LGTM, ACK 1e17b74207
PastaPastaPasta:
utACK 1e17b74207
Tree-SHA512: 83c1c07d0066e26202fd21942a09e41c3560c4d32229b44390946c4acb22319b32aa61a13b9106d20fc8cc197dd2a8ab5fdfcfdeaf3da76af062fc0fd7646972
d6946aaba1 fix: offset fee for 1 duff in commission in wallet_basic.py due to missing bitcoin/bitcoin#22949 (Konstantin Akimov)
3ba99b9c42 Merge bitcoin/bitcoin#21786: wallet: ensure sat/vB feerates are in range (mantissa of 3) (MarcoFalke)
22435f1898 Merge bitcoin/bitcoin#21787: test: fix off-by-ones in rpc_fundrawtransaction assertions (W. J. van der Laan)
ccac35c89c Merge #21083: wallet: Avoid requesting fee rates multiple times during coin selection (Samuel Dobson)
9e9975f83b Merge #21201: rpc: Disallow sendtoaddress and sendmany when private keys disabled (Samuel Dobson)
5ad8a489a5 Merge #20573: wallet, bugfix: allow send with string fee_rate amounts (MarcoFalke)
db4a2169bb Merge #20410: wallet: Do not treat default constructed types as None-type (MarcoFalke)
01e41aa1fb Merge #20426: wallet: allow zero-fee fundrawtransaction/walletcreatefundedpsbt and other fixes (MarcoFalke)
f436c20bc4 Merge #20305: wallet: introduce fee_rate sat/vB param/option (MarcoFalke)
0fa19226cb Merge #20220: wallet, rpc: explicit fee rate follow-ups/fixes for 0.21 (Samuel Dobson)
d530b73016 Merge #18275: wallet: error if an explicit fee rate was given but the needed fee rate differed (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Just regular backports from bitcoin v0.21, v22; mostly wallet+fee related
## What was done?
- bitcoin/bitcoin#18275
- bitcoin/bitcoin#20220
- bitcoin/bitcoin#20305
- bitcoin/bitcoin#20426
- bitcoin/bitcoin#20410
- bitcoin/bitcoin#20573
- bitcoin/bitcoin#21201
- bitcoin/bitcoin#21083
- bitcoin/bitcoin#21787
## How Has This Been Tested?
Run unit and functional tests
## Breaking Changes
Some wallet rpc (sendtoaddress, sendmany, send) have a new argument `fee_rate` which is inserted before `verbose`.
Release notes will be provided in a new PR once scope of backports and fixes in this PR is finalized by merging it to develop/
## 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
ACKs for top commit:
UdjinM6:
utACK d6946aaba1
PastaPastaPasta:
utACK d6946aaba1
Tree-SHA512: 8826e1453fe84e3d21f789fab62c23ea13299ce13a7bf1132f70831c3255e823437b6ddd63f69a8e8a0dae95a2638a4454d727e91177b53a5d331872528b92e8
5a77abd4e657458852875a07692898982f4b1db5 [style] Clean up BroadcastTransaction() (John Newbery)
7282d4c0363ab5152baa34af626cb49afbfddc32 [test] Allow rebroadcast for same-txid-different-wtxid transactions (glozow)
cd48372b67d961fe661990a2c6d3cc3d91478924 [mempool] Allow rebroadcast for same-txid-different-wtxid transactions (John Newbery)
847b6ed48d7bacec9024618922e9b339d2d97676 [test] Test transactions are not re-added to unbroadcast set (Duncan Dean)
2837a9f1eaa2c6bf402d1d9891d9aa84c4a56033 [mempool] Only add a transaction to the unbroadcast set when it's added to the mempool (John Newbery)
Pull request description:
1. Only add a transaction to the unbroadcast set when it's added to the mempool
Currently, if BroadcastTransaction() is called to rebroadcast a
transaction (e.g. by ResendWalletTransactions()), then we add the
transaction to the unbroadcast set. That transaction has already been
broadcast in the past, so peers are unlikely to request it again,
meaning RemoveUnbroadcastTx() won't be called and it won't be removed
from m_unbroadcast_txids.
Net processing will therefore continue to attempt rebroadcast for the
transaction every 10-15 minutes. This will most likely continue until
the node connects to a new peer which hasn't yet seen the transaction
(or perhaps indefinitely).
Fix by only adding the transaction to the broadcast set when it's added to the mempool.
2. Allow rebroadcast for same-txid-different-wtxid transactions
There is some slightly unexpected behaviour when:
- there is already transaction in the mempool (the "mempool tx")
- BroadcastTransaction() is called for a transaction with the same txid
as the mempool transaction but a different witness (the "new tx")
Prior to this commit, if BroadcastTransaction() is called with
relay=true, then it'll call RelayTransaction() using the txid/wtxid of
the new tx, not the txid/wtxid of the mempool tx. For wtxid relay peers,
in SendMessages(), the wtxid of the new tx will be taken from
setInventoryTxToSend, but will then be filtered out from the vector of
wtxids to announce, since m_mempool.info() won't find the transaction
(the mempool contains the mempool tx, which has a different wtxid from
the new tx).
Fix this by calling RelayTransaction() with the wtxid of the mempool
transaction in this case.
The third commit is a comment/whitespace only change to tidy up the BroadcastTransaction() function.
ACKs for top commit:
duncandean:
reACK 5a77abd
naumenkogs:
ACK 5a77abd4e657458852875a07692898982f4b1db5
theStack:
re-ACK 5a77abd4e657458852875a07692898982f4b1db5
lsilva01:
re-ACK 5a77abd4e6
Tree-SHA512: d1a46d32a9f975220e5b432ff6633fac9be01ea41925b4958395b8d641680500dc44476b12d18852e5b674d2d87e4d0160b4483e45d3d149176bdff9f4dc8516
874ef8cda2 fix: mine_quorum_no_checks -> mine_quorum_less_checks: do some checks to make sure quorums are mined correctly (UdjinM6)
4f636f47b4 fix: re-order functional tests: move governance to 60+seconds category (Konstantin Akimov)
fe49f3f178 refactor: removed dead and commented code from test_framework.py (Konstantin Akimov)
cd1958c82a perf: removed sleep(6) from mine_cycle_quorum in functional tests (Konstantin Akimov)
3f17a01a83 fix: bump mocktime in simplepose when generating blocks to improve robustness (Konstantin Akimov)
132d95e651 perf: remove sleep(1) from each step of quorum creation in functional tests (Konstantin Akimov)
4c57ad1c05 chore: increase batch size from 10 to 50 for faster block generation in functional tests (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Functional tests take too long time to run.
(PR is recreated from https://github.com/dashpay/dash/pull/6268 because CI is broken)
## What was done?
- increased robustness `feature_llmq_simplepose.py` by adding missing bump for mocktime during block generations
- removed sleep(1) from each stage of mine_quorum
- removed sleep(6) from final stage of mine_cycled_quorum
- size of batch for block generation in `feature_asset_locks.py` and in `activate_fork_by_name()` increased from 10 blocks to 50 blocks
- moved governance's functional tests to "60 seconds+" category because they always the last one to wait if running more than 10 jobs at once
Plus extra refactoring which removes dead and commented code from test_framework.py
## How Has This Been Tested?
Locally, the functional tests speed up with these fixes for 15% for overall time and 20% for accumulated time
`test/functional/test_runner.py -j20`
Before:
```
ALL | ✓ Passed | 7860 s (accumulated)
Runtime: 481 s
```
After:
```
ALL | ✓ Passed | 6237 s (accumulated)
Runtime: 416 s
```
---
CI tsan job speeds up for 5 minutes in absolute time (~5%) and 1000 seconds in accumulated time.
```
ALL | ✓ Passed | 23854 s (accumulated)
Runtime: 6249 s
```
↑ [old version](https://gitlab.com/dashpay/dash/-/jobs/7822664869) vs [new version](https://gitlab.com/dashpay/dash/-/jobs/7825461091) ↓
```
ALL | ✓ Passed | 22901 s (accumulated)
Runtime: 5962 s
```
## 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
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
utACK 874ef8cda2
PastaPastaPasta:
utACK 874ef8cda2
Tree-SHA512: 514fa2fb32abd59c90f63b68fccc8c3d3b6d16b0b6ad7459c4a348825815e7d3012177565dea1f70b8a1f28ede1a297f91361365454d1be85955e77260451cf5
74a5a9f984 style: apply clang-format (Konstantin Akimov)
db9798f5e4 refactor: move call GetQuorumMembers inside Init() (Konstantin Akimov)
9593566802 refactor: move pQuorumBaseBlockIndex from Init() to constructor in CDKGSession (Konstantin Akimov)
9f3eb6bbb8 perf: check DIP0003 before CDKGSession initialization (Konstantin Akimov)
7f815cb501 refactor: remove unused constructor of CDKGLogger (Konstantin Akimov)
c82672af94 refactor: remove dependency of CDKGSession on PeerManager (Konstantin Akimov)
fb78b0cc94 refactor: remove retBan flag from ReceiveMessage (Konstantin Akimov)
d26d4ab0bc refactor: remove dependency of dkgsessionmgr on dkgsession (Konstantin Akimov)
d361b11e5b refactor: moved including llmq/dkgsession.h from dkgsessionmgr.h to cpp file (Konstantin Akimov)
e77aeb321b refactor: removed including quorums.h from chainlocks.h (Konstantin Akimov)
6f7068ef42 refactor: remove exceeding evodb.h from headers (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
We have 72 circular dependencies of dash specific code. This PR removes 2 of them, over dkgsession.
## What was done?
Refactor dkgsession initialization, message processing, dropped unused arguments, re-distributed code between functions and modules... See each commit.
Also optimized headers: excluded evo/evodb.h and llmq/quorums.h from the headers where they are not needed.
## How Has This Been Tested?
Run `test/lint/lint-circular-dependencies.sh`
Run unit/functional tests
## 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
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
utACK 74a5a9f984
PastaPastaPasta:
utACK 74a5a9f984
Tree-SHA512: f3adabe6a7bc6c4dcae6430be6857b31a0722d7f605d6651f3ff93c6fcb350a7312a9a0ecbda8ac131ffef6fbf2499700112de3990c7512ddb057bde7cc42665
a3d6ec5bb567481a634638cea7ae37c355119a7b test: move rpc_rawtransaction tests to < 30s group (Jon Atack)
5a1ed96077852c739034c21d399da65db09e7714 test: whitelist rpc_rawtransaction peers to speed up tests (Jon Atack)
Pull request description:
Speed up the somewhat slow `rpc_rawtransaction.py` test by more than 3x (from 45-55 seconds to 15 seconds on a laptop running 2 x 2.5GHz).
ACKs for top commit:
mjdietzx:
ACK a3d6ec5bb567481a634638cea7ae37c355119a7b
kristapsk:
ACK a3d6ec5bb567481a634638cea7ae37c355119a7b
theStack:
ACK a3d6ec5bb567481a634638cea7ae37c355119a7b 🐎
brunoerg:
tACK a3d6ec5bb567481a634638cea7ae37c355119a7b
Tree-SHA512: f1d105594c9b5b257a7096b631a6fa5aeb50e330a351f75c2d6ffa7dd73abdb6e1f596a78c16d204a9bac3fe506e0519f9ad96bb8477ab6424c8e18125ccb659
BACKPORT NOTICE: it includes missing commit: d6ef3543ae16847d5a91fa9271acee9bd2164b32
lint: Run mypy with --show-error-codes
When using mypy ignore directives, the error code needs to be specified.
Somehow mypy doesn't print it by default...
71689fe6dc (partial) Merge bitcoin/bitcoin#22981: doc: Fix incorrect C++ named args (fanquake)
2b71a9b030 Merge bitcoin/bitcoin#23755: rpc: Quote user supplied strings in error messages (MarcoFalke)
5a441b38de (partial) Merge bitcoin-core/gui#409: Fix window title of wallet loading window (Hennadii Stepanov)
49c87e93a6 Merge bitcoin/bitcoin#23142: Return false on corrupt tx rather than asserting (W. J. van der Laan)
Pull request description:
backports from bitcoin
ACKs for top commit:
UdjinM6:
utACK 71689fe6dc
knst:
utACK 71689fe6dc
Tree-SHA512: c68e2a1be5669f4fd8b02001ea81310b41fcac2cc5cc660e67b5140b334669c9a071a4bd5b33232580215607f323af5f87218a3465493675a633e112984296eb
073d6d6b2a Merge bitcoin/bitcoin#23723: test: Replace hashlib.new with named constructor (MarcoFalke)
674dcf9a55 Merge bitcoin/bitcoin#23547: Bugfix: RPC/mining: Fail properly in estimatesmartfee if smart fee data is unavailable (MarcoFalke)
546e548755 Merge bitcoin/bitcoin#24153: test: remove unused sanitizer suppressions (fanquake)
78b06a4dd2 Merge bitcoin/bitcoin#23591: refactor: Use underlying type of isminetype for isminefilter (W. J. van der Laan)
Pull request description:
backporting
ACKs for top commit:
UdjinM6:
utACK 073d6d6b2a
knst:
utACK 073d6d6b2a
Tree-SHA512: 5c5af5b795ec86f2b98cf9884e1275b8d3a5e7942f8b6632d74ecb799b1b7fe34071c052ac9af15abac14bad1b886ead5d0478f5e03fe0b461b7b40a7defef9e
4e621037c5 test: move `EXPECTED_STDERR_NO_GOV`{`_PRUNE`} and use it more (Kittywhiskers Van Gogh)
77ce6af5c1 chore: remove ancient setting migration logic (Kittywhiskers Van Gogh)
061aa05cf0 chore: remove old llmq db migration code (Kittywhiskers Van Gogh)
438cb85ece ci: set UBSan to halt on error and provide more information (Kittywhiskers Van Gogh)
3a1743fc7f trivial: avoid unneeded copy when iterating through `mapDenomCount` (Kittywhiskers Van Gogh)
cba650953a test: simplify `pow_test`'s `get_next_work` block index construction (Kittywhiskers Van Gogh)
dacf859218 refactor: make `pdsNotificationInterface` a `unique_ptr`, rename (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
Collection of miscellaneous changes collected from work on earlier pull requests that don't fit into pull requests in the immediate future but are nonetheless useful.
## Checklist
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
re-utACK 4e621037c5
UdjinM6:
utACK 4e621037c5
Tree-SHA512: 5af4e5afdc34840905ffbf375f53cb12b682053cc135ff190dd02d245da9903a7f3e6af6fb1f1727546bf5034cbe42243477342f775e05d15bcfc5d5957616e9
63b13aa519 merge bitcoin#28525: Drop v2 garbage authentication packet (Kittywhiskers Van Gogh)
662394cd49 merge bitcoin#28489: fix incorrect assumption in v2transport_test (Kittywhiskers Van Gogh)
98782c62df merge bitcoin#28433: Follow-up to BIP324 connection support (Kittywhiskers Van Gogh)
f9825168fb merge bitcoin#28196: BIP324 connection support (Kittywhiskers Van Gogh)
3087275039 merge bitcoin#28419: introduce and use `ConsumePrivateKey` helper (Kittywhiskers Van Gogh)
dccd395a4e merge bitcoin#27577: give seednodes time before falling back to fixed seeds (Kittywhiskers Van Gogh)
eb4f01f931 merge bitcoin#26584: include local ("unreachable") peers in -netinfo table (Kittywhiskers Van Gogh)
10dc874136 merge bitcoin#26982: bitcoin#25880 fixups (Kittywhiskers Van Gogh)
a36f8f2a1a merge bitcoin#25880: Make stalling timeout adaptive during IBD (Kittywhiskers Van Gogh)
1d77f3ff55 merge bitcoin#26519: Add getpeerinfo test for missing version message (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* When backporting [bitcoin#25880](https://github.com/bitcoin/bitcoin/pull/25880), changes needed to be made to `p2p_ibd_stalling.py` to help it pass.
* DIP3 activation had to be delayed to a block beyond the range of the test. This is to prevent block rejection arising from a missing DIP3-compliant coinbase (done with `-dip3params=2000:2000`)
* Mock time was disabled to ensure nodes in the test do not resort to direct fetching (with mock time enabled, nodes would be considered close apart in time, which would prevent the primary node from fetching in parallel, which is behavior this test relies on) (done with `self.disable_mocktime = True`)
* The nodes connected do not report compressed headers support (the test relies on sending `headers` messages and reworking it to use compressed headers has little benefit) (done with `services = NODE_NETWORK | NODE_BLOOM`)
* When backporting [bitcoin#28196](https://github.com/bitcoin/bitcoin/pull/28196), in the `v2transport_test` unit test, references to `4000000` were substituted with `MAX_PROTOCOL_MESSAGE_LENGTH` as Dash messages have a protocol limit of 3MiB ([source](d754799580/src/net.h (L79-L80))) while Bitcoin messages have a protocol limit of 4MB ([source](225718eda8/src/net.h (L62-L63))).
* Additionally note that short message IDs as defined in the BIP324 spec ([source](22660ad307/bip-0324.mediawiki (v2-bitcoin-p2p-message-structure))) have not been changed to include Dash-specific messages, meaning, Dash-specific messages will always take 13 bytes.
* As `FEEFILTER` is not supported by Dash, it has been replaced with a blank string in the short IDs table. It was not removed as doing so would disturb the table's arrangement as specified in spec and require readjustment of tests to account for the change in layout.
## Breaking Changes
None expected.
## Checklist
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK 63b13aa519
knst:
utACK 63b13aa519
Tree-SHA512: c41d2c6b1f145be513e285c8f91d00ac31fff4e4d24f611a1fdab24c8740f453b0bb28912021cdf8be4f5ce93dcff8579864727ee14b8e45894b56df524ab48d
It significantly improve speed of forks activation because reduces overhead for block generations
Bigger batch size can cause time-outs for RPC for tsan job (time-out is 30 seconds)
48c7f98b1a doc: drop trailing whitespace (pasta)
697743d77b test: add missing import (UdjinM6)
cfe99fd289 docs: add release notes for 6239 (pasta)
a6bbaacfaa fix: GetHeadersLimit is used for getheaders(2) and headers(2), refactor it to accept `compressed` instead of `msg_type` (UdjinM6)
b224f3f6ca bump p2p_version in tests (PastaPastaPasta)
b423f42aae refactor: sort imports (UdjinM6)
f6c68ba71b refactor: simplify _compute_requested_block_headers (UdjinM6)
07876b2c4a use `MAX_HEADERS_UNCOMPRESSED_RESULT` not `MAX_HEADERS_UNCOMPRESSED_RESULTS` ; use `MAX_HEADERS_UNCOMPRESSED_RESULT` in RPC to avoid breaking changes (pasta)
b137280df4 change to _COMPRESSED or _UNCOMPRESSED (pasta)
303bc7af99 fix: increase it for headers2 only (UdjinM6)
e23410ffdd trivial: rename `MAX_HEADERS_RESULTS_NEW` to `MAX_HEADERS_RESULTS` (Kittywhiskers Van Gogh)
bcf0320691 trivial: move the headers limit determination to `GetHeadersLimit()` (Kittywhiskers Van Gogh)
993c7c0f90 feat: increase the number of block headers able to be downloaded at once to 8000 in protocol version `70234` (pasta)
Pull request description:
## Issue being fixed or feature implemented
We did some testing quite a while ago that found that sending 8000 headers at a time could speed stuff up. But we wanted to wait until compressed headers were implemented. Well, they've been implemented!
## What was done?
Bump 2000 -> 8000 triggered by protocol version
## How Has This Been Tested?
Hasn't, we should setup a few nodes running this and sync them from each other
## Breaking Changes
New protocol version, not breaking but should add notes? I should probably add release notes
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [ ] 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)_
ACKs for top commit:
UdjinM6:
light ACK 48c7f98b1a
knst:
utACK 48c7f98b1a
Tree-SHA512: 54c68b9496131ab7f32504d44398d776a151df809d0120d093bbabb18904a783bd9b58796820209f5d75552df5476e30eaa09d68f7c5057882f94b5766a64f4c
847288df07b45ca535c849e518b22818ab492896 test: fee rate values that cannot be represented as sat/vB (Jon Atack)
06a90fa0381c790f7bde2ab9bf47d2b22acef4a5 rpc: for sat/vB fee rates, limit ParseFixedPoint decimals to 3 (Jon Atack)
0742c7840f03505597fd2de87db97f12597ef667 rpc: enable passing decimals to AmountFromValue, add doxygen (Jon Atack)
8ce3ef57a3e9ad13c0aaa4648e8584241d53592d test: ParseFixedPoint with 3 decimals for sat/vB fee rates (Jon Atack)
b5033275979a2a495b02b25f70cadbdcc8b6eb6a test: type error and out of range fee rates where missing (Jon Atack)
c5fd4344f7fcc257062a610c8ff26ffcc9b53953 test: explicit fee rates with invalid amounts (Jon Atack)
ea6f76b66ecc52360719053489e0ec9f9a673eab test: improve zero-value explicit fee rate coverage (Jon Atack)
Pull request description:
- Improve/close gaps in existing test coverage before making the change
- Enable passing `decimals` to `ParseFixedPoint()` when calling `AmountFromValue()`
- Limit explicit fee rates in sat/vB passed in by users to 3 decimals, and raise otherwise
- Add regression test coverage
Closes#20534.
ACKs for top commit:
MarcoFalke:
review ACK 847288df07b45ca535c849e518b22818ab492896 🔷
Tree-SHA512: c539d07ae9b21c0d6c8ea460beb9c8dad5559445518aace560abc3c05c588907bae189b6fd7602b3b397de4a42356136c3ec6f960d3dcf2d5d16377aef4ab5a2
f09e6b2585200040be2e8ee44fa79b86b1970d70 test: fix off-by-ones in rpc_fundrawtransaction (Jon Atack)
Pull request description:
The variables in these assertions should be the same within each line.
ACKs for top commit:
laanwj:
ACK f09e6b2585200040be2e8ee44fa79b86b1970d70
theStack:
ACK f09e6b2585200040be2e8ee44fa79b86b1970d70
Tree-SHA512: 7ac754eaadd8cb00a725afa55bccbb8de7547dedac9350d79a9a470918245617e075c56a91adc36fb653bbe8a0a325d59b00443155a7e1a81ebf22e4e4cf56d9
6bfbc97d716faad38c87603ac6049d222236d623 test: disallow sendtoaddress/sendmany when private keys disabled (Jon Atack)
0997019e7681efb00847a7246c15ac8f235128d8 Disallow sendtoaddress and sendmany when private keys disabled (Andrew Chow)
Pull request description:
Since `sendtoaddress` and `sendmany` (which use the `SendMoney` function) create and commit a transaction, they should not do anything when the wallet does not have private keys. Otherwise a valid transaction cannot be made.
Fixes#21104
ACKs for top commit:
jonatack:
ACK 6bfbc97d716faad38c87603ac6049d222236d623
meshcollider:
utACK 6bfbc97d716faad38c87603ac6049d222236d623
kristapsk:
ACK 6bfbc97d716faad38c87603ac6049d222236d623. "Error: Private keys are disabled for this wallet" is definitely a better error message than "Insufficient funds" here. Hopefully change of error code from -6 to -4 doesn't break any software using Bitcoin JSON-RPC API.
Tree-SHA512: f277d6b5252e43942d568614032596f2c0827f00cd0cb71e44ffcb9822bfb15a71730a3e3688f31e59ba4eb7d275250c4e65ad4b6b3e96be6314c56a672432fb
6fa72ceb8021c3b5aea62f6cfe92665c29212923 test: add coverage for passing fee rate as a string (Jon Atack)
ce207d6b93d35bc02fcd2dd28f1fd95869261d43 wallet, bugfix: allow send to take string fee rate values (Jon Atack)
Pull request description:
RPC send currently only accepts fee rates as numbers, which is a user-facing bug. It should accept fee rates as an amount, e.g. a string or a number, as documented in its help and like sendtoaddress, sendmany, fundrawtransaction, walletcreatefundedpsbt, and bumpfee. Provide a fix and regression test coverage.
ACKs for top commit:
MarcoFalke:
review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923
achow101:
Code review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923
promag:
Code review ACK 6fa72ceb8021c3b5aea62f6cfe92665c29212923.
Tree-SHA512: 735f9269cb1b81953764b5283449c0b154bd62de034225be5bcedc515c84faf767fe8fe0741008679fe412922c847b00d116cb11aab775236b779c847ba87167
fa69c2c78455fd0dc436018fece9ff7fc83a180d wallet: Do not treat default constructed types as None-type (MarcoFalke)
fac4e136fa3d0fab7fde900a6be921313e16e7a6 refactor: Change pointer to reference because it can not be null (MarcoFalke)
Pull request description:
Equating `0==None` and `""==None` is confusing, unneeded and undocumented
ACKs for top commit:
jonatack:
ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
achow101:
ACK fa69c2c78455fd0dc436018fece9ff7fc83a180d
Sjors:
tACK fa69c2c78455fd0dc436018fece9ff7fc83a180d modulo `unset`
Tree-SHA512: c4c8d0ad80c6697621d356a9545caf28ca2facc82bb2fa8e70eceb52372d25f0685237c73688c4b01da0e75d213c77c0d45011a8bdfe81ea783d85f045786dac
9f08780dd7946b63476e9736745131db8e7f4e93 Use the correct incremental fee constant in bumpfee help (Jon Atack)
3f1e10b2b1cd11f7112fbad6355464bd4adbbc5c Update feeRate (BTC/kvB) to fee_rate (sat/vB) in wallet_bumpfee (Jon Atack)
1b3d7009280595108eb22ac1188bc4367804fc5d Allow zero-fee fundrawtxn and walletcreatefundedpsbt calls (Jon Atack)
Pull request description:
- Fixes https://github.com/bitcoin/bitcoin/pull/20305/files#r525406176. A check to raise an error on zero-fee txns was mistakenly extended in a0d4957 from the bumpfee and send{toaddress, many} RPCs to also include fundrawtransaction and walletcreatefundedpsbt. This commit re-overrides zero fee rate checking for these two RPCs, not only for the feeRate (BTC/kvB) arg to return to previous behavior, but also for the new fee_rate (sat/vB) arg. Negative fee rates will still raise "amount out of range" by the MoneyRange check in src/bitcoin-tx.cpp::AmountFromValue.
- Updates a wallet bumpfee test from feeRate (BTC/kvB) to fee_rate (sat/vB)
- Fixes https://github.com/bitcoin/bitcoin/pull/20305/files#r525405363 to use the correct incremental fee rate constant in the bumpfee help (thanks Marco Falke for the catch) and rectifies "1.000 sat/vB sat/vB" in the help to "1.000 sat/vB"
ACKs for top commit:
MarcoFalke:
review ACK 9f08780dd7946b63476e9736745131db8e7f4e93 🐾
promag:
Code review ACK 9f08780dd7946b63476e9736745131db8e7f4e93.
Xekyo:
Code review reACK 9f08780dd7946b63476e9736745131db8e7f4e93.
Tree-SHA512: 413dfb4f23ebaf3d2ef210dd04610a843272e64eabba428699f5de4d646a86ac4911dab66b5e2f5ebea53b76e4be8347ef40824c1592c750d5eaa12579d3cdf6
05e82d86b09d914ebce05dbc92a7299cb026847b wallet: override minfee checks (fOverrideFeeRate) for fee_rate (Jon Atack)
9a670b4f07a6140de809d73cbd7f3e614eb6ea74 wallet: update sendtoaddress, send RPC examples with fee_rate (Jon Atack)
be481b72e24fb6834bd674cd8daee67c6938b42d wallet: use MIN_RELAY_TX_FEE in bumpfee help (Jon Atack)
449b730579566459e350703611629e63e54657ed wallet: provide valid values if invalid estimate mode passed (Jon Atack)
6da3afbaee5809ebf6d88efaa3958c505c2d71c7 wallet: update remaining rpcwallet fee rate units to BTC/kvB (Jon Atack)
173b5b5fe07d45be5a1e5bc7a5df996f20ab1e85 wallet: update fee rate units, use sat/vB for fee_rate error messages (Jon Atack)
7f9835a05abf3e168ad93e7195cbaa4bf61b9b07 wallet: remove fee rates from conf_target helps (Jon Atack)
b7994c01e9a3251536fe6538a22f614774eec82d wallet: add fee_rate unit warnings to bumpfee (Jon Atack)
410e471fa42d3db04e8879c71f8c824dcc151a83 wallet: remove redundant bumpfee fee_rate checks (Jon Atack)
a0d495747320c79b27a83c216dcc526ac8df8f24 wallet: introduce fee_rate (sat/vB) param/option (Jon Atack)
e21212f01b7c41eba13b0479b252053cf482bc1f wallet: remove unneeded WALLET_BTC_KB_TO_SAT_B constant (Jon Atack)
6112cf20d43b0be34fe0edce2ac3e6b27cae1bbe wallet: add CFeeRate ctor doxygen documentation (Jon Atack)
3f7279161347543ce4e997d78ea89a4043491145 wallet: fix bug in RPC send options (Jon Atack)
Pull request description:
This PR builds on #11413 and #20220 to address #19543.
- replace overloading the conf_target and estimate_mode params with `fee_rate` in sat/vB in the sendtoaddress, sendmany, send, fundrawtransaction, walletcreatefundedpsbt, and bumpfee RPCs
- allow non-actionable conf_target value of `0` and estimate_mode value of `""` to be passed to use `fee_rate` as a positional argument, in addition to as a named argument
- fix a bug in the experimental send RPC described in https://github.com/bitcoin/bitcoin/pull/20220#discussion_r513789526 where args were not being passed correctly into the options values
- update the feerate error message units for these RPCs from BTC/kB to sat/vB
- update the test coverage, help docs, doxygen docs, and some of the RPC examples
- other changes to address the excellent review feedback
See this wallet meeting log for more context: http://www.erisian.com.au/bitcoin-core-dev/log-2020-11-06.html#l-309
ACKs for top commit:
achow101:
re-ACK 05e82d8
MarcoFalke:
review ACK 05e82d86b0 did not test and found a few style nits, which can be fixed later 🍯
Xekyo:
tACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Sjors:
utACK 05e82d86b09d914ebce05dbc92a7299cb026847b
Tree-SHA512: a4ee5f184ada53f1840b2923d25873bda88c5a2ae48e67eeea2417a0b35154798cfdb3c147b05dd56bd6608a784e1b91623bb985ee2ab9ef2baaec22206d0a9c
0be29000c011dec0722481dbebb159873da6fa54 rpc: update conf_target helps for correctness/consistency (Jon Atack)
778b9be40667876c705e586849ea9c9e44cf451c wallet, rpc: fix send subtract_fee_from_outputs help (Jon Atack)
603c0050837ec65765208dd54dde354145fbe098 wallet: add rpc send explicit fee rate coverage (Jon Atack)
dd341e602d5160fc621c0299179b91403756b61d wallet: add sendtoaddress/sendmany explicit fee rate coverage (Jon Atack)
44e7bfa60313e4ae67da49e5ba4535038b71b453 wallet: add walletcreatefundedpsbt explicit fee rate coverage (Jon Atack)
6e1ea4273e52fdcd86c87628aa595c03a071ca8c test: refactor for walletcreatefundedpsbt fee rate coverage (Jon Atack)
3ac7b0c6f1c68e74a84d868a454f508bada6b09d wallet: fundrawtx fee rate coverage, fixup ParseConfirmTarget() (Jon Atack)
2d8eba8f8425a2515022d51f1f5b4911329fbf55 wallet: combine redundant bumpfee invalid params and args tests (Jon Atack)
1697a40b6f841a54ee0d9744ed7fd09034b0ddad wallet: improve bumpfee error/help, add explicit fee rate coverage (Jon Atack)
fc5721723d34f76f9e1ffd2e31f274ea6b22f894 wallet: fix SetFeeEstimateMode() error message (Jon Atack)
052427eef1c9da84c474c5161b1910d3328ef0da wallet, bugfix: fix bumpfee with explicit fee rate modes (Jon Atack)
Pull request description:
Follow-up to #11413 providing a base to build on for #19543:
- bugfix for `bumpfee` raising a JSON error with explicit feerates, fixes issue #20219
- adds explicit feerate test coverage for `bumpfee`, `fundrawtransaction`, `walletcreatefundedpsbt`, `send`, `sendtoaddress`, and `sendmany`
- improves a few related RPC error messages and `ParseConfirmTarget()` / error message
- fixes/improves the explicit fee rate information in the 6 RPC helps, of which 2 were also missing `conf_target` sat/B units
This provides a spec and regression coverage for the potential next step of a universal `sat/vB` feerate argument (see #19543), as well as immediate coverage and minimum fixes for 0.21.
ACKs for top commit:
kallewoof:
Concept/Tested ACK 0be29000c011dec0722481dbebb159873da6fa54
meshcollider:
Code review + functional test run ACK 0be29000c011dec0722481dbebb159873da6fa54
Tree-SHA512: efd965003e991cba51d4504e2940f06ab3d742e34022e96a673606b44fad85596aa03a8c1809f06df7ebcf21a38e18a891e54392fe3d6fb4d120bbe4ea0cf5e0
fa1b63c01887adff83f16b1bbba3bd159dc51104 test: Replace hashlib.new with named constructor (MarcoFalke)
Pull request description:
A small refactor that doesn't matter too much, but it using the named constructor is nice because:
* It clarifies that it is a built-in function
* It is (trivially) faster and less code.
ACKs for top commit:
Zero-1729:
ACK fa1b63c01887adff83f16b1bbba3bd159dc51104
w0xlt:
ACK fa1b63c
Tree-SHA512: d23dc4552c1e6fc1f90f8272e47e4efcbe727f0b66a6f6a264db8a50ee6cb6d57a2809befcb95fda6725136672268633817a03dd1859f2298d20e3f9e0ca4a7f
We need to disable mocktime so that the node doesn't resort to direct
fetching. We also need to delay DIP3's activation so that blocks don't
get rejected for not having a valid DIP3-compliant coinbase.
056d869571 refactor: use testdummy in feature_mnehf functional test, removed useless checks (Konstantin Akimov)
0351469bb5 refactor: removed duplicated meaningless condition from Check mnhftx (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Using mn_rr in feature_mnehf.py is a blocker for burying mn_rr fork.
## What was done?
Removed useless conditions, uses testdummy fork instead mn_rr in ehf functional test.
## How Has This Been Tested?
Run it `test/functional/feature_mnehf.py`
## 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
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
utACK 056d869571
PastaPastaPasta:
utACK 056d869571
Tree-SHA512: aa016dc21fee3afdb5a4172e757cdd0b92867c23eb5241287f641d3c97d363ab1c46eba06423481ecc7ba05f6c0010b65eadfe77d4a1fd6074706cc6f3a71309
fa3bdbd37be81b613e48a10aa77dfd3bcede61e1 test: remove unused sanitizer suppressions (MarcoFalke)
Pull request description:
Looks like those are not needed (anymore)
ACKs for top commit:
fanquake:
ACK fa3bdbd37be81b613e48a10aa77dfd3bcede61e1
Tree-SHA512: 4bedb6363aba8ea7763291ee0cd074e6bfd77e691bb32999c3959393864dc396bacba1eced2b10d9d600b66e8b83b91f7bc6692331dbd113bbaa87e72d11e2e8
In upcoming commits, message sending will be split off into a separate
file and stats capabilities will be fleshed out. Prepare for that by
giving it its own directory.
Also get rid of `statsd` namespace, it is entirely unnecessary.
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
9bdda50151dd808cbad094d457bf0ed7939a7c87 Enable TLS in links in documentation (Jeremy Rand)
Pull request description:
This PR enables TLS in several documentation links, which improves security.
ACKs for top commit:
fanquake:
ACK 9bdda50151dd808cbad094d457bf0ed7939a7c87
Tree-SHA512: 9d04d8771a9daf3c3b9914ff324e2eabfdf3ff5ae7f7dc92b84a1f3527010ceb860e73873a8f24d6051763eb472d9ea324ccbd6129a40318a520ca88c05f0586
faa75fa19335e3e826efa4f2280609a2db34425d Avoid unsigned integer overflow in bitcoin-tx (MarcoFalke)
Pull request description:
While `npos` means "largest unsigned value" and adding `1` to it yields `0`, it may be clearer to just assign `0` to it and only increment otherwise.
This also allows to remove a file-wide suppression for `unsigned-integer-overflow`.
ACKs for top commit:
hebasto:
ACK faa75fa19335e3e826efa4f2280609a2db34425d, I have reviewed the code and it looks OK, I agree it can be merged.
theStack:
Code-review ACK faa75fa19335e3e826efa4f2280609a2db34425d
Tree-SHA512: c24436641e5d801341c948b812c7f711d5dff70efdf04af00fd3221f4b81d93f25608dddaa36230ba81ca7ab0d18bdd957095d4561e22621e4d69017934f0a16
fa24a3df8796cbf4eeb35d950a4c848d605e5b22 rpc: Quote user supplied strings in error messages (MarcoFalke)
Pull request description:
I can't see a downside doing this and this fixes a fuzzing crash
Background:
This is a follow-up to commit 926fc2a0d4ff64cf2ff8e1dfa64eca2ebd24e090, which introduced the "starts_with-hack". Maybe an alternative to the hack would be to assign a unique error code to internal bugs? However, I think this can be done in an separate pull request and the changes here make sense even on their own.
ACKs for top commit:
fanquake:
ACK fa24a3df8796cbf4eeb35d950a4c848d605e5b22 - to fix the fuzzers.
Tree-SHA512: d998626406a64396a037a6d1fce22fce3dadb7567c2f9638e450ebe8fb8ae77d134e15dd02555326732208f698d77b0028bc62be9ceee9c43282b61fe95fccbd
cc998abec1 fmt: apply `clang-format-diff.py` suggestions (Kittywhiskers Van Gogh)
0401c581eb stats: `const`-ify variables and arguments (Kittywhiskers Van Gogh)
9f96723774 stats: stop using error codes, switch over to `bool` (Kittywhiskers Van Gogh)
1a81979c1e stats: initialize socket after we have a valid socket address (Kittywhiskers Van Gogh)
dbbfc8d766 stats: use `Socks` wrapper, use `CService` to generate our `sockaddr` (Kittywhiskers Van Gogh)
2def905044 stats: move init logic into constructor (Kittywhiskers Van Gogh)
4bc727cd6c stats: clean up randomization code, move `FastRandomContext` inward (Kittywhiskers Van Gogh)
840241eefd stats: cleanup error logging, improve code sanity (Kittywhiskers Van Gogh)
85890ddb13 docs: add copyright notice to source file, update notice in header (Kittywhiskers Van Gogh)
a9d1b1494d stats: move `_StatsdClientData` variables into `StatsdClient` (Kittywhiskers Van Gogh)
30c30c1397 stats: fetch all arguments needed when constructing `g_stats_client` (Kittywhiskers Van Gogh)
5133d88415 stats: s/statsClient/g_stats_client/g (Kittywhiskers Van Gogh)
f81951dd00 stats: make `statsClient` a `std::unique_ptr`, denote as global variable (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
Support for transmitting stats to a Statsd server has been courtesy of Statoshi ([repo](https://github.com/jlopp/statoshi)), implemented Dec, 2020 by [dash#2515](https://github.com/dashpay/dash/pull/2515) but since then, it hasn't gotten much attention aside from benefiting from codebase-wide changes and the occasional compiler appeasement. This pull request aims to give our statistics code some TLC.
Changes include:
* Limiting initialization to solely during construction and moving the responsibility of fetching arguments outside of `statsd::StatsdClient`.
* Using the RAII `Socks` wrapper as early as possible (we still need to construct a raw socket ourselves but this is done in the initializer and control is moved to the wrapper and everywhere else, the wrapper is used)
* Utilizing existing networking code to generate the socket address
* This lets us trivially allow IPv6 connections as the responsibility to construct it safely is moved to `CService`.
* Using `std::string` and our string manipulation capabilities (replacing `snprintf` with `strprintf`), replacing platform-specific types (replacing `short` with `uint16_t`).
## Breaking Changes
None observed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK [cc998ab](cc998abec1)
UdjinM6:
utACK cc998abec1
Tree-SHA512: 433c92160d6ac7ebb8582ada3cbb65ead7913618266b773619a528c90dfe0e286aafa46dc3b0bca62f246938e5948a732080e2cddba942d3627f007ca6efcc1f
6c7335e002 merge bitcoin#24331: Revert back `MoveFileExW` call for MinGW-w64 (Kittywhiskers Van Gogh)
15e794bdd8 merge bitcoin#24238: use arc4random on OpenBSD (Kittywhiskers Van Gogh)
e039aecbdc merge bitcoin#23936: Add and use EnsureArgsman helper (Kittywhiskers Van Gogh)
b4bfacfd24 merge bitcoin#23769: Disallow copies of CChain (Kittywhiskers Van Gogh)
5b66688160 merge bitcoin#22362: Drop only invalid entries when reading banlist.json (Kittywhiskers Van Gogh)
109c963f6a merge bitcoin#23175: Add CJDNS network to -addrinfo and -netinfo (Kittywhiskers Van Gogh)
d57c96ea37 merge bitcoin#23398: add return message to savemempool RPC (Kittywhiskers Van Gogh)
22e59fb464 merge bitcoin#23054: Use C++11 member initializer in CTxMemPoolEntry (Kittywhiskers Van Gogh)
d158063b6d merge bitcoin#22653: Rename JoinErrors and re-use it (Kittywhiskers Van Gogh)
e24324d266 merge bitcoin#22221: Pass block reference instead of pointer to PeerManagerImpl::BlockRequested (Kittywhiskers Van Gogh)
68657efc03 merge bitcoin#22141: net processing: Remove hash and fValidatedHeaders from QueuedBlock (Kittywhiskers Van Gogh)
c0e6792e27 merge bitcoin#20018: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* When backporting [bitcoin#23054](https://github.com/bitcoin/bitcoin/pull/23054), `sigOpCount` and `nSigOpCountWithAncestors` were switched from `unsigned int` to `int64_t`. This change was done to prevent integer narrowing when accepting the `int64_t` taken from the constructor.
This isn't a problem upstream as the same changes were as part of [bitcoin#8149](https://github.com/bitcoin/bitcoin/pull/8149/files#diff-8a2230436880b65a205db9299ab2e4e4adb1d4069146791b5db566f3fb752adaL90-L107), which was omitted but the type changes remain valid as sigop count cannot be a negative number.
## Breaking Changes
None observed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK 6c7335e002
UdjinM6:
utACK 6c7335e002
Tree-SHA512: 29cae42dd82235305d3558562bae346170e742ce0b65897e396b331294b39cad0dd831fa9a09b34780a67844e55292e5b4e784cf544a894cc3f8897afe617ca1
- Use `uint16_t` instead of `short`, `int64_t` instead of `size_t`
- Get rid of the `errmsg` buffer and use `LogPrintf` to report errors
- Use `strprintf` instead of `snprintf`
- Rephrase networking error logs to allow inclusion of error strings
0c5179462e test: rework feature_governance_cl.py (UdjinM6)
11ac0819da feat: bump_mocktime also bumps schedulers now (UdjinM6)
1937f503fe feat: regtest-only: do not auto-reset mnsync after 1h (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
1. The original idea behind forced reset was to help desktop clients which go into sleep/hibernation mode from time to time to sync with no issues once they are online again. For regtest however it doesn't do anything good and only causes issues.
2. We rely on schedulers a lot, bumping them should let nodes behave more like on a real network.
3. Forcing mnsync to skip governance votes doesn't always work as we'd expect cause we don't control connection creation. To make it more deterministic create a proposal that should get into one superblock only. This way both the proposal and the trigger will expire/be deleted after another superblock meaning we can be 100% sure the isolated node never gets any of them and can only sync thanks to chainlock.
## What was done?
## How Has This Been Tested?
run tests
## 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 0c5179462e
Tree-SHA512: 9a40eeaba880f3f27f86736e92afa19a4ecb2a2d157bc42b65dd9da0d4109c9cd1d83a5abdf1ec16be2f8a8b31821fb700a7f0d2265c094fd4fdff7f18bc6ec7
57fb0874ef feat: broadcast dsq messages using the inventory system (pasta)
Pull request description:
## Issue being fixed or feature implemented
DSQ messages are 142 bytes.
Previously, assuming a relatively highly connected masternode hosting 100 connection, each round of coinjoin will result in 14.2KB (100*142) of inbound and outbound traffic each.
## What was done?
Now, using the inventory system, a message will first use 36 bytes per peer (sending and receiving), plus the size of a `getdata` message and the actual message itself. As a result, bandwidth usage for 1 round of mixing would be closer to 36 * 100 + 142 (dsq) + 36 (getdata) = ~3.8KB, a reduction of around ~73%
## How Has This Been Tested?
Has not been; @UdjinM6 especially please review well :)
## Breaking Changes
Does introduce a new protocol version, but in a backwards compatible way. I don't think this would need to be delayed to v22 for any reason.
## 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
- [ ] 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)_
Top commit has no ACKs.
Tree-SHA512: 3dc39a339cba29d8cf207cec76ecace5ad0e11d1892ca0f65f9253a2b1d90313da21c6c178c2476756c5566ece0fab777006cd609b7984df906a9206c25d921d
DSQ messages are 142 bytes.
Previously, assuming a relatively highly connected masternode hosting 100 connection, each round of coinjoin will result in 14.2KB (100*142) of inbound and outbound traffic each.
Now, using the inventory system, a message will first use 36 bytes per peer (sending and receiving), plus the size of a `getdata` message and the actual message itself. As a result, bandwidth usage for 1 round of mixing would be closer to 36 * 100 + 142 (dsq) + 36 (getdata) = ~3.8KB, a reduction of around ~73%
23812555b1 fix: possible deadlock during calculation of signals for historical blocks during re-index (Konstantin Akimov)
1087489fd4 feat: bury v20 deployment (Konstantin Akimov)
64cedb30bd feat: actually test something EHF unit tests (Konstantin Akimov)
762a808b8c chore: drop irrelevant bip9 code from feature_llmq_rotation.py (Konstantin Akimov)
7735631aad fix: remove v20 from test feature_llmq_evo as far as mn_rr used (Konstantin Akimov)
ca83b26815 fix: crash in CreditPool: it meant to check that DIP0003 is activated (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
V20 is activated on mainnet: time to bury it!
https://github.com/dashpay/dash/issues/6186
## What was done?
Hard-fork v20 is buried and it requires to implement multiple fixes, simplifications, refactoring:
- some tests for EHF moved from functional tests to unit tests
- fixed crash in Credit Pool if DIP3 is not activated yet
- added a requirement for v20 activation for `CMNHFManager::GetSignalsStage`
- removed useless code from functional test feature_llmq_rotation
- renamed variables "v20" to "mn_rr" in feature_llmq_evo.py so far as actually used fork is mn_rr
## How Has This Been Tested?
Some unit and functional tests to succeed.
Done reindex (just in case):
src/qt/dash-qt -reindex -assumevalid=0
src/qt/dash-qt -reindex -assumevalid=0 -testnet
## 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
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
ACK 23812555b1
PastaPastaPasta:
utACK 23812555b1
Tree-SHA512: eec35745baa695f3f286d39b6a61fa0a9f34820b13d1dd4cfbd1efe707850283892c39bf7fe49c49c812e0c02465d64df11480b3f12aa7f21b59a71eeae7260e
e92aad7cff test: make sure MNs don't vote twice even when they are allowed to (UdjinM6)
3d75390e4e fix: correct conditions for YES voting (UdjinM6)
ec1392c6de chore: make clang-format and linter happy (UdjinM6)
a6320865c4 fix: avoid voting for the same trigger multiple times (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
We just had a sb voting period and I noticed that the network is way too spammy and produces too many votes (10x+ the expected numbers). It turns out that relying on `ProcessVoteAndRelay` on mainnet is not enough because rate-check expires too soon and MNs are able to vote again and again. On testnet it was never an issue because the voting period there is really short.
## What was done?
Check known votes to make sure we never voted earlier.
## How Has This Been Tested?
Run tests, run a MN on mainnet and check logs.
## 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK e92aad7cff
Tree-SHA512: 142e23d3a19fa9527fa5257eb790e558d3507a7a857f17c6e02fd58eeb5643fcfb48d824d227e0ea7cd3dae6a6d7d871b3af88b13077f5af074ed1911e42bb28
0a7a234bd3 merge bitcoin#28110: correct Fedora systemtap dep (Kittywhiskers Van Gogh)
c92eb67c40 merge bitcoin#27458: Detect USDT the same way how it is used in the code (Kittywhiskers Van Gogh)
f4a53ba8ce merge bitcoin#26945: systemtap 4.8 (Kittywhiskers Van Gogh)
88696129f3 merge bitcoin#25794: don't rely on block_connected USDT event order in tests (Kittywhiskers Van Gogh)
457bbd3f8b merge bitcoin#25360: SystemTap 4.7 (RISC-V support) (Kittywhiskers Van Gogh)
f3b219ad0d merge bitcoin#24358: USDT tracepoint interface tests (Kittywhiskers Van Gogh)
5b10a5a2fd merge bitcoin#23907: utxocache tracepoints follow up (Kittywhiskers Van Gogh)
c3d7e3a192 merge bitcoin#26944: fix systemtap download URL (Kittywhiskers Van Gogh)
264e02fcc7 merge bitcoin#23724: add systemtap's sys/sdt.h as depends for GUIX builds with USDT tracepoints (Kittywhiskers Van Gogh)
6cc596b99a merge bitcoin#22902: utxocache tracepoints (Kittywhiskers Van Gogh)
644a47ef9a merge bitcoin#23302: drop GetHash().ToString() argument from the `validation:block_connected` tracepoint (Kittywhiskers Van Gogh)
bfdc9ad364 merge bitcoin#23375: more deterministic coin selection for coinbase UTXOs (oldest first) (Kittywhiskers Van Gogh)
5718716cd2 merge bitcoin#22955: Rename fBlocksOnly, Add test (Kittywhiskers Van Gogh)
cacc31213b merge bitcoin#22006: first tracepoints and documentation on User-Space, Statically Defined Tracing (USDT) (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* In [bitcoin#22955](https://github.com/bitcoin/bitcoin/pull/22955), `fBlocksOnly` has not been renamed to `reject_tx_invs` as Dash uses the inventory system beyond relaying transaction data with the block-relay-only blocklist have a greater set of prohibited messages. This renders the new name misleading but coining a new name may be a source of confusion, making retaining the legacy name desirable.
* Additionally, because the word "transaction" isn't hardcoded into the log message (instead opting to use `CInv::GetCommand()`'s output instead, which for transactions, are "tx"), the expected log message in `p2p_blocksonly.py` has been adjusted accordingly.
* [bitcoin#24358](https://github.com/bitcoin/bitcoin/pull/24358) depends on [bitcoin#22955](https://github.com/bitcoin/bitcoin/pull/22955) and [bitcoin#23375](https://github.com/bitcoin/bitcoin/pull/23375) in order to work as without the latter backport, `interface_usdt_utxocache.py` will return the following error
<details>
<summary>Test run:</summary>
```
debian@debian:~/dash$ sudo ./test/functional/interface_usdt_utxocache.py
[sudo] password for debian:
2024-08-26T17:08:05.234000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_n5rb0xy4
2024-08-26T17:08:07.023000Z TestFramework (INFO): testing the utxocache:uncache tracepoint API
2024-08-26T17:08:07.026000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/debian/dash/test/functional/test_framework/test_framework.py", line 160, in main
self.run_test()
File "/home/debian/dash/./test/functional/interface_usdt_utxocache.py", line 149, in run_test
self.test_uncache()
File "/home/debian/dash/./test/functional/interface_usdt_utxocache.py", line 172, in test_uncache
invalid_tx = self.wallet.create_self_transfer(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/debian/dash/test/functional/test_framework/wallet.py", line 166, in create_self_transfer
assert_equal(mempool_valid, tx_info['allowed'])
File "/home/debian/dash/test/functional/test_framework/util.py", line 51, in assert_equal
raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
AssertionError: not(True == False)
2024-08-26T17:08:07.533000Z TestFramework (INFO): Stopping nodes
2024-08-26T17:08:08.535000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_n5rb0xy4
2024-08-26T17:08:08.535000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_n5rb0xy4/test_framework.log
2024-08-26T17:08:08.535000Z TestFramework (ERROR):
2024-08-26T17:08:08.535000Z TestFramework (ERROR): Hint: Call /home/debian/dash/test/functional/combine_logs.py '/tmp/dash_func_test_n5rb0xy4' to consolidate all logs
2024-08-26T17:08:08.536000Z TestFramework (ERROR):
2024-08-26T17:08:08.536000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-08-26T17:08:08.536000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
2024-08-26T17:08:08.536000Z TestFramework (ERROR):
```
</details>
with the underlying error that has been alluded to in d2c4904
```
{'txid': '44b58b10e69321dacf00724f1893c9ecb50fc1f89ed7c70a6c0b3c08f7dc750f', 'allowed': False, 'reject-reason': 'bad-txns-premature-spend-of-coinbase'}
```
* In [bitcoin#24358](https://github.com/bitcoin/bitcoin/pull/24358), all `interface_usdt_*.py` tests needed minor syntax changes to account for how we invoke certain RPCs. `interface_usdt_utxocache.py` required tweaking with the blocks needed to be eligible for pruning (`450` vs. `350` upstream) and stop the node explicitly to account for the governance validation warning .
This is important as the placement assumes that `test_flush()` is the last test, should this change, the node may need to be manually started before continuing on with the tests after.
* Some `TRACE*` entries may not exactly match the backports they come from because the variables or functions they access may have been amended by a backport done _after_ the backport that introduce the trace entry.
## Breaking Changes
None observed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] 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)_
ACKs for top commit:
UdjinM6:
utACK 0a7a234bd3
PastaPastaPasta:
utACK [0a7a234](0a7a234bd3)
Tree-SHA512: 8df11392dd8d152c18d55ac70a446d1eec336bdf1a984cbf41c3202c353358180e05ba4b7182ec2962ea09eefa41d1dc3cd383d358f9b3dec57ce8b67c6e6afd
9876c2d78b docs: add partial release notes (UdjinM6)
b330318db7 refactor: drop circular dependency (UdjinM6)
e54fe42ce8 refactor: use `key_to_p2pkh_script` in more places (UdjinM6)
3ed6246889 test: check `creditOutputs` format (UdjinM6)
ba0e64505b fix: `creditOutputs` in AssetLock tx json output should be an array of objects, not debug strings (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
Txout-s in `creditOutputs` for AssetLock txes should be shown the way txout-s are shown in other places. We should not be using debug strings there.
Example: `getrawtransaction 50757f651f335e22c5a810bd05c1e5aac0d95b132f6454e2a72683f88e3983f3 1`
develop:
```
"assetLockTx": {
"version": 1,
"creditOutputs": [
"CTxOut(nValue=0.01000000, scriptPubKey=76a914cdfca4ae1cf2333056659a2c)"
]
},
```
This PR:
```
"assetLockTx": {
"version": 1,
"creditOutputs": [
{
"value": 0.01000000,
"valueSat": 1000000,
"scriptPubKey": {
"asm": "OP_DUP OP_HASH160 cdfca4ae1cf2333056659a2c8dc656f36d228402 OP_EQUALVERIFY OP_CHECKSIG",
"hex": "76a914cdfca4ae1cf2333056659a2c8dc656f36d22840288ac",
"address": "yf6c2VSpWGXUgmjQSHRpfEcTPsbqN4oL4c",
"type": "pubkeyhash"
}
}
]
},
```
kudos to @coolaj86 for finding the issue
## What was done?
Change `CAssetLockPayload::ToJson()` output to be closer to [`TxToUniv()`](https://github.com/dashpay/dash/blob/develop/src/core_write.cpp#L262-L272)
NOTE: `refactor: use key_to_p2pkh_script in more places` commit is a bit unrelated but I decided to add it anyway to make it easier to follow assetlock creation vs getrawtransaction rpc check.
## How Has This Been Tested?
Try example above, run tests
## Breaking Changes
RPC output is different for AssetLock txes
## 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
- [ ] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK 9876c2d78b
Tree-SHA512: 158c98ac9e4979bb29c4f54cb1b71806f22aaec92218d92cd2b2e9b9f74df721563e7a6c5f517ea358ac74659fa79f51d1b683002a1cdceb1b8ee80f8fd79375
`fBlocksOnly` has not been renamed as Dash uses the inv system for
relaying far more than transaction data and the blocklist of invs in
block-relay-only mode extends beyond what is conveyed by `reject_tx_invs`
f032119456 merge bitcoin#22910: Encapsulate asmap in NetGroupManager (Kittywhiskers Van Gogh)
8020bfa8c1 merge bitcoin#24665: document clang tidy named args (Kittywhiskers Van Gogh)
40a22e457a merge bitcoin#24201: Avoid InitError when downgrading peers.dat (Kittywhiskers Van Gogh)
cdcaf2278c merge bitcoin#23373: Parse command line arguments from unit and fuzz tests, make addrman consistency check ratio easier to change (Kittywhiskers Van Gogh)
b30f0fa441 test: remove `connman` local from `BasicTestingSetup` (Kittywhiskers Van Gogh)
df43565464 merge bitcoin#23826: Make AddrMan unit tests use public interface, extend coverage (Kittywhiskers Van Gogh)
c14a54089f merge bitcoin#23780: update `addrman_tests.cpp` to use output from `AddrMan::Good()` (Kittywhiskers Van Gogh)
8b2db6bce4 merge bitcoin#23713: refactor addrman_tried_collisions test to directly check for collisions (Kittywhiskers Van Gogh)
5b5dd39f45 merge bitcoin#23492: tidy up addrman unit tests (Kittywhiskers Van Gogh)
aba0ebd400 merge bitcoin#23477: tidy up unit tests (Kittywhiskers Van Gogh)
cdc8321c4d merge bitcoin#22872: improve checkaddrman logging with duration in milliseconds (Kittywhiskers Van Gogh)
8d22fe9945 merge bitcoin#23084: avoid non-determinism in asmap-addrman test (Kittywhiskers Van Gogh)
ba4696718e partial bitcoin#23025: update nanobench add `-min_time` (Kittywhiskers Van Gogh)
c28b05c5ca merge bitcoin#22831: add addpeeraddress "tried", test addrman checks on restart with asmap (Kittywhiskers Van Gogh)
c4fe6085c8 merge bitcoin#22226: add unittest core dump instructions (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* In [bitcoin#22831](https://github.com/bitcoin/bitcoin/pull/22831), when restarting the node in `rpc_net.py`'s `test_addpeeraddress()`, existing commands need to be appended to `extra_args` to ensure they're retained when the node is restarted (default behavior is to overwrite the argument list with `extra_args`) to prevent the test from hanging and then failing due to missing fast DIP3 activation params from the arguments list.
Missing arguments result in a block database sanity check failure on restart due to `bad-qc-premature` arising from the activation height being higher than the height of a block with a quorum commitment.
* `NodeContext` was moved from `TestingSetup` to `BasicTestingSetup` in [bitcoin#18571](https://github.com/bitcoin/bitcoin/pull/18571) ([dash#4844](https://github.com/dashpay/dash/pull/4844)) but `connman` as a `BasicTestingSetup` variable still stuck around (despite `NodeContext`'s presence making this unnecessary).
To prepare for [bitcoin#22910](https://github.com/bitcoin/bitcoin/pull/22910), the remnant variable has been replaced with `m_node.connman` and adjustments have been made to that effect.
## Breaking Changes
None observed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] 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)_
ACKs for top commit:
UdjinM6:
utACK f032119456
PastaPastaPasta:
utACK f032119456
Tree-SHA512: b29c292ecda54cda8301ea804b433f80476a1cdbb72bd48740cc9b2e885a4ff52350e5e42f112426856282bd6d961f0e37f1b23020c52f07238413070bbc504a
`p2p_ibd_txrelay.py` was introduced in bitcoin#19423 but not backported
as Dash doesn't have feefilter capabilities but this backport has the
test check for additional cases which are within Dash's capabilities, so
the test has been committed in with the feefilter portions minimally
stripped out
ef4d74a669 test: remove dead code from `p2p_initial_headers_sync.py` to favor of disable mocktime (UdjinM6)
4d9837c21e refactor: add a new flag disable_mocktime to set_test_params() (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
To disable mocktime you should re-implement setup_nodes(). It seems as bug-friendly solution
## What was done?
This PR introduce a new flag "disable_mocktime" which can be set in `set_test_params`.
It seems more error prune and the code is shorter
## How Has This Been Tested?
Run unit/functional tests including future changes from https://github.com/dashpay/dash/pull/6235
## 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
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
utACK ef4d74a669
kwvg:
utACK ef4d74a669
PastaPastaPasta:
utACK ef4d74a669
Tree-SHA512: c6be8002cae4d7824e150938957464c156931d0b6f7fc41c430a83d662865431f1b56cb11df73f56db54a140f36b0addd68b2917e25c5c941fae52e8d322bc25
We need to continue inheriting the existing set of arguments to prevent
block invalidation due to missing arguments that allow for fast DIP3
activation (will manifest as `bad-qc-premature`)
be5c84f41d Merge bitcoin/bitcoin#22089: test: MiniWallet: fix fee calculation for P2PK and check tx vsize (MarcoFalke)
247141d32c Merge bitcoin/bitcoin#22210: test: Use MiniWallet in test_no_inherited_signaling RBF test (MarcoFalke)
dad3ae3f33 Merge bitcoin/bitcoin#22130: test: refactor: dedup utility function chain_transaction() (MarcoFalke)
9dff334f47 Merge bitcoin-core/gui#361: Fix gui segfault caused by bitcoin/bitcoin#22216 (Hennadii Stepanov)
1087849955 Merge bitcoin/bitcoin#22216: refactor: Make SetupServerArgs callable without NodeContext (MarcoFalke)
fd94de6888 Merge bitcoin/bitcoin#21178: test: run mempool_reorg.py even with wallet disabled (MarcoFalke)
f1f5723fcf fix: missing changes from bitcoin#14123 (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Regular backports from bitcoin v22
## What was done?
See commits for list of backported changes. It also have some missing changes from bitcoin#14123
## How Has This Been Tested?
Run unit/functional tests
## 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
ACKs for top commit:
UdjinM6:
utACK be5c84f41d
PastaPastaPasta:
utACK be5c84f41d
Tree-SHA512: 553ffde63c8409799cf6b3b87bf1ee285fbf58b13c08d04cdac29bc0e4dd75059feaa2f163803084ae85175397512517b68a6e0e0cc602d981f38ac70d96e393
b75e83b298 merge bitcoin#24218: Fix implicit-integer-sign-change (Kittywhiskers Van Gogh)
8ecc22f51f merge bitcoin#23471: Improve ZMQ documentation (Kittywhiskers Van Gogh)
2965093c4a merge bitcoin#22079: Add support to listen on IPv6 addresses (Kittywhiskers Van Gogh)
3ac3714957 merge bitcoin#21310: fix sync-up by matching notification to generated block (Kittywhiskers Van Gogh)
7b0c725c59 merge bitcoin#21008: fix zmq test flakiness, improve speed (Kittywhiskers Van Gogh)
5e87efd04b merge bitcoin#20523: deduplicate 'sequence' publisher message creation/sending (Kittywhiskers Van Gogh)
99c730f0f3 merge bitcoin#20953: dedup zmq test setup code (node restart, topics subscription) (Kittywhiskers Van Gogh)
982c1f03d4 merge bitcoin#19572: Create "sequence" notifier, enabling client-side mempool tracking (Kittywhiskers Van Gogh)
b0b4e0fa7f zmq: Make `g_zmq_notification_interface` a smart pointer (Kittywhiskers Van Gogh)
0a1ffd30b9 zmq: extend appending address to log msg for Dash-specific notifications (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* [bitcoin#19572](https://github.com/bitcoin/bitcoin/pull/19572) introduces tests in `interface_zmq.py` that validate newly introduced "sequence" reporting of all message types (`C`, `D`, `R` and `A`). The `R` message type (removed from mempool) is tested by leveraging the RBF mechanism, which isn't present in Dash.
In order to allow the tests to successfully pass, all fee bumping and RBF-specific code had to be removed from the tests. This test also involves creating a new block to test the `C` message (connected block) that would return a `time-too-new` error and fail unless mocktime has been disabled (which has been done in this test).
* When backporting [bitcoin#18309](https://github.com/bitcoin/bitcoin/pull/18309) ([dash#5728](https://github.com/dashpay/dash/pull/5728)), Dash-specific ZMQ notifications did not have those changes applied to them and that particular backport was merged upstream *after* [bitcoin#19572](https://github.com/bitcoin/bitcoin/pull/19572), meaning, for completion, the changes from [bitcoin#18309](https://github.com/bitcoin/bitcoin/pull/18309) have been integrated into [bitcoin#19572](https://github.com/bitcoin/bitcoin/pull/19572)'s backport.
As for the Dash-specific notifications, they have been covered to ensure uniformity in a separate commit.
* The ZMQ notification interface has been converted to a smart pointer in the interest of safety (this change is eventually done upstream in 8ed4ff8e05d61a8e954d72cebdc2e1d1ab24fb84 ([bitcoin#27125](https://github.com/bitcoin/bitcoin/pull/27125)))
## Breaking Changes
None expected.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] 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)_
ACKs for top commit:
UdjinM6:
utACK b75e83b298
PastaPastaPasta:
utACK b75e83b298
knst:
utACK b75e83b298
Tree-SHA512: 9f860d1203bebe0914a5102f101f646873d14754830d651fb91ed0d1285a6c1a58ffc492b07d4768324d94f53171c9a4da974cf4a0b1e5c665979eace289f6f0
2eadcf2f68 partial Merge bitcoin/bitcoin#29007: test: create deterministic addrman in the functional tests (stratospher)
f95ca4ed3e fix: unify with bitcoin: removed requirement of txindex=0 (Konstantin Akimov)
bf46e7a8e1 partial Merge bitcoin/bitcoin#28799: wallet: cache descriptor ID to avoid repeated descriptor string creation (Andrew Chow)
05e5966199 partial Merge bitcoin/bitcoin#27920: wallet: bugfix, always use apostrophe for spkm descriptor ID (furszy)
Pull request description:
## Issue being fixed or feature implemented
Lately our CI for tsan is flapping many functional tests and take long times.
This PR has several important changes backported from the latest bitcoin's version to improve CI experience
## What was done?
This PR has several backports that improved CI experience drastically.
**Firstly, it aims to fix flapping test p2p_node_network_limited.py**
For example: https://gitlab.com/dashpay/dash/-/jobs/7692635307
<details>
<summary>p2p_node_network_limited.py | ✖ Failed | 28 s</summary>
```
test 2024-08-29T02:50:53.929000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/builds/dashpay/dash/build-ci/dashcore-linux64_tsan/test/functional/test_framework/test_framework.py", line 158, in main
self.run_test()
File "/builds/dashpay/dash/build-ci/dashcore-linux64_tsan/test/functional/p2p_node_network_limited.py", line 79, in run_test
self.nodes[0].disconnect_p2ps()
File "/builds/dashpay/dash/build-ci/dashcore-linux64_tsan/test/functional/test_framework/test_node.py", line 611, in disconnect_p2ps
wait_until_helper(check_peers, timeout=5)
File "/builds/dashpay/dash/build-ci/dashcore-linux64_tsan/test/functional/test_framework/util.py", line 262, in wait_until_helper
raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
AssertionError: Predicate ''''
def check_peers():
for p in self.getpeerinfo():
for p2p in self.p2ps:
if p['subver'] == p2p.strSubVer:
return False
return True
''' not true after 5.0 seconds
```
</details>
**Secondly, it improves performance of Descriptor wallets significantly for case of `tsan` CI**. It is tiny improvement for Release build and local runs, but some fucnctional tests run as fast as twice:
https://gitlab.com/dashpay/dash/-/jobs/7694458953https://gitlab.com/dashpay/dash/-/jobs/7665132625
```
wallet_create_tx.py --descriptors | ✓ Passed | 236 s <-- new version
wallet_create_tx.py --legacy-wallet | ✓ Passed | 108 s
wallet_basic.py --descriptors | ✓ Passed | 135 s <---- new version
wallet_basic.py --legacy-wallet | ✓ Passed | 97 s
wallet_create_tx.py --descriptors | ✓ Passed | 456 s <-- old version
wallet_create_tx.py --legacy-wallet | ✓ Passed | 98 s
wallet_basic.py --descriptors | ✓ Passed | 189 s <--- old version
wallet_basic.py --legacy-wallet | ✓ Passed | 131 s
```
See performance investigation here: https://github.com/dashpay/dash/pull/6226
## How Has This Been Tested?
Run unit/functional tests
## 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
ACKs for top commit:
PastaPastaPasta:
utACK 2eadcf2f68
UdjinM6:
utACK 2eadcf2f68
Tree-SHA512: 127fbaa65c160aa95e2145a6b40d3811f7c42e36fbee9ce98a9ac021abd9cbe6edc7791870b331a54855ba891e3804885db7936ef212647b693f50f79a60d232
BACKPORT NOTICE
It includes only this commit: be25ac3092b7755e26e1ec6c33a27cd0e3dd9eac
[init] Remove -addrmantest command line arg
-addrmantest is only used in `p2p_node_network_limited.py` test to
test if the node self-advertises a hard-coded local address
(which wouldn't be advertised in the tests because it's unroutable
without the test-only code path) to check pruning-related services
are correct in that addr.
Remove -addrmantest because the self advertisement happens because
of hard coded test path logic, and expected services are nominal
due to how easily the test-only code could diverge from mainnet
logic. It's also being used only in 1 test.
0f7dc893ea1776515173dcd0bfe6826e963c90f3 test: compare `/chaininfo` response with `getblockchaininfo` RPC (brunoerg)
Pull request description:
The `/chaininfo` REST endpoint gets its infos from `getblockchaininfo` RPC, so this PR adds an `assert_equal` (in `interface_rest`) to ensure both responses are the same. Obs: other endpoints do the same for their respective RPC.
ACKs for top commit:
0xB10C:
Concept and Code Review ACK 0f7dc893ea1776515173dcd0bfe6826e963c90f3. Belts-and-spenders.
Tree-SHA512: 51cbcf988090272e406a47dc869710740b74e2222af29c05ddcbf53bd49765cdc59efb525e970867f091b3d2efec4fb13371a342d9e484e51144b760265bc5b8
Signed-off-by: Vijay <vijaydas.mp@gmail.com>
47c48b5f35b4910fcf87caa6e37407e67d879c80 test: only use verbose for getrawmempool when necessary in functional tests (Michael Dietz)
77349713b189e80f2c140db4df50177353a1cb83 test: use getmempoolentry instead of getrawmempool in functional tests when appropriate (Michael Dietz)
86dbd54ae8a8f9c693c0ea67114bbff24a0754df test: improve mempool_updatefrom efficiency by using getmempoolentry for specific txns (Michael Dietz)
Pull request description:
I don't think this changes the intention of the test. But it does shave ~30 seconds off the time it takes to run. From what I've seen our CI `macOS 11 native [gui] [no depends]` runs `mempool_updatefrom.py` in ~135 seconds. After this PR it should run in ~105 seconds
I noticed this improvement should probably be made when testing performance/runtimes of https://github.com/bitcoin/bitcoin/pull/22698. But I wanted to separate this out from that PR so the affects of each is decoupled
Edit: The major change in this PR is improving mempool_updatefrom.py's runtime as this is a very long running test. Then made the same efficiency improvements across all the functional tests as it made since to do that here
ACKs for top commit:
theStack:
Tested ACK 47c48b5f35b4910fcf87caa6e37407e67d879c80
Tree-SHA512: 40f553715f3d4649dc18c2738554eafaca9ea800c4b028c099217896cc1c466ff457ae814d59cf8564c782a8964d8fac3eda60c1b6ffb08bbee1439b2d34434b
Signed-off-by: Vijay <vijaydas.mp@gmail.com>
accf3d5868460b4b14ab607fd66ac985b086fbb3 [test] mempool package ancestor/descendant limits (glozow)
2b6b26e57c24d2f0abd442c1c33098e3121572ce [test] parameterizable fee for make_chain and create_child_with_parents (glozow)
313c09f7b7beddfdb74c284720d209c81dfdb94f [test] helper function to increase transaction weight (glozow)
f8253d69d6f02850995a11eeb71fedc22e6f6575 extract/rename helper functions from rpc_packages.py (glozow)
3cd663a5d33aa7ef87994e452bced7f192d021a0 [policy] ancestor/descendant limits for packages (glozow)
c6e016aa139c8363e9b38bbc1ba0dca55700b8a7 [mempool] check ancestor/descendant limits for packages (glozow)
f551841d3ec080a2d7a7988c7b35088dff6c5830 [refactor] pass size/count instead of entry to CalculateAncestorsAndCheckLimits (glozow)
97dd1c729d2bbedf9527b914c0cc8267b8a7c21b MOVEONLY: add helper function for calculating ancestors and checking limits (glozow)
f95bbf58aaf72aab8a9c5827b1f162f3b8ac38f4 misc package validation doc improvements (glozow)
Pull request description:
This PR implements a function to calculate mempool ancestors for a package and enforces ancestor/descendant limits on them as a whole. It reuses a portion of `CalculateMemPoolAncestors()`; there's also a small refactor to move the reused code into a generic helper function. Instead of calculating ancestors and descendants on every single transaction in the package and their ancestors, we use a "worst case" heuristic, treating every transaction in the package as each other's ancestor and descendant. This may overestimate everyone's counts, but is still pretty accurate in the our main package use cases, in which at least one of the transactions in the package is directly related to all the others (e.g. 1 parent + 1 child, multiple parents with 1 child, or chains).
Note on Terminology: While "package" is often used to describe groups of related transactions _within_ the mempool, here, I only use package to mean the group of not-in-mempool transactions we are currently validating.
#### Motivation
It would be a potential DoS vector to allow submission of packages to mempool without a proper guard for mempool ancestors/descendants. In general, the purpose of mempool ancestor/descendant limits is to limit the computational complexity of dealing with families during removals and additions. We want to be able to validate multiple transactions on top of the mempool, but also avoid these scenarios:
- We underestimate the ancestors/descendants during package validation and end up with extremely complex families in our mempool (potentially a DoS vector).
- We expend an unreasonable amount of resources calculating everyone's ancestors and descendants during package validation.
ACKs for top commit:
JeremyRubin:
utACK accf3d5
ariard:
ACK accf3d5.
Tree-SHA512: 0d18ce4b77398fe872e0b7c2cc66d3aac2135e561b64029584339e1f4de2a6a16ebab3dd5784f376e119cbafc4d50168b28d3bd95d0b3d01158714ade2e3624d
Signed-off-by: Vijay <vijaydas.mp@gmail.com>
1f4e1a17ed test: add test for governance inv expiration (UdjinM6)
06b4dba0d3 feat: make CInv python implementation aware of governance invs (UdjinM6)
c7c930ece6 fix: use correct condition in logs (UdjinM6)
d41d87a5be fix: use `std::chrono::seconds` (UdjinM6)
d6fe7146ff chore: make clang-format and linter happy (UdjinM6)
b57a9220c1 refactor: sqash 2 hash maps into one, use proper naming (UdjinM6)
4116ba3253 fix: let internal govobj/vote inv request trackers expire after 60 sec (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
Another issue noticed during recent sb voting period. Lots of inv are coming from peers that never send the object/vote they announced. Let's not wait for these forever, 60 seconds should be enough.
## What was done?
Add a "timer" to each request.
## How Has This Been Tested?
Run tests, run a MN on mainnet and check logs.
## 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
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 1f4e1a17ed
Tree-SHA512: f5c5a896f61b5aed27c6f42c926254156c604edb4efd2a3cd3738a7e8d1ed7bafffadc584148e4a5c1a172c2b3d61a06884a1f6d56eb4ebe37a4b0dbc050edd6
d6d2ab984547be4a9f7ba859a2a4c9ac9bfbf206 test: MiniWallet: fix fee calculation for P2PK and check tx vsize (Sebastian Falbesoner)
ce024b1c0ef2dcd307023aaaab40373c8bf17db1 test: MiniWallet: force P2PK signature to have fixed size (71 bytes) (Sebastian Falbesoner)
Pull request description:
This PR is a follow-up to #21945. It aims to both fix the fee calculation for P2PK mode transactions and enable its vsize check. Currently, the latter assumes a fixed tx length, which is fine for anyone-can-spend txs but doesn't apply to P2PK output spends due to varying DER signature size; the vsize check is therefore disabled for P2PK mode on master branch.
Creating one million DER signatures with MiniWallet shows the following distribution of sizes (smart people with better math skills probably could deduce the ratios without trying, but hey):
| DER signature size [bytes] | #occurences (ratio) |
| ------------- | ------------- |
| 71 | 498893 (49.89%) |
| 70 | 497244 (49.72%) |
| 69 | 3837 (0.38%) |
| 68 | 22 (0.0022%) |
Note that even smaller signatures are possible (for smaller R and S values with leading zero bytes), it's just that the probability decreases exponentially. Instead of choosing a large vsize check range and hoping that smaller signatures are never created (potentially leading to flaky tests), the proposed solution is ~~to limit the signature size to the two most common sizes 71 and 70 (>99.6% probability) and then accordingly only check for two vsize values; the value to be used for fee calculation is a decimal right between the two possible sizes (167.5 vbytes) and for the vsize check it's rounded down/up integer values are used.~~ to simply grind the signature to a fixed size of 71 bytes (49.89% probability, i.e. on average each call to `sign_tx()`, on average two ECC signing operations are needed).
~~The idea of grinding signatures to a fixed size (similar to https://github.com/bitcoin/bitcoin/pull/13666 which grinds to low-R values) would be counter-productive, as the signature creation in the test suite is quite expensive and this would significantly slow down tests that calculate hundreds of signatures (like e.g. feature_csv_activation.py).~~
For more about transaction sizes on different input/output types, see the following interesting article: https://medium.com/coinmonks/on-bitcoin-transaction-sizes-97e31bc9d816
ACKs for top commit:
MarcoFalke:
Concept ACK d6d2ab984547be4a9f7ba859a2a4c9ac9bfbf206
Tree-SHA512: 011c70ee0e4adf9ba12902e4b6c411db9ae96bdd8bc810bf1d67713367998e28ea328394503371fc1f5087a819547ddaea56c073b28db893ae1c0031d7927f32
fa7d71f270b89c9d06230d4ff262646f9ea29f4a test: Run pep-8 on touched test (MarcoFalke)
fab7e99c2a4b02a41b7448b45f0e6cdfdbb53ac3 test: Use MiniWallet in test_no_inherited_signaling RBF test (MarcoFalke)
fab871f649e3da4a5a5f6cffac3fc748bb1ca900 test: Remove unused generate() from test (MarcoFalke)
faff3f35b778d9af3d649b303d7edab49bfe40b4 test: Add txin.sequence option to MiniWallet (MarcoFalke)
Pull request description:
This comes with nice benefits:
* Less code and complexity
* Test can be run without wallet compiled in
Also add some additional checks for `getmempoolentry` (#22209) and other cleanups 🎨
ACKs for top commit:
mjdietzx:
Tested ACK fa7d71f270b89c9d06230d4ff262646f9ea29f4a thanks for the explanations, nicely done
theStack:
ACK fa7d71f270b89c9d06230d4ff262646f9ea29f4a 🍷
Tree-SHA512: 0e9b8fe985779d8d7034d256deed627125bb374b6ae2972c461b3a220739a51061c6147ad69339bee16282f82716c7f3f8a7a89c693ceb1e47ea50709272332a
01eedf3821f2c3ee6bab8733f8549531c844add7 test: doc: improve doc for chain_transaction() helper (Sebastian Falbesoner)
6e63e366d609312ab984b4439de2d59bb618620b test: refactor: dedup utility function chain_transaction() (Sebastian Falbesoner)
Pull request description:
Both tests `mempool_packages.py` and `mempool_package_onemore.py` define a utility function `chain_transaction` with a similar implementation. This PR deduplicates it by moving it into the util package and keeping the more general properties:
* pass a list of parent_txids/vouts instead of single values
* always mark the BIP125-replaceable flag for txs, created via `createrawtransaction` (this is needed by the `mempool_package_onemore.py` test, but doesn't hurt the other one)
This is a low-hanging fruit; as a potential follow-up one could probably also deduplicate the function `chain_transaction` in `rpc_packages.py`, which looks a bit different, as it also takes the parent locking script into account and doesn't send the tx.
ACKs for top commit:
mjdietzx:
reACK 01eedf3821f2c3ee6bab8733f8549531c844add7
klementtan:
Code review ACK 01eedf3821f2c3ee6bab8733f8549531c844add7
MarcoFalke:
review ACK 01eedf3821f2c3ee6bab8733f8549531c844add7 🙅
Tree-SHA512: ac7105d02c23f53d76d4ec9dc8de1074dd8faefeecd44b107921b78665279498966152fed312ecbe252a1c34a9643d531166329a4fea0e773df3bb75d43092b0
a3f0cbf82ddae2dd83001a9cc3a7948dcfb6fa47 test: run mempool_reorg.py even with wallet disabled (Darius Parvin)
Pull request description:
Run mempool_reorg.py test even when the wallet is disabled, as discussed in #20078.
As part of this PR I created a new method in `MiniWallet`, `create_self_transfer`, to return a raw tx (without broadcasting it) and its associated utxo.
ACKs for top commit:
MarcoFalke:
cr ACK a3f0cbf82ddae2dd83001a9cc3a7948dcfb6fa47
Tree-SHA512: 316a38faffadcb87499c1d6eca21e9696cef65362bbffcf621788a9b771bb1fa2971b1c7835cbd34b952d7612ad83afbca824cd8be39ecd6b994e8963027f991
5a1bef60a03b57de708a1a751bd90b8245fd8b83 test: refactor: remove binascii from test_framework (Zero-1729)
Pull request description:
This PR continues the work started in PR #22593, regarding using the `bytes` built-in module. In this PR specifically, instances of `binascii`'s methods `hexlify`, `unhexlify`, and `a2b_hex` have been replaced with the build-in `bytes` module's `hex` and `fromhex` methods where appropriate to make bytes <-> hex-string conversions consistent across the functional test files and test_framework.
Additionally, certain changes made are based on the following assumption:
```
bytes.hex(data) == binascii.hexlify(data).decode()
bytes.hex(data).encode() == binascii.hexlify(data)
```
Ran the functional tests to ensure behaviour is still consistent and changes didn't break existing tests.
closes#22605
ACKs for top commit:
theStack:
Code-review ACK 5a1bef60a03b57de708a1a751bd90b8245fd8b83 🔢
Tree-SHA512: 8f28076cf0580a0d02a156f3e1e94c9badd3d41c3fbdfb2b87cd8a761dde2c94faa5f4c448d6747b1ccc9111c3ef1a1d7b42a11c806b241fa0410b7529e2445f
Signed-off-by: Vijay <vijaydas.mp@gmail.com>
715bc1af66 test: check `masternode winners` before and after mn_rr (UdjinM6)
9d47cd2226 fix: adjust payee predictions after mn_rr activation (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
Payment predictions in GUI are wrong when mn_rr is active, `masternode winners` RPC is affected by the same issue too. Actual payments aren't affected.
## What was done?
Adjust calculations, add tests for `masternode winners`.
## How Has This Been Tested?
Run dash-qt on testnet, check "Next Payment" on "Masternode" tab. Run tests.
## 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
- [ ] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK 715bc1a
Tree-SHA512: 293c77974bcb50c6f9c51449d7bb12f89ad8db5871cad3a6083fe1951fe77e0deba8de7688b2f600fabe977bdc7390a66a984a6a076be19183c23742e00e27bf
c8092fb7fc Merge bitcoin/bitcoin#25214: multiprocess build fix: ipc/capnp/init.capnp.h: No such file or directory (fanquake)
1c40b5d161 Merge bitcoin/bitcoin#25088: Wallet: Ensure m_attaching_chain is set before registering for signals (Andrew Chow)
247fe418d6 Merge bitcoin/bitcoin#24984: wallet: ignore chainStateFlushed notifications while attaching chain (Andrew Chow)
93e7c38788 Merge bitcoin/bitcoin#24338: util: Work around libstdc++ create_directories issue (fanquake)
e80c2a383a Merge bitcoin/bitcoin#24308: util: use stronger-guarantee rename method (MarcoFalke)
f3775d96b0 Merge bitcoin/bitcoin#24251: Re-enable windows path tests disabled by #20744 (fanquake)
f2f298286e Merge bitcoin/bitcoin#23707: fuzz: Fix RPC internal bug detection (MarcoFalke)
d06b693864 Merge bitcoin/bitcoin#23654: fuzz: Rework rpc fuzz target (MarcoFalke)
e45229de76 Merge bitcoin/bitcoin#23385: refactor: get wallet path relative to wallet_dir (fanquake)
9d9d97d4fd Merge bitcoin/bitcoin#22218: multiprocess: Add ipc::Context and ipc::capnp::Context structs (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Batch of trivial backports
## What was done?
## How Has This Been Tested?
Built and tests locally
## Breaking Changes
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [ ] 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)_
ACKs for top commit:
knst:
utACK c8092fb7fc
UdjinM6:
utACK c8092fb7fc
Tree-SHA512: 7e3f95737b8c945e737396f130cddca24f71aa4d7f5ccb8ead62b8ba90e187af7036c1be0984e7f000a058606cc74e5381768dc2aad6e56d07a8238934198dcc
f4ba2bb769 feat: enforce DIP0001 from first block on regtest and drop fDIP0001ActiveAtTip (Konstantin Akimov)
5fa64bc4f8 feat: put brr activation to height=1 (Konstantin Akimov)
593c6cff14 feat: instant activation dip-0008 on regtest on first block (Konstantin Akimov)
cfd7ea2bc3 feat: activate DIP0020 on regtest from block 1 (Konstantin Akimov)
d1676b0280 Merge bitcoin/bitcoin#22818: test: Activate all regtest softforks at height 1, unless overridden (merge-script)
Pull request description:
## Issue being fixed or feature implemented
## What was done?
Backport bitcoin#22818 which helped to activate all forks from block-1 at regtest.
Activate next dash's softforks at block 1:
- DIP-0001 (blocksize 2mb)
- DIP-0020 (opcodes)
- DIP-0008 (chainlocks)
- BRR (block reward reallocation)
## How Has This Been Tested?
## Breaking Changes
All changes are relevant to RegTest only
## 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
ACKs for top commit:
PastaPastaPasta:
utACK f4ba2bb769
Tree-SHA512: 8d095365ff9e06ddcf47dbd457310ea2326998f0627d409651ab2fd35f6c1407cd3d2a23a4c636de359547782f4c43821944528229f3ea800cc65d3537595ea8
f4b896ef5f merge bitcoin#24266: Avoid buggy std::filesystem:::create_directories() call (Kittywhiskers Van Gogh)
b02d5e34ff merge bitcoin#30217: Update Boost download link (Kittywhiskers Van Gogh)
9e80893a3c partial bitcoin#28622: use macOS 14 SDK (Xcode 15.0) (Kittywhiskers Van Gogh)
05fb206f76 merge bitcoin#25898: remove WSL 1 workaround in fs (Kittywhiskers Van Gogh)
da5b433909 merge bitcoin#26952: Avoid `BOOST_NO_CXX98_FUNCTION_BASE` macro redefinition (Kittywhiskers Van Gogh)
7a1f48e53c merge bitcoin#25465: remove boost library detection (Kittywhiskers Van Gogh)
1d8b8900ef merge bitcoin#26557: Update Boost to 1.81.0 in depends (Kittywhiskers Van Gogh)
1ad64dadca merge bitcoin#25873: Boost 1.80.0 (Kittywhiskers Van Gogh)
d2c968bf91 merge bitcoin#25808: work around u8path deprecated-declaration warnings with libc++ (Kittywhiskers Van Gogh)
aa361b2717 merge bitcoin#24301: header-only Boost (Kittywhiskers Van Gogh)
357d1b6792 merge bitcoin#24252: Represent paths with fs::path instead of std::string (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Depends on https://github.com/dashpay/dash/pull/6138
## Breaking Changes
None observed.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
UdjinM6:
utACK f4b896ef5f
Tree-SHA512: a23b97a4e2ff5749b8b964a76fab40aa670d1b2c43debdd125d9a747f7e411523b080dc222ef5e2c6dcd9b190afd757392c33eb9e003dc1408d33f52b2e0ecb6
The flag fDIP0001ActiveAtTip shows a status of activation of DIP0001
This flag is used currently only in net.cpp for setup block buffer size.
Assume the bigger by default so far as DIP0001 is activated years ago
fa4db8671bb604e11b43a837f91de8866226f166 test: Activate all regtest softforks at height 1, unless overridden (MarcoFalke)
faad1e5ffda255aecf1b0ea2152cd4f6805e678f Introduce -testactivationheight=name@height setting (MarcoFalke)
fadb2ef2fa8561882db463f35df9b8a0e9609658 test: Add extra_args argument to TestChain100Setup constructor (MarcoFalke)
faa46986aaec69e4cf016101ae517ce8778e2ac5 test: Remove version argument from build_next_block in p2p_segwit test (MarcoFalke)
fa086ef5398b5ffded86e4f0d6633c523cb774e9 test: Remove unused ~TestChain100Setup (MarcoFalke)
Pull request description:
All softforks that are active at the tip of mainnet, should also be active from genesis in regtest. Otherwise their rules might not be enforced in user testing, thus making their testing less useful.
To still allow tests to check pre-softfork rules, a runtime argument can change the activation height.
ACKs for top commit:
laanwj:
Code review ACK fa4db8671bb604e11b43a837f91de8866226f166
theStack:
re-ACK fa4db8671bb604e11b43a837f91de8866226f166
Tree-SHA512: 6397d46ff56ebc48c007a4cda633904d6ac085bc76b4ecf83097c546c7eec93ac0c44b88083b2611b9091c8d1fb8ee1e314065de078ef15e922c015de7ade8bf
e4e7c440f4 fix: use proper chain instead using ActiveChain for test framework (Konstantin Akimov)
65b92fa093 Merge bitcoin/bitcoin#21862: test: Set regtest.BIP65Height = 111 to speed up tests (fanquake)
adcf095ab9 Merge bitcoin/bitcoin#22718: doc: Add missing PR 16333 release note (MarcoFalke)
101a863399 Merge bitcoin/bitcoin#16333: test: Set BIP34Height = 2 for regtest (MarcoFalke)
71af8816ef Merge bitcoin/bitcoin#22907: test: Avoid intermittent test failure in feature_csv_activation.py (merge-script)
fc25503cbc Merge bitcoin/bitcoin#22632: test: Set regtest.BIP66Height = 102 to speed up tests (W. J. van der Laan)
cbd2be8e18 Merge bitcoin/bitcoin#22597: consensus/params: simplify ValidDeployment check to avoid gcc warning (MarcoFalke)
8928146bfa Merge bitcoin/bitcoin#22550: test: improve `test_signing_with_{csv,cltv}` subtests (speed, prevent timeout) (MarcoFalke)
fb00431b7c Merge bitcoin/bitcoin#22385: refactor: Use DeploymentEnabled to hide VB deployments (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Backports from bitcoin related to hard-fork mechanism and accelerated action
## What was done?
see commits for backports.
Also fixed an issue of using from ChainState on RegTest: should be used specified chain, not the "best chain".
## How Has This Been Tested?
Run unit & functional tests
## Breaking Changes
That's a breaking changes for RegTest
## 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
ACKs for top commit:
PastaPastaPasta:
utACK e4e7c440f4
Tree-SHA512: c02813179177781a0da3beaff65732e12017f0056565b897891b2c5a06fda125a4e032571a02fad9d8640728df82c117393bd78bc6d406e0a30f72587b2d7344
b223c3c21e89f6af76b5401413880923f7c444d6 test: Add functional test for symlinked blocks directory (laanwj)
ddb75c2e87a60ed24065bdf0c3bfabf4e058cef1 test: Add fs_tests/create_directories unit test (Hennadii Stepanov)
1f46b6e46e1454b91ff7ceb31853bc440952f8eb util: Work around libstdc++ create_directories issue (laanwj)
Pull request description:
Work around libstdc++ issue [PR101510] with create_directories where the leaf already exists as a symlink. Fixes#24257, introduced by the switch to `std::filesystem`. It is meant to be more thorough than #24266, which worked around one instance of the problem.
The issue was [fixed upstream](https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=124eaa50e0a34f5f89572c1aa812c50979da58fc), but unfortunately we'll have to carry a fix for it for a while.
This introduces a function `fs::create_directories` which wraps
`std::filesystem::create_directories`. This allows easiliy reverting the
workaround when it is no longer necessary.
ACKs for top commit:
jonatack:
re-ACK b223c3c21e89f6af76b5401413880923f7c444d6 per `git range-diff df08250 67019cd b223c3c`
hebasto:
re-ACK b223c3c21e89f6af76b5401413880923f7c444d6
w0xlt:
re-ACK b223c3c
vasild:
ACK b223c3c21e89f6af76b5401413880923f7c444d6
Tree-SHA512: 028321717c8b10d16185c3711b35da6b05fb7aa31cee1c8c7e754e92bf5a0b02719a3785cd0f6f8bf052b3bd759f644af212320672baabc9e44e0b93ba464abc
40c0e06047 fix: use coinstatsindex instead blockfilterindex in feature_prunning (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
This is follow-up changes for backports bitcoin#15946 and bitcoin#19521
It fixes failure:
TestFramework (INFO): Test invalid pruning command line options
TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "dashtest/functional/test_framework/test_node.py", line 511, in assert_start_raises_init_error
ret = self.process.wait(timeout=self.rpc_timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/subprocess.py", line 1264, in wait
return self._wait(timeout=timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/subprocess.py", line 2038, in _wait
raise TimeoutExpired(self.args, timeout)
subprocess.TimeoutExpired: Command '['dashsrc/dashd', '-datadir=/tmp/dash_func_test_m8w6q7a2/node0', '-logtimemicros', '-debug', '-debugexclude=libevent', '-debugexclude=leveldb', '-mocktime=1417713337', '-uacomment=testnode0', '-logthreadnames', '-logsourcelocations', '-createwalletbackups=0', '-prune=550', '-blockfilterindex', '-mocktime=1417713337']' timed out after 120 seconds
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "dashtest/functional/test_framework/test_framework.py", line 159, in main
self.run_test()
File "dashtest/functional/feature_pruning.py", line 495, in run_test
self.test_invalid_command_line_options()
File "dashtest/functional/feature_pruning.py", line 149, in test_invalid_command_line_options
self.nodes[0].assert_start_raises_init_error(
File "dashtest/functional/test_framework/test_node.py", line 541, in assert_start_raises_init_error
self._raise_assertion_error(assert_msg)
File "dashtest/functional/test_framework/test_node.py", line 180, in _raise_assertion_error
raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] dashd should have exited within 120s with expected error Error: Prune mode is incompatible with -blockfilterindex.
TestFramework (INFO): Stopping nodes
## What was done?
Removed `blockfilterindex` test, enabled `coinstatsindex` test.
## How Has This Been Tested?
Run manually: `test/functional/feature_pruning.py`
This test is not run by our CI
## 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
ACKs for top commit:
PastaPastaPasta:
utACK 40c0e06047
UdjinM6:
ACK 40c0e06047
Tree-SHA512: 94005c52a39cc02be8df65898844584fb247b71abab36341c3d8b4e5018902931ed8554734f27bb5d54fae2b7867ba40f136fc089fd6f9f0e57852203a5d4011
That follow-up changes for backports bitcoin#15946 and bitcoin#19521
It fixes failure:
TestFramework (INFO): Test invalid pruning command line options
TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "dashtest/functional/test_framework/test_node.py", line 511, in assert_start_raises_init_error
ret = self.process.wait(timeout=self.rpc_timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/subprocess.py", line 1264, in wait
return self._wait(timeout=timeout)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.11/subprocess.py", line 2038, in _wait
raise TimeoutExpired(self.args, timeout)
subprocess.TimeoutExpired: Command '['dashsrc/dashd', '-datadir=/tmp/dash_func_test_m8w6q7a2/node0', '-logtimemicros', '-debug', '-debugexclude=libevent', '-debugexclude=leveldb', '-mocktime=1417713337', '-uacomment=testnode0', '-logthreadnames', '-logsourcelocations', '-createwalletbackups=0', '-prune=550', '-blockfilterindex', '-mocktime=1417713337']' timed out after 120 seconds
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "dashtest/functional/test_framework/test_framework.py", line 159, in main
self.run_test()
File "dashtest/functional/feature_pruning.py", line 495, in run_test
self.test_invalid_command_line_options()
File "dashtest/functional/feature_pruning.py", line 149, in test_invalid_command_line_options
self.nodes[0].assert_start_raises_init_error(
File "dashtest/functional/test_framework/test_node.py", line 541, in assert_start_raises_init_error
self._raise_assertion_error(assert_msg)
File "dashtest/functional/test_framework/test_node.py", line 180, in _raise_assertion_error
raise AssertionError(self._node_msg(msg))
AssertionError: [node 0] dashd should have exited within 120s with expected error Error: Prune mode is incompatible with -blockfilterindex.
TestFramework (INFO): Stopping nodes
c467cfffcebb30f829eeb8160166a6b941d97ed6 test: add coverage for `purpose` arg in `listlabels` (brunoerg)
Pull request description:
This PR adds test coverage for `listlabels` command when specifying the `purpose` (send and receive).
dcdfd72861/src/wallet/rpc/addresses.cpp (L698-L704)
ACKs for top commit:
kristapsk:
ACK c467cfffcebb30f829eeb8160166a6b941d97ed6
Tree-SHA512: 7e7143c1264692f7b22952e7c70dbe9ed3f5dcd2e3b69962a47be9f9c21b3f4a9089ca87962fbc8ff9116e7d2dbeb7f36d6a132c9ac13724a255cfe1b32373a8
