Commit Graph

26182 Commits

Author SHA1 Message Date
pasta
50a3b7c552
Merge #6095: fix: createwallet to require 'load_on_startup' for descriptor wallets
a42e9df06f fix: createwallet to require 'load_on_startup' for descriptor wallets (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

  RPC `createwallet` has changed list of arguments: `createwallet "wallet_name" ( disable_private_keys blank "passphrase" avoid_reuse descriptors load_on_startup )` since https://github.com/dashpay/dash/pull/5965
  `load_on_startup` used to be an argument 5 but now it has a number 6. Both arguments 5 and 6 are boolean and it can confuse an user: they may even not notice that something wrong when it meant to be "load on startup" but got "descriptor wallet" which is not expected.

  See also previous attempt to resolve issue: https://github.com/dashpay/dash/pull/6029

  ## What was done?
  To prevent confusion if user is not aware about this breaking changes, the RPC createwallet throws an exception if user trying to create descriptor wallet but has not mentioned load_on_startup. This requirement can be removed when major amount of users updated to v21.

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

  Tested with CLI:
  ```
  $ createwallet "tmp-create" true true "" false true
  RPC createwallet would not accept creating descriptor wallets without specifying 'load_on_startup' flag. It is required explicitly in v21 due to breaking changes in createwallet RPC (code -8)
  $ createwallet "tmp-create" true true "" false true true
  {
    "name": "tmp-create",
    "warning": "Empty string given as passphrase, wallet will not be encrypted.\nWallet is an experimental descriptor wallet"
  }
  ```

  ## Breaking Changes
  You can't more pass 'descriptor=NN' without  https://github.com/dashpay/dash/pull/5965 which has not been released yet.

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

ACKs for top commit:
  UdjinM6:
    utACK a42e9df06f
  PastaPastaPasta:
    utACK a42e9df06f
  thephez:
    utACK a42e9df06f

Tree-SHA512: bf57fed40d04a32e756e4f8bfabbe39c0cbf87275546c92f4efc19523bc3c5dd3ddc5a884d67285971dc301a6c5808bef979db52c35645ca2db0810046fe1bdc
2024-07-08 12:55:50 -05:00
pasta
60e36b786a
fix: adjust incorrect parameter description that says there is a default that doesn't exist 2024-07-08 12:53:26 -05:00
Konstantin Akimov
0133c9866d
feat: add functional test for submitchainlock far ahead in future 2024-07-09 00:10:07 +07:00
Konstantin Akimov
6004e06769
feat: return enum in RecoveredSig verifying code, apply for RPC submitchainlock
It helps to detect case of Tip is way too far behind: the signature can be
still valid but core can't verify it yet but later it may become a valid signature.
2024-07-09 00:09:49 +07:00
MarcoFalke
264e7f9e62
Merge #18638: net: Use mockable time for ping/pong, add tests
fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 net: Use mockable time for ping/pong, add tests (MarcoFalke)
faab4aaf2fa1153c6d76efc8113fa01b06943ece util: Add count_microseconds helper (MarcoFalke)

Pull request description:

  Switch `CNode::m_ping_start` and `CNetMessage::m_time` to mockable time, so that tests can be added.

  Mockable time is also type-safe, since it uses `std::chrono`

ACKs for top commit:
  jonatack:
    Code review re-ACK fa33654 re-read code, verified rebase per `git range-diff 4b5c919 fa94d6f fa33654`, previous tested ACKs still valid
  troygiorshev:
    ACK fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3

Tree-SHA512: 7d632bd6019ce7c882029e71b667a61517e783af82755a85dd979ef09380934e172dec8b8f91d57b200a30a6e096aeaf01f19fee7f3aed0e0e871c72eb44d70e
2024-07-08 23:57:01 +07:00
Konstantin Akimov
d3e842f605
refactor: remove dead code which has no use since composite commands are refactored 2024-07-08 23:53:00 +07:00
Konstantin Akimov
58c5d431fe
fix: follow-up to #6017 - enable one more assert in wallet_descriptor test 2024-07-08 23:23:45 +07:00
Konstantin Akimov
dbed4a31af
fix: update comment for wallet_keypool_hd due to bitcoin#17681 DNM 2024-07-08 23:23:45 +07:00
Konstantin Akimov
3d8ff0cd2d
fix: cppcheck warning about coinst in rpc/coinjoin
It fixes this failure:
```
src/rpc/coinjoin.cpp:73:28: warning: Variable 'chainman' can be declared as reference to const [constVariableReference]

Advice not applicable in this specific case? Add an exception by updating
IGNORED_WARNINGS in test/lint/lint-cppcheck-dash.sh
```
2024-07-08 20:32:40 +07:00
Konstantin Akimov
63dfdd7d42
fix: workaround for buggy cppcheck in masternode/utils
It shows this error:
```
src/masternode/utils.cpp:0:0: warning: Internal Error. MathLib::toLongNumber: input was not completely consumed: 5s [cppcheckError]

Advice not applicable in this specific case? Add an exception by updating
IGNORED_WARNINGS in test/lint/lint-cppcheck-dash.sh
```

It doesn't seems as error in our code, but I don't like this warning
2024-07-08 20:31:18 +07:00
Konstantin Akimov
4741bcc5c3
chore: remove outdated todo - removed by bitcoin#16898 2024-07-08 18:23:22 +07:00
Konstantin Akimov
a42e9df06f
fix: createwallet to require 'load_on_startup' for descriptor wallets
createwallet has changed list of arguments: createwallet "wallet_name" ( disable_private_keys blank "passphrase" avoid_reuse descriptors load_on_startup )
load_on_startup used to be an argument 5 but now has a number 6.
Both arguments 5 and 6 are boolean and it can confuse an user.

To prevent confusion if user is not aware about this breaking changes,
the RPC createwallet throws an exception if user trying to create descriptor wallet but has not mentioned load_on_startup.
This requirement can be removed when major amount of users updated to v21
2024-07-07 21:56:16 +07:00
Konstantin Akimov
130b6d1e96
refactor: replace static private member method to static method 2024-07-05 16:13:05 +07:00
Konstantin Akimov
c72ec70fdf
feat: implement governance RPCs votealias and votemany for descriptor wallets 2024-07-05 15:37:29 +07:00
Konstantin Akimov
490832959d
refactor: new method to generate a signing message in CGovernanceVote 2024-07-05 01:35:34 +07:00
Konstantin Akimov
c944908399
refactor: simplify implementation of function CWallet::IsLocked 2024-07-03 15:35:29 +07:00
Konstantin Akimov
685cf34cb9
fix: unlock descriptor wallet for mixing-only 2024-07-03 15:35:28 +07:00
pasta
6e5d3f1d1f
Merge #6090: fix: auto backup issue with descriptor wallets for CJ
bebea4b9b6 fix: auto backup issue with descriptor wallets for CJ (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Qt CoinJoin session has problems (https://github.com/dashpay/dash/pull/5579#pullrequestreview-1912946808):
   - Autobackup problems
   - False keypool depletion reporting

  https://github.com/dashpay/dash-issues/issues/59

  ## What was done?
  Disables check for "remaining keys left" and "auto-backups" for non-legacy wallet.

  ## How Has This Been Tested?
  Run unit/functional test
  Try to run CJ mixing for descriptor wallet.

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

Tree-SHA512: 610551001d054c447ddca9451ac6d94f3d063ecf3ccfab437d99324efc5f99ff86e59d80a36f4ff4983d3c8107aa19292c021cb3210fcf51e79919387169c414
2024-07-02 08:47:24 -05:00
pasta
85ba35c6f5
Merge #6088: feat: add a marker experimental for descriptor wallets
684503db6b feat: add a marker experimental for descriptor wallets (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Descriptor wallets pass most of functional tests now, but they are not really tested in-the-real-environment by multiple users.

  ## What was done?
  Add a marker "experimental" for 'createwallet' rpc, for create wallet UI form.

  ## How Has This Been Tested?
  Run and check

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

Tree-SHA512: e843e5b8080c7cd4273ea532038f3ec65e0260419b2c2a3db07ee1fadd856a7ce47f367705b4ddb19c5fc91cee5a529a54d580dbf53428c3e5e8b433c96ec0c9
2024-07-02 08:43:56 -05:00
pasta
f33474240e
Merge #6089: fix: crash if try to upgradetohd descriptor wallet
e708a4b047 fix: crash if try to upgradetohd descriptor wallet (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

  If `upgradetohd` is called for blank descriptor wallet it can cause a crash:
  ```
  Posix Signal: Segmentation fault
     0#: (0x55A0C7658814) stl_vector.h:115        - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_copy_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data const&)
     1#: (0x55A0C7658814) stl_vector.h:127        - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_swap_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data&)
     2#: (0x55A0C7658814) stl_vector.h:1959       - std::vector<unsigned long, std::allocator<unsigned long> >::_M_move_assign(std::vector<unsigned long, std::allocator<unsigned long> >&&, std::integral_constant<bool, true>)
     3#: (0x55A0C7658814) stl_vector.h:768        - std::vector<unsigned long, std::allocator<unsigned long> >::operator=(std::vector<unsigned long, std::allocator<unsigned long> >&&)
     4#: (0x55A0C7658814) stacktraces.cpp:777     - HandlePosixSignal
     5#: (0x744F89C42990) libc_sigaction.c        - ???
     6#: (0x55A0C7789926) scriptpubkeyman.cpp:418 - LegacyScriptPubKeyMan::GenerateNewHDChain(std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&)
     7#: (0x55A0C77BEC2E) wallet.cpp:5033         - CWallet::UpgradeToHD(std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, bilingual_str&)
     8#: (0x55A0C775B85F) rpcwallet.cpp:2839      - operator()
     9#: (0x55A0C775BBB4) std_function.h:292      - _M_invoke
    10#: (0x55A0C72D4BC2) univalue.h:17           - UniValue::operator=(UniValue&&)
    11#: (0x55A0C72D4BC2) server.h:110            - CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)(), std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}::operator()(JSONRPCRequest const&, UniValue&, bool) const
    12#: (0x55A0C76ED57C) basic_string.h:792      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string()
    13#: (0x55A0C76ED57C) request.h:29            - JSONRPCRequest::~JSONRPCRequest()
    14#: (0x55A0C76ED57C) interfaces.cpp:576      - operator()
    15#: (0x55A0C723B3D9) std_function.h:292      - _M_invoke
    16#: (0x55A0C73D4FFC) std_function.h:591      - std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const
    17#: (0x55A0C73D4FFC) server.cpp:609          - ExecuteCommand
    18#: (0x55A0C73D4FFC) server.cpp:528          - CRPCTable::execute(JSONRPCRequest const&) const
    19#: (0x55A0C72421FB) basic_string.h:223      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_data() const
    20#: (0x55A0C72421FB) basic_string.h:264      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_is_local() const
    21#: (0x55A0C72421FB) basic_string.h:282      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose()
    22#: (0x55A0C72421FB) basic_string.h:792      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string()
    23#: (0x55A0C72421FB) request.h:29            - JSONRPCRequest::~JSONRPCRequest()
    24#: (0x55A0C72421FB) interfaces.cpp:487      - executeRpc
    25#: (0x55A0C6F37700) univalue.h:17           - UniValue::operator=(UniValue&&)
    26#: (0x55A0C6F37700) rpcconsole.cpp:338      - RPCConsole::RPCParseCommandLine(interfaces::Node*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, WalletModel const*)
    27#: (0x55A0C6F38475) rpcconsole.cpp:444      - RPCExecutor::request(QString const&, WalletModel const*)
    28#: (0x55A0C8064490) <unknown-file>          - ???
    29#: (0x55A0C82E95B2) <unknown-file>          - ???
  Aborted (core dumped)
  ```

  ## What was done?
  Added a guard in UpgradeToHD that wallet is not descriptor wallet.

  ## How Has This Been Tested?
  Create wallet "Blank" + "Descriptor".
  Call rpc "upgradetohd" -> no crash

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

Tree-SHA512: 1963606b52e9aed13ba0f4d27f5c2e590351970842333f54c39612d4dc2d47f0ad7ac87cbff2473d2b3544d1f21dded10f9d84c109c2224f4e8070309655b053
2024-07-02 08:38:54 -05:00
Konstantin Akimov
684503db6b
feat: add a marker experimental for descriptor wallets 2024-07-02 01:11:15 +07:00
Konstantin Akimov
bebea4b9b6
fix: auto backup issue with descriptor wallets for CJ 2024-07-02 00:23:19 +07:00
pasta
acc42267d4
Merge #6080: feat: bump protocol version to distinguish v21 from v20.1
375838378a feat: bump protocol version to distinguish v21 from v20.1 (pasta)

Pull request description:

  ## Issue being fixed or feature implemented
  See commit

  ## What was done?

  ## How Has This Been Tested?

  ## Breaking Changes
  None really; will require MNs to upgrade in order to not be banned

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

Top commit has no ACKs.

Tree-SHA512: c4a61e3c9a1ccda9b00a8982eaaff6ea7672bf59d9538342b0e45a16977554224b44b34db48d3355d28e2b26a01ab3133a5e0c38bb00a3d7a1410cfd1fcfc4ae
2024-07-01 11:45:29 -05:00
pasta
d2bbff3927
Merge #6087: feat: stricter bestCLHeightDiff checks
6c5246803d test: add tests for both current and future behaviour (UdjinM6)
4e86bda4dc feat: stricter bestCLHeightDiff checks (UdjinM6)

Pull request description:

  ## Issue being fixed or feature implemented
  Current `bestCLHeightDiff` checks are too relaxed

  ## What was done?
  Make`bestCLHeightDiff` checks stricter

  ## How Has This Been Tested?
  Run a node on mainnet/testet, run tests

  ## Breaking Changes
  Old nodes aren't aware of this new logic so it's activated via `mn_rr` hardfork

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

ACKs for top commit:
  PastaPastaPasta:
    utACK 6c5246803d

Tree-SHA512: 2028d0ceb00a2270c92831ef38488d009d0bac47be4fc6a23ac93efdcf74847f1b9e99a529863fb4e14c65f120adda4e12c5b9e084d0f667d5f0fbaf80e3701d
2024-07-01 11:41:19 -05:00
UdjinM6
6c5246803d
test: add tests for both current and future behaviour 2024-07-01 11:38:26 -05:00
Konstantin Akimov
e708a4b047
fix: crash if try to upgradetohd descriptor wallet
Posix Signal: Segmentation fault
   0#: (0x55A0C7658814) stl_vector.h:115        - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_copy_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data const&)
   1#: (0x55A0C7658814) stl_vector.h:127        - std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data::_M_swap_data(std::_Vector_base<unsigned long, std::allocator<unsigned long> >::_Vector_impl_data&)
   2#: (0x55A0C7658814) stl_vector.h:1959       - std::vector<unsigned long, std::allocator<unsigned long> >::_M_move_assign(std::vector<unsigned long, std::allocator<unsigned long> >&&, std::integral_constant<bool, true>)
   3#: (0x55A0C7658814) stl_vector.h:768        - std::vector<unsigned long, std::allocator<unsigned long> >::operator=(std::vector<unsigned long, std::allocator<unsigned long> >&&)
   4#: (0x55A0C7658814) stacktraces.cpp:777     - HandlePosixSignal
   5#: (0x744F89C42990) libc_sigaction.c        - ???
   6#: (0x55A0C7789926) scriptpubkeyman.cpp:418 - LegacyScriptPubKeyMan::GenerateNewHDChain(std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&)
   7#: (0x55A0C77BEC2E) wallet.cpp:5033         - CWallet::UpgradeToHD(std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, secure_allocator<char> > const&, bilingual_str&)
   8#: (0x55A0C775B85F) rpcwallet.cpp:2839      - operator()
   9#: (0x55A0C775BBB4) std_function.h:292      - _M_invoke
  10#: (0x55A0C72D4BC2) univalue.h:17           - UniValue::operator=(UniValue&&)
  11#: (0x55A0C72D4BC2) server.h:110            - CRPCCommand::CRPCCommand(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, RPCHelpMan (*)(), std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >)::{lambda(JSONRPCRequest const&, UniValue&, bool)#1}::operator()(JSONRPCRequest const&, UniValue&, bool) const
  12#: (0x55A0C76ED57C) basic_string.h:792      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string()
  13#: (0x55A0C76ED57C) request.h:29            - JSONRPCRequest::~JSONRPCRequest()
  14#: (0x55A0C76ED57C) interfaces.cpp:576      - operator()
  15#: (0x55A0C723B3D9) std_function.h:292      - _M_invoke
  16#: (0x55A0C73D4FFC) std_function.h:591      - std::function<bool (JSONRPCRequest const&, UniValue&, bool)>::operator()(JSONRPCRequest const&, UniValue&, bool) const
  17#: (0x55A0C73D4FFC) server.cpp:609          - ExecuteCommand
  18#: (0x55A0C73D4FFC) server.cpp:528          - CRPCTable::execute(JSONRPCRequest const&) const
  19#: (0x55A0C72421FB) basic_string.h:223      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_data() const
  20#: (0x55A0C72421FB) basic_string.h:264      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_is_local() const
  21#: (0x55A0C72421FB) basic_string.h:282      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_dispose()
  22#: (0x55A0C72421FB) basic_string.h:792      - std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::~basic_string()
  23#: (0x55A0C72421FB) request.h:29            - JSONRPCRequest::~JSONRPCRequest()
  24#: (0x55A0C72421FB) interfaces.cpp:487      - executeRpc
  25#: (0x55A0C6F37700) univalue.h:17           - UniValue::operator=(UniValue&&)
  26#: (0x55A0C6F37700) rpcconsole.cpp:338      - RPCConsole::RPCParseCommandLine(interfaces::Node*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >*, WalletModel const*)
  27#: (0x55A0C6F38475) rpcconsole.cpp:444      - RPCExecutor::request(QString const&, WalletModel const*)
  28#: (0x55A0C8064490) <unknown-file>          - ???
  29#: (0x55A0C82E95B2) <unknown-file>          - ???
Aborted (core dumped)
2024-07-01 16:37:47 +07:00
Konstantin Akimov
1a691bd100
feat: add logging for RPC HTTP requests: command, user, http-code, time of running 2024-06-30 13:29:25 +07:00
pasta
c1de83bf8f
Merge #6083: refactor: move Get*Index to rpc/index_util.cpp, const-ify functions and arguments, add lock annotations and some minor housekeeping
8fb863008e refactor: inline sorting and make available through argument (Kittywhiskers Van Gogh)
3e0fcf471f refactor: move accessing CBlockTreeDB global out of Get*Index (Kittywhiskers Van Gogh)
ee9d11214e refactor: move pair value swapping out of CTxMemPool::getAddressIndex() (Kittywhiskers Van Gogh)
808842b1a3 refactor: define all key/value pairings as *Entry and use them instead (Kittywhiskers Van Gogh)
488f0474a8 rpc: extend error-on-failure to GetSpentIndex (Kittywhiskers Van Gogh)
9a6503d9e8 refactor: make CBlockTreeDB::Read*Index arguments const (Kittywhiskers Van Gogh)
625982e8d2 refactor: make CTxMemPool::get*Index functions and arguments const (Kittywhiskers Van Gogh)
5ad49ad668 refactor: move Get{Address*, Timestamp, Spent}Index out of validation (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  This pull request is motivated by [bitcoin#22371](https://github.com/bitcoin/bitcoin/pull/22371), which gets rid of the `pblocktree` global.

  The sole usage of `pblocktree` introduced by Dash is for managing our {address, spent, timestamp} indexes with most of invocations within `BlockManager` or `CChainState`, granting them internal access to `pblocktree` (now `m_block_tree_db`). The sole exception being `Get*Index`, that relies on accessing the global and has no direct internal access.

  This pull request aims to refactor code associated with `Get*Index` with the eventual aim of moving gaining access to the global out of the function. `Get*Index` is called exclusively called through RPC, which makes giving it access to `ChainstateManager` quite easy, which makes switching from the global to accessing it through `ChainstateManager` when backporting  [bitcoin#22371](https://github.com/bitcoin/bitcoin/pull/22371) a drop-in replacement.

  Alongside that, the surrounding code has been given some TLC:

  * Moving code out of `validation.cpp` and into `rpc/index_util.cpp`. The code is exclusively used in RPC logic and doesn't aid in validation.
  * Add lock annotations for accessing `pblocktree` (while already protected by `::cs_main`, said protection is currently not enforced but will be once moved into `BlockManager` in the backport)
  * `const`-ing input arguments and using pass-by-value for input arguments that can be written inline (i.e. types like `CSpentIndexKey` are still pass-by-ref).
    * While `const`ing `CTxMemPool` functions were possible (courtesy of the presence of `const_iterator`s), the same is currently not possible with `CBlockTreeDB` functions as the iterator is non-`const`.
  * Extend error messages to `GetSpentIndex` to bring it line with other `Get*Index`es.
  * Define key-value pairings as a `*Entry` typedef and replacing all explicit type constructions with it.
  * Make `CTxMemPool::getAddressIndex` indifferent to how `CMempoolAddressDeltaKey` is constructed.
    * Current behaviour is to accept a `std::pair<uint160, AddressType>` and construct the `CMempoolAddressDeltaKey` internally, this was presumably done to account for the return type of `getAddressesFromParams` in the sole call for the `CTxMemPool::getAddressIndex`.
    * This has been changed, moving the construction into the RPC call.
   * Moving {height, timestamp} sorting out of RPC and into the applicable `Get*Index` functions.

  ## Breaking Changes

  None expected.

  ## 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 (note: N/A)
  - [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:
  PastaPastaPasta:
    utACK 8fb863008e
  UdjinM6:
    utACK 8fb863008e

Tree-SHA512: 425a383e8284bbd74a5e9bcda4a9d7988221197055f43faa591e6f0d579625cee28f6a6046dab951e7afa0c3e33af1778fb4bb5f0a2e1e5792fe0d9396897a14
2024-06-29 14:48:46 -05:00
pasta
3e342d797e
Merge #6086: backport: bitcoin/bitcoin#27610: Improve performance of p2p inv to send queues
489b44c647 Merge bitcoin/bitcoin#27610: Improve performance of p2p inv to send queues (fanquake)

Pull request description:

  ## Issue being fixed or feature implemented
  Couple of performance improvements when draining the inventory-to-send queue:

     * drop txs that have already been evicted from the mempool (or included in a block) immediately, rather than at the end of processing
     * marginally increase outgoing trickle rate during spikes in tx volume

  ## What was done?
  Backport bitcoin#27610

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

  ## Breaking Changes
  N/A

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

ACKs for top commit:
  UdjinM6:
    utACK 489b44c647
  PastaPastaPasta:
    utACK 489b44c647

Tree-SHA512: 4afca4705b0f9680d147f0bef006fb82a6fd9692fef898dd50aede8570d02b6fece367ec30ab2caa973279df28d90348006a1f78b550dd8b0f7e72dbcb1bba5b
2024-06-29 14:45:33 -05:00
fanquake
489b44c647
Merge bitcoin/bitcoin#27610: Improve performance of p2p inv to send queues
5b3406094f2679dfb3763de4414257268565b943 net_processing: Boost inv trickle rate (Anthony Towns)
228e9201efb5574b1b96bb924de1d2e8dd1317f3 txmempool: have CompareDepthAndScore sort missing txs first (Anthony Towns)

Pull request description:

  Couple of performance improvements when draining the inventory-to-send queue:

   * drop txs that have already been evicted from the mempool (or included in a block) immediately, rather than at the end of processing
   * marginally increase outgoing trickle rate during spikes in tx volume

ACKs for top commit:
  willcl-ark:
    ACK 5b34060
  instagibbs:
    ACK 5b3406094f
  darosior:
    utACK 5b3406094f2679dfb3763de4414257268565b943
  glozow:
    code review ACK 5b3406094f2679dfb3763de4414257268565b943
  dergoegge:
    utACK 5b3406094f2679dfb3763de4414257268565b943

Tree-SHA512: 155cd3b5d150ba3417c1cd126f2be734497742e85358a19c9d365f4f97c555ff9e846405bbeada13c3575b3713c3a7eb2f780879a828cbbf032ad9a6e5416b30
2024-06-29 22:57:17 +07:00
UdjinM6
4e86bda4dc
feat: stricter bestCLHeightDiff checks 2024-06-28 20:37:47 +03:00
Kittywhiskers Van Gogh
8fb863008e
refactor: inline sorting and make available through argument 2024-06-28 08:17:42 +00:00
Kittywhiskers Van Gogh
3e0fcf471f
refactor: move accessing CBlockTreeDB global out of Get*Index 2024-06-28 08:17:05 +00:00
Kittywhiskers Van Gogh
ee9d11214e
refactor: move pair value swapping out of CTxMemPool::getAddressIndex() 2024-06-28 08:14:30 +00:00
Kittywhiskers Van Gogh
808842b1a3
refactor: define all key/value pairings as *Entry and use them instead 2024-06-28 08:14:30 +00:00
Kittywhiskers Van Gogh
488f0474a8
rpc: extend error-on-failure to GetSpentIndex 2024-06-28 08:14:30 +00:00
Kittywhiskers Van Gogh
9a6503d9e8
refactor: make CBlockTreeDB::Read*Index arguments const
With bonus code formatting. We cannot make the functions themselves
const as the iterators are non-const.
2024-06-28 08:14:30 +00:00
Kittywhiskers Van Gogh
625982e8d2
refactor: make CTxMemPool::get*Index functions and arguments const 2024-06-28 08:14:29 +00:00
Kittywhiskers Van Gogh
5ad49ad668
refactor: move Get{Address*, Timestamp, Spent}Index out of validation
With bonus const'ing of CTxMemPool::get*Index
2024-06-28 08:14:29 +00:00
pasta
37e026a038
Merge #6074: backport: merge bitcoin#18344, #20867, #20286, #21359, #21910, #19651, #21934, #22722, #20295, #23702, partial bitcoin#23706 (rpc backports)
b23d94b14f partial bitcoin#23706: getblockfrompeer followups (Kittywhiskers Van Gogh)
7e5cc5e375 merge bitcoin#23702: Add missing optional to getblockfrompeer (Kittywhiskers Van Gogh)
c294457b52 merge bitcoin#20295: getblockfrompeer (Kittywhiskers Van Gogh)
63ac87f011 merge bitcoin#22722: update estimatesmartfee rpc to return max of estimateSmartFee, mempoolMinFee and minRelayTxFee. (Kittywhiskers Van Gogh)
07e4c2cdd0 merge bitcoin#21934: Include versionbits signalling details during LOCKED_IN (Kittywhiskers Van Gogh)
960e7687d4 merge bitcoin#19651: importdescriptors update existing (Kittywhiskers Van Gogh)
1f31823fed merge bitcoin#21910: remove redundant fOnlySafe argument (Kittywhiskers Van Gogh)
69c5aa8947 merge bitcoin#21359: include_unsafe option for fundrawtransaction (Kittywhiskers Van Gogh)
169dce7e50 merge bitcoin#20286: deprecate addresses and reqSigs from rpc outputs (Kittywhiskers Van Gogh)
7cddf70c58 merge bitcoin#20867: Support up to 20 keys for multisig under Segwit context (Kittywhiskers Van Gogh)
7c59923845 merge bitcoin#18344: Fix nit in getblockchaininfo (Kittywhiskers Van Gogh)
ec0803a0f5 refactor: align `TxToUniv` argument list with upstream (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Closes https://github.com/dashpay/dash/issues/6000

  * `TxToUniv`'s argument list needed to be rearranged to match upstream as closely as possible (i.e. placing Dash-specific arguments at the end of the list to allow for code to be backported unmodified, relying on default arguments instead of having to modify each invocation to insert the default argument in between).

    This was due to a new `TxToUniv` variant being introduced in [bitcoin#20286](https://github.com/bitcoin/bitcoin/pull/20286)

  * The maximum number of public keys in a multisig remains the same. The upper limit for bare multisigs is and always has been `MAX_PUBKEYS_PER_MULTISIG` ([source](19512988c6/src/script/interpreter.cpp (L1143-L1144))), which has always been 20 ([source](19512988c6/src/script/script.h (L28-L29))). The limit of up to 16 comes from P2SH overhead ([source](19512988c6/src/script/descriptor.cpp (L877-L880))) and that hasn't changed.

    In effect, what [bitcoin#20867](https://github.com/bitcoin/bitcoin/pull/20867) does to Dash Core is change the error from "too many public keys" (as we'll be testing against the bare multisig limit) to "excessive redeemScript size" ([source](19512988c6/src/rpc/util.cpp (L223-L225))) (which is the _true_ limitation).

  * Backporting [bitcoin#21934](https://github.com/bitcoin/bitcoin/pull/21934) required a minor change in the condition needed to emit `activation_height` as `has_signal` in the preceding block will evaluate true for both `STARTED` and `LOCKED_IN` while earlier it would only for `STARTED`.

  * In [bitcoin#22722](https://github.com/bitcoin/bitcoin/pull/22722), a `self.stop_node` had to be added to account for the absurd fee warning that a new test condition introduces.

  * [bitcoin#23706](https://github.com/bitcoin/bitcoin/pull/23706) is partial due to commits in it that rely on RPC type enforcement, which is currently not implemented in Dash Core.

  * `feature_dip3_deterministicmns.py` depends on the reporting of `addresses` to validate multisigs containing expected payees. There is no replacement RPC call to report the pubkeys that compose a multisig address. In the interm, the `-deprecatedrpc=addresses` flag has been enabled to allow the test to run otherwise unmodified.

  ## Breaking Changes

  * The following RPCs:  `gettxout`, `getrawtransaction`, `decoderawtransaction`, `decodescript`, `gettransaction`, and REST endpoints: `/rest/tx`, `/rest/getutxos`, `/rest/block` deprecated the following fields (which are no longer returned in the responses by default): `addresses`, `reqSigs`.

    The `-deprecatedrpc=addresses` flag must be passed for these fields to be included in the RPC response. Note that these fields are attributes of the `scriptPubKey` object returned in the RPC response. However, in the response of `decodescript` these fields are top-level attributes, and included again as attributes of the `scriptPubKey` object.

  * When creating a hex-encoded Dash transaction using the `dash-tx` utility with the `-json` option set, the following fields: `addresses`, `reqSigs` are no longer returned in the tx output of the response.

  * The error message for attempts at making multisigs with >16 pubkeys will change to an "excessive redeemScript size" instead of the previous "too many public keys".

  ## Checklist:

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

ACKs for top commit:
  UdjinM6:
    utACK b23d94b14f
  PastaPastaPasta:
    utACK b23d94b14f

Tree-SHA512: 659d9ba3a0a9f8594b307a7056ab172309d5a0d9efec605bc4b345f7e0fb1032ad303af9e8f51dbd6549e82d0b738ad41eae8a5b3aebf59081f39df0d4b5ec7c
2024-06-27 17:37:09 -05:00
pasta
f542e8e6ff
Merge #6076: refactor: use new type of composite commands for governance NNN
91aae7b2fe refactor: use properly RPCHelpMan for governance RPC getsuperblockbudget (Konstantin Akimov)
e92e5ef526 refactor: use properly RPCHelpMan for governance RPC voteraw (Konstantin Akimov)
87b3011e55 refactor: duplicated code between governance RPC list and diff (Konstantin Akimov)
faa61795fb docs: move comments from the old code to the new implementation of governance RPC (Konstantin Akimov)
39cd5e41b1 fix: format code for rpc governance (Konstantin Akimov)
006242114f refactor: use new style of composite commands for RPC gobject NNN (Konstantin Akimov)
42e0b37cb2 refactor: use properly RPCHelpMan for governance RPC getgovernanceinfo (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  See #6051

  ## What was done?
  Commands starting from 'governance ...' uses new a new way to make composite commands.

  This PR includes also a refactoring to remove common code between `gobject list` and `gobject diff`
  This PR includes also a refactoring to properly use RPCHelpMan for remaining governance's RPC.

  ## 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
  - [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 91aae7b2fe
  PastaPastaPasta:
    utACK 91aae7b2fe

Tree-SHA512: 46c00df924a7d2c5af799c605bfc479034019eed346f9d487827b0688993aa712ba3dc528d9bdd09b64f0e07b6940078337ca1baf0a1355f87eb792406c676ca
2024-06-27 17:07:44 -05:00
pasta
b0426f396d
Merge #6072: refactor: use new type of composite commands for protx NNN
7b44af26ca refactor: move common code inside protx_register_common_wrapper (Konstantin Akimov)
89d5e26473 refactor: replace bunch of bool flags to the enum (Konstantin Akimov)
1351bc9aad refactor: move common code to the wrapper (Konstantin Akimov)
8d4c28a983 refactor: remove unused protx code from old implementation (Konstantin Akimov)
2a837f8243 refactor: use new composite for RPC 'protx NNN' (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  See #6051

  ## What was done?
  Commands starting from 'protx ...' uses new a new way to make composite commands.

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

Tree-SHA512: a5124121de93ba566bfa6c7c58a5217446301dd027839d137e056df00388282af6be0feb0b81ea0217310e12231a68e3b405fa82b66df93a3fb159d341d7a199
2024-06-27 16:24:58 -05:00
pasta
1f00538ed6
Merge #6084: fix: backport bitcoin#26909, allow for silent overwriting of inconsistent peers.dat
adba60924c addrman: allow for silent overwriting of inconsistent peers.dat (Kittywhiskers Van Gogh)
fbb2b51d75 merge bitcoin#26909: prevent peers.dat corruptions by only serializing once (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  [bitcoin#22762](https://github.com/bitcoin/bitcoin/pull/22762) (backported as part of [dash#6043](https://github.com/dashpay/dash/pull/6043)) did away with then-existing behaviour of overwriting `peers.dat` silently if found corrupt with the rationale of preventing situations where the wrong file is pointed at or the file is written by a higher version of Core. Alongside a change in behaviour, refactoring also took place and further changes were built on top of them.

  Since then, there have been reports of an increasing number of "Corrupt data. Consistency check failed with code -5: iostream error" errors from builds based on `develop`. Reverting the pull request that introduced this change in behaviour is non-trivial due to the number of backports that build on top of the refactoring brought along with it.

  Nor were any other error messages found except for the one mentioned above. The tendency for `peers.dat` to corrupt itself has also been documented upstream ([bitcoin#26599](https://github.com/bitcoin/bitcoin/issues/26599)), with the issue marked as closed with the merger of [bitcoin#26909](https://github.com/bitcoin/bitcoin/pull/26909).

  Therefore, to remedy the above problem, alongside backporting [bitcoin#26909](https://github.com/bitcoin/bitcoin/pull/26909), to avoid inconvenience, instead of reverting all progress made from backporting (as the benefits of not overwriting `peers.dat` for having the wrong magic altogether, for example, is something that doesn't need to be reverted), only inconsistent `peers.dat` files will be overwritten and the action logged with no user intervention required.

  ## Breaking Changes

  None expected.

  ## 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 **(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 adba60924c
  knst:
    utACK adba60924c
  PastaPastaPasta:
    utACK adba60924c

Tree-SHA512: 3e09e7a77c82cce333fe9f02f137485e362f7816c450aef3d18950b9fd57276b4a21cbd1fe90b3eefd62ede0947970ed367c5917930a0656833bc38c0629e408
2024-06-27 16:01:15 -05:00
pasta
2b95b607c3
Merge #6079: backport: merge bitcoin#22433, #22464, #22845, #23007, #21920, #21430, #22133, #23269, #23494, #20201, #23947, #22088, #22093, partial bitcoin#20938 (build backports)
cc55ebbf93 merge bitcoin#22093: Try posix-specific CXX first for mingw32 host (Kittywhiskers Van Gogh)
638806b2cc merge bitcoin#22088: improve note on choosing posix mingw32 (Kittywhiskers Van Gogh)
7ad0141d66 merge bitcoin#23947: use host_os instead of TARGET_OS in configure output (Kittywhiskers Van Gogh)
9126006c18 merge bitcoin#20201: pkg-config related cleanup (Kittywhiskers Van Gogh)
77862d8f5f merge bitcoin#23494: minor boost tidyups (Kittywhiskers Van Gogh)
ef4b35060d merge bitcoin#23269: remove redundant warning flags (Kittywhiskers Van Gogh)
183d08f1d9 merge bitcoin#22133: Make QWindowsVistaStylePlugin available again (Kittywhiskers Van Gogh)
1fbdd009cd merge bitcoin#21430: Add -Werror=implicit-fallthrough compile flag (Kittywhiskers Van Gogh)
cae5496d0b merge bitcoin#21920: improve macro for testing -latomic requirement (Kittywhiskers Van Gogh)
78db324970 partial bitcoin#20938: fix linking against -latomic when building for riscv (Kittywhiskers Van Gogh)
972b4198d7 merge bitcoin#23007: remove WSL install instructions and point to upstream (Kittywhiskers Van Gogh)
54be58b494 merge bitcoin#22845: improve check for ::(w)system (Kittywhiskers Van Gogh)
5b86009d40 merge bitcoin#22464: fix 32-bit narrowing warning in bench/peer_eviction.cpp (Kittywhiskers Van Gogh)
a42202d86f merge bitcoin#22433: remove straggling boost thread_group related code (Kittywhiskers Van Gogh)
abdf4d7b9f build: use enough padding to match with labels in options printout (Kittywhiskers Van Gogh)
e22f4216cb doc: clean up `build-windows.md` (Kittywhiskers Van Gogh)

Pull request description:

  ## Breaking Changes

  None expected.

  ## 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 **(note: N/A)**
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  PastaPastaPasta:
    utACK [cc55ebb](cc55ebbf93)

Tree-SHA512: f4cf6506120facabd5fc2e968c1b14a1fca02cf5858f63cce9e9743aa5327b18a7855b2584829eb568d6664a8e12c4bbfdda2f954ea44e29aaaba1776b3d1535
2024-06-27 15:53:03 -05:00
Kittywhiskers Van Gogh
b23d94b14f
partial bitcoin#23706: getblockfrompeer followups
excludes:
- 8d1a3e6498de6087501969a9d243b0697ca3fe97
- 809d66bb65aa78048e27c2a878d6f7becaecfe11
- 60243cac7286e4c4bdda7094bef4cf6d1564b583
2024-06-27 19:28:32 +00:00
Kittywhiskers Van Gogh
7e5cc5e375
merge bitcoin#23702: Add missing optional to getblockfrompeer 2024-06-27 19:28:32 +00:00
Kittywhiskers Van Gogh
c294457b52
merge bitcoin#20295: getblockfrompeer 2024-06-27 19:28:32 +00:00
Kittywhiskers Van Gogh
63ac87f011
merge bitcoin#22722: update estimatesmartfee rpc to return max of estimateSmartFee, mempoolMinFee and minRelayTxFee. 2024-06-27 19:27:38 +00:00
Kittywhiskers Van Gogh
07e4c2cdd0
merge bitcoin#21934: Include versionbits signalling details during LOCKED_IN 2024-06-27 19:27:38 +00:00
Kittywhiskers Van Gogh
960e7687d4
merge bitcoin#19651: importdescriptors update existing 2024-06-27 19:27:37 +00:00