Commit Graph

24347 Commits

Author SHA1 Message Date
PastaPastaPasta
1c29238296
Merge pull request #5676 from knst/fix-ubuntu-23-10-backports
backport:  bitcoin#26189, bitcoin-core/gui#137, gui#46, partial bitcoin#25233
2023-11-07 07:44:34 -06:00
MacroFake
17d017ec88 partial Merge bitcoin/bitcoin#25233: compat: remove glibcxx sanity checks
It's partial due to this missing changes:
```
https://github.com/bitcoin/bitcoin/pull/25233/files#diff-4cb884d03ebb901069e4ee5de5d02538c40dd9b39919c615d8eaa9d364bbbd77L632
```

cc61bc2e19b1c8cb32778ef42746d32b02cc2671 compat: remove glibcxx sanity checks (fanquake)

Pull request description:

  These checks were added in #4339, (see also #4081), to test
  our back-compat stubs, however, those stubs no-longer exist (#22930),
  meaning that these checks are now just testing some specific standard
  library behaviour, without a particular rationale, or reason, compared
  to any other standard library functionlity we use.

  There has also been some discussion about our sanity checks in the
  context of the libbitcoinkernel refactoring, see https://github.com/bitcoin/bitcoin/pull/25065#discussion_r880668218.
  Removing the checks removes the need to worry about atleast the
  glibcxx checks.

  Also remove the list of checks from the doc in `init.h`, because it is
  incomplete, and anyone who wants to know what checks are included can
  look at the function.

  Guix Build (arm64):
  ```bash
  e18a81e25b4707cbe113fb4d3ba2459013c1178e7cecfe446e4f14ee5ecd2ce8  guix-build-cc61bc2e19b1/output/arm-linux-gnueabihf/SHA256SUMS.part
  9928cc38b79f827018cba0bdde98666b31806afcc79dd95a00acb8e153c36eec  guix-build-cc61bc2e19b1/output/arm-linux-gnueabihf/bitcoin-cc61bc2e19b1-arm-linux-gnueabihf-debug.tar.gz
  ebf4635ba4688899ae62e4bb17ebb2afb25c538c4a8068ef515920fd4e43754e  guix-build-cc61bc2e19b1/output/arm-linux-gnueabihf/bitcoin-cc61bc2e19b1-arm-linux-gnueabihf.tar.gz
  74c7e35b47c6d101fda7205f144d37150329b4c360db09d37b8c1437f3390898  guix-build-cc61bc2e19b1/output/arm64-apple-darwin/SHA256SUMS.part
  6e12643b17be9326f1d873dfe51a52c082671540792877af624b42ca9f6e1791  guix-build-cc61bc2e19b1/output/arm64-apple-darwin/bitcoin-cc61bc2e19b1-arm64-apple-darwin-unsigned.dmg
  1d86d0416c7a50afd7bd8d850f416b7c7277464ccc95e4dae53b5b59415fc83d  guix-build-cc61bc2e19b1/output/arm64-apple-darwin/bitcoin-cc61bc2e19b1-arm64-apple-darwin-unsigned.tar.gz
  84070843f23839e7191ad3a667eb63c45f668eb95afbfb3fcdfb8363320f67d4  guix-build-cc61bc2e19b1/output/arm64-apple-darwin/bitcoin-cc61bc2e19b1-arm64-apple-darwin.tar.gz
  bf6ccd7b8c40476b1dc52b491757313ea3e96c43a01c8aabaf39f94dc1837329  guix-build-cc61bc2e19b1/output/dist-archive/bitcoin-cc61bc2e19b1.tar.gz
  25e7e1ff7d8de38632abf9926343c8ba556209f3d03109c92864ffe72813a05f  guix-build-cc61bc2e19b1/output/powerpc64-linux-gnu/SHA256SUMS.part
  d0398de83841607b1bf921d4553b30ad5e2d70d0570e96a2eaaf2762e1103c79  guix-build-cc61bc2e19b1/output/powerpc64-linux-gnu/bitcoin-cc61bc2e19b1-powerpc64-linux-gnu-debug.tar.gz
  f09cdc2ac2a2bb644f4749f3d74b5210ddb531594c33d127a907f0223e7793e5  guix-build-cc61bc2e19b1/output/powerpc64-linux-gnu/bitcoin-cc61bc2e19b1-powerpc64-linux-gnu.tar.gz
  ef36a68ef4e5ee9b311df40062cd2296f897e7b1550e39e0643601cd7d469010  guix-build-cc61bc2e19b1/output/powerpc64le-linux-gnu/SHA256SUMS.part
  937b600a2b86304ccc5b6c71a7eaf8aa5e2020592724cef6a933a1955995480b  guix-build-cc61bc2e19b1/output/powerpc64le-linux-gnu/bitcoin-cc61bc2e19b1-powerpc64le-linux-gnu-debug.tar.gz
  eca4eec41e71fdf7a7b0fa4065afa49c47d3b9541ed2cb4d083ad4a0de102e37  guix-build-cc61bc2e19b1/output/powerpc64le-linux-gnu/bitcoin-cc61bc2e19b1-powerpc64le-linux-gnu.tar.gz
  981c0968c19905925a599cff357ec259c1e806bdb7691c7b52039be450bdad7c  guix-build-cc61bc2e19b1/output/riscv64-linux-gnu/SHA256SUMS.part
  89c709967f9a157256281fbf682aad246f2eaad9c2f1797c2787253cbabe12f8  guix-build-cc61bc2e19b1/output/riscv64-linux-gnu/bitcoin-cc61bc2e19b1-riscv64-linux-gnu-debug.tar.gz
  454cd830dd382e176f5a23041fc33f93937668245481b0dd29fc04882d9528eb  guix-build-cc61bc2e19b1/output/riscv64-linux-gnu/bitcoin-cc61bc2e19b1-riscv64-linux-gnu.tar.gz
  e0812c2dc492e5c5f06e3685d19da8fb29ed38d3b32821d293ef01cb4fefbd79  guix-build-cc61bc2e19b1/output/x86_64-apple-darwin/SHA256SUMS.part
  0e7d4241d8ac882a8091fa00a7813db87a3e5afec59627e45b6c910cfdd4a7b0  guix-build-cc61bc2e19b1/output/x86_64-apple-darwin/bitcoin-cc61bc2e19b1-x86_64-apple-darwin-unsigned.dmg
  3faaca046cbb2642445a7dd1f389ed7bf94a65de8372441c36d5cb79c030ce31  guix-build-cc61bc2e19b1/output/x86_64-apple-darwin/bitcoin-cc61bc2e19b1-x86_64-apple-darwin-unsigned.tar.gz
  73080f032a42db679baf0d09619671ac5b9d85be84a68bdd6b6709eb0e6465bd  guix-build-cc61bc2e19b1/output/x86_64-apple-darwin/bitcoin-cc61bc2e19b1-x86_64-apple-darwin.tar.gz
  07b6e1b6291404bca1044df4a45b6958b882ffb88c143ba98f1959960a394897  guix-build-cc61bc2e19b1/output/x86_64-linux-gnu/SHA256SUMS.part
  16b455f62398f4aa0d3821abb1cceb8151e31c2664e3f974a764a5b8702b50f3  guix-build-cc61bc2e19b1/output/x86_64-linux-gnu/bitcoin-cc61bc2e19b1-x86_64-linux-gnu-debug.tar.gz
  3c1a3a6a343f17b83f3b3d47e9426eccd2d0bcc7f824cd958fcf2cf06cdc3276  guix-build-cc61bc2e19b1/output/x86_64-linux-gnu/bitcoin-cc61bc2e19b1-x86_64-linux-gnu.tar.gz
  f05afa688ea7211b0049555385fb2acc26986e24d8d00893389160e07037e693  guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/SHA256SUMS.part
  8bcbae67dd0746c42e1e7c7db67725a69289b08e9aa97b873d443d0aa355615d  guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/bitcoin-cc61bc2e19b1-win64-debug.zip
  efa45e3b76e5ae08a8392d58e741325df572d92c7dd69b65d876cdcda541d2fc  guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/bitcoin-cc61bc2e19b1-win64-setup-unsigned.exe
  3a8c2461ca826138c3017d06279a79b4c6bee2a507ad362aa6e424f76678596c  guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/bitcoin-cc61bc2e19b1-win64-unsigned.tar.gz
  e56ae4f609d4e6a3ca5917a4bb763c91012ece2d236d6b62a666358791e43525  guix-build-cc61bc2e19b1/output/x86_64-w64-mingw32/bitcoin-cc61bc2e19b1-win64.zip
  ```

ACKs for top commit:
  MarcoFalke:
    lgtm ACK cc61bc2e19b1c8cb32778ef42746d32b02cc2671
  laanwj:
    Concept and code review ACK cc61bc2e19b1c8cb32778ef42746d32b02cc2671

Tree-SHA512: 3da6aba44eef3f864fcbe897db1faa964923756e68c6a713e444b5d01c6d3542c3d7ca26678760e81a7a9e3cd40bd90622d0a7b697c27166817ba4f1023661ef
2023-11-07 07:44:05 -06:00
MarcoFalke
bb976499bf Merge bitcoin-core/gui#46: refactor: Fix deprecation warnings when building against Qt 5.15
705c1f0648c72aa97e0ee699ff9a3da23fc9bd61 qt, refactor: Fix 'buttonClicked is deprecated' warnings (Hennadii Stepanov)
c2f4e5ea1d6f01713ac69aaf6018884028aa55bd qt, refactor: Fix 'split is deprecated' warnings (Hennadii Stepanov)
8e12d6996116e786e928077b22d9f47cee27319e qt, refactor: Fix 'QFlags is deprecated' warnings (Hennadii Stepanov)
fa5749c805878304c107bcae0ae5ffa401dc7c4d qt, refactor: Fix 'pixmap is deprecated' warnings (Hennadii Stepanov)
b02264cb5dfcef50eec8a6346471cbaa25370e00 qt, refactor: Fix 'QDateTime is deprecated' warnings (Hennadii Stepanov)

Pull request description:

  [What's New in Qt 5.15](https://doc.qt.io/qt-5/whatsnew515.html#deprecated-modules):
  > To help preparing for the transition to Qt 6, numerous classes and member functions that will be removed from Qt 6.0 have been marked as deprecated in the Qt 5.15 release.

  Fixes #36

ACKs for top commit:
  jonasschnelli:
    utACK 705c1f0648c72aa97e0ee699ff9a3da23fc9bd61
  promag:
    Tested ACK 705c1f0648c72aa97e0ee699ff9a3da23fc9bd61 on macos with Apple clang version 11.0.3 (clang-1103.0.32.62) and brew qt 5.15.1.

Tree-SHA512: 29e00535b4583ceec0dfb29612e86ee29bdea13651b548c6d22167917a4a10464af49160a12b05151030699f690f437ebb9c4ae9f130f66a722415222165b44f
2023-11-07 07:44:05 -06:00
Jonas Schnelli
f40b0f2a9c Merge bitcoin-core/gui#137: refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date
86b1ab64b1a5b56518787ef16ea54ddbbc97d83e refactor: Replace deprecated Qt::SystemLocale{Short,Long}Date (Hennadii Stepanov)

Pull request description:

  As all deprecated warning in Qt 5.15.0 were eliminated in #46, Qt 5.15.1 introduced another one that is fixed in this PR.

  Required for https://github.com/bitcoin/bitcoin/pull/20182.

  Details in Qt docs:
  - https://doc.qt.io/qt-5/qdatetime.html#toString-1
  - https://doc.qt.io/qt-5/qdate.html#toString-1

ACKs for top commit:
  jarolrod:
    Tested ACK 86b1ab6 on MacOS 10.15.7 and Arch Linux both with Qt 5.15.1
  jonasschnelli:
    Tested ACK 86b1ab64b1a5b56518787ef16ea54ddbbc97d83e

Tree-SHA512: 1dbba8ee70c895bf58317172a9901cdbe5503b1d6258f51caaae88d88d332d9fbd4697c995192d31e3618ddfd532c5f5881289b3af1184422e5a9263a1224115
2023-11-07 07:44:05 -06:00
fanquake
96a8ecae50 Merge bitcoin/bitcoin#26189: refactor: Do not discard try_lock() return value
30cc1c6609ad7868f73e88afe0b0233d395ec08c refactor: Drop `owns_lock()` call (Hennadii Stepanov)
bff4e068b69edd40a00466156f860bde2df29268 refactor: Do not discard `try_lock()` return value (Hennadii Stepanov)

Pull request description:

  Microsoft's C++ Standard Library uses the `[[nodiscard]]` attribute for `try_lock()`.
  See: https://github.com/microsoft/STL/blob/main/stl/inc/mutex

  This change allows to drop the current suppression for the warning C4838 and helps to prevent the upcoming warning C4858.
  See: 539c26c923

  Fixes bitcoin/bitcoin#26017.

  Split from bitcoin/bitcoin#25819.

ACKs for top commit:
  vasild:
    ACK 30cc1c6609ad7868f73e88afe0b0233d395ec08c

Tree-SHA512: ce17404e1c78af4f763129753caf8e5a0e1c91ba398778fe912f9fcc56a847e8112460d1a1a35bf905a593b7d8e0b16c6b099ad74976b67dca5f4f3eda6ff621
2023-11-07 07:44:05 -06:00
UdjinM6
6253aa2fec
chore: only report "bad" connection when it's actually bad (#5680)
## Issue being fixed or feature implemented
Having `<protxhash> is not connected to us, badConnection=0` doesn't
help when we don't expect it to be connected 🤷‍♂️

## What was done?


## 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)_
2023-11-07 07:41:27 -06:00
UdjinM6
c61fe0aacd
fix: actually vote NO on triggers we don't like, some additional cleanups and tests (#5670)
## 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)_
2023-11-06 23:45:42 +03:00
UdjinM6
322e332942
chore: bump chainparams on testnet (#5679)
## Issue being fixed or feature implemented

## What was done?

## How Has This Been Tested?
reindexed

## 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)_
2023-11-06 13:32:55 -06:00
Odysseas Gabrielides
a612b31aab
test: getblockchaininfo projected activation_height test (#5665)
## 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>
2023-11-06 13:32:05 -06:00
Konstantin Akimov
cae3fa5619
feat: reduce spamming logs with messages from v20 features (#5669)
## Issue being fixed or feature implemented
There's too much spamming log items related to new v20 features: credit
pool, asset locks, EHF manager, EHF Signaling for MN_RR.

Some logs are still spamming after this PR but related code is not
changed here https://github.com/dashpay/dash/pull/5658

## What was done?
 - Removed some log items, tidy-up other.
- logs that supposed to appear for each block are moved to new
categories EHF and CREDITPOOL

## How Has This Been Tested?
Run unit/functional tests, reviewed log output

## 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
2023-11-06 09:26:36 -06:00
UdjinM6
1b8d9ecc6b
docs: update tor and i2p docs (#5673)
## Issue being fixed or feature implemented
Fix Dash Core version in i2p.md and update list of nodes in tor.md (we
do not support v2 tor anymore).

## What was done?
ran a node with `-onlynet=oinion` and picked 8 nodes

## How Has This Been Tested?
n/a

## 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)_
2023-11-06 09:23:20 -06:00
UdjinM6
e20cb56bde
chore: avoid useless PoSePunish log spam (#5678)
## Issue being fixed or feature implemented
No need to log things like `punished MN <protxhash>, penalty 515->515
(max=515)`

(check block 907818 on testnet, it has a lot of these)

## What was done?

## How Has This Been Tested?
n/a

## 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)_
2023-11-06 09:16:43 -06:00
PastaPastaPasta
8f597a3f2b
Merge pull request #5675 from knst/fix-ubuntu-23-10
fix: reduce warnings during build on Ubuntu 23.10
2023-11-06 09:15:49 -06:00
Konstantin Akimov
0905bed3fb fix: replaced qrand() to QrandomGenerator due to deprecation
It fixes this and similar warnings:
```
qt/test/trafficgraphdatatests.cpp:145:23: warning: ‘int qrand()’ is deprecated: use QRandomGenerator instead [-Wdeprecated-declarations]
  145 |         int in = qrand() % 1000;
      |                  ~~~~~^~
```

Call of `qsrand` is not needed because QRandomGenerator is already randomly initialized
2023-11-06 09:15:29 -06:00
Konstantin Akimov
f2e8431f4a fix: use GUIUtil::TextWidth() instead deprecated width()
It fixes this warning:
```
qt/trafficgraphwidget.cpp:177:42: warning: ‘int QFontMetrics::width(const QString&, int) const’ is deprecated: Use QFontMetrics::horizontalAdvance [-Wdeprecated-declarations]
  177 |     const int nWidthBytes = fmInOut.width("1000 GB") + 2 * nPadding;
```
2023-11-06 09:15:29 -06:00
Konstantin Akimov
66432fd04e fix: add cast to void to avoid gcc warning inside string_cast.cpp
It fixes this warning:
```
bench/string_cast.cpp:85:84: warning: ignoring return value of ‘std::__cxx11::basic_string<_CharT, _Traits, _Allocator> std::operator+(__cxx11::basic_string<_CharT, _Traits, _Allocator>&&, __cxx11::basic_string<_CharT, _Traits, _Allocator>&&) [with _CharT = char; _Traits = char_traits<char>; _Alloc = allocator<char>]’, declared with attribute ‘nodiscard’ [-Wunused-result]
```
2023-11-06 09:15:29 -06:00
UdjinM6
0c26093915
fix: a couple of fixes for the way mnauth and probe nodes work (#5660)
## Issue being fixed or feature implemented
1. inactive MNs (`activeMasternodeInfo.proTxHash.IsNull() == true`)
should simply drop duplicated connections like regular nodes do.
2. we should not instantly drop inbound (potentially probe) connections
(even if `DeterministicOutboundConnection` results would say so), should
let `CMasternodeUtils::DoMaintenance` do that. This way a probing peer
should have a chance to get our `mnauth` back and mark this attempt as a
success. This should hopefully reduce the number of random unexplained
pose-punishments.
3. probe nodes must be disconnected ignoring everything else, quorum
nodes and relay members connect using their own logic which should not
interfere with the way probe nodes work. (meaningful changes only:
9134d964a0)

## What was done?
pls see individual commits

as a side-effect `activeMasternodeInfoCs` lock is moved out of
`ForEachNode`

## How Has This Been Tested?
run tests, run a testnet mn

## 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)_
2023-11-03 10:34:40 -05:00
UdjinM6
66223aed51
fix: FundTransaction should follow the same bip69 rules CreateTransaction does (#5667)
## 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)_
2023-11-03 10:05:37 -05:00
Odysseas Gabrielides
d910b3e465
chore(rpc): renamed from bitcoin to coins in help text (#5659)
## Issue being fixed or feature implemented


## What was done?
Renamed `bitcoin` to `coins` in help texts of mining RPCs.

## How Has This Been Tested?


## 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
- [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: UdjinM6 <UdjinM6@users.noreply.github.com>
2023-11-03 09:06:02 -05:00
UdjinM6
25ee1677ca
test: fix feature_governance.py (#5657)
## 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)_
2023-10-31 09:01:05 -05:00
PastaPastaPasta
0c8cc9777e
Merge pull request #5652 from PastaPastaPasta/mini-backport
backport: merge bitcoin#25803, #23082, #25079, #24974 (modernization PRs)
2023-10-31 09:00:40 -05:00
fanquake
3a669676ec Merge bitcoin/bitcoin#24974: refactor: Make FEELER_SLEEP_WINDOW type safe (std::chrono)
fa74e726c414f5f7a1e63126a69463491f66e0ec refactor: Make FEELER_SLEEP_WINDOW type safe (std::chrono) (MacroFake)
fa3b3cb9b5d944d34b1d5ac3e102ac333482a475 Expose underlying clock in CThreadInterrupt (MacroFake)

Pull request description:

  This gets rid of the `value*1000` manual conversion.

ACKs for top commit:
  naumenkogs:
    utACK fa74e726c414f5f7a1e63126a69463491f66e0ec
  dergoegge:
    Code review ACK fa74e726c414f5f7a1e63126a69463491f66e0ec

Tree-SHA512: 90409c05c25f0dd2f1c4dead78f707ebfd78b7d84ea4db9fcefd9c4958a1a3338ac657cd9e99eb8b47d52d4485fa3c947dce4ee1559fb56ae65878685e1ed9a3
2023-10-31 08:40:25 -05:00
MacroFake
6d534eebbc Merge bitcoin/bitcoin#25079: index: Change sync variables to use std::chrono::steady_clock
92b35aba224ad4440f3ea6c01c841596a6a3d6f4 index, refactor: Change sync variables to use `std::chrono::steady_clock` (w0xlt)

Pull request description:

  This PR refactors the sync variables to use `std::chrono::steady_clock` as it is best suitable for measuring intervals.

ACKs for top commit:
  jonatack:
    utACK 92b35aba224ad4440f3ea6c01c841596a6a3d6f4
  ajtowns:
    ACK 92b35aba224ad4440f3ea6c01c841596a6a3d6f4 - code review only

Tree-SHA512: cd4bafde47b30beb88c0aac247e41b4dced2ff2845c67a7043619da058dcff4f84374a7c704a698f3055c888d076d25503c2f38ace8fbc5456f624e0efe1e188
2023-10-31 08:40:25 -05:00
W. J. van der Laan
fb3d9a16bf Merge bitcoin/bitcoin#23082: build: improve gexauxval() detection, remove getauxval() weak linking
4446ef0a549d567a88d82b606aa8c47f115673f9 build: remove support for weak linking getauxval() (fanquake)
e56100c5b4daf2285dde9807bf654599aa19bd6b build: remove arm includes from getauxval() check (fanquake)

Pull request description:

  It was [pointed out in #23030](https://github.com/bitcoin/bitcoin/pull/23030#issuecomment-922893367) that we might be able to get rid of our weak linking of [`getauxval()`](https://man7.org/linux/man-pages/man3/getauxval.3.html) (`HAVE_WEAK_GETAUXVAL`) entirely, with only Android being a potential holdout:
  > I wonder if it's time to get rid of HAVE_WEAK_GETAUXVAL. I think it's confusing. Either we build against a C library that has this functionality, or not. We don't do this weak linking thing for any other symbols and recently got rid of the other glibc backwards compatibility stuff.
  > Unless there is still a current platform that really needs it (Android?), I'd prefer to remove it from the build system, it has caused enough issues.

  After looking at Android further, it would seem that given we are moving to using `std::filesystem`, which [requires NDK version 22 and later](https://github.com/android/ndk/wiki/Changelog-r22), and `getauxval` has been available in the since [API version 18](https://developer.android.com/ndk/guides/cpu-features#features_using_libcs_getauxval3), that shouldn't really be an issue. Support for API levels < 19 will be dropped with the NDK 24 release, and according to [one website](https://apilevels.com/), supporting API level 18+ will cover ~99% of devices. Note that in the CI we currently build with NDK version 22 and API level 28.

  The other change in this PR is removing the include of headers for ARM intrinsics, from the check for strong `getauxval()` support in configure, as they shouldn't be needed. Including these headers also meant that the check would basically only succeed when building for ARM. This would be an issue if we remove weak linking, as we wouldn't detect `getauxval()` as supported on other platforms. Note that we also use `getauxval()` in our RNG when it's available.

  I've checked that with these changes we detect support for strong `getauxval()` on Alpine (muslibc). On Linux, previously we'd be detecting support for weak getauxval(), now we detect strong support. Note that we already require glibc 2.17, and `getauxval()` was introduced in `2.16`.

  This is an alternative / supersedes #23030.

ACKs for top commit:
  laanwj:
    Code review and tested ACK 4446ef0a549d567a88d82b606aa8c47f115673f9

Tree-SHA512: 5f2a9e9cc2d63bddab73f0dcb169d4d6beda74622af82bc0439722f1189f81d052e2fc1eaf27056a7a606320d5ddc4c11075f0d051dd93d77c5e1c15337f354a
2023-10-31 08:40:25 -05:00
fanquake
51ccdca24d Merge bitcoin/bitcoin#25803: refactor: Drop boost/algorithm/string/replace.hpp dependency
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
2023-10-31 08:40:25 -05:00
PastaPastaPasta
5e69a5654e
chore: bump version to rc.2 (#5656)
## Issue being fixed or feature implemented
Bump version

## What was done?


## How Has This Been Tested?
Na

## Breaking Changes
Na

## 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)_
2023-10-30 10:19:50 -05:00
PastaPastaPasta
0a9302e4d8
refactor: additional if-init usage (#5593)
# Issue being fixed or feature implemented
Fixed some clang-tidy warnings

## What was done?
used more if-init

## How Has This Been Tested?
built

## 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)_

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2023-10-30 10:14:18 -05:00
UdjinM6
965f5b2063
fix: adjust GetPaymentsLimit to work correctly with historical blocks, adjust sb params on regtest, tweak tests (#5641)
## 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)_
2023-10-30 18:12:07 +03:00
UdjinM6
fa19c5ffee
fix: adjust LLMQ_TEST_DIP0024 params, mine_cycle_quorum should use correct size (#5655)
## 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)_
2023-10-30 10:03:22 -05:00
Konstantin Akimov
e0f0d865e2
fix: chain halt if some invalid asset lock transactions are in mempool (#5648)
## Issue being fixed or feature implemented
As discovered during platform testing by @shumkov , it seems as the
chain can halt in miner if somehow mempool would have several
transactions that are somehow invalid (maybe too low fee or something
else). They can't be mined, but miner can't prepare a valid block with
correct Credit Pool amount.

It is indeed can happen although I haven't reproduced it with functional
tests at the moment 🤷‍♂️

## What was done?
Refactored and simplified a logic of Credit Pool amount of validation
and added one more layer of validation: after all transaction are
actually added to block by miner, it is recalculated one more time.
Also used correct `pindexPrev` instead Tip() for EHF signals.

## How Has This Been Tested?
Before this changes platform failed with this error and chain halt:
```
2023-10-20T06:20:16Z (mocktime: 2023-10-20T06:28:29Z) ERROR: ConnectBlock(DASH): CheckCreditPoolDiffForBlock for block 9d635e1fd0d7a8a5bf16ce158d3a39cbf903864bb6d671769836ea7db6055230 failed with bad-cbtx-asse locked-amount
```
With changes from this PR platform is generate the asset-lock
transactions that are included to block and chain is not halt:
```
2023-10-27T10:45:37Z (mocktime: 2023-10-27T14:37:22Z) GetCreditPoolDiffForBlock: CCreditPool is CCreditPool(locked=32100015, currentLimit=32100015)
```

unit/functional tests are succeed.


## Breaking Changes
N/A; no 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
- [ ] 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>
2023-10-30 09:57:20 -05:00
PastaPastaPasta
7440078f5d
Merge pull request #5587 from PastaPastaPasta/backport-uint256-modern
backport: merge bitcoin#26105, #26345 (uint256 modern)
2023-10-30 09:50:34 -05:00
pasta
a2c3031dcc refactor: extend changes to uint512 2023-10-30 09:50:10 -05:00
Andrew Chow
5ad6088c93 Merge bitcoin/bitcoin#26345: refactor: modernize the implementation of uint256.*
935acdcc79d1dc5ac04a83b92e5919ddbfa29329 refactor: modernize the implementation of uint256.* (pasta)

Pull request description:

  - Constructors of uint256 to utilize Span instead of requiring a std::vector
  - converts m_data into a std::array
  - Prefers using `WIDTH` instead of `sizeof(m_data)`
  - make all the things constexpr
  - replace C style functions with c++ equivalents
      - memset -> std::fill
          This may also be replaced by std::memset, but I think that std::fill is more idiomatic of modern c++ and readable.
      - memcpy -> std::copy
          Note: In practice, implementations of std::copy avoid multiple assignments and use bulk copy functions such as std::memmove if the value type is TriviallyCopyable and the iterator types satisfy LegacyContiguousIterator. (https://en.cppreference.com/w/cpp/algorithm/copy)
          This could also likely be replaced by std::memcpy, but as said above, I believe the using std::copy is the more c++ way to do anything and is almost guaranteed to compile to the same asm
      - memcmp -> std::memcmp

ACKs for top commit:
  achow101:
    ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329
  hebasto:
    Approach ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329.
  aureleoules:
    reACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329
  john-moffett:
    ACK 935acdcc79d1dc5ac04a83b92e5919ddbfa29329
  stickies-v:
    Approach ACK 935acdcc7

Tree-SHA512: 4f1ba54ff2198eea0e505d41e73d552c84c60f6878d5c85a94a8ab57f39afc94ef8d79258e7afd01fa84ec2a99f4404bb877eecd671f65e1ee9273f3129fc650
2023-10-30 09:50:10 -05:00
fanquake
44e2edff1d Merge bitcoin/bitcoin#26105: Use ReadLE64 in uint256::GetUint64 instead of duplicating logic
04fee75bacb9ec3bceff1246ba6c8ed8a8759548 Use ReadLE64 in uint256::GetUint64() instead of duplicating logic (Pieter Wuille)

Pull request description:

  No need to have a (naive) copy of the `ReadLE64` logic inside `uint256::GetUint64`, when we have an optimized function for exactly that.

ACKs for top commit:
  davidgumberg:
    ACK 04fee75bacb9ec3bceff1246ba6c8ed8a8759548
  jonatack:
    ACK 04fee75bacb9ec3bceff1246ba6c8ed8a8759548 review, this use of ReadLE64() is similar to the existing invocation by Num3072::Num3072(), sanity checked that before and after this change GetUint64() returns the same result (debug build, clang 13)

Tree-SHA512: 0fc2681536a18d82408411bcc6d5c6445fb96793fa43ff4021cd2933d46514c725318da35884f428d1799023921f33f8af091ef428ceb96a50866ac53a345356
2023-10-30 09:50:10 -05:00
UdjinM6
d8db2e9125
revert: 5636, introduce -llmqtestinstantsend and -llmqtestinstantsenddip0024 (#5654)
## Issue being fixed or feature implemented
This reverts #5636 and introduces 2 similar cmd-line/config params which
are made specifically for regtest. Turned out Platform guys actually
still need smth like that for local testing #5259.

## What was done?
pls see individual commits

## How Has This Been Tested?
run tests but we don't really have(/need?) tests for this.

## 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)_

cc @shumkov
2023-10-30 09:34:46 -05:00
UdjinM6
7d1e3d4d0d
fix: do not trim values in payment_amounts (#5647)
## 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)_
2023-10-27 19:59:44 -05:00
Konstantin Akimov
a0c8c9f0a5
fix: possible assert call if nHeight in CDeterministicMNList is higher then Tip (#5590)
## Issue being fixed or feature implemented
fix: possible assert call if nHeight in CDeterministicMNListDiff is
higher than Tip
    
Example of new log:
```
2023-09-28T17:35:50Z GetProjectedMNPayeesAtChainTip WARNING pindex is nullptr due to height=914160 chain height=914159
```
    
instead assert call:
```
...
     #6  0x00007ffff7a33b86 in __assert_fail (assertion=0x55555783afd2 "pindex", file=0x5555577f2ed8 "llmq/utils.cpp", line=730,
            function=0x5555577f2448 "bool llmq::utils::IsMNRewardReallocationActive(const CBlockIndex*)") at ./assert/assert.c:101
     #7  0x0000555555ab7daf in llmq::utils::IsMNRewardReallocationActive (pindex=<optimized out>) at llmq/utils.cpp:730
     #8  0x00005555559458ad in CDeterministicMNList::GetProjectedMNPayees (this=this@entry=0x7fffffffc690, pindex=0x0, nCount=<optimized out>, nCount@entry=2147483647)
            at evo/deterministicmns.cpp:231
     #9  0x000055555594614f in CDeterministicMNList::GetProjectedMNPayeesAtChainTip (this=this@entry=0x7fffffffc690, nCount=nCount@entry=2147483647) at evo/deterministicmns.cpp:216
     #10 0x00005555558c9f51 in MasternodeList::updateDIP3List (this=this@entry=0x55555908cfd0) at qt/masternodelist.cpp:194
     #11 0x00005555558ca9a0 in MasternodeList::updateDIP3ListScheduled (this=0x55555908cfd0) at qt/masternodelist.cpp:157
     #12 0x000055555684a60f in void doActivate<false>(QObject*, int, void**) ()
     #13 0x00005555568525b1 in QTimer::timerEvent(QTimerEvent*) ()
     #14 0x0000555556844ce5 in QObject::event(QEvent*) ()
     #15 0x0000555556ac3252 in QApplicationPrivate::notify_helper(QObject*, QEvent*) ()
     #16 0x000055555681e6b8 in QCoreApplication::sendEvent(QObject*, QEvent*) ()
     #17 0x000055555686de2a in QTimerInfoList::activateTimers() ()
     #18 0x000055555686be84 in QEventDispatcherUNIX::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
     #19 0x00005555569bf8a2 in QXcbUnixEventDispatcher::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) ()
     #20 0x000055555681caf6 in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) ()
     #21 0x0000555556825f8a in QCoreApplication::exec() ()
...
```

## What was done?
ClientModel returns now a pair: MNList and CBlockIndex; so, we always
know the which one has been used even if current chain is switched.

## How Has This Been Tested?
Run on my localhost from `c034ff0c2606142ba3e8894bc74f693b87374e5c` -
aborted with backtrace like above.
With both of commit - no assert more.


## 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

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2023-10-27 19:53:27 -05:00
UdjinM6
3e732a9522
fix: expire triggers that are too far into the future (#5646)
## Issue being fixed or feature implemented
`gobject count all`

before:
>Governance Objects: 1195 (Proposals: 9, Triggers: 1186, Other: 0;
Erased: 1), Votes: 135064

after (in 10-ish minutes after gov sync is done):
>Governance Objects: 11 (Proposals: 9, Triggers: 2, Other: 0; Erased:
1), Votes: 702

I _think_ it happens when a node can't follow the right chain for some
reason but it keeps receiving triggers and votes from other nodes which
means triggers never expire on such node. This wouldn't be a problem for
us if we wouldn't reorg testnet/devnets from time to time. Once we reorg
the stuck node happily spams us with all the triggers it saved in the
meantime.

## What was done?
2 sb cycles into the future should be enough for all legit triggers,
drop the ones that have their height even higher

## How Has This Been Tested?
run a node, check rpc

## 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)_
2023-10-25 21:39:59 -05:00
PastaPastaPasta
297b50ae6c
chore: bump rc to 1 (#5645)
## Issue being fixed or feature implemented
Bump version to rc.1

## What was done?
bump version

## How Has This Been Tested?


## Breaking Changes


## 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)_
2023-10-23 12:39:23 -05:00
Odysseas Gabrielides
5f7d5fee1a
chore: Testnet re-organization required changes (#5619)
## Issue being fixed or feature implemented
Dropped all changes made so far to be able to sync Testnet.

## What was done?


## How Has This Been Tested?


## Breaking Changes
Testnet syncing obviously

## 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)_
2023-10-23 12:35:15 -05:00
Konstantin Akimov
5c08afd80e
fix!: mn_rr features only for v21+ (#5642)
## Issue being fixed or feature implemented
Should not be 2 forks in one version

## What was done?
- Asset Unlock transactions (withdrawals) should be available only in
MN_RR fork
- MN_RR should not be auto-activated on Main net without intentional
release of code (and not by spork), but they are need on test net to
test platform.

## How Has This Been Tested?
Run unit/functional tests

## Breaking Changes
Yes (see "what was done")


## 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
2023-10-23 12:26:45 -05:00
Odysseas Gabrielides
f2cfb88c68
feat!: Block reward reallocation activation at v20 (#5639)
## Issue being fixed or feature implemented
Implementation of accepted proposal:
https://www.dashcentral.org/p/expedite-60-20-20-reallocation

## What was done?
Activates changers brought in #5588 on `v20` hard fork instead of
`mn_rr`.

## How Has This Been Tested?
run tests

## Breaking Changes
Again, Testnet sync is broken

## 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: UdjinM6 <UdjinM6@users.noreply.github.com>
2023-10-23 11:57:32 -05:00
Odysseas Gabrielides
1717d2f607
feat(rpc): return activation_height in getblockchaininfo for BIP9 softforks (#5624)
## Issue being fixed or feature implemented
When expecting a hard fork, we manually calculate activation heights.

## What was done?
Returning expected activation height for BIP9 softporks in `locked_in`
status in `getblockchaininfo` RPC.

## 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)_
2023-10-23 10:56:10 -05:00
UdjinM6
c1504ce970
fix: ConnectBlock/Tip stats are in ms (5616 follow-up) (#5629)
## Issue being fixed or feature implemented
We went from milliseconds to microseconds for these silently in #5616 🙈 

## What was done?
make it milliseconds again

## How Has This Been Tested?
n/a

## 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)_
2023-10-23 10:52:41 -05:00
PastaPastaPasta
dab44cd0b0
Merge pull request #5626 from knst/v20-todo-backports
backport:  bitcoin#23416,#18997,#18996, partial #27483 (TODO cleanup)
2023-10-23 10:48:51 -05:00
MarcoFalke
0fa3aa65ca Merge #18996: net: Remove un-actionable TODO
fabea6d404571d046365f4f083da3569d2cbf4f7 net: Run clang-format on protocol.h (MarcoFalke)
facdeea2b25ef36e37b6ada58ea390a72d11a4b2 net: Remove un-actionable TODO (MarcoFalke)

Pull request description:

  The first commit removes a TODO that is infeasible to solve. Currently, most (de)serializable classes in Bitcoin Core have public members. For example `CMessageHeader`, `FlatFilePos`, `CBlock`, `CTransaction`, `CCoin`, ...

  So either this TODO comment should apply to all classes or to none. Fix that discrepancy by removing it from the source code for now. If deemed important, the TODO can be discussed in a brainstorming issue later.

  Also run clang format on the header file in a new commit. Happy to drop this commit if it is too controversial, but I think it is trivial to review and makes the workflow of developers using clang-format-diff easier.

ACKs for top commit:
  practicalswift:
    ACK fabea6d404571d046365f4f083da3569d2cbf4f7
  naumenkogs:
    ACK fabea6d. Not sure why that TODO was there in the first place, but Marco's justification seems correct.
  hebasto:
    ACK fabea6d404571d046365f4f083da3569d2cbf4f7, agree with both changes: removing TODO and applying the `clang-format-diff.py`.

Tree-SHA512: b79ae07be27e5a40fc9f411a5e9ae91aecb2fdedbcbf74699614a1004f4ef816bf396903ec6c06eb1395fd83a2047620c7583acbaadfb8c4e613319a63062c3c
2023-10-23 10:48:39 -05:00
MarcoFalke
01adc81ebf Merge #18997: gui: Remove un-actionable TODO
4444dbf4d5047dd1c92973f7167a74a0779e61a3 gui: Remove un-actionable TODO (MarcoFalke)

Pull request description:

  With encryption turned on by default for all wallets in consideration (#18889), I believe that wallet decryption will not be implemented ever or at least any time soon. So remove that TODO comment for now. If deemed important, a brainstorming issue can be opened instead.

  Also remove some TODOs in the RPC console, which I don't understand. Maybe the gui was meant to show the debug log interactively? In any case, if deemed important, this should be filed as a brainstorming feature request, so that trade-offs of different solutions can be discussed.

ACKs for top commit:
  laanwj:
    Thanks. ACK 4444dbf4d5047dd1c92973f7167a74a0779e61a3
  achow101:
    ACK 4444dbf4d5047dd1c92973f7167a74a0779e61a3

Tree-SHA512: f7ddb37a14178f575da5409ea1c34e34bde37d79b2b56eaaf606a069e2b91c9d7b734529f5c68664b2fa5aa831117c8d19cce823743671cd6c31b81d68b8c70c
2023-10-23 10:48:39 -05:00
laanwj
bbcb2d3998 Merge bitcoin/bitcoin#23416: doc: Remove fee delta TODO from txmempool.cpp
fa32cc0682a0aa3420e6a11031721fcb6c50fa44 doc: Remove fee delta TODO from txmempool.cpp (MarcoFalke)

Pull request description:

  This refactor request was added in commit eb306664e7, though it didn't explain why the refactor is needed and what the goal is. Given that this wasn't touched for more than 5 years, it doesn't seem critical. Generally, non-trivial `TODO`s make more sense as GitHub issues, so that they can be discussed and triaged more easily.

ACKs for top commit:
  laanwj:
    Code review ACK fa32cc0682a0aa3420e6a11031721fcb6c50fa44

Tree-SHA512: 6629fef543e815136c82c38aa8ba2c4de68a5fe94c6954f2559e468f7e59052e02dd7c221d3b159be0314eaf0dbb18f74814297c58f76e2289c47e8d4f49be4e
2023-10-23 10:48:39 -05:00
fanquake
6e7b402fe9 partial Merge bitcoin/bitcoin#27483: Bump python minimum version to 3.8
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
2023-10-23 10:48:39 -05:00
PastaPastaPasta
d5041b6f6e
Merge pull request #5625 from knst/v20-fix-todo
chore: TODO related fixes for old PRs
2023-10-23 10:47:06 -05:00