Commit Graph

26221 Commits

Author SHA1 Message Date
Konstantin Akimov
006242114f
refactor: use new style of composite commands for RPC gobject NNN 2024-06-28 02:05:41 +07:00
Konstantin Akimov
42e0b37cb2
refactor: use properly RPCHelpMan for governance RPC getgovernanceinfo 2024-06-28 02:04:19 +07:00
pasta
b316be7680
Merge #6078: refactor: drop usage of chainstate globals in Dash-specific code, merge bitcoin#21866 (goodbye to a global chainstate)
0213fbebe6 merge bitcoin#21866: Farewell, global Chainstate! (Kittywhiskers Van Gogh)
e3687f790a test, bench: remove globals vCoins and testWallet from test and bench (Kittywhiskers Van Gogh)
0f4184cd70 refactor: drop usage of chainstate globals in spork logic (Kittywhiskers Van Gogh)
208b1c079b refactor: drop usage of chainstate globals in masternode logic (Kittywhiskers Van Gogh)
303c6bb4db refactor: drop usage of chainstate globals in llmq logic (Kittywhiskers Van Gogh)
fa20718b4f refactor: drop usage of chainstate globals in asset locks logic (Kittywhiskers Van Gogh)
21cc12c62a refactor: drop usage of chainstate globals in governance logic (Kittywhiskers Van Gogh)
a475f5f4e5 refactor: drop usage of chainstate globals in coinjoin logic (Kittywhiskers Van Gogh)
ed56dbdbc4 refactor: don't use globals to access members we can directly access (Kittywhiskers Van Gogh)
c48c0e79f3 refactor: stop using `::ChainstateActive()` in `GetBlockHash` (Kittywhiskers Van Gogh)
6abf7f8b63 refactor: stop using `::Chain`{`state`}`Active()` in `GetUTXO*` (Kittywhiskers Van Gogh)
f6f7df3731 rpc: don't use GetUTXOCoin in CDeterministicMN::ToJson() (Kittywhiskers Van Gogh)

Pull request description:

  ```
  Thank you, I'll say goodbye soon
  Though its the end of these globals, don't blame yourself now
  And if its true, I will surround you and give life to a chainstate
  That's our own
  ```

  ## Additional Information

  * In `CDeterministicMN::ToJson()`, `collateralAddress` is extracted by finding the `scriptPubKey` of a transaction output for a masternode, originally this used `GetUTXOCoin` but doesn't work for spent tranasction outputs (as they're _not_ UTXOs), so in [dash#5607](https://github.com/dashpay/dash/pull/5607), a fallback was introduced that looks through the general transaction set if going through the UTXO set yielded nothing.

     `GetUTXOCoin` accesses the active chainstate to get ahold of the UTXO set, this was done through globals. The removal of chainstate globals meant that whoever was calling `GetUTXOCoin` should have access to the chainstate handy. This is trivial in RPC code where `ToJson()` is used ([source](5baa522225/src/rpc/evo.cpp (L1286))) through `Ensure`(`Any`)`Chainman`. Not the case in Qt code ([source](5baa522225/src/qt/masternodelist.cpp (L369))), which is supposed to be given restricted access to information by the interface.

    As the fallback seems to be capable of fetching UTXOs and spent outputs, we can remove the `GetUTXOCoin` method and make the fallback the only method.

  * In `develop`, as of this writing, `CChainState` members `FlushStateToDisk` and {`Enforce`, `Invalidate`, `MarkConflicting`}`Block` were accessing their internals through the global, despite having direct access to them. As the globals they were calling are going to be bid farewell, they needed to be changed to access its members instead.

    The reason for going the roundabout way is unknown.

  * `CDSNotificationInterface` takes in a `ChainstateManager` (instead of the `CChainState` it actually requires) as at the time of interface initialization ([source](5baa522225/src/init.cpp (L1915-L1918))), the active chainstate hasn't been loaded in yet as that happens further down ([source](5baa522225/src/init.cpp (L1988-L1991))).

    As `CDSNotificationInterface::InitializeCurrentBlockTip()` is called well after it is initialized, we can resolve to the active chainstate in there.

  * As `GetCreditPoolDiffForBlock` requires access to `ChainstateManager` as `GetCreditPoolDiffForBlock` > `ProcessLockUnlockTransaction` > `CheckAssetLockUnlockTx` > `CheckAssetUnlockTx` > `ChainstateManager::m_blockman.LookupBlockIndex()` and `BlockAssembler` only has `CChainState`, it had to be reworked around `ChainstateManager`.

    ~~`CChainState` is passed as a direct argument while `ChainstateManager` can be fetched from `NodeContext`. Unlike `CTxMemPool`, which can be passed custom instances ([source](5baa522225/src/rpc/mining.cpp (L381-L382)), [source](5baa522225/src/test/util/setup_common.cpp (L391-L392))), `CChainState`'s argument value is taken from `NodeContext::chainstate.ActiveChainstate()` and since we're now accepting `ChainstateManager` wholesale, we can dispense with accepting `CChainState` as an argument.~~

    ~~Changes to that effect have been made.~~

    AssumeUTXO introduces the need to be able to use different `CChainState`s, so this underlying assumption no longer holds true, the above described changes have been reverted. Asset locks code has been refactored to use `BlockManager` directly (which does come with the downside of needing to hold `cs_main` for longer than strictly necessary, this is why only asset locks uses `BlockManager` directly while other cases still benefit from having `ChainstateManager` as a whole).

  * `CMNHFManager::ConnectManagers` will be taking in a `ChainstateManager` pointer due to the `GetSignalsStage` > `GetForBlock` > `ProcessBlock` > `extractSignals` > `CheckMNHFTx` > `ChainstateManager::m_blockman.LookupBlockIndex()` chain.

  * The use of a bespoke `NodeContext` in `coinselector_tests` breaks tests if any interface call relies on a chainstate as `testNode` doesn't initialize one. For the most part, this was masked by `WalletTestingSetup` populating the chainstate globals from its own `NodeContext` even if the tests themselves preferred to use their own stripped down `testNode`.

    Though, removing the chainstate globals meant that they can no longer rely on `WalletTestingSetup`'s `NodeContext` to mask the barebones `testNode` global being used in the test (specifically, `addCoins` > `listMNCollaterials` > `ChainActive()` worked because `ChainActive()` accessed `WalletTestingSetup`'s `NodeContext` but when `ChainActive()` was gone and replaced with `NodeContext::chainman.ActiveChain()`, it uses `testNode`'s `ChainstateManager`, which doesn't exist, which causes it to crash).

    To remedy this, a5595b13 and 5e54aa9b from [bitcoin#23288](https://github.com/bitcoin/bitcoin/pull/23288) were adapted for the limited purpose of eliminating `testNode` and using `WalletTestingSetup`'s `NodeContext` instead. This comes with the unfortunate effect of skipping a lot of the refactoring, cleanups and optimizations done before and adapting the ones after them non-trivial.

    It is therefore best recommended that the commit be reverted and changes implemented step-by-step in a pull request at some point in the future. For now, it's kept around here for the sake of this pull request, which, if merged, should prevent more chainstate globals use from leaking into the codebase.

      <details>

      <summary>Pre-fix crash stacktrace: </summary>

      ```
      dash@71aecd6afb45:/src/dash$ lldb-16 ./src/test/test_dash
      (lldb) target create "./src/test/test_dash"
      Current executable set to '/src/dash/src/test/test_dash' (x86_64).
      (lldb) r -t coinselector_tests
      Process 395006 launched: '/src/dash/src/test/test_dash' (x86_64)
      Running 4 test cases...
      node/interfaces.cpp:711 chainman: Assertion `m_node.chainman' failed.
      Process 395006 stopped
      * thread #1, name = 'd-test', stop reason = signal SIGABRT
          frame #0: 0x00007ffff7a7300b libc.so.6`__GI_raise(sig=<unavailable>) at raise.c:51:1
      (lldb) bt
      * thread #1, name = 'd-test', stop reason = signal SIGABRT
      * frame #0: 0x00007ffff7a7300b libc.so.6`__GI_raise(sig=<unavailable>) at raise.c:51:1
          frame #1: 0x00007ffff7a52859 libc.so.6`__GI_abort at abort.c:79:7
          frame #2: 0x00005555563cba33 test_dash`assertion_fail(file="node/interfaces.cpp", line=711, func="chainman", assertion="m_node.chainman") at check.cpp:13:5
          frame #3: 0x0000555555fb47aa test_dash`node::(anonymous namespace)::ChainImpl::listMNCollaterials(std::vector<std::pair<std::shared_ptr<CTransaction const> const&, unsigned int>, std::allocator<std::pair<std::shared_ptr<CTransaction const> const&, unsigned int>>> const&) [inlined] std::unique_ptr<ChainstateManager, std::default_delete<ChainstateManager>>& inline_assertion_check<true, std::unique_ptr<ChainstateManager, std::default_delete<ChainstateManager>>&>(val=nullptr, file=<unavailable>, line=711, func=<unavailable>, assertion=<unavailable>) at check.h:62:13
          frame #4: 0x0000555555fb4781 test_dash`node::(anonymous namespace)::ChainImpl::listMNCollaterials(std::vector<std::pair<std::shared_ptr<CTransaction const> const&, unsigned int>, std::allocator<std::pair<std::shared_ptr<CTransaction const> const&, unsigned int>>> const&) [inlined] node::(anonymous namespace)::ChainImpl::chainman(this=0x000055555723e830)at interfaces.cpp:711:45
          frame #5: 0x0000555555fb477d test_dash`node::(anonymous namespace)::ChainImpl::listMNCollaterials(std::vector<std::pair<std::shared_ptr<CTransaction const> const&, unsigned int>, std::allocator<std::pair<std::shared_ptr<CTransaction const> const&, unsigned int>>> const&) [inlined] node::(anonymous namespace)::ChainImpl::listMNCollaterials(this=<unavailable>)::'lambda'()::operator()() const at interfaces.cpp:788:34
          frame #6: 0x0000555555fb474f test_dash`node::(anonymous namespace)::ChainImpl::listMNCollaterials(this=0x000055555723e830, outputs=size=0) at interfaces.cpp:788:34
          frame #7: 0x00005555565bcd07 test_dash`CWallet::AddToWallet(this=0x00005555571701e0, tx=<unavailable>, confirm=<unavailable>, update_wtx=<unavailable>, fFlushOnClose=<unavailable>) at wallet.cpp:886:46
          frame #8: 0x0000555555bed3ef test_dash`coinselector_tests::add_coin(wallet=0x00005555571701e0, nValue=0x00007fffffffc7c0, nAge=144, fIsFromMe=false, nInput=0, spendable=<unavailable>) at coinselector_tests.cpp:77:29
          frame #9: 0x0000555555bead3e test_dash`coinselector_tests::bnb_search_test::test_method() [inlined] coinselector_tests::add_coin(nValue=0x00007fffffffc7c0, nAge=144, fIsFromMe=false, nInput=0, spendable=false) at coinselector_tests.cpp:88:5
          frame #10: 0x0000555555bead20 test_dash`coinselector_tests::bnb_search_test::test_method(this=0x00007fffffffcad0) at coinselector_tests.cpp:278:5
          frame #11: 0x0000555555be6607 test_dash`coinselector_tests::bnb_search_test_invoker() at coinselector_tests.cpp:138:1
      ```

      </details>

  ## Breaking Changes

  * Backporting `coinselector_tests` changes are now much more annoying.

  * The following RPCs, `protx list`, `protx listdiff`, `protx info` will no longer report `collateralAddress` if the transaction index has been disabled (`txindex=0`).

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation **(note: N/A)**
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 0213fbebe6
  knst:
    utACK 0213fbebe6
  PastaPastaPasta:
    utACK 0213fbebe6

Tree-SHA512: 839f3f5b2af018520f330c4f4727622471d6225640c98853f28c3d88c4b6c728091b5e0c35b320e82979e5cd1357902fa1212afa4b6977967f05c636a25cc3b0
2024-06-27 12:58:07 -05:00
Kittywhiskers Van Gogh
adba60924c
addrman: allow for silent overwriting of inconsistent peers.dat 2024-06-27 06:09:30 +00:00
Kittywhiskers Van Gogh
fbb2b51d75
merge bitcoin#26909: prevent peers.dat corruptions by only serializing once 2024-06-26 20:56:42 +00:00
Kittywhiskers Van Gogh
0213fbebe6
merge bitcoin#21866: Farewell, global Chainstate! 2024-06-26 13:50:50 +00:00
Kittywhiskers Van Gogh
e3687f790a
test, bench: remove globals vCoins and testWallet from test and bench
adapted from a5595b13 and 5e54aa9b in bitcoin#23288. changes to
coinselector_tests must be reverted before backports related to it
are done as these changes are primarily motivated by bitcoin#21866
taking away the global chainstate, which breaks coinselector_tests and
these changes skips over a lot of backporting, making them incompatible
with backporting efforts (or even the commits its adapted from).

the existing behaviour was that it creates its own testNode but doesn't
populate it with a ChainstateManager (or much at all) while the rest of
the client uses WalletTestingSetup's values. for the longest time this
was fine because addCoins > listMNCollaterials > ChainActive() meant that
it was using the fallback, but when chainstate globals are removed,
WalletTestingSetup's values can't serve as a fallback anymore as it'd
now be looking for NodeContext::chainman::m_chain, and chainman wasn't
setup with testNode and the tests crash.
2024-06-26 13:50:49 +00:00
Kittywhiskers Van Gogh
0f4184cd70
refactor: drop usage of chainstate globals in spork logic 2024-06-26 13:50:49 +00:00
Kittywhiskers Van Gogh
208b1c079b
refactor: drop usage of chainstate globals in masternode logic 2024-06-26 13:50:49 +00:00
Kittywhiskers Van Gogh
303c6bb4db
refactor: drop usage of chainstate globals in llmq logic 2024-06-26 13:50:49 +00:00
Kittywhiskers Van Gogh
fa20718b4f
refactor: drop usage of chainstate globals in asset locks logic 2024-06-26 13:50:48 +00:00
Kittywhiskers Van Gogh
21cc12c62a
refactor: drop usage of chainstate globals in governance logic 2024-06-26 11:57:21 +00:00
Kittywhiskers Van Gogh
a475f5f4e5
refactor: drop usage of chainstate globals in coinjoin logic 2024-06-26 11:57:21 +00:00
Kittywhiskers Van Gogh
ed56dbdbc4
refactor: don't use globals to access members we can directly access
FlushStateToDisk and {Enforce, Invalidate, MarkConflicting}Block are all
CChainState functions, no need to access our own members through
chainstate globals when we can access them directly.
2024-06-26 11:49:48 +00:00
Kittywhiskers Van Gogh
c48c0e79f3
refactor: stop using ::ChainstateActive() in GetBlockHash 2024-06-26 11:49:47 +00:00
Kittywhiskers Van Gogh
6abf7f8b63
refactor: stop using ::Chain{state}Active() in GetUTXO* 2024-06-26 11:25:07 +00:00
Kittywhiskers Van Gogh
f6f7df3731
rpc: don't use GetUTXOCoin in CDeterministicMN::ToJson()
The fallback introduced in dash#5607 for spent UTXOs also works for
unspent UTXOs, no reason to leave it as fallback when it can be made
primary.

Also, if we did want to keep GetUTXOCoin around, it would need access
to CChainState due to the refactoring due in the next commit, which is
possible in its RPC invocations but isn't so readily available in Qt
code, where it is also called. The fallback code has the benefit of not
relying on CChainState.
2024-06-26 11:25:05 +00:00
Konstantin Akimov
7b44af26ca
refactor: move common code inside protx_register_common_wrapper 2024-06-25 22:36:55 +07:00
Konstantin Akimov
89d5e26473
refactor: replace bunch of bool flags to the enum 2024-06-25 22:36:55 +07:00
Konstantin Akimov
1351bc9aad
refactor: move common code to the wrapper 2024-06-25 22:36:55 +07:00
Konstantin Akimov
8d4c28a983
refactor: remove unused protx code from old implementation 2024-06-25 22:36:54 +07:00
Konstantin Akimov
2a837f8243
refactor: use new composite for RPC 'protx NNN' 2024-06-25 22:36:53 +07:00
pasta
4a520991db
Merge #6066: feat: support descriptor wallets for RPC protx updateregistar
c9a600e0fa fix: linkage error - message signer better to be common code rather than libconsensus (Konstantin Akimov)
8299b3b369 feat: protxregistar implementation for descriptor wallets (Konstantin Akimov)
6f45432f76 refactor: removed unused SignSpecialTxPayloadByString (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

  RPC `protx updateregistar` uses forcely LegacyScriptPubKeyMan instead using CWallet's interface.
  It causes a failures such as
  ```
  test_framework.authproxy.JSONRPCException: This type of wallet does not support this command (-4)
  ```

  ## What was done?
  New method `SignSpecialTxPayloadByHash` is implemented in interface instead exporting raw private key for some address.

  See https://github.com/dashpay/dash-issues/issues/59 to track progress

  ## How Has This Been Tested?
  Functional test `feature_dip3_deterministicmns.py` to run by both ways - legacy and descriptor wallets.

  Run unit and functional tests.

  Extra test done locally:
  ```diff
  --- a/test/functional/test_framework/test_framework.py
  +++ b/test/functional/test_framework/test_framework.py
  @@ -242,10 +242,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):

           if self.options.descriptors is None:
               # Prefer BDB unless it isn't available
  -            if self.is_bdb_compiled():
  -                self.options.descriptors = False
  -            elif self.is_sqlite_compiled():
  +            if self.is_sqlite_compiled():
                   self.options.descriptors = True
  +            elif self.is_bdb_compiled():
  +                self.options.descriptors = False
  ```

  to flip flag descriptor wallets/legacy wallets for all functional tests.

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK c9a600e0fa

Tree-SHA512: 00aea1cd9db3537b9a9dcdee096d47ea48337edeea3f52ad54aea91781678b8641ac2dd86b67f61f87e3912945bcb5361a42a3279b6c08bb8d9f096bed8fe842
2024-06-25 09:46:52 -05:00
pasta
fc11cd8362
Merge #6077: test: functional tests for RPC getgovernanceinfo
3971613285 feat: functional tests for RPC getgovernanceinfo (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  https://github.com/dashpay/dash-issues/issues/63

  ## What was done?
  It adds functional test for `getgovernanceinfo`

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

  Check output of `test/functional/test_runner.py -j20 --previous-releases --coverage --extended`
  ```
  Uncovered RPC commands:
    - cleardiscouraged
    - debug
    - getblockheaders
    - getmerkleblocks
    - getpoolinfo
    - voteraw
  ```

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK 3971613285

Tree-SHA512: f2d93db9068a707f2b33dd841b14f266db226fc5e907f19a49cf7a856b840f0c2c1f521c462996888933bb0ed636c288db12b1d36978c124cb536ea1ab9f3892
2024-06-25 09:27:13 -05:00
pasta
375838378a
feat: bump protocol version to distinguish v21 from v20.1 2024-06-25 09:19:50 -05:00
pasta
6cb8dcd75b
Merge #6065: fix: add missing checks for not nullptr in rpc for dash specific code
dc15420470 refactor: use EnsureConnman and EnsurePeerman when possible (Konstantin Akimov)
dc01f07f74 fix: add missing checks for not nullptr in rpc for dash specific code (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  There're some usages of unique pointers in RPC code that are not guarded by non-nullptr checks

  ## What was done?
  Added missing `CHECK_NON_FATAL` and refactored some of them to `EnsureConnman` and `EnsurePeerman`

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

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK dc15420470

Tree-SHA512: 0483d9208f38648cad55804791415b2facfd165514651463f9b48c44d1963be2888471bb6b1f51015bb9ddd160f272b86f42ee67aea66e640c410175412f5f75
2024-06-25 08:46:04 -05:00
pasta
d2af69fb2b
Merge #6058: fix: reset rounds to 0 when tx fee is not 0
0365b06fab fix: reset rounds to 0 when tx fee is not 0 (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  > a coinjoin send with say 10x 1 + 0.001 denom -> 10 denom, and this doesn’t reset rounds to 0

  reported by @PastaPastaPasta 👍

  ## What was done?
  only real coinjoin tx can be a 0 fee tx with denoms, use that fact to avoid the edge case described above

  ## How Has This Been Tested?
  reproduced the issue

  ## Breaking Changes
  n/a

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    ACK 0365b06fab

Tree-SHA512: b72ac5c01cc77c81945c3ae00b80fbdc4884acf3cfadc3fbcf4b78ff5655cce3b2ece4885478e5bf7476d351411df88b967d2baa3925155cffdc6054dd3bb502
2024-06-25 08:44:14 -05:00
Kittywhiskers Van Gogh
cc55ebbf93
merge bitcoin#22093: Try posix-specific CXX first for mingw32 host 2024-06-25 13:39:57 +00:00
Kittywhiskers Van Gogh
638806b2cc
merge bitcoin#22088: improve note on choosing posix mingw32 2024-06-25 13:39:57 +00:00
Kittywhiskers Van Gogh
7ad0141d66
merge bitcoin#23947: use host_os instead of TARGET_OS in configure output 2024-06-25 13:39:57 +00:00
Kittywhiskers Van Gogh
9126006c18
merge bitcoin#20201: pkg-config related cleanup 2024-06-25 13:39:57 +00:00
Kittywhiskers Van Gogh
77862d8f5f
merge bitcoin#23494: minor boost tidyups 2024-06-25 13:39:57 +00:00
Kittywhiskers Van Gogh
ef4b35060d
merge bitcoin#23269: remove redundant warning flags 2024-06-25 13:39:56 +00:00
Kittywhiskers Van Gogh
183d08f1d9
merge bitcoin#22133: Make QWindowsVistaStylePlugin available again 2024-06-25 13:39:56 +00:00
Kittywhiskers Van Gogh
1fbdd009cd
merge bitcoin#21430: Add -Werror=implicit-fallthrough compile flag 2024-06-25 13:39:56 +00:00
Kittywhiskers Van Gogh
cae5496d0b
merge bitcoin#21920: improve macro for testing -latomic requirement 2024-06-25 13:22:14 +00:00
Kittywhiskers Van Gogh
78db324970
partial bitcoin#20938: fix linking against -latomic when building for riscv
includes:
- 54ce4fac80689621dcbcc76169b2b00b179ee743
2024-06-25 13:22:13 +00:00
Kittywhiskers Van Gogh
972b4198d7
merge bitcoin#23007: remove WSL install instructions and point to upstream 2024-06-25 13:22:13 +00:00
Kittywhiskers Van Gogh
54be58b494
merge bitcoin#22845: improve check for ::(w)system 2024-06-25 13:22:13 +00:00
Kittywhiskers Van Gogh
5b86009d40
merge bitcoin#22464: fix 32-bit narrowing warning in bench/peer_eviction.cpp 2024-06-25 13:22:13 +00:00
Kittywhiskers Van Gogh
a42202d86f
merge bitcoin#22433: remove straggling boost thread_group related code 2024-06-25 13:22:13 +00:00
Kittywhiskers Van Gogh
abdf4d7b9f
build: use enough padding to match with labels in options printout 2024-06-25 13:22:12 +00:00
Kittywhiskers Van Gogh
e22f4216cb
doc: clean up build-windows.md
These cleanups include:
- Removing a leftover protobuf reference
- Removing instructions on how to build for 32-bit Windows
  - As backports assumed that there are only instructions for 64-bit
    Windows persist, their diff only made one change instead of two.
    But as the 32-bit instructions precede 64-bit, only they received
    changes. The following were affected.
    - Removal of tarball comment
    - Addition of WSL-specific instructions
- Removing references to MSVC as we do not support it
- Adding instructions for setting the default compiler for mingw32 to
  POSIX
- Adding instructions for escaping %PATH%s with spaces
- Adding instructions for cloning Dash Core into the mounted filesystem
  if using WSL
- Adding reference to `dependencies.md`
- Adding suggestion to use `-j`
- Realigning some language with upstream
- Add git as a dependency
2024-06-25 13:21:26 +00:00
pasta
5baa522225
Merge #6045: feat: one more queue for "external" requests from 3rd parties
241f073932 feat: rpc external users are comma separated list (Konstantin Akimov)
68def970ad refactor: re-order arguments options alphabetically (Konstantin Akimov)
c7efd56a07 feat: rpc external users: use 2 queues but no extra threads (Konstantin Akimov)
c575a5808a feat: change handler to '/' for external users, use only rpc user name to choose queue (Konstantin Akimov)
f1c1fd873e feat: implementation for /external handler for RPC (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  To avoid struggling to response to critical rpc requests, and split them from 3rd parties who uses a node as an external service, there are introduced one more queue of requests that will be served without throttling for instance consensus important rpcs

  ## What was done?
  new command line arguments:
   - `rpcexternaluser` - List of comma-separated usernames for JSON-RPC external connections. If not specified, there's no special queue is created, all requests in one queue
   - `rpcexternalworkqueue=<n>` - Set the depth of the work queue to service external RPC calls

  ## How Has This Been Tested?
  Functional test `rpc_platform_filter.py` is updated to test new functionality

  ## Breaking Changes
  NA

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK 241f073932
  PastaPastaPasta:
    utACK 241f073932

Tree-SHA512: 15b371f24f5302853b85419e2b20c29749d6aae1c98a541d7471f1d3a681643063302c2a5ecce04dfad2da9101ea69d2f08a7e0e11a28609c6011d78273c57a7
2024-06-24 11:54:43 -05:00
pasta
7ca4812b18
Merge #6036: feat: skip governance checks for blocks below the best chainlock
18328279ec fix: force mnsync to skip gov obj sync on reconnection (UdjinM6)
08331bb950 fix: apply suggestions (UdjinM6)
3c3489d7a1 test: add test (UdjinM6)
41ab95dbf8 feat: skip governance checks for blocks below the best chainlock (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  A node can miss governance trigger sometimes and then it would stuck not being able to sync any further. This issue can be fixed manually by resetting sync status and reconsidering the "invalid" block. However, that's inconvenient. Also, what it does under the hood is it simply disables some parts of block validation. We could do that automagically and more precise if we would trust ChainLocks instead.

  ## What was done?
  Skip governance checks for blocks below the best known chainlock, add tests.

  ## How Has This Been Tested?
  Run tests.

  ## Breaking Changes
  n/a

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  knst:
    utACK 18328279ec
  PastaPastaPasta:
    utACK 18328279ec

Tree-SHA512: 3cc4e2707e24b36c9f64502561667d0cb66eced7019db7941781ab1b84cfd267b3dab4684c71b059e074450ea76dc8e342744bffdd1ca1be6ccceb34b3580659
2024-06-24 11:52:39 -05:00
Konstantin Akimov
3971613285
feat: functional tests for RPC getgovernanceinfo 2024-06-24 18:09:55 +07:00
pasta
91e9dd41bd
Merge #6069: backport: bitcoin#19698, #20079, #20566, #20715, #20868, #21012, #21188, #21200, #21205, #21211, #21293
4e81732c57 Merge #21200: test: Speed up rpc_blockchain.py by removing miniwallet.generate() (Wladimir J. van der Laan)
4ab6aa4598 Merge #21293: test: Replace accidentally placed bit-OR with logical-OR (Wladimir J. van der Laan)
5a77c02188 Merge #19698: test: apply strict verification flags for transaction tests and assert backwards compatibility (Wladimir J. van der Laan)
259a767a38 Merge #21211: test: Move P2WSH_OP_TRUE to shared test library (MarcoFalke)
91e0359df4 Merge #21205: build: actually fail when Boost is missing (fanquake)
e3e69290a2 Merge #21188: scripted-diff: Remove redundant lock annotations in net processing (MarcoFalke)
5ec99ff3b4 Merge #20715: util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet (MarcoFalke)
292861a9bc Merge #20868: validation: remove redundant check on pindex (MarcoFalke)
e5249fb307 Merge #21012: ci: Fuzz with integer sanitizer (MarcoFalke)
785f7310ed Merge #20079: p2p: Treat handshake misbehavior like unknown message (MarcoFalke)
147d391c74 Merge #20566: refactor: Use C++17 std::array where possible (MarcoFalke)

Pull request description:

  ## What was done?
  Backports from bitcoin v22

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

  ## Breaking Changes
  N/A

  ## Checklist:
  - [ ] I have performed a self-review of my own code
  - [ ] I have commented my code, particularly in hard-to-understand areas
  - [ ] I have added or updated relevant unit/integration/functional/e2e tests
  - [ ] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK 4e81732c57b9513d0cc3de53d4a61918b064a3e7; LGTM
  UdjinM6:
    utACK 4e81732

Tree-SHA512: f18a73427b7f86bdc4cf9a3c2569b970d814a68705ed386196a143914860b52362365f24283f5b69bf1684ac35bbec5051a9728c475795ffdf3cf3a6ce217c00
2024-06-21 00:39:37 -05:00
Wladimir J. van der Laan
4e81732c57
Merge #21200: test: Speed up rpc_blockchain.py by removing miniwallet.generate()
faa137eb9eac5554504b062a6dc865ca87fd572b test: Speed up rpc_blockchain.py by removing miniwallet.generate() (MarcoFalke)
fa1fe80c757df0adcbfaf41b5c5c8a468bc07b6f test: Change address type from P2PKH to P2WSH in rpc_blockchain (MarcoFalke)
fa4d8f3169e38cbdbae20258efebe7070c49f522 test: Cache 25 mature coins for ADDRESS_BCRT1_P2WSH_OP_TRUE (MarcoFalke)
fad25153f5c8e88f72cf666b16b0b0dbdc45d3b1 test: Remove unused bug workaround (MarcoFalke)
faabce7d07c5776e4116b1a7ad1f6c408a4a4e46 test: Start only the number of nodes that are needed (MarcoFalke)

Pull request description:

  Speed up various tests:

  * Remove unused nodes, which only consume time on start/stop
  * Remove unused "bug workarounds"
  * Remove the need for `miniwallet.generate()` by adding `miniwallet.scan_blocks()`. (On my system, with valgrind, generating 105 blocks takes 3.31 seconds. Rescanning 5 blocks takes 0.11 seconds.)

ACKs for top commit:
  laanwj:
    Code review ACK faa137eb9eac5554504b062a6dc865ca87fd572b

Tree-SHA512: ead1988d5aaa748ef9f8520af1e0bf812cf1d72e281ad22fbd172b7306d850053040526f8adbcec0b9a971c697a0ee7ee8962684644d65b791663eedd505a025
2024-06-20 12:23:14 +07:00
Wladimir J. van der Laan
4ab6aa4598
Merge #21293: test: Replace accidentally placed bit-OR with logical-OR
df8f2a11dcadd08840c5311631132a522650a83e test: Replace accidentally placed bit-OR with logical-OR (Hennadii Stepanov)

Pull request description:

  This PR is a follow up of #19698.

ACKs for top commit:
  glozow:
    utACK df8f2a11dc

Tree-SHA512: 36aba3e952850deafe78dd39775a568e89e867c8a352f511f152bce7062f614f5bb4f23266dbb33da5292c9ee6da5ccefce117e3168621c71d2140c8e7f58460
2024-06-20 12:23:13 +07:00
Wladimir J. van der Laan
5a77c02188
Merge #19698: test: apply strict verification flags for transaction tests and assert backwards compatibility
5786a818e1a96bc1dd65b0e81b05998876357a74 Verify that all validation flags are backward compatible (gzhao408)
b10ce9aa48c8937cb91fca05e29c68098a364d93 [test] check verification flags are minimal/maximal (gzhao408)
a260c22cad0672dda11f42f649ebdc7cfa53b16a [test] Check for invalid flag combinations (gzhao408)
a7098a2a8d2d23ee3be1d71ab8c71475bf5a31ee [refactor] use CheckTxScripts, TrimFlags, FillFlags (gzhao408)
7a77727b2f66f3d723e03e917f0cabb459c49d62 Apply minimal validation flags to tx_invalid tests (gzhao408)
9532591bedaecf7c2debe779dec0a0debec2623b [test] add BADTX setting for invalid txns that fail CheckTransaction (gzhao408)
4c06ebf1281f0f387ab7493fe15176a05247525e [test] fix two witness tests in invalid tests with empty vout (gzhao408)
158a0b268ca2f73a5d504791359d1eff2cf27715 Apply maximal validation flags to tx_valid tests (gzhao408)
0a76a39b633760d4668d39859605c05629ee0025 [test] fix CSV test missing OP_ADD (gzhao408)
19db590d044efe7d474a16720e5b56e7b55db54c [test] remove unnecessary OP_1s from CSV and CLTV tests (gzhao408)

Pull request description:

  This uses the first 4 commits of #15045, rebased and added some comments. The diff is quite large already and I want to make it easy to review, so I'm splitting it into 2 PRs (transaction and script). Script one is WIP, I'll link it when I open it.

  Interpretation of scripts is dependent on the script verification flags passed in.
  In tests, we should always apply **maximal** verification flags when checking that a transaction is **valid**; any additional flags should invalidate the transaction. A transaction should not be valid because we forgot to include a flag, and we should apply all flags by default.
  We should apply **minimal** verification flags when asserting that a transaction is **invalid**; if verification flags are applied, removing any one of them should mean the transaction is valid.
  New verify flags must be backwards compatible; tests should check backwards compatibility and apply the new flags by default. All `tx_invalid` tests should continue to be invalid with the exact same verify flags. All `tx_valid` tests that don't pass with new flags should _explicitly_ indicate that the flags need to be excluded, and fail otherwise.

  1. Flip the meaning of `verifyFlags` in tx_valid.json to mean _excluded_ verification flags instead of included flags. Edit the test data accordingly.
  2. Trim unneeded flags from tx_invalid.json.
  3. Add check to verify that tx_valid tests have maximal flags and tx_invalid tests have minimal flags.
  4. Add checks to verify that flags are soft forks (#10699) i.e. adding any flag should only decrease the number of acceptable scripts. Test by adding/removing random flags.

ACKs for top commit:
  achow101:
    ACK 5786a818e1a96bc1dd65b0e81b05998876357a74
  laanwj:
    ACK 5786a818e1a96bc1dd65b0e81b05998876357a74

Tree-SHA512: 19195d8cf3299e62f47dd3443ae4a95430c5c9d497993a18ab80de9e24b1869787af972774993bf05717784879bc4592fdabaae0fddebd437963d8f3c96d9a73
2024-06-20 12:23:13 +07:00