fbeb8c43bc5bce131e15eb9e162ea457bfe2b83e test: add type annotations to util.get_rpc_proxy (fanquake)
Pull request description:
Split out from #22092 while we address the functional test failure.
ACKs for top commit:
instagibbs:
ACK fbeb8c43bc
Tree-SHA512: 031ef8703202ae5271787719fc3fea8693574b2eb937ccf852875de95798d7fa3c39a8db7c91993d0c946b45d9b4d6de570bd1102e0344348784723bd84803a8
f685a13bef0418663015ea6d8f448f075510c0ec doc: GetTransaction()/getrawtransaction follow-ups to #22383 (John Newbery)
abc57e1f0882a1a2bb20474648419979af6e383d refactor: move `GetTransaction(...)` to node/transaction.cpp (Sebastian Falbesoner)
Pull request description:
~This PR is based on #22383, which should be reviewed first~ (merged by now).
In [yesterday's PR review club session to PR 22383](https://bitcoincore.reviews/22383), the idea of moving the function `GetTransaction(...)` from src/validation.cpp to src/node/transaction.cpp came up. With this, the circular dependency "index/txindex -> validation -> index/txindex" is removed (see change in `lint-circular-dependencies.sh`). Thanks to jnewbery for suggesting and to sipa for providing historical background.
Relevant IRC log:
```
17:52 <jnewbery> Was anyone surprised that GetTransaction() is in validation.cpp? It seems to me that node/transaction.cpp would be a more appropriate place for it.
17:53 <raj_> jnewbery, +1
17:53 <stickies-v> agreed!
17:54 <glozow> jnewbery ya
17:54 <jnewbery> seems weird that validation would call into txindex. I wonder if we remove this function, then validation would no longer need to #include txindex
17:54 <sipa> GetTransaction predates node/transaction.cpp, and even the generic index framework itself :)
17:55 <sipa> (before 0.8, validation itself used the txindex)
17:55 <jnewbery> (and GetTransaction() seems like a natural sibling to BroadcastTransaction(), which is already in node/transaction.cpp)
17:55 <jnewbery> sipa: right, this is not meant as a criticism of course. Just wondering if we can organize things a bit more rationally now that we have better separation between things.
17:55 <sipa> jnewbery: sure, just providing background
17:56 <sipa> seems very reasonable to move it elsewhere now
```
The commit should be trivial to review with `--color-moved`.
ACKs for top commit:
jnewbery:
Code review ACK f685a13bef0418663015ea6d8f448f075510c0ec
rajarshimaitra:
tACK f685a13bef
mjdietzx:
crACK f685a13bef0418663015ea6d8f448f075510c0ec
LarryRuane:
Code review, test ACK f685a13bef0418663015ea6d8f448f075510c0ec
Tree-SHA512: 0e844a6ecb1be04c638b55bc4478c2949549a4fcae01c984eee078de74d176fb19d508fc09360a62ad130677bfa7daf703b67870800e55942838d7313246248c
2ebf2fe0e4727a5a57a03f4283bdf1e263855803 test: check for RPC error 'Transaction already in block chain' (-27) (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for the RPC error "Transaction already in block chain" (error code `RPC_VERIFY_ALREADY_IN_CHAIN` = `RPC_TRANSACTION_ALREADY_IN_CHAIN` = -27), which is thrown in the function `BroadcastTransaction` (src/node/transaction.cpp).
ACKs for top commit:
kristapsk:
ACK 2ebf2fe0e4727a5a57a03f4283bdf1e263855803 (ran linter, looked at changes and ran modified test and checked code in `src/node/transaction.cpp`)
darosior:
ACK 2ebf2fe0e4727a5a57a03f4283bdf1e263855803
Tree-SHA512: 8bfbd3ff3da0cb3b8745f69b8ca2377f85fa99f0270750840b60e6ae43b5645c5c59b236993e8b2ad0444ec4171484e4f1ee23fa7e81b79d4222bcb623666fa5
20edf4bcf61e9fa310c3d7f3cac0c80a04df5364 rpc: Return block time in getblockchaininfo (João Barbosa)
Pull request description:
Return tip time in `getblockchaininfo`, for some use cases this can save a call to `getblock`.
ACKs for top commit:
naumenkogs:
ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364
theStack:
re-ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364
0xB10C:
ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364
kristapsk:
ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364
Zero-1729:
re-ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364
Tree-SHA512: 29a920cfff1ef53e0af601c3f93f8f9171f3be47fc84b0fa293cb865b824976e8c1510b17b27d17daf0b8e658dd77d9dc388373395f0919fc4a23cd5019642d5
489ebfd7a16443d8263c048d55622da297df7c39 tests: feature_backwards_compatibility.py test downgrade after upgrade (Andrew Chow)
Pull request description:
After upgrading the node, try to go back to the original version to make sure that using a newer node version does not prevent the wallet file from being downgraded again.
ACKs for top commit:
MarcoFalke:
ACK 489ebfd7a16443d8263c048d55622da297df7c39
Tree-SHA512: 86231de6514b3657912fd9d6621212166fd2b29b591fc97120092c548babcf1d6f50b5bd103b28cecde395a26809134f01c1a198725596c3626420de3fd1f017
80d4423f997e15780bfa3f91bf4b4bf656b8ea45 Test buffered valid message (Troy Giorshev)
Pull request description:
This PR is a tweak of #19302. This sends a valid message.
Additionally, this test includes logging in the same vein as #19272.
ACKs for top commit:
MarcoFalke:
tested ACK 80d4423f997e15780bfa3f91bf4b4bf656b8ea45 (added an assert(false) to observe deterministic coverage) 🌦
gzhao408:
ACK 80d4423f99👊
Tree-SHA512: 3b1aa5ec480a1661917354788923d64595e2886448c9697ec0606a81293e8b4a4642b2b3cc9afb2206ce6f74e5c6d687308c5ad19cb73c5b354d3071ad8496f8
Adds missing changes from p2p_invalid_tx.py but one assert is still disabled
fa4c29bc1d2425f861845bae4f3816d9817e622a test: Add various low-level p2p tests (MarcoFalke)
Pull request description:
ACKs for top commit:
jonatack:
ACK fa4c29bc1d242
Tree-SHA512: 842821b97359d4747c763398f7013415858c18a300cd882887bc812d039b5cbb67b9aa6f68434575dbc3c52f7eb8c43d1b293a59555a7242c0ca615cf44dc0aa
dc12f2e212dfacbe238cf68eb454b9ec71169bbc test: improve error msg on previous release tarball extraction failure (kdmukai)
7121fd8fa7de50ff67157f81f9e0f267b9795dbb test: self-sign previous release binaries for arm64 macOS (kdmukai)
Pull request description:
## The Problem
If you run `test/get_previous_releases.py -b` on an M1 or M2 mac, you'll get an unsigned v23.0 binary in the arm64 tarball. macOS [sets stricter requirements on ARM binaries](https://news.ycombinator.com/item?id=26996578) so the unsigned arm64 binary is apparently completely unusable without being signed/notarized(?).
This means that any test that depends on a previous release (e.g. `wallet_backwards_compatibility.py`) will fail because the v23.0 node cannot launch:
```
TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/Users/kdmukai/dev/bitcoin-core/test/functional/test_framework/test_framework.py", line 563, in start_nodes
node.wait_for_rpc_connection()
File "/Users/kdmukai/dev/bitcoin-core/test/functional/test_framework/test_node.py", line 231, in wait_for_rpc_connection
raise FailedToStartError(self._node_msg(
test_framework.test_node.FailedToStartError: [node 2] bitcoind exited with status -9 during initialization
```
This can also be confirmed by downloading bitcoin-23.0-arm64-apple-darwin.tar.gz (https://bitcoincore.org/bin/bitcoin-core-23.0/) and trying to run any of the binaries manually on an M1 or M2 mac.
## Solution in this PR
(UPDATED) Per @ hebasto, we can self-sign the arm64 binaries. This PR checks each binary in the previous release's "bin/" and verifies if the arm64 binary is signed. If not, attempt to self-sign and confirm success.
(note: an earlier version of this PR downloaded the x86_64 binary as a workaround but this approach has been discarded)
## Longer term solution
If possible, produce signed arm64 binaries in a future v23.x tarball?
Note that this same problem affects the new v24.0.1 arm64 tarball so perhaps a signed v24.x.x tarball would also be ideal?
That being said, this PR will check all current and future arm64 binaries and self-sign as needed, so perhaps we need not worry about pre-signing the tarball binaries. And I did test a version of `get_previous_releases.py` that includes the new v24.0.1 binaries and it successfully self-signed both v23.0 and v24.0.1, as expected.
## Further info:
Somewhat related to: https://github.com/bitcoin/bitcoin/issues/15774#issuecomment-1265164753
And @ fanquake noted on IRC that you can confirm which binaries are or are not signed via:
```
$ codesign -v -d bitcoin-qt
bitcoin-qt: code object is not signed at all
```
ACKs for top commit:
hebasto:
ACK dc12f2e212dfacbe238cf68eb454b9ec71169bbc
Tree-SHA512: 644895f8e97f5ffb3c4754c1db2c48abd77fa100c2058e3c896af04806596fc2b9c807a3f3a2add5be53301ad40ca2b8171585bd254e691f6eb38714d938396b
dba123167236a172d2d33861d58aa94a19729671 test: previous releases: add v23.0 (Sjors Provoost)
Pull request description:
Follows the same pattern as d8b705f1caeb3b4a6790cb26e4e5584ca791d965 (v22.0) and 8a57a06a5062dd8dfdefca4e404d0ddbd2a3da1d (v0.21.0).
Starting from v23.0 there is a separate macOS release for x86_64 and aarch64.
ACKs for top commit:
prusnak:
Approach ACK dba123167236a172d2d33861d58aa94a19729671
Tree-SHA512: 249aeddd5e80e163578581e5c8e9b6579f3694abc3d1fb68dddb7b42d75021ad85266688ec4a365a6631d82a65a19873aff7ba61c0ea59d21f8adbe4b772dc16
## Issue being fixed or feature implemented
Non-deterministic IS locks aren't used anymore since v18 dip24.
We should drop that support to make code simpler.
## What was done?
Dropped non-deterministic IS code, `evo_instantsend_tests` and
`feature_llmq_is_migration.py` (don't need it anymore), adjusted func
tests.
## How Has This Been Tested?
all tests, synced Testnet
## Breaking Changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] 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)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: Konstantin Akimov <545784+knst@users.noreply.github.com>
511aa4f1c7508f15cab8d7e58007900ad6fd3d5d Add unit test for ChaCha20's new caching (Pieter Wuille)
fb243d25f754da8f01793b41e2d225b917f3e5d7 Improve test vectors for ChaCha20 (Pieter Wuille)
93aee8bbdad808b7009279b67470d496cc26b936 Inline ChaCha20 32-byte specific constants (Pieter Wuille)
62ec713961ade7b58e90c905395558a41e8a59f0 Only support 32-byte keys in ChaCha20{,Aligned} (Pieter Wuille)
f21994a02e1cc46d41995581b54222abc655be93 Use ChaCha20Aligned in MuHash3072 code (Pieter Wuille)
5d16f757639e2cc6e81db6e07bc1d5dd74abca6c Use ChaCha20 caching in FastRandomContext (Pieter Wuille)
38eaece67b1bc37b2f502348c5d7537480a34346 Add fuzz test for testing that ChaCha20 works as a stream (Pieter Wuille)
5f05b27841af0bed1b6e7de5f46ffe33e5919e4d Add xoroshiro128++ PRNG (Martin Leitner-Ankerl)
12ff72476ac0dbf8add736ad3fb5fad2eeab156c Make unrestricted ChaCha20 cipher not waste keystream bytes (Pieter Wuille)
6babf402130a8f3ef3058594750aeaa50b8f5044 Rename ChaCha20::Seek -> Seek64 to clarify multiple of 64 (Pieter Wuille)
e37bcaa0a6dbb334ab6e817efcb609ccee6edc39 Split ChaCha20 into aligned/unaligned variants (Pieter Wuille)
Pull request description:
This is an alternative to #25354 (by my benchmarking, somewhat faster), subsumes #25712, and adds additional test vectors.
It separates the multiple-of-64-bytes-only "core" logic (which becomes simpler) from a layer around which performs caching/slicing to support arbitrary byte amounts. Both have their uses (in particular, the MuHash3072 code can benefit from multiple-of-64-bytes assumptions), plus the separation results in more readable code. Also, since FastRandomContext effectively had its own (more naive) caching on top of ChaCha20, that can be dropped in favor of ChaCha20's new built-in caching.
I thought about rebasing #25712 on top of this, but the changes before are fairly extensive, so redid it instead.
ACKs for top commit:
ajtowns:
ut reACK 511aa4f1c7508f15cab8d7e58007900ad6fd3d5d
dhruv:
tACK crACK 511aa4f1c7
Tree-SHA512: 3aa80971322a93e780c75a8d35bd39da3a9ea570fbae4491eaf0c45242f5f670a24a592c50ad870d5fd09b9f88ec06e274e8aa3cefd9561d623c63f7198cf2c7
## Issue being fixed or feature implemented
Some headers include other heavy headers, such as `logging.h`,
`tinyformat.h`, `iostream`. These headers are heavy and increase
compilation time on scale of whole project drastically because can be
used in many other headers.
## What was done?
Moved many heavy includes from headers to cpp files to optimize
compilation time.
In some places added forward declarations if it is reasonable.
As side effect removed 2 circular dependencies:
```
"llmq/debug -> llmq/dkgsessionhandler -> llmq/debug"
"llmq/debug -> llmq/dkgsessionhandler -> llmq/dkgsession -> llmq/debug"
```
## How Has This Been Tested?
Run build 2 times before refactoring and after refactoring: `make clean
&& sleep 10s; time make -j18`
Before refactoring:
```
real 5m37,826s
user 77m12,075s
sys 6m20,547s
real 5m32,626s
user 76m51,143s
sys 6m24,511s
```
After refactoring:
```
real 5m18,509s
user 73m32,133s
sys 6m21,590s
real 5m14,466s
user 73m20,942s
sys 6m17,868s
```
~5% of improvement for compilation time. That's not huge, but that's
worth to get merged
There're several more refactorings TODO but better to do them later by
backports:
- bitcoin/bitcoin#27636
- bitcoin/bitcoin#26286
- bitcoin/bitcoin#27238
- and maybe this one: bitcoin/bitcoin#28200
## 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
## Issue being fixed or feature implemented
`ConnectBlock` can fail after `ProcessSpecialTxsInBlock`, we shouldn't
be notifying too early. Same for `DisconnectBlock` but that's less of an
issue imo.
## What was done?
Move notifications to the end of `ConnectBlock`/`DisconnectBlock`. There
is no `connman` in `CChainState` and I don't want to pass it in updates
struct so I changed `NotifyMasternodeListChanged` and used `connman`
from `CDSNotificationInterface` instead.
## How Has This Been Tested?
run unit test, run testnet qt wallet
## 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
- [ ] 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
Be more explicit about the fact that spork24 is for non-mainnet only,
enforce it in code.
NOTE: I know we have EHF signalling disabled for mainnet in v20 but I
think it still makes sense to make sure spork24 condition won't slip
into mainnet in some future version accidentally.
## What was done?
pls see individual commits
## How Has This Been Tested?
## 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)_
## Issue being fixed or feature implemented
1. we _should not_ skip masternode payments checks below
nSuperblockStartBlock or when governance is disabled
2. we _should_ skip superblock payee checks while we aren't synced yet
(should help recovering from missed triggers)
## What was done?
pls see individual commits.
## How Has This Been Tested?
run tests, sync w/ and w/out `--disablegovernance`, reindexed on testnet
## 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)_
## Issue being fixed or feature implemented
With DIP29 added to v20, miners include best CL Signature in CbTx.
The purpose of this test, is to ensure that mining is still possible
when CL information isn't available.
In such case, miners are expected to copy best CL Signature from CbTx of
previous block.
## What was done?
Two scenarios are implemented:
- Add dynamically a node, make sure `getbestchainlock()` fails, let it
mine a block.
- Disable `SPORK_19_CHAINLOCKS_ENABLED`, add dynamically a node, make
sure `getbestchainlock()` fails, let it mine a block.
In both tests, we make sure the block is accepted by everyone and that
the `bestCLSignature` in CbTx is copied from previous block.
## How Has This Been Tested?
`feature_llmq_chainlocks.py`
## Breaking Changes
no
## 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)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Addressed issues and comments from [PR
comment](https://github.com/dashpay/dash/pull/5469#discussion_r1317886678)
and [PR
comment](https://github.com/dashpay/dash/pull/5469#discussion_r1338704082)
`Params()` should be const; global variable `CMNHFManager` is a better
out-come.
## What was done?
The helpers and direct calls of `UpdateMNParams` for each block to
update non-constant member in `Params()` is not needed anymore. Instead
`CMNHFManager` takes cares about status of Signals for each block,
update them dynamically and save in evo db.
## How Has This Been Tested?
Run unit/functional tests.
## Breaking Changes
Changed rpc `getblockchaininfo`.
the field `ehf` changed meaning: it's now only a flag -1/0; but it is
introduced a new field `ehf_height` now that a height.
## 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
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: thephez <thephez@users.noreply.github.com>
## Issue being fixed or feature implemented
Currently, on functional tests v20 activates at height 1440 which is
later than needed.
## What was done?
Reduced the window size of v20 from 480 to 400 which activates v20 at
1200.
Adjusted tests to this change.
Note regarding the window analysis for MN payments in
`feature_llmq_evo.py` (reduced from 256 to 48 blocks):
48 window is enough to analyse 4 MNs and 5 EvoNodes (Weighted count=24)
On my machine using develop:
`python3 feature_llmq_rotation.py 145.45s user 30.00s system 68% cpu
4:16.93 total`
With this PR:
`python3 feature_llmq_rotation.py 119.26s user 24.61s system 62% cpu
3:50.89 total`
## How Has This Been Tested?
all tests
## Breaking Changes
no
## 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)_
## Issue being fixed or feature implemented
MNs don't really vote NO on triggers that do not match their local
candidates because:
1. they bail out too early when they see that they are not the payee
2. the hash for objects to vote NO on was picked incorrectly.
## What was done?
Moved voting out of `CreateGovernanceTrigger` and into its own
`VoteGovernanceTriggers`. Refactored related code to use `optional`
while at it, dropped useless/misleading `IsValid()` call. Added some
safety belts, logging, tests.
## How Has This Been Tested?
Run 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
- [ ] 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
https://github.com/dashpay/dash/issues/5640
## What was done?
Tests that `activation_height` projected by `getblockchaininfo` during
locked_in phase.
Now, this test is only possible with v20 activation since v19, dip0024
are buried and mn_rr uses MNEF.
Enabled this test only in `feature_llmq_rotation.py`.
## How Has This Been Tested?
tests
## Breaking Changes
no
## 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)_
---------
Co-authored-by: Konstantin Akimov <knstqq@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
fixes#5666
kudos to @tinshen for discovering the issue 👍
## What was done?
add missing logic in FundTransaction
## How Has This Been Tested?
implement/run tests, test rpc manually
## 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
- [ ] 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
```
test/functional/feature_governance.py:205:59: F821 undefined name 'p0_amount'
test/functional/feature_governance.py:205:95: F821 undefined name 'p1_amount'
test/functional/feature_governance.py:205:131: F821 undefined name 'p2_amount'
```
## What was done?
add missing `self.`
## How Has This Been Tested?
run linter and `feature_governance.py`
## 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)_
fea75ad3caa29972db32d3ce7e0fe125ec77a0eb refactor: Drop `boost/algorithm/string/replace.hpp` dependency (Hennadii Stepanov)
857526e8cbb0847a865e9c2509425960d458f535 test: Add test case for `ReplaceAll()` function (Hennadii Stepanov)
Pull request description:
A new implementation of the `ReplaceAll()` seems enough for all of our purposes.
ACKs for top commit:
adam2k:
ACK Tested fea75ad3caa29972db32d3ce7e0fe125ec77a0eb
theStack:
Code-review ACK fea75ad3caa29972db32d3ce7e0fe125ec77a0eb
Tree-SHA512: dacfffc9d2bd1fb9f034baf8c045b1e8657b766db2f0a7f8ef7e25ee6cd888f315b0124c54aba7a29ae59186b176ef9868a8b709dc995ea215c6b4ce58e174d9
## Issue being fixed or feature implemented
Noticed a couple of things while I was trying to figure out if an
[issue](https://github.com/dashpay/dash/pull/5627#discussion_r1367153099)
@knst mentioned in #5627 could actually exist:
1. `GetPaymentsLimit()` won't work correctly with historical blocks rn.
We don't use it that way internally but it could be done via rpc and it
should provide correct results.
2. superblock params on regtest are too small to test them properly
3. because of (2) and a huge v20 activation window (comparing to sb
params) `feature_governance.py` doesn't test v20 switching states.
There's also no "sb on v20 activation block" test.
~NOTE: based on #5639 atm~
## What was done?
fix it, pls see individual commits
## How Has This Been Tested?
run 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
- [ ] 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
Small dip0024 related cleanups, regtest only.
## What was done?
pls see individual commits
## 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)_
## Issue being fixed or feature implemented
sb produced by sentinel:
>"DataString": ... \"payment_amounts\": \"20.00000000|20.00000000\", ...
>...
> "YesCount": 83,
sb produced by core:
>"DataString": ... \"payment_amounts\": \"20.00|20.00\", ...
> "YesCount": 13,
These 2 triggers are for the same block (900552), proposal hashes and
addresses are also the same but the difference in `payment_amounts`
format makes it look like a different trigger for core and this creates
a race.
## What was done?
Use `ValueFromAmount` instead of `FormatMoney` to avoid trimming
## 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)_
fac395e5eb2cd3210ba6345f777a586a9bec84e3 ci: Bump ci/lint/Dockerfile (MarcoFalke)
fa6eb6516727a8675dc6e46634d8343e282528ab test: Use python3.8 pow() (MarcoFalke)
88881cf7ac029aea660c2413ca8e2a5136fcd41b Bump python minimum version to 3.8 (MarcoFalke)
Pull request description:
There is no pressing reason to drop support for 3.7, however there are several maintenance issues:
* There is no supported operating system that ships 3.7 by default. (debian:buster is EOL and unmaintained to the extent that it doesn't run in the CI environment. See https://github.com/bitcoin/bitcoin/pull/27340#issuecomment-1484988445)
* Compiling python 3.7 from source is also unsupported on at least macos, according to https://github.com/bitcoin/bitcoin/pull/24017#issuecomment-1107820790
* Recent versions of lief require 3.8, see https://github.com/bitcoin/bitcoin/pull/27507#issuecomment-1517561645
Fix all maintenance issues by bumping the minimum.
ACKs for top commit:
RandyMcMillan:
ACK fac395e
fjahr:
ACK fac395e5eb2cd3210ba6345f777a586a9bec84e3
fanquake:
ACK fac395e5eb2cd3210ba6345f777a586a9bec84e3
Tree-SHA512: c198decdbbe29d186d73ea3f6549d8a38479383495d14a965a2f9211ce39637b43f13a4c2a5d3bf56e2d468be4bbe49b4ee8e8e19ec69936ff43ddf2b714c712
96299a9d6c0a6b9125a58a63ee3147e55d1b086b Test: Move common function assert_approx() into util.py (fridokus)
Pull request description:
To reduce code duplication, move `assert_approx` into common framework `util.py`.
`assert_approx()` is used in two functional tests.
ACKs for top commit:
theStack:
ACK 96299a9
practicalswift:
ACK 96299a9d6c0a6b9125a58a63ee3147e55d1b086b -- DRY is good and diff looks correct
fanquake:
ACK 96299a9d6c0a6b9125a58a63ee3147e55d1b086b - thanks for contributing 🍻
Tree-SHA512: 8e9d397222c49536c7b3d6d0756cc5af17113e5af8707ac48a500fff1811167fb2e03f3c0445b0b9e80f34935f4d57cfb935c4790f6f5463a32a67df5f736939
## Issue being fixed or feature implemented
The bug was introduced in the original PR #5026 and refactored later
(which is good actually cause we shouldn't mix refactoring and
bug-fixing :) )
## What was done?
fix conditions, add tests
## How Has This Been Tested?
`feature_asset_locks.py`
## 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)_
## Issue being fixed or feature implemented
Current implementation relies either on asserts or sometimes checks then
returning a special value; In the case of asserts (or no assert where we
use the value without checks) it'd be better to make it explicit to
function caller that the ptr must be not_null; otherwise gsl::not_null
will call terminate.
See
https://github.com/microsoft/GSL/blob/main/docs/headers.md#user-content-H-pointers-not_null
and
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-nullptr
I'm interested in a conceptual review; specifically on if this is
beneficial over just converting these ptrs to be a reference?
## What was done?
*Partial* implementation on using gsl::not_null in dash code
## How Has This Been Tested?
Building
## Breaking Changes
None
## 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)_
---------
Signed-off-by: pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Implementation EHF mechanism, part 4. Previous changes are:
- https://github.com/dashpay/dash/pull/4577
- https://github.com/dashpay/dash/pull/5505
- https://github.com/dashpay/dash/pull/5469
## Issue being fixed or feature implemented
Currently MN_RR is activated automatically by soft-fork activation after
v20 is activated.
It is not flexible enough, because platform may not be released by that
time yet or in opposite it can be too long to wait.
Also, any signal of EHF requires manual actions from MN owners to sign
EHF signal - it is automated here.
## What was done?
New spork `SPORK_24_MN_RR_READY`; new EHF manager that sign EHF signals
semi-automatically without manual actions; and send transaction with EHF
signal when signal is signed to network.
Updated rpc `getblockchaininfo` to return information about of EHF
activated forks.
Fixed function `IsTxSafeForMining` in chainlock's handler to skip
transactions without inputs (empty `vin`).
## How Has This Been Tested?
Run unit/functional tests. Some tests have been updated due to new way
of MN_RR activation: `feature_asset_locks.py`, `feature_mnehf.py`,
`feature_llmq_evo.py` and unit test `block_reward_reallocation_tests`.
## Breaking Changes
New way of MN_RR activation.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
## Issue being fixed or feature implemented
Calculation of `platformReward` should ignore fees and rely only on
Block subsidy.
cc @QuantumExplorer
## What was done?
From now on, the following formula is applied:
```
blockReward = blockSubsidy + feeReward
masternodeReward = masternodeShare(blockSubsidy)
platformReward = platformShare(masternodeReward)
masternodeReward += masternodeShare(feeReward)
```
## How Has This Been Tested?
## Breaking Changes
`plaftormReward` differs in networks where `mn_rr` is already active
## 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
- [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)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Currently, the `nSubsidyBase` calculation relies on difficulty. This
leads to variable Block Subsidity.
When Platform will be live, it would constantly require blocks
difficulty in order to calculate the `platformReward` (which relies on
Block Subsidy)
cc @QuantumExplorer
## What was done?
Starting from v20 activation, `nSubsidyBase` will no longer rely on
difficulty and will be constant to 5.
## How Has This Been Tested?
## Breaking Changes
Block rewards will differ.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
## Issue being fixed or feature implemented
Fixed a problem forgotten in #5588 in feature_asset_locks.py.
## What was done?
Avoid floating operations when calculating `coinbasevalue`
## 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
- [ ] 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)_
---------
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
## Issue being fixed or feature implemented
Historical masternode data returned via rpcs like `protx listdiff` can
be broken because some collaterals might be spent already and
`GetUTXOCoin` wasn't able to get any info.
## What was done?
Use `GetTransaction` as a fallback.
## 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)_
## Issue being fixed or feature implemented
avoid potential discrepancies in block reward calculations
## What was done?
use integers (int64_t) only when dealing with block rewards, no
float/double
## How Has This Been Tested?
run tests
## Breaking Changes
might fork off on devnets that use previous version
## 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)_
