fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6 Remove unused bits from service flags enum (MarcoFalke)
Pull request description:
Remove service bits that haven't been observed on the active network for years and won't ever be observed on the network with this meaning. Keeping this dead assignment in our source code forever doesn't add any value.
I somehow forgot to do this in commit fa0d0ff6e1bee60fde63724ae28a51aac5a94d4a.
ACKs for top commit:
laanwj:
Code review ACK fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6
practicalswift:
cr ACK fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6
fanquake:
ACK fa40168ab3102b9ad850f967a0e7fa22dbfbd0c6
Tree-SHA512: 376e5ac05940493cf2209fea60515c843e978c4b476f2524f6bf7a37a646d237c3ddcf6c0fa23641f9ba550f625609703d9b51b4be631a7f2a90e1092b557232
## Issue being fixed or feature implemented
Current implementation of MnEhfTx is not matched with DIP-0023, this PR
fixes it. It is a prior work for
https://github.com/dashpay/dash/pull/5469
## What was done?
- requestID is fixed from `clsig{quorumHeight}` to `mnhf{versionBit}` +
fixes for signature validation properly
- v20 is minimal height to accept MnEHF special transactions
- versionBit is not BLS version - removed unrelated wrong code and
validations
- TxMempool will accept MnEHF transaction even if inputs/outputs are
zeroes and no fee
- implemented python's serialization/deserialization of MnEHF
transactions for future using in functional tests
## How Has This Been Tested?
Run functional/unit tests. Beside that there's new functional test in
https://github.com/dashpay/dash/pull/5469 that actually test format of
transaction and signature validation - to be merged later.
## Breaking Changes
Payload of MnEhf tx is changed, related consensus rules are changed.
## 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
## Issue being fixed or feature implemented
This is an implementation of DIP0027 "Credit Asset Locks".
It's a mechanism to fluidly exchange between Dash and credits.
## What was done?
This pull request includes:
- Asset Lock transaction
- Asset Unlock transaction (withdrawal)
- Credit Pool in coinbase
- Unit tests for Asset Lock/Unlock tx
- New functional test `feature_asset_locks.py`
RPC: currently locked amount (credit pool) is available through rpc call
`getblock`.
## How Has This Been Tested?
There added new unit tests for basic checks of transaction validity
(asset lock/unlock).
Also added new functional test "feature_asset_locks.py" that cover
typical cases, but not all corner cases yet.
## Breaking Changes
This feature should be activated as hard-fork because:
- It adds 2 new special transaction and one of them [asset unlock tx]
requires update consensus rulels
- It adds new data in coinbase tx (credit pool)
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**To release DIP 0027**
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
0956e46bff7f0b6da65a4de6d4f8261fe9d7055c test: use zero-argument super() shortcut (Python 3.0+) (Sebastian Falbesoner)
Pull request description:
This mini-PR replaces all calls to `super(...)` with arguments with the zero-argument shortcut `super()` where applicable. See [PEP 3135](https://www.python.org/dev/peps/pep-3135/#specification):
> The new syntax:
>
> super()
>
> is equivalent to:
>
> super(__class__, <firstarg>)
>
> where __class__ is the class that the method was defined in, and <firstarg> is
> the first parameter of the method (normally self for instance methods, and cls
> for class methods).
ACKs for top commit:
fanquake:
ACK 0956e46bff7f0b6da65a4de6d4f8261fe9d7055c
Tree-SHA512: 4ac66fe7ab2be2e8a514e5fcfc41dbb298f21b23ebb7b7b0310d704b0b3cef8adf287a8d80346d1ea9418998c597b4f0ff1f66148d0d806bb43db6607e0fe1cf
## Issue being fixed or feature implemented
Implementation of Randomness Beacon Part 3.
Starting from v20 activation fork, members for quorums are sorted using
(if available) the best CL signature found in Coinbase.
If no CL signature is present yet, then the usual way is used (By using
Blockhash instead)
The actual new way to shuffle is already implemented in
https://github.com/dashpay/dash/pull/5366.
SPV clients also need to calculate members, but they only know block
headers.
Since Coinbase is in the actual block, then they lack the required
information to correctly calculate quorum members.
## What was done?
- Message `MNLISTIDFF` is enriched with a new field `quorumsCLSigs`.
This field holds the Chainlock Signature required for each set of
indexes corresponding to quorums in field `newQuorums`.
- Protocol version has been bumped to `70230`.
- Clients with protocol version greater or equal to `70230` will receive
the new field `quorumsCLSigs`.
- The same field is returned in `protx diff` RPC.
Note:
- Field `quorumsCLSigs` will populated only after v20 activation
- If for one or more quorums, no non-null CL sig was found in CbTx then
a null signature is returned in `quorumsCLSigs`.
## How Has This Been Tested?
- Functional test mininode's protocol version was bumped to `70230`.
- `feature_llmq_rotation.py` checks that `quorumsCLSigs` match in both
P2P and RPC messages.
## Breaking Changes
No
## 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 _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: thephez <thephez@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
## Issue being fixed or feature implemented
Version field should always be the first field of a message for better
readibility.
## What was done?
- Introduced new protocol version `MNLISTDIFF_VERSION_ORDER` (`70229`).
- `nVersion` serialisation order is changed for clients with protocol
version greater than or equal to `70229`.
- For clients with protocol version >= `70225` and < `70229` the old
order is used: can be deprecated in the future.
- Increased functional test P2P mininode's protocol version to `70229`.
## How Has This Been Tested?
`feature_llmq_rotation.py` with new protocol version.
## Breaking Changes
## 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 _(for repository
code-owners and collaborators only)_
## Issue being fixed or feature implemented
Mobile wallets would have to convert 4k+ pubkeys at the V19 fork point
and it's a pretty hard job for them that can easily take 10-15 seconds
if not more. Also after the HF, if a masternode list is requested from
before the HF, the operator keys come in basic scheme, but the
merkelroot was calculated with legacy. From mobile team work it wasn't
possible to convert all operator keys to legacy and then calculate the
correct merkleroot.
~This PR builds on top of ~#5392~ #5403 (changes that belong to this PR:
26f7e966500bdea4c604f1d16716b40b366fc707 and
4b42dc8fcee3354afd82ce7e3a72ebe1659f5f22) and aims to solve both of
these issues.~
cc @hashengineering @QuantumExplorer
## What was done?
Introduce `nVersion` on p2p level for every CSimplifiedMNListEntry. Set
`nVersion` to the same value we have it in CDeterministicMNState i.e.
pubkey serialization would not be via basic scheme only after the V19
fork, it would match the way it’s serialized on-chain/in
CDeterministicMNState for that specific MN.
## How Has This Been Tested?
run tests
## Breaking Changes
NOTE: `testnet` is going to re-fork at v19 forkpoint because
`merkleRootMNList` is not going to match
## 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)_
0ed2d8e07d3806d78d03a77d2153f22f9d733a07 test: add BIP37 remote crash bug [CVE-2013-5700] test to p2p_filter.py (Sebastian Falbesoner)
Pull request description:
Integrates the missing message type `filteradd` to the test framework and checks that the BIP37 implementation is not vulnerable to the "remote crash bug" [CVE-2013-5700](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-5700) anymore. Prior to v.0.8.4, it was possible to trigger a division-by-zero error on the following line in the function `CBloomFilter::Hash()`:
f0d6487e29/src/bloom.cpp (L45)
By setting a zero-length filter via `filterload`, `vData.size()` is 0, so the modulo operation above, called on any .insert() or .contains() operation then crashed the node. The test uses the approach of just sending an arbitrary `filteradd` message after, which calls `CBloomFilter::insert()` (and in turn `CBloomFilter::Hash()`) on the node. The vulnerability was fixed by commit 37c6389c5a (an intentional covert fix, [according to gmaxwell](https://github.com/bitcoin/bitcoin/issues/18483#issuecomment-608224095)), which introduced flags `isEmpty`/`isFull` that wouldn't call the `Hash()` member function if `isFull` is true (set to true by default constructor).
To validate that the test fails if the implementation is vulnerable, one can simply set the flags to false in the member function `UpdateEmptyFull()` (that is called after a filter received via `filterload` is constructed), which activates the vulnerable code path calling `Hash` in any case on adding or testing for data in the filter:
```diff
diff --git a/src/bloom.cpp b/src/bloom.cpp
index bd6069b..ef294a3 100644
--- a/src/bloom.cpp
+++ b/src/bloom.cpp
@@ -199,8 +199,8 @@ void CBloomFilter::UpdateEmptyFull()
full &= vData[i] == 0xff;
empty &= vData[i] == 0;
}
- isFull = full;
- isEmpty = empty;
+ isFull = false;
+ isEmpty = false;
}
```
Resulting in:
```
$ ./p2p_filter.py
[...]
2020-04-03T14:38:59.593000Z TestFramework (INFO): Check that division-by-zero remote crash bug [CVE-2013-5700] is fixed
2020-04-03T14:38:59.695000Z TestFramework (ERROR): Assertion failed
[...]
[... some exceptions following ...]
```
ACKs for top commit:
naumenkogs:
utACK 0ed2d8e07d3806d78d03a77d2153f22f9d733a07
Tree-SHA512: 02d0253d13eab70c4bd007b0750c56a5a92d05d419d53033523eeb3ed80318bc95196ab90f7745ea3ac9ebae7caee3adbf2a055a40a4124e0915226e49018fe8
00559229588feb19de2a0cb7506f70c483a1f433 test: add BIP37 'filterclear' test to p2p_filter.py (Sebastian Falbesoner)
Pull request description:
Integrates the message type `filterclear` to the test framework and adds a simple test to `p2p_filter.py`, checking that arbitrary txs get relayed again after deleting the filter.
ACKs for top commit:
naumenkogs:
utACK 00559229588feb19de2a0cb7506f70c483a1f433
Tree-SHA512: fe64e99a526865770707d8077b9968d3923f248045ec7fa56cd380dba85ac77a71a473d244ef3aede2fc0d287b8d7c6bc0156b6033b0c949c2058cc08e255697
## Issue being fixed or feature implemented
## What was done?
- Bumped version of `CbTx`. Added fields `bestCLHeightDiff`,
`bestCLSignature`
- Miner starting from v20 fork, includes best CL signature in `CbTx` (if
available) or null signature.
- All nodes should verify included CL signature before accepting the
block.
## How Has This Been Tested?
Basically, activated v20 on in the beginning of
`feature_llmq_chainlocks.py`
## Breaking Changes
Yes, new version of CbTx
## 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
This was reported/requested by @HashEngineering:
> Older versions of our App won't sync due to if (obj.nVersion ==
BASIC_BLS_VERSION) . Older versions don't know what version a SML Entry
is. As such, they will never read the type field. On the android client
this causes an offset problem when reading the mnlistdiff and it will
throw an exception that bans the peer that supplied it. Soon enough, no
peers will be left to connect to because they will all give the android
client bad data.
## What was done?
With this PR, SML will serialise the new v19 fields (`nType`,
`platformHTTPPort`, `platformNodeID`) if the client's version is at
least equal to `70227`.
Note: Serialisation for hashing skips the above rule.
Also, functional test mininode protocol version is set to `70227`.
## How Has This Been Tested?
## Breaking Changes
## 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
638441928a446726ce3a7fb20433a5478e7585bb test: add parameterized constructor for msg_sendcmpct() (Sebastian Falbesoner)
Pull request description:
While working on the test for #19776 I noticed that creating a `sendcmpct` message is quite cumbersome -- due to the lack of a parameterized constructor, one needs to create an empty (that is, initialized with default values) object and then set the two fields one by one. This PR replaces the default constructor with a parameterized constructor and uses it in the test `p2p_compactblocks.py`, reducing LOC. No need to pollute the namespace with temporary throw-away message objects anymore.
ACKs for top commit:
guggero:
Code review ACK 638441928a446726ce3a7fb20433a5478e7585bb.
epson121:
Code review ACK 638441928a446726ce3a7fb20433a5478e7585bb
Tree-SHA512: 3b58d276d714b73abc6cc98d1d52dec5f6026b33f03faaeb7dcbc5d83ac377555179f98b159b2b9ecc8957999c35a1dc082e3c69299c5fde4e35f1bd0587ce9d
854382885f18aa9a95cdde3d11591b05c305ad3f refactor: test: improve wait_for{header,merkleblock} interface (Sebastian Falbesoner)
1356a45ef042e7bd3d539fbb606d6b1be547d00f test: complete impl. of msg_merkleblock and wait_for_merkleblock (Sebastian Falbesoner)
Pull request description:
Implements the missing initialization/serialization methods for `msg_merkleblock`, based on the already present class `CMerkleBlock`. Also changes the method `wait_for_merkleblock()` to be more precise by waiting for a merkleblock with a specified blockhash instead of an arbitrary one.
In the BIP37 test `p2p_filter.py`, this new method is used to make the test of receiving merkleblock and tx if a filter is set to be more precise, by checking if they also arrive in the right order.
In the course of this PR, also the interface for the methods `wait_for_merkleblock()` and `wait_for_header()` are improved to take a hex string instead of an integer, which is more typesafe and less of a burden to the caller.
ACKs for top commit:
MarcoFalke:
ACK 854382885f18aa9a95cdde3d11591b05c305ad3f
Tree-SHA512: adaf0ac728ef0b9929cb417a7a7b4c1346c400b2d365bf6914515c67b6cfe8f4a7ecc62fb514afdce9792f0bed833416f6bca6b9620f3d5dcdc66e4d5b0b7ea3
e4f4ea47ebf7774fb6f445adde7bf7ea71fa05a1 lint: Catch use of [] or {} as default parameter values in Python functions (practicalswift)
25dd86715039586d92176eee16e9c6644d2547f0 Avoid using mutable default parameter values (practicalswift)
Pull request description:
Avoid common Python default parameter gotcha when mutable `dict`/`list`:s are used as default parameter values.
Examples of this gotcha caught during review:
* https://github.com/bitcoin/bitcoin/pull/16673#discussion_r317415261
* https://github.com/bitcoin/bitcoin/pull/14565#discussion_r241942304
Perhaps surprisingly this is how mutable list and dictionary default parameter values behave in Python:
```
>>> def f(i, j=[], k={}):
... j.append(i)
... k[i] = True
... return j, k
...
>>> f(1)
([1], {1: True})
>>> f(1)
([1, 1], {1: True})
>>> f(2)
([1, 1, 2], {1: True, 2: True})
```
In contrast to:
```
>>> def f(i, j=None, k=None):
... if j is None:
... j = []
... if k is None:
... k = {}
... j.append(i)
... k[i] = True
... return j, k
...
>>> f(1)
([1], {1: True})
>>> f(1)
([1], {1: True})
>>> f(2)
([2], {2: True})
```
The latter is typically the intended behaviour.
This PR fixes two instances of this and adds a check guarding against this gotcha going forward :-)
ACKs for top commit:
Sjors:
Oh Python... ACK e4f4ea47ebf7774fb6f445adde7bf7ea71fa05a1. Testing tip: swap the two commits.
Tree-SHA512: 56e14d24fc866211a20185c9fdb274ed046c3aed2dc0e07699e58b6f9fa3b79f6d0c880fb02d72b7fe5cc5eb7c0ff6da0ead33123344e1a872209370c2e49e3f
fa156999695ddaeb016d8320bee62f8d96679d55 test: Add basic test for BIP 37 (MarcoFalke)
Pull request description:
This does not add full coverage, but should be a good start and can be extended in the future. Currently, none of the BIP 37 p2p code has test coverage.
ACKs for top commit:
practicalswift:
Code review ACK fa156999695ddaeb016d8320bee62f8d96679d55 -- more testing coverage is better than less testing coverage
Tree-SHA512: d52e8be79240dffb769105c087ae0ae9305d599282546e4ca7379c4c7add2dbcd668265b46670aa07c357638044cf0f61a6fab7dba8971dd0f80c8f99768686e
## Issue being fixed or feature implemented
## What was done?
1. Increased protocol version of mininode to match v19 changes in
`MNLISTDIFF` P2P message
2. Added verification of MNs and HPMNs (dip4) in `feature_llmq_hpmn.py`
## How Has This Been Tested?
## Breaking Changes
## 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
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
b651ef7e1c submitheader: more directly test missing prev block header (Gregory Sanders)
1e7f741745 remove some magic mining constants in functional tests (Gregory Sanders)
Pull request description:
The fewer magic numbers the better.
Also more directly tested a `submitheader` case of bad previous blockhash.
Tree-SHA512: 52b01a6aa199fa909eea4e9e84409a901933e545724e33149cc4132c82168199fd678809b6d94d95c9ff6ad02238a9552363620d13b8beaa5d4b67ade9ef425c
* Compressed headers implementation.
First header is always compressed in a headers2 msg
Version is uncompressed if it’s not matched within the last 7 unique versions to be sent in the current msg
Service flag to signal that the peer supports compressed headers
If compressed headers services is active, the peer will receive headers compressed
If both sendheaders and sendheaders2 are sent, the peer will respond with compressed headers
Functional tests as for uncompressed headers
Updates regarding the existing functional tests to use the compressed headers if the NODE_HEADERS_COMPRESSED service flag is active
* style: add missing comma
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
When receiving an islock, propagate it as islock.
When creating/receiving and isdlock, propagate it as isdlock to peers which support it and as islock to peers which don't.
Functional tests to cover both islock and isdlock scenarios.
c82190cdb6 tests: Add Python dead code linter (vulture) (practicalswift)
590a57fdec tests: Remove unused testing code (practicalswift)
Pull request description:
Add Python dead code linter (`vulture`) to Travis.
Rationale for allowing dead code only after explicit opt-in (via `--ignore-names`):
* Less is more :-)
* Unused code is by definition "untested"
* Unused code can be an indication of bugs/logical errors. By making the contributor aware of newly introduced unused code it gives him/her an opportunity to investigate if the unused code they introduce is malignant or benign :-)
* Unused code is hard to spot for humans and is thus often missed during manual review
* [YAGNI](https://en.wikipedia.org/wiki/You_aren%27t_gonna_need_it)
Based on #14312 to make linter job pass.
Tree-SHA512: 4c581df7c34986e226e4ade479e0d3c549daf38f4a4dc4564b25564d63e773a1830ba55d1289c771b1fa325483e8855b82b56e61859fe8e4b7dfa54034b093b6
e460232876 Document fixed attribute behavior in critical test framework classes. (Justin Turner Arthur)
17b42f4122 Check for specific tx acceptance failures based on script signature (Justin Turner Arthur)
3a4449e9ad Strictly enforce instance attrs in critical functional test classes. (Justin Turner Arthur)
1d0ce94a54 Fix for incorrect version attr set on functional test segwit block. (Justin Turner Arthur)
ba923e32a0 test: Fix broken segwit test (practicalswift)
Pull request description:
No extra attributes will be able to be added to instances of the C++ class ports or of other critical classes without causing an exception. Helps prevent adding or depending on attributes that aren't in the intended object structure. It may prevent issues such as the one fixed in bitcoin/bitcoin#14300.
This request fixes the erroneous version attribute used in the p2p_segwit.py functional tests. This pull includes the commit from bitcoin/bitcoin#14300.
Tree-SHA512: 1b8c58e7aa0f71075ed5ff3e5be0a5182599108d8cd9bce682feac3b1842508124288e9335432b16a43f40f159c9710899e6d84af1b5868f48c947bc6f3e07ec
fa78a2fc67 [tests] Test that nodes respond to getdata with notfound (MarcoFalke)
Pull request description:
If a node has not announced a tx at all, then it should respond to
getdata messages for that tx with notfound, to avoid leaking tx
origination privacy.
In the future this could be adjusted such that a node responds with
notfound when a tx has not been announced to us, but that seems
to be a more involved change. See e.g.
https://github.com/jnewbery/bitcoin/commits/pr14220.1
Tree-SHA512: 6244afa5bd5d8fec9b89dfc02c9958bc370195145a0f3715f33200d6cf73a376c94193d44bf4523867196e6591c53ede8f9b6a77cb296b48c114a117b8c8b1fa
fa85c985ed qa: Add p2p_invalid_locator test (MarcoFalke)
Pull request description:
Should not be merged *before* #13907
Tree-SHA512: a67ca407854c421ed20a184d0b0dc90085aed3e3431d9652a107fa3022244767e67f67e50449b7e95721f56906836b134615875f28a21e8a012eb22cfe6a66a5
b55555d rpc: Add testmempoolaccept (MarcoFalke)
Pull request description:
To check if a single raw transaction makes it into the current transaction pool, one had to call `sendrawtransaction`. However, on success, this adds the transaction to the mempool with no easy way to undo.
The call `testmempoolaccept` is introduced to provide a way to solely check the result without changing the mempool state.
Tree-SHA512: 5afd9311190135cee8fc1f229c7d39bf893f1028f29e28d34f70df820198ff97b4bf86b41cbbd6e6c36a5c30073cefa92d541c74a4939c7a2a6fa283dfd41b63
* version: Bump PROTOCOL_VERSION and MIN_MASTERNODE_PROTO_VERSION
* version: Introduce LLMQ_DATA_MESSAGES_VERSION for QGETDATA/QDATA support
* test: Bump MY_VERSION to 70219 (LLMQ_DATA_MESSAGES_VERSION)
* llmq: Introduce CQuorumDataRequest as wrapper for QGETDATA requests
* llmq: Implement CQuorum::{SetVerificationVector, SetSecretKeyShare}
* llmq|net|protocol: Implement QGETDATA/QDATA P2P messages
* llmq: Restrict processing QGETDATA/QDATA to masternodes only
* llmq: Implement request limiting for QGETDATA/QDATA
* llmq: Implement CQuorumManger::RequestQuorumData
* rpc: Implement "quorum getdata" as wrapper around QGETDATA
Allows to trigger sending QGETDATA messages to connected peers by RPC.
* test: Handle QGETDATA/QDATA messages in mininode
* test: Add data structures to support QGETDATA/QDATA
* test: Add some helper in test_framework.py
* test: Implement tests for QGETDATA/QDATA in p2p_quorum_data.py
* test: Add p2p_quorum_data.py to BASE_SCRIPTS
* llmq|test: Add QWATCH support for QGETDATA/QDATA
* llmq: Store CQuorumPtr in cache, not CQuorumCPtr
* llmq: Fix cache usage after recent changes
* Use uacomment to create/find specific p2ps
* No need to use network adjusted time here, GetTime should be enough
* rpc: check proTxHash
* minor tweaks
* test: Adjustments after 4e27d6513e
* llmq: Rename and improve error lambda in CQuorumManager::ProcessMessage
* llmq: Process QDATA if -watchquorums is enabled
* test: Handle qwatch messages in mininode
* test: Add test for -watchquorums support
* test: Just some empty lines
* test: Properly stop the p2p network thread at the end of the test
* rpc: Adjust "quorum getdata" parameter descriptions
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
* rpc: Fix optionality of proTxHash in "quorum getdata" command
* test: Test optionality of proTxHash for "quorum getdata" command
* test: Be more specific about imports in p2p_quorum_data.py
* llmq|rpc: Add some comments about the request.GetDataMask checks
* test: Some more empty lines
* rpc: One more parameter description
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
* test: Unify assert statements / drop parentheses for all of them
* fix typo
Signed-off-by: pasta <pasta@dashboost.org>
* adjust some line wrapping to 80 chars
Signed-off-by: pasta <pasta@dashboost.org>
* tests: Seperate out into dif atomic methods, add logging
Signed-off-by: pasta <pasta@dashboost.org>
* test: Avoid restarting masternodes, just let available requests expire
Just takes a lot time and isn't required imo.
* test: Drop redundant code/tests after separation
This was introduced in 9e224ec2f2
* test: Merge three tests
"test_mnauth_restriction", "test_invalid_messages" and "test_invalid_unexpected_qdata" with the resulting name "test_basics" because i don't feel like DKG recovery thing should be part of a test called "test_invalid_messages" and giving it an own test probably wouldn't make a lot sense because it would still depend on "test_invalid_messages". I also think there is no need for a separated "test_invalid_unexpected_qdata".
* test: Rename test_ratelimiting_banscore -> test_request_limit
* test: Apply python style
* test: Wrap all at 120 characters
Thats the default "draw annoying warnings" setting for PyCharm (and IMO a reasonable line length).
* test: Move some variables
* test: Optimize for speed
* tests: use wait_until in get_mininode_id
* test: Don't use `!=` to check for `None`
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
* llmq: Split CSigShare creation/processing in CSigSharesManager
* rpc: Add "submit" parameter to "quorum sign"
* test: Add CSigShare and msg_qsigshare to messages.py
* test: Test the optional "submit" parameter of "quorum sign"
d280617bf569f84457eaea546541dc74c67cd1e4 [qa] Add a test for merkle proof malleation (Suhas Daftuar)
ed82f1700006830b6fe34572b66245c1487ccd29 have verifytxoutproof check the number of txns in proof structure (Gregory Sanders)
Pull request description:
Recent publication of a weakness in Bitcoin's merkle tree construction demonstrates many SPV applications vulnerable to an expensive to pull off yet still plausible attack: https://bitslog.wordpress.com/2018/06/09/leaf-node-weakness-in-bitcoin-merkle-tree-design/
This change would at least allow `verifytxoutproof` to properly validate that the proof matches a known block, with known number of transactions any time after the full block is processed. This should neuter the attack entirely.
The negative is that a header-only processed block/future syncing mode would cause this to fail until the node has imported the data required.
related: #13451
`importprunedfunds` needs this check as well. Can expand it to cover this if people like the idea.
Tree-SHA512: 0682ec2b622a38b29f3f635323e0a8b6fc071e8a6fd134c954579926ee7b516e642966bafa667016744ce49c16e19b24dbc8801f982a36ad0a6a4aff6d93f82b
* test: Add more C++ representing classes in message.py
* test: Add interface_zmq_dash.py
* test: Add interface_zmq_dash.py to BASE_SCRIPTS in test_runner.py
* test: Adjust hashrecoveredsig parsing
* Force node0 to finish syncing
* Avoid `uint256_to_string(uint256_from_str())`
* Be more specific in imports
* Plural for "publisher" when makes sense
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* test: Introduce uint256_to_string
* test: Use uint256_to_string in some places
* Avoid converting back and forth, reuse known string represntations of hashes for blocks and txes instead of converting sha256
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
