fa04f9b4ddffc5ef23c2ee7f3cc72a7c2ae49204 rpc: Remove duplicate name and argNames from CRPCCommand (MarcoFalke)
fa92912b4bb4629addcbfdfb7cc000be701614af rpc: Use RPCHelpMan for check-rpc-mappings linter (MarcoFalke)
faf835680be39811827504f77005b6603165f53e rpc: [refactor] Use concise C++11 code in CRPCConvertTable constructor (MarcoFalke)
Pull request description:
Currently, the RPC argument names are specified twice to simplify consistency linting. To avoid having to specify the argnames twice when adding new arguments, remove the linter and add an equivalent test based on RPCHelpMan.
ACKs for top commit:
laanwj:
ACK fa04f9b4ddffc5ef23c2ee7f3cc72a7c2ae49204
Tree-SHA512: 3f5f32f5a09b22d879f24aa67031639d2612cff481d6aebc6cfe6fd757cafb3e7bf72120b30466f59292a260747b71e57322c189d5478b668519b9f32fcde31a
2db69d7b81 chore: add release notes for "quorum platformsign" (Konstantin Akimov)
283c5f89a2 feat: create new composite command "quorum platformsign" (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
It splits from #6100
With just whitelist it is impossible to limit the RPC `quorum sign` to use only one specific quorum type, this PR aim to provide ability for quorum signing for platform quorum only.
## What was done?
Implemented a new composite command "quorum platformsign"
This composite command let to limit quorum type for signing for case of whitelist.
After that old way to limit platform commands can be deprecated - #6105
## How Has This Been Tested?
Updated a functional tests to use platform signing for Asset Unlocks feature.
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
utACK 2db69d7b81
PastaPastaPasta:
utACK 2db69d7b81
Tree-SHA512: b0dff9934137c4faa85664058e1e77f85067cc8d931e6d76ee5b9e610164ac8b0609736d5f09475256cb78d65bf92466624d784f0b13d20136df7e75613662cb
85abbb97b4 chore: add release notes for composite command for whitelist (Konstantin Akimov)
78ad778bb0 feat: test composite commands in functional test for whitelist (Konstantin Akimov)
a102a59787 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/66https://github.com/dashpay/dash-issues/issues/65
## What was done?
Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see https://github.com/dashpay/dash/pull/6052https://github.com/dashpay/dash/pull/6051https://github.com/dashpay/dash/pull/6055 and other related PRs
This PR makes whitelist feature to be compatible with composite commands.
Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things.
Platform at their side can use config such as this one (created based on shumkov's example):
```
rpc: {
host: '127.0.0.1',
port: 9998,
users: [
{
user: 'dashmate',
password: 'rpcpassword',
whitelist: null,
lowPriority: false,
},
{
username: 'platform-dapi',
password: 'rpcpassword',
whitelist: [],
lowPriority: true,
},
{
username: 'platform-drive-consensus',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
lowPriority: false,
},
{
username: 'platform-drive-other',
password: 'rpcpassword',
whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
],
lowPriority: true,
},
],
allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'],
},
```
## How Has This Been Tested?
Updated functional tests, see commits
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
LGTM, utACK 85abbb97b4
PastaPastaPasta:
utACK 85abbb97b4
Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
1840c9441a fix: drop extra pings - follow up for #18638 (UdjinM6)
264e7f9e62 Merge #18638: net: Use mockable time for ping/pong, add tests (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Split from https://github.com/dashpay/dash/pull/6102
## What was done?
So far as bitcoin#19499 is backported, bitcoin#18638 can be finished and removed related workarounds.
## 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 1840c9441a
PastaPastaPasta:
utACK 1840c9441a
Tree-SHA512: f34657c14514d6ee596175b3faf9ee44e58e8b0339939a0708d58ab2d119786830c183f9b236ed87c08ef8e1dbd031a38fc596b5aa4d38e10521658df4330e79
d3e842f605 refactor: remove dead code which has no use since composite commands are refactored (Konstantin Akimov)
58c5d431fe fix: follow-up to #6017 - enable one more assert in wallet_descriptor test (Konstantin Akimov)
dbed4a31af fix: update comment for wallet_keypool_hd due to bitcoin#17681 DNM (Konstantin Akimov)
4741bcc5c3 chore: remove outdated todo - removed by bitcoin#16898 (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Multiple TODO is reviewed and fixes in this PR
## What was done?
See commits
## 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 d3e842f605
PastaPastaPasta:
utACK d3e842f605
Tree-SHA512: 25080abcce8fa3dbb4e4c71d321b24cb9d23c073e6caf75366be58623023af50d28bc1cdac4098eb78a87b4fe0f3c33d39bb06257f0590b3e42e5fcad161ea2d
0133c9866d feat: add functional test for submitchainlock far ahead in future (Konstantin Akimov)
6004e06769 feat: return enum in RecoveredSig verifying code, apply for RPC submitchainlock (Konstantin Akimov)
130b6d1e96 refactor: replace static private member method to static method (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
Currently by result of `submitchainlock` impossible to distinct a situation when a signature is invalid and when a core is far behind and just doesn't know about signing quorum yet.
This PR aims to fix this issue, as requested by shumkov for needs of platform:
> mailformed signature and can’t verify signature due to unknown quorum is the same error?
> possible to distingush ?
## What was done?
Return enum in CL verifying code `chainlock_handler.VerifyChainLock`.
The RPC `submitchainlock` now returns error with code=-1 and message `no quorum found. Current tip height: {N} hash: {HASH}`
## How Has This Been Tested?
Functional test `feature_llmq_chainlocks.py` is updated
## Breaking Changes
`submitchainlock` return one more error code - not really a breaking change though, because v21 hasn't released yet.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
utACK 0133c9866d
PastaPastaPasta:
utACK 0133c9866d
Tree-SHA512: 794ba410efa57aaa66c47a67914deed97c1d060326e5d11a722c9233a8447f5e9215aa4a5ca401cb2199b8fc445144b2b2a692fc35494bf3296a74e9e115bda7
fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 net: Use mockable time for ping/pong, add tests (MarcoFalke)
faab4aaf2fa1153c6d76efc8113fa01b06943ece util: Add count_microseconds helper (MarcoFalke)
Pull request description:
Switch `CNode::m_ping_start` and `CNetMessage::m_time` to mockable time, so that tests can be added.
Mockable time is also type-safe, since it uses `std::chrono`
ACKs for top commit:
jonatack:
Code review re-ACK fa33654 re-read code, verified rebase per `git range-diff 4b5c919 fa94d6f fa33654`, previous tested ACKs still valid
troygiorshev:
ACK fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3
Tree-SHA512: 7d632bd6019ce7c882029e71b667a61517e783af82755a85dd979ef09380934e172dec8b8f91d57b200a30a6e096aeaf01f19fee7f3aed0e0e871c72eb44d70e
createwallet has changed list of arguments: createwallet "wallet_name" ( disable_private_keys blank "passphrase" avoid_reuse descriptors load_on_startup )
load_on_startup used to be an argument 5 but now has a number 6.
Both arguments 5 and 6 are boolean and it can confuse an user.
To prevent confusion if user is not aware about this breaking changes,
the RPC createwallet throws an exception if user trying to create descriptor wallet but has not mentioned load_on_startup.
This requirement can be removed when major amount of users updated to v21
6c5246803d test: add tests for both current and future behaviour (UdjinM6)
4e86bda4dc feat: stricter bestCLHeightDiff checks (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
Current `bestCLHeightDiff` checks are too relaxed
## What was done?
Make`bestCLHeightDiff` checks stricter
## How Has This Been Tested?
Run a node on mainnet/testet, run tests
## Breaking Changes
Old nodes aren't aware of this new logic so it's activated via `mn_rr` hardfork
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK 6c5246803d
Tree-SHA512: 2028d0ceb00a2270c92831ef38488d009d0bac47be4fc6a23ac93efdcf74847f1b9e99a529863fb4e14c65f120adda4e12c5b9e084d0f667d5f0fbaf80e3701d
b23d94b14f partial bitcoin#23706: getblockfrompeer followups (Kittywhiskers Van Gogh)
7e5cc5e375 merge bitcoin#23702: Add missing optional to getblockfrompeer (Kittywhiskers Van Gogh)
c294457b52 merge bitcoin#20295: getblockfrompeer (Kittywhiskers Van Gogh)
63ac87f011 merge bitcoin#22722: update estimatesmartfee rpc to return max of estimateSmartFee, mempoolMinFee and minRelayTxFee. (Kittywhiskers Van Gogh)
07e4c2cdd0 merge bitcoin#21934: Include versionbits signalling details during LOCKED_IN (Kittywhiskers Van Gogh)
960e7687d4 merge bitcoin#19651: importdescriptors update existing (Kittywhiskers Van Gogh)
1f31823fed merge bitcoin#21910: remove redundant fOnlySafe argument (Kittywhiskers Van Gogh)
69c5aa8947 merge bitcoin#21359: include_unsafe option for fundrawtransaction (Kittywhiskers Van Gogh)
169dce7e50 merge bitcoin#20286: deprecate addresses and reqSigs from rpc outputs (Kittywhiskers Van Gogh)
7cddf70c58 merge bitcoin#20867: Support up to 20 keys for multisig under Segwit context (Kittywhiskers Van Gogh)
7c59923845 merge bitcoin#18344: Fix nit in getblockchaininfo (Kittywhiskers Van Gogh)
ec0803a0f5 refactor: align `TxToUniv` argument list with upstream (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Closes https://github.com/dashpay/dash/issues/6000
* `TxToUniv`'s argument list needed to be rearranged to match upstream as closely as possible (i.e. placing Dash-specific arguments at the end of the list to allow for code to be backported unmodified, relying on default arguments instead of having to modify each invocation to insert the default argument in between).
This was due to a new `TxToUniv` variant being introduced in [bitcoin#20286](https://github.com/bitcoin/bitcoin/pull/20286)
* The maximum number of public keys in a multisig remains the same. The upper limit for bare multisigs is and always has been `MAX_PUBKEYS_PER_MULTISIG` ([source](19512988c6/src/script/interpreter.cpp (L1143-L1144))), which has always been 20 ([source](19512988c6/src/script/script.h (L28-L29))). The limit of up to 16 comes from P2SH overhead ([source](19512988c6/src/script/descriptor.cpp (L877-L880))) and that hasn't changed.
In effect, what [bitcoin#20867](https://github.com/bitcoin/bitcoin/pull/20867) does to Dash Core is change the error from "too many public keys" (as we'll be testing against the bare multisig limit) to "excessive redeemScript size" ([source](19512988c6/src/rpc/util.cpp (L223-L225))) (which is the _true_ limitation).
* Backporting [bitcoin#21934](https://github.com/bitcoin/bitcoin/pull/21934) required a minor change in the condition needed to emit `activation_height` as `has_signal` in the preceding block will evaluate true for both `STARTED` and `LOCKED_IN` while earlier it would only for `STARTED`.
* In [bitcoin#22722](https://github.com/bitcoin/bitcoin/pull/22722), a `self.stop_node` had to be added to account for the absurd fee warning that a new test condition introduces.
* [bitcoin#23706](https://github.com/bitcoin/bitcoin/pull/23706) is partial due to commits in it that rely on RPC type enforcement, which is currently not implemented in Dash Core.
* `feature_dip3_deterministicmns.py` depends on the reporting of `addresses` to validate multisigs containing expected payees. There is no replacement RPC call to report the pubkeys that compose a multisig address. In the interm, the `-deprecatedrpc=addresses` flag has been enabled to allow the test to run otherwise unmodified.
## Breaking Changes
* The following RPCs: `gettxout`, `getrawtransaction`, `decoderawtransaction`, `decodescript`, `gettransaction`, and REST endpoints: `/rest/tx`, `/rest/getutxos`, `/rest/block` deprecated the following fields (which are no longer returned in the responses by default): `addresses`, `reqSigs`.
The `-deprecatedrpc=addresses` flag must be passed for these fields to be included in the RPC response. Note that these fields are attributes of the `scriptPubKey` object returned in the RPC response. However, in the response of `decodescript` these fields are top-level attributes, and included again as attributes of the `scriptPubKey` object.
* When creating a hex-encoded Dash transaction using the `dash-tx` utility with the `-json` option set, the following fields: `addresses`, `reqSigs` are no longer returned in the tx output of the response.
* The error message for attempts at making multisigs with >16 pubkeys will change to an "excessive redeemScript size" instead of the previous "too many public keys".
## 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 b23d94b14f
PastaPastaPasta:
utACK b23d94b14f
Tree-SHA512: 659d9ba3a0a9f8594b307a7056ab172309d5a0d9efec605bc4b345f7e0fb1032ad303af9e8f51dbd6549e82d0b738ad41eae8a5b3aebf59081f39df0d4b5ec7c
c9a600e0fa fix: linkage error - message signer better to be common code rather than libconsensus (Konstantin Akimov)
8299b3b369 feat: protxregistar implementation for descriptor wallets (Konstantin Akimov)
6f45432f76 refactor: removed unused SignSpecialTxPayloadByString (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
RPC `protx updateregistar` uses forcely LegacyScriptPubKeyMan instead using CWallet's interface.
It causes a failures such as
```
test_framework.authproxy.JSONRPCException: This type of wallet does not support this command (-4)
```
## What was done?
New method `SignSpecialTxPayloadByHash` is implemented in interface instead exporting raw private key for some address.
See https://github.com/dashpay/dash-issues/issues/59 to track progress
## How Has This Been Tested?
Functional test `feature_dip3_deterministicmns.py` to run by both ways - legacy and descriptor wallets.
Run unit and functional tests.
Extra test done locally:
```diff
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -242,10 +242,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
if self.options.descriptors is None:
# Prefer BDB unless it isn't available
- if self.is_bdb_compiled():
- self.options.descriptors = False
- elif self.is_sqlite_compiled():
+ if self.is_sqlite_compiled():
self.options.descriptors = True
+ elif self.is_bdb_compiled():
+ self.options.descriptors = False
```
to flip flag descriptor wallets/legacy wallets for all 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
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK c9a600e0fa
Tree-SHA512: 00aea1cd9db3537b9a9dcdee096d47ea48337edeea3f52ad54aea91781678b8641ac2dd86b67f61f87e3912945bcb5361a42a3279b6c08bb8d9f096bed8fe842
3971613285 feat: functional tests for RPC getgovernanceinfo (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
https://github.com/dashpay/dash-issues/issues/63
## What was done?
It adds functional test for `getgovernanceinfo`
## How Has This Been Tested?
Run unit/functional tests
Check output of `test/functional/test_runner.py -j20 --previous-releases --coverage --extended`
```
Uncovered RPC commands:
- cleardiscouraged
- debug
- getblockheaders
- getmerkleblocks
- getpoolinfo
- voteraw
```
## 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
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK 3971613285
Tree-SHA512: f2d93db9068a707f2b33dd841b14f266db226fc5e907f19a49cf7a856b840f0c2c1f521c462996888933bb0ed636c288db12b1d36978c124cb536ea1ab9f3892
241f073932 feat: rpc external users are comma separated list (Konstantin Akimov)
68def970ad refactor: re-order arguments options alphabetically (Konstantin Akimov)
c7efd56a07 feat: rpc external users: use 2 queues but no extra threads (Konstantin Akimov)
c575a5808a feat: change handler to '/' for external users, use only rpc user name to choose queue (Konstantin Akimov)
f1c1fd873e feat: implementation for /external handler for RPC (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
To avoid struggling to response to critical rpc requests, and split them from 3rd parties who uses a node as an external service, there are introduced one more queue of requests that will be served without throttling for instance consensus important rpcs
## What was done?
new command line arguments:
- `rpcexternaluser` - List of comma-separated usernames for JSON-RPC external connections. If not specified, there's no special queue is created, all requests in one queue
- `rpcexternalworkqueue=<n>` - Set the depth of the work queue to service external RPC calls
## How Has This Been Tested?
Functional test `rpc_platform_filter.py` is updated to test new functionality
## Breaking Changes
NA
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
UdjinM6:
utACK 241f073932
PastaPastaPasta:
utACK 241f073932
Tree-SHA512: 15b371f24f5302853b85419e2b20c29749d6aae1c98a541d7471f1d3a681643063302c2a5ecce04dfad2da9101ea69d2f08a7e0e11a28609c6011d78273c57a7
18328279ec fix: force mnsync to skip gov obj sync on reconnection (UdjinM6)
08331bb950 fix: apply suggestions (UdjinM6)
3c3489d7a1 test: add test (UdjinM6)
41ab95dbf8 feat: skip governance checks for blocks below the best chainlock (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
A node can miss governance trigger sometimes and then it would stuck not being able to sync any further. This issue can be fixed manually by resetting sync status and reconsidering the "invalid" block. However, that's inconvenient. Also, what it does under the hood is it simply disables some parts of block validation. We could do that automagically and more precise if we would trust ChainLocks instead.
## What was done?
Skip governance checks for blocks below the best known chainlock, add tests.
## 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 _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 18328279ec
PastaPastaPasta:
utACK 18328279ec
Tree-SHA512: 3cc4e2707e24b36c9f64502561667d0cb66eced7019db7941781ab1b84cfd267b3dab4684c71b059e074450ea76dc8e342744bffdd1ca1be6ccceb34b3580659
faa137eb9eac5554504b062a6dc865ca87fd572b test: Speed up rpc_blockchain.py by removing miniwallet.generate() (MarcoFalke)
fa1fe80c757df0adcbfaf41b5c5c8a468bc07b6f test: Change address type from P2PKH to P2WSH in rpc_blockchain (MarcoFalke)
fa4d8f3169e38cbdbae20258efebe7070c49f522 test: Cache 25 mature coins for ADDRESS_BCRT1_P2WSH_OP_TRUE (MarcoFalke)
fad25153f5c8e88f72cf666b16b0b0dbdc45d3b1 test: Remove unused bug workaround (MarcoFalke)
faabce7d07c5776e4116b1a7ad1f6c408a4a4e46 test: Start only the number of nodes that are needed (MarcoFalke)
Pull request description:
Speed up various tests:
* Remove unused nodes, which only consume time on start/stop
* Remove unused "bug workarounds"
* Remove the need for `miniwallet.generate()` by adding `miniwallet.scan_blocks()`. (On my system, with valgrind, generating 105 blocks takes 3.31 seconds. Rescanning 5 blocks takes 0.11 seconds.)
ACKs for top commit:
laanwj:
Code review ACK faa137eb9eac5554504b062a6dc865ca87fd572b
Tree-SHA512: ead1988d5aaa748ef9f8520af1e0bf812cf1d72e281ad22fbd172b7306d850053040526f8adbcec0b9a971c697a0ee7ee8962684644d65b791663eedd505a025
fa61b9d1a68820758f9540653920deaeae6abe79 util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet (MarcoFalke)
7777105a24a36b62df35d12ecf6c6370671568c8 refactor: Move all command dependend checks to ExecuteWalletToolFunc (MarcoFalke)
fa06bce4ac17f93decd4ee38c956e7aa55983f0d test: Add tests (MarcoFalke)
fac05ccdade8b34c969b9cd9b37b355bc0aabf9c wallet: [refactor] Pass ArgsManager to WalletAppInit (MarcoFalke)
Pull request description:
This not only moves the parsing responsibility out from the wallet tool, but it also makes it easier to implement bitcoin-util #19937Fixes: #20902
ACKs for top commit:
ajtowns:
ACK fa61b9d1a68820758f9540653920deaeae6abe79
fjahr:
Code review ACK fa61b9d1a68820758f9540653920deaeae6abe79
Tree-SHA512: 79622b806e8bf9dcd0dc24a8a6687345710df57720992e83a41cd8d6762a6dc112044ebc58fcf6e8fbf45de29a79b04873c5b8c2494a1eaaf902a2884703e47b
faaad1bbac46cfeb22654b4c59f0aac7a680c03a p2p: Ignore version msgs after initial version msg (MarcoFalke)
fad68afcff731153d1c83f7f56c91ecbb264b59a p2p: Ignore non-version msgs before version msg (MarcoFalke)
Pull request description:
Handshake misbehaviour doesn't cost us more than any other unknown message, so it seems odd to treat it differently
ACKs for top commit:
jnewbery:
utACK faaad1bbac46cfeb22654b4c59f0aac7a680c03a
practicalswift:
ACK faaad1bbac46cfeb22654b4c59f0aac7a680c03a: patch looks correct
Tree-SHA512: 9f30c3b5c1f6604fd02cff878f10999956152419a3dd9825f8267cbdeff7d06787418b41c7fde8a00a5e557fe89204546e05d5689042dbf7b07fbb7eb95cddff
fb8a4db8f6 Merge bitcoin/bitcoin#26717: test: Improve `check-doc.py` pattern (MarcoFalke)
349cad2865 Merge bitcoin/bitcoin#26708: clang-tidy: Fix `modernize-use-nullptr` in headers (MarcoFalke)
6bf786d168 Merge bitcoin/bitcoin#25735: net: remove useless call to IsReachable() from CConnman::Bind() (fanquake)
012b0b7169 Merge bitcoin/bitcoin#24258: test: check localaddresses in getnetworkinfo for nodes with proxy (MarcoFalke)
c67f527b0b Merge bitcoin-core/gui#448: Add helper to load font (Hennadii Stepanov)
8e0abeb1c1 Merge bitcoin-core/gui#345: Connection Type Translator Comments (Hennadii Stepanov)
688b66e9d1 Merge bitcoin-core/gui#266: Doc: Copyright: Fix embedded font file location (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Trivial backports
## What was done?
## How Has This Been Tested?
Built and ran tests locally; p2p_addr_relay.py fails locally. Not sure why
## 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:
UdjinM6:
utACK fb8a4db8f6
knst:
utACK fb8a4db8f6
Tree-SHA512: abb9469f25c6d45acea01da6d2b9cb6df94822f61d06f80e3008b32d2522016370a1e0b0c9ff95b92df22c4f227fc40f7765d76f1987eac7603155fe2d894593
89bb25d22a0e1c700dba4e3b754984c9b2b14836 test: check localaddresses in getnetworkinfo for nodes with proxy (brunoerg)
Pull request description:
This PR adds test coverage for the field `localaddresses` for `getnetworkinfo`. In this case, it verifies if this field is empty for all nodes since they are using proxy.
Reference:
515200298b/src/init.cpp (L449)
ACKs for top commit:
jonatack:
ACK 89bb25d22a0e1c700dba4e3b754984c9b2b14836
Tree-SHA512: 3c765c7060b6972c1ae5a1104734cd7669b650b5f6aa4f623f4299567732260da5083fef306a7c1e71c931f5d1396f24abad251d95c3d82b1f3ee0efee7fcd1f
1cbf3b9a53 merge bitcoin-core/gui#206: Display fRelayTxes and bip152_highbandwidth_{to, from} in peer details (Kittywhiskers Van Gogh)
239062192e merge bitcoin#20764: cli -netinfo peer connections dashboard updates (Kittywhiskers Van Gogh)
06a6f8444c merge bitcoin#25147: follow ups to #20799 (removing support for v1 compact blocks) (Kittywhiskers Van Gogh)
6274a571b7 merge bitcoin#20799: Only support version 2 compact blocks (Kittywhiskers Van Gogh)
f4ce573538 merge bitcoin#22340: Use legacy relaying to download blocks in blocks-only mode (Kittywhiskers Van Gogh)
73b8f84fdb merge bitcoin#22147: p2p: Protect last outbound HB compact block peer (Kittywhiskers Van Gogh)
2ce481849a merge bitcoin#20599: Tolerate sendheaders and sendcmpct messages before verack (Kittywhiskers Van Gogh)
799214b2c8 merge bitcoin#19776: expose high bandwidth mode state via getpeerinfo (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Version 2 of BIP152 high-bandwidth mode/compact blocks implements SegWit support.
As Dash does not implement SegWit, there has never been a need to implement v2 (and therefore, have all the code necessary to support both v1 and v2, that gets removed as part of making support v2 only).
* Despite that, the changes surrounding removing support for both versions (that in our case, do not apply as we never have supported v2) refactor the code in other ways and influence their behaviour. In the interest of upstream alignment, those changes have been backported.
* [bitcoin#19776](https://github.com/bitcoin/bitcoin/pull/19776) doesn't seem to work on its own without successive backports, specifically [bitcoin#20799](https://github.com/bitcoin/bitcoin/pull/20799), despite the latter being a later backport.
<details>
<summary>19776-only p2p_compactblocks.py run (9f2c868947cc254d021e1a9bd00eb7bc80061e81)</summary>
```
dash@825a14c32b73:/src/dash$ ./test/functional/p2p_compactblocks.py
2024-06-09T12:29:09.777000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_kb2nr5oe
2024-06-09T12:29:16.341000Z TestFramework (INFO): Testing SENDCMPCT p2p message...
2024-06-09T12:29:31.432000Z TestFramework (INFO): Testing compactblock construction...
2024-06-09T12:29:40.068000Z TestFramework (INFO): Testing compactblock requests...
2024-06-09T12:29:44.597000Z TestFramework (INFO): Testing getblocktxn handler...
2024-06-09T12:29:59.808000Z TestFramework (INFO): Testing compactblock requests/announcements not at chain tip...
2024-06-09T12:30:03.855000Z TestFramework (INFO): Testing handling of incorrect blocktxn responses...
2024-06-09T12:30:05.868000Z TestFramework (INFO): Testing reconstructing compact blocks from all peers...
2024-06-09T12:30:09.389000Z TestFramework (INFO): Testing end-to-end block relay...
2024-06-09T12:30:10.404000Z TestFramework (INFO): Testing handling of invalid compact blocks...
2024-06-09T12:30:12.418000Z TestFramework (INFO): Testing invalid index in cmpctblock message...
2024-06-09T12:30:14.384000Z TestFramework (INFO): Testing high-bandwidth mode states via getpeerinfo...
2024-06-09T12:30:16.893000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/src/dash/test/functional/test_framework/test_framework.py", line 158, in main
self.run_test()
File "./test/functional/p2p_compactblocks.py", line 849, in run_test
self.test_highbandwidth_mode_states_via_getpeerinfo()
File "./test/functional/p2p_compactblocks.py", line 791, in test_highbandwidth_mode_states_via_getpeerinfo
hb_test_node.send_and_ping(msg_block(block))
File "/src/dash/test/functional/test_framework/p2p.py", line 579, in send_and_ping
self.sync_with_ping(timeout=timeout)
File "/src/dash/test/functional/test_framework/p2p.py", line 596, in sync_with_ping
self.wait_until(test_function, timeout=timeout)
File "/src/dash/test/functional/test_framework/p2p.py", line 487, in wait_until
wait_until_helper(test_function, timeout=timeout, lock=p2p_lock, timeout_factor=self.timeout_factor)
File "/src/dash/test/functional/test_framework/util.py", line 249, in wait_until_helper
if predicate():
File "/src/dash/test/functional/test_framework/p2p.py", line 484, in test_function
assert self.is_connected
AssertionError
2024-06-09T12:30:17.396000Z TestFramework (INFO): Stopping nodes
2024-06-09T12:30:18.400000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_kb2nr5oe
2024-06-09T12:30:18.401000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_kb2nr5oe/test_framework.log
2024-06-09T12:30:18.401000Z TestFramework (ERROR):
2024-06-09T12:30:18.401000Z TestFramework (ERROR): Hint: Call /src/dash/test/functional/combine_logs.py '/tmp/dash_func_test_kb2nr5oe' to consolidate all logs
2024-06-09T12:30:18.401000Z TestFramework (ERROR):
2024-06-09T12:30:18.401000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-06-09T12:30:18.402000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
2024-06-09T12:30:18.402000Z TestFramework (ERROR):
```
</details>
<details>
<summary>20799-incl p2p_compactblocks.py run (aa116c4f0b4753b615f9483aa03adec5ee4fd655)</summary>
```
dash@825a14c32b73:/src/dash$ ./test/functional/p2p_compactblocks.py
2024-06-09T12:34:27.169000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_7d65lmhz
2024-06-09T12:34:32.695000Z TestFramework (INFO): Testing SENDCMPCT p2p message...
2024-06-09T12:34:51.288000Z TestFramework (INFO): Testing compactblock construction...
2024-06-09T12:34:55.325000Z TestFramework (INFO): Testing compactblock requests...
2024-06-09T12:34:59.861000Z TestFramework (INFO): Testing getblocktxn handler...
2024-06-09T12:35:07.460000Z TestFramework (INFO): Testing compactblock requests/announcements not at chain tip...
2024-06-09T12:35:09.503000Z TestFramework (INFO): Testing handling of incorrect blocktxn responses...
2024-06-09T12:35:11.519000Z TestFramework (INFO): Testing reconstructing compact blocks from all peers...
2024-06-09T12:35:15.039000Z TestFramework (INFO): Testing end-to-end block relay...
2024-06-09T12:35:16.055000Z TestFramework (INFO): Testing handling of invalid compact blocks...
2024-06-09T12:35:17.062000Z TestFramework (INFO): Testing invalid index in cmpctblock message...
2024-06-09T12:35:19.139000Z TestFramework (INFO): Testing high-bandwidth mode states via getpeerinfo...
2024-06-09T12:35:22.159000Z TestFramework (INFO): Stopping nodes
2024-06-09T12:35:23.163000Z TestFramework (INFO): Cleaning up /tmp/dash_func_test_7d65lmhz on exit
2024-06-09T12:35:23.163000Z TestFramework (INFO): Tests successful
```
</details>
* The backport of [bitcoin-core/gui#206](https://github.com/bitcoin-core/gui/pull/206) is a continuation of 3e8ba24c87 from [dash#5964](https://github.com/dashpay/dash/pull/5964)
* The backport of [bitcoin#20764](https://github.com/bitcoin/bitcoin/pull/20764) is a continuation of bd934c71eb from [dash#6034](https://github.com/dashpay/dash/pull/6034)
## Breaking changes
* The `getpeerinfo` RPC returns two new boolean fields, `bip152_hb_to` and `bip152_hb_from`, that respectively indicate whether we selected a peer to be in compact blocks high-bandwidth mode or whether a peer selected us as a compact blocks high-bandwidth peer.
High-bandwidth peers send new block announcements via a `cmpctblock` message rather than the usual inv/headers announcements. See BIP 152 for more details.
* Blocks-only mode will use legacy relaying instead of BIP152 high-bandwidth mode
## 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 1cbf3b9a53
PastaPastaPasta:
utACK 1cbf3b9a53
knst:
utACK 1cbf3b9a53
Tree-SHA512: 5947b622d8d57a1dc9445cd6e07d4ad690379416d0fcf04ed574269975d1beb704691a79ff081341f3c800cf11869d401f1ed90baa5449f371f9ce658f2d2e95
a93fec6f2d merge bitcoin#23380: Fix AddrMan::Add() return semantics and logging (Kittywhiskers Van Gogh)
d1a4b14b48 merge bitcoin#23354: Introduce new V4 format addrman (Kittywhiskers Van Gogh)
7a97aabfe0 test: restore pre-bitcoin#23306 tests to validate port distinguishment (Kittywhiskers Van Gogh)
1a050d6cb4 merge bitcoin#23306: Make AddrMan support multiple ports per IP (Kittywhiskers Van Gogh)
d56702aa0c merge bitcoin#23140: Make CAddrman::Select_ select buckets, not positions, first (Kittywhiskers Van Gogh)
19b0145379 merge bitcoin#22839: improve addrman logging (Kittywhiskers Van Gogh)
3910c68028 merge bitcoin#23053: Use public methods in addrman fuzz tests (Kittywhiskers Van Gogh)
b6ec8ab6df merge bitcoin#22950: Pimpl AddrMan to abstract implementation details (Kittywhiskers Van Gogh)
236cf36d88 merge bitcoin#22734: Avoid crash on corrupt data, Force Check after deserialize (Kittywhiskers Van Gogh)
2420ac9e53 merge bitcoin#23041: Add addrman deserialization error tests (Kittywhiskers Van Gogh)
8aefa9b93a merge bitcoin#22762: Raise InitError when peers.dat is invalid or corrupted (Kittywhiskers Van Gogh)
c9a645f814 merge bitcoin#22879: Fix format string in deserialize error (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependent on https://github.com/dashpay/dash/pull/6040.
* Dash already introduced support for storage of address-port pairs (referred to as "port discrimination") and allowed the usage of non-default ports in P2P with [dash#2168](https://github.com/dashpay/dash/pull/2168).
* Albeit this was only permitted for networks with `fAllowMultiplePorts` enabled (which at the time was `devnet` and as it stands currently, on every network except `mainnet`).
* Keeping in line with the above policy (discussion on lifting such restrictions on `mainnet` is outside the scope of this PR), when backporting [bitcoin#23306](https://github.com/bitcoin/bitcoin/pull/23306), changes have been made to retain the effects of `discriminate_ports`.
* This involves appending placing a `!m_discriminate_ports` condition to behaviour that otherwise would be _removed_ entirely.
* Additionally, in line with upstream backports, changes have been made that render port distinguishment _enabled_ as the new default in `addrman_tests` (the old default was to keep it _disabled_, to mirror `mainnet` and pre-change upstream behaviour).
* To ensure distinguishment _disabled_ works as expected, affected pre-backport tests were reintroduced with the `_nondiscriminate` suffix.
---
I would propose at some point to rename the flag to `ignore_port`/`suppress_port` (if not remove it altogether should the `mainnet` restriction be lifted) as discriminate (or distinguish) isn't immediately clear with if address entries will be discriminated/distinguished _using_ ports (i.e. considered) or ports will be discriminated _against_ (i.e. ignored).
## Breaking Changes
It's unclear if these backports result in serialization issues for older versions, as Dash Core technically supported address-port pairs since 0.12 and suppressed it on `mainnet` by setting zero-ing out the port ([source](19512988c6/src/addrman.cpp (L135-L138))), meaning even with port discrimination _disabled_, the serialization format should remain the same.
Regardless, following upstream backports, a new version has been introduced (v4) that marks the AddrMan format incompatible with older versions of Dash Core.
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(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 a93fec6f2d
PastaPastaPasta:
utACK a93fec6f2d
Tree-SHA512: 49b35af3e4eb660249ce9a65d5a539205d852e9c728da22dc88779f6b3b15c13cf91522896a313bfe2a91889fedf3b6b2cebdea12cc2bbe865ec3b85b6a5dfa8
6777ab73a2 Merge bitcoin-core/gui#682: Don't directly delete abandoned txes from GUI (Hennadii Stepanov)
6e1a8c1fdc Merge bitcoin/bitcoin#26628: RPC: Reject RPC requests with same named parameter specified multiple times (MarcoFalke)
7c28b01c78 Merge bitcoin/bitcoin#26666: refactor: Deleted unreachable code in httpserver.cpp (MarcoFalke)
478fe51ead Merge bitcoin/bitcoin#26100: doc: clarify that NetPermissionFlags::Implicit is only about whitelists (MarcoFalke)
69b19cbfc0 Merge bitcoin/bitcoin#26546: test: remove unused class `NodePongAdd1` (fanquake)
245df942c4 Merge bitcoin/bitcoin#26380: Revert "test: check importing wallets when blocks are pruned throw an error" (MacroFake)
3db3bd0d31 Merge bitcoin/bitcoin#24269: test: add functional test for `-discover` (Andrew Chow)
c72ef299da Merge bitcoin/bitcoin#25896: wallet: Log when Wallet::SetMinVersion sets a different minversion (Andrew Chow)
12b438cb46 Merge bitcoin/bitcoin#25925: doc: add `{import,list}descriptors` to list of descriptor RPCs (Andrew Chow)
73d64c48a9 Merge bitcoin/bitcoin#25738: depends: use a patch instead of sed in libxcb (fanquake)
1fae0c2bbb Merge bitcoin/bitcoin#25333: test: Fix out-of-range port collisions (MacroFake)
b75089667c Merge bitcoin/bitcoin#25231: ci: Install documented packages for "Win64" CI task (fanquake)
Pull request description:
## Issue being fixed or feature implemented
Batch of trivial backports
## What was done?
Trivial backports
## How Has This Been Tested?
Built; ran tests locally
## Breaking Changes
None
## 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:
utACK 6777ab73a2
Tree-SHA512: 60d68c8d0fb9875d0b2421cad97b46a9d4deb79e03ca011d66cc0595ed68bb7ad207f2fd95973a3b7a0b9bca2c2f0deaf0e1116f0312a4d9d315f009228fe485
