Commit Graph

2507 Commits

Author SHA1 Message Date
fanquake
dd26a7a806
Merge bitcoin/bitcoin#22797: test, doc: refer to the correct variable names in p2p_invalid_tx.py
0d9fdd329e81cb171d687042290f4e6b1507d7f4 test, doc: refer to the correct variable names in p2p_invalid_tx.py (aitorjs)

Pull request description:

  _tx_orphan_no_fee_ and _tx_orphan_invalid_ don't exist as transactions.

  Have been replaced by _tx_orphan_2_no_fee_ and _tx_orphan_2_invalid_ respectively.

  **Motivation**: Comments are more accurate and easy understandable under the tests context (I think).

ACKs for top commit:
  kristapsk:
    utACK 0d9fdd329e81cb171d687042290f4e6b1507d7f4
  theStack:
    ACK 0d9fdd329e81cb171d687042290f4e6b1507d7f4 📃

Tree-SHA512: a4cafd931e51fe2a67085e10e9c61178c864c14982664d112b76327e040af08cd1de04eca4a8ae980fad57ba7078017ce02fc60e7658f38380e8172c2ae28b77
2024-07-23 10:59:39 -05:00
MarcoFalke
56c3f844dc
Merge bitcoin/bitcoin#22622: util: Check if specified config file cannot be opened
127b4608e9dbb8217c74c9332e82fcec8c326fa8 test: Check if specified config file cannot be opened (nthumann)
6bb54708e6457f21596793a7149dc6dfea1dc871 util: Check if specified config file cannot be opened (nthumann)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/22612.
  When running e.g. `./src/bitcoind -datadir=/tmp/bitcoin -regtest -conf=/tmp/bitcoin/regtest/bitcoin.conf` and the specified config cannot be opened (doesn't exist, permission denied, ...), the initialization silently uses the default config.

  As voidburn already noted:
  > I can't think of a situation in which a config file is specified explicitly (in the startup options, as per service unit linked above), but inaccessible, where the fail condition should be to keep booting using defaults instead.

  With this patch applied, the initialization will fail immediately, if the specified config file cannot be opened. If no config file is explicitly specified, the behavior is unchanged. This not only affects `bitcoind`, but also `bitcoin-cli` and `bitcoin-qt`.

  In the example below the datadir is accessible, but the config file is not due to insufficient permissions:
  ```
  $ ./src/bitcoind -datadir=/tmp/bitcoin -regtest --debug=1 -conf=/tmp/bitcoin/regtest/bitcoin.conf
  Error: Error reading configuration file: specified config file "/tmp/bitcoin/regtest/bitcoin.conf" could not be opened.
  ```

ACKs for top commit:
  0xB10C:
    ACK 127b4608e9dbb8217c74c9332e82fcec8c326fa8
  Zero-1729:
    tACK 127b4608e9dbb8217c74c9332e82fcec8c326fa8
  theStack:
    Tested ACK 127b4608e9dbb8217c74c9332e82fcec8c326fa8

Tree-SHA512: 4fe487921485426f1d1da8d256c388af517b984b639d776aec7b159b3e23b669824093d3bdd31139d9415ed5f5de405b3e6a51b110c8ab471f12b9c99ac67cc1
2024-07-23 10:59:37 -05:00
pasta
1e9694d0d9
Merge #6085: backport: merge bitcoin#21727, #22371, #21526, #23174, #23785, #23581, #23974, #22932, #24050, #24515 (blockstorage backports)
1bf0bf492f merge bitcoin#24515: Only load BlockMan in BlockMan member functions (Kittywhiskers Van Gogh)
5c1eb67c42 merge bitcoin#24050: Give m_block_index ownership of CBlockIndexes (Kittywhiskers Van Gogh)
c440304c85 merge bitcoin#22932: Add CBlockIndex lock annotations, guard nStatus/nFile/nDataPos/nUndoPos by cs_main (Kittywhiskers Van Gogh)
e303a4ec45 merge bitcoin#23974: Make blockstorage globals private members of BlockManager (Kittywhiskers Van Gogh)
301163c65e merge bitcoin#23581: Move BlockManager to node/blockstorage (Kittywhiskers Van Gogh)
732e871a6b merge bitcoin#23785: Move stuff to ChainstateManager (Kittywhiskers Van Gogh)
b402fd57fa merge bitcoin#23174: have LoadBlockIndex account for snapshot use (Kittywhiskers Van Gogh)
a08f2f48bf merge bitcoin#21526: UpdateTip/CheckBlockIndex assumeutxo support (Kittywhiskers Van Gogh)
472caa048a merge bitcoin#22371: Move pblocktree global to BlockManager (Kittywhiskers Van Gogh)
d69ca833df merge bitcoin#21727: Move more stuff to blockstorage (Kittywhiskers Van Gogh)
6df927fc60 chore: exclude underscore placeholder from shadowing linter warnings (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on https://github.com/dashpay/dash/pull/6078

  * Dependent on https://github.com/dashpay/dash/pull/6074

  * Dependent on https://github.com/dashpay/dash/pull/6083

  * Dependent on https://github.com/dashpay/dash/pull/6119

  * Dependency for https://github.com/dashpay/dash/pull/6138

  * In [bitcoin#24050](https://github.com/bitcoin/bitcoin/pull/24050), `BlockMap` is given ownership of the `CBlockIndex` instance contained within the `unordered_map`. The same has not been done for `PrevBlockMap` as `PrevBlockMap` is populated with `pprev` pointers and doing so seems to break validation logic.

  * Dash has a specific linter for all Dash-specific code present in Core. The introduction of `util/translation.h` into `validation.h` has caused the linter to trigger shadowing warnings due to a conflict between the common use of `_` as a placeholder/throwaway name ([source](37e026a038/src/spork.cpp (L44))) and upstream's usage of it to process translatable strings ([source](37e026a038/src/util/translation.h (L55-L62))).

    Neither C++17 nor C++20 have an _official_ placeholder/throwaway term or annotation for structured bindings (which cannot use `[[maybe_unused]` or `std::ignore`) but [P2169](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2169r4.pdf) is a proposal put forth to make it the official placeholder, in that light, the linter will silence shadowing warnings involving an underscore.

  ## 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
  - [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 1bf0bf492f (with one nit)
  knst:
    utACK 1bf0bf492f
  PastaPastaPasta:
    utACK 1bf0bf492f

Tree-SHA512: 875fff34fe91916722f017526135697466e521d7179c473a5c0c444e3aa873369019b804dee9f5f795fc7ebed5c2481b5ce2d895b2950782a37de7b098157ad4
2024-07-23 09:30:59 -05:00
Kittywhiskers Van Gogh
6df927fc60
chore: exclude underscore placeholder from shadowing linter warnings
Bitcoin uses underscore in util/translation.h for translatable strings
but underscores are also a placeholder used in multiple languages, incl.
C++. The next commit will be introducing backports, one of them will be
placing util/translation.h into validation.h, which is a header used by
Dash-specific code.

This causes a conflict between the normal usage of underscore as a
placeholder and Bitcoin's usage of it as a function name, which is
reported by the Dash-specific linter. We need to exclude shadowing
warnings from the linter to account for this.
2024-07-19 17:17:47 +00:00
MarcoFalke
044ddb4c80
Merge bitcoin/bitcoin#21777: test: Fix feature_notifications.py intermittent issue
fa4aec2b26696cc16dc44c6425f7dca3ef91c8ee test: Fix feature_notifications.py intermittent issue (MarcoFalke)

Pull request description:

  Fixes #21683

Top commit has no ACKs.

Tree-SHA512: 256806d82877477f4b3d795658f61127c0de4eff07216f6071f40a8ec1f5d43f3c587f35dd436d480dc261ef6646ac5547db104d22f3fcfeeb61bbdbe04bcc31
2024-07-20 00:05:26 +07:00
MarcoFalke
5336f42ea8
Merge bitcoin/bitcoin#19801: test: check for all possible OP_CLTV fail reasons in feature_cltv.py (BIP 65)
b01cd9471f435bb36b8ed5211a56baad51111ad2 test: check that _all_ invalid-CLTV txs are rejected after BIP65 activation (Sebastian Falbesoner)
dbc19814743cb12960a99793197c811e2750a06b test: check that _all_ invalid-CLTV txs are allowed in a block pre-BIP65 (Sebastian Falbesoner)
8d0ce50c4826529a2d30ffc850bce4d44da6019b test: prepare cltv_invalidate to test all failure reasons in feature_cltv.py (Sebastian Falbesoner)
ce994e1202c4820b1ad5c375d3d671fd0a18e092 test: add tx modfication helper function in feature_cltv.py (Sebastian Falbesoner)

Pull request description:

  The functional test for [BIP65](https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki) / `OP_CHECKLOCKTIMEVERIFY` (`feature_cltv.py`) currently only tests one out of five conditions that lead to failure of the op-code -- by prepending the script `OP_1NEGATE OP_CHECKLOCKTIMEVERIFY OP_DROP` to a tx's first input's scriptSig, the case of "_the top item on the stack is less than 0_" is checked:

  f8462a6d27/test/functional/feature_cltv.py (L26-L35)

  This PR adds the other cases (5 in total) by taking an integer argument to the function `cltv_invalidate` that is called in a loop instead of only once per testing scenario. Here is the full list of failure conditions and how they are tested (note that the scriptSig should still be valid before activation of BIP65, when `OP_CLTV` is simply a no-op):
  * _the stack is empty_
  ➡️  prepending `OP_CHECKLOCKTIMEVERIFY` to scriptSig
  * _the top item on the stack is less than 0_
  ➡️  prepending `OP_1NEGATE OP_CHECKLOCKTIMEVERIFY OP_DROP` to scriptSig
  * _the lock-time type (height vs. timestamp) of the top stack item and the nLockTime field are not the same_
  ➡️  prepending `OPNum(1000) OP_CHECKLOCKTIMEVERIFY OP_DROP` to scriptSig
  ➡️ setting tx.vin[0].nSequence=0 and tx.nCheckTimeLock=1296688602 (genesis block timestamp)
  * _the top stack item is greater than the transaction's nLockTime field_
  ➡️  prepending `OPNum(1000) OP_CHECKLOCKTIMEVERIFY OP_DROP` to scriptSig
  ➡️ setting tx.vin[0].nSequence=0 and tx.nCheckTimeLock=500
  * _the nSequence field of the txin is 0xffffffff_
  ➡️  prepending `OPNum(500) OP_CHECKLOCKTIMEVERIFY OP_DROP` to scriptSig
  ➡️ setting tx.vin[0].nSequence=0xffffffff and tx.nCheckTimeLock=500

  The first commit creates a helper function for the tx modification and also includes some tidying up like turning single-line to multi-line Python imports where necessary and cleaning up some PEP8 warnings. The second commit prepares the invalidation function `cltv_invalidate` and the third and the fourth use it and check for the expected reject reason strings ("Operation not valid with the current stack size", "Negative locktime" and "Locktime requirement not satisfied").

ACKs for top commit:
  MarcoFalke:
    review ACK b01cd9471f435bb36b8ed5211a56baad51111ad2 🐣

Tree-SHA512: dd82ae86e2bc4f3ab9bb1cfc9f04e4431b2b59c8aaf2a9f4b28654a1577e003fb43c500f99d76ff57e96262168e1cad7c1a0d71158e4b01063737e8f4be1e07d
2024-07-20 00:05:26 +07:00
Wladimir J. van der Laan
709652bff7
Merge #21141: wallet: Add new format string placeholders for walletnotify
06e1fb0b170a69996a7ce1ef5203785a7bc6b278 Add new format string placeholders for walletnotify to include relevant block information for transactions (Maayan Keshet)

Pull request description:

  This patch includes two new format placeholders for walletnotify:
  %b - the hash of the block containting the transaction (zeroed if a mempool transaction)
  %h - the height of the block containing the transaction (zero if a mempool transaction)

  I've included test suite changes to check and validate the above functional requirements as well as doc/help description changes.

  **Motivation**
  The walletnotify option is used to be notified of new transactions relevant to the wallet of the node.
  A common usage pattern is to perform afterwards additional RPC calls to determine:
  1. If this is a mempool transaction or not (i.e. are there any confirmations?)
  2. What block was it included in?
  3. Did this transaction was seen before and is now seen again because of a fork?

  All of these questions can be answered with the current features, but the resulting RPC calls may be expensive in a heavily used node. As this information is readily available when calling the walletnotify callback, it makes sense to save expensive round trips by optionally sending this information at that point in time. I can definitely say we would like to use it in Fireblocks, my employer.

  Please let me know of any questions and suggestions.

ACKs for top commit:
  laanwj:
    ACK 06e1fb0b170a69996a7ce1ef5203785a7bc6b278

Tree-SHA512: d2744e2a7a883f9c3a9fd32237110e048c4b6b11fea8221c33d10b74157f65bbc4351211f441e8c1a4af5d5d38e2ba6b1943a7673dc18860c0553d7b41e00775
2024-07-20 00:05:26 +07:00
pasta
e07431b7d3
Merge #6110: backport: bitcoin#18531, #20012, #21035, #21572, #21574 (rpc command) and related fixes
bfc083e9b7 Merge #21574: Drop JSONRPCRequest constructors after #21366 (MarcoFalke)
c0cdb0488b Merge #21572: Fix wrong wallet RPC context set after #21366 (MarcoFalke)
2f7814acdd Merge #21035: Remove pointer cast in CRPCTable::dumpArgMap (MarcoFalke)
d3b1ef374c refactor: simplify implementation of RPC composite commands (Konstantin Akimov)
3270becc9b chore: add TODO to make client parsing for composite commands (Konstantin Akimov)
d55759fa79 Merge #20012: rpc: Remove duplicate name and argNames from CRPCCommand (Wladimir J. van der Laan)
1d87ce4e86 Merge #18531: rpc: remove deprecated CRPCCommand constructor (MarcoFalke)
a7e538d7ae fix: missing changes from bitcoin#19250 (Konstantin Akimov)
68c5da41dc feat: fix help message - all subcommands support it now! (Konstantin Akimov)
d3e181f516 fix: add missing client's argument parsing for RPC commands (Konstantin Akimov)
37bd4009c1 refactor: use monostate instead std::optional in CoreContext (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Backports from bitcoin v22 rpc command related

  ## What was done?
  See commits for backports.
  Also:
   - refactored and significantly simplified implementation of composite commands
   - added missing changes from bitcoin#19250
   - fix  help message for rpc `help` - all subcommands support it now
   - add missing client's argument parsing for RPC commands
   - CoreContext uses std::monostate instead nullopt which is best-practice for std::variant

  ## How Has This Been Tested?
  Run unit/functional tests.
  Checked autocomplete for various commands
  Checked help for various commands

  ## 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 bfc083e9b7
  UdjinM6:
    utACK bfc083e9b7

Tree-SHA512: b7b586eac2f848a6808c677252a5965577bc783778fd70d3025b8d510113de2b177d423d72ea5f61ddd8905673bf3458e55810ada371ee235fbaa19de8d2d36f
2024-07-19 11:35:58 -05:00
pasta
f5bf5ce77a
Merge #6116: fix: mitigate crashes associated with some upgradetohd edge cases
69c37f4ec2 rpc: make sure `upgradetohd` always has the passphrase for `UpgradeToHD` (Kittywhiskers Van Gogh)
619b640a77 wallet: unify HD chain generation in CWallet (Kittywhiskers Van Gogh)
163d31861c wallet: unify HD chain generation in LegacyScriptPubKeyMan (Kittywhiskers Van Gogh)

Pull request description:

  ## Motivation

  When filming demo footage for https://github.com/dashpay/dash/pull/6093, I realized that if I tried to create an encrypted blank legacy wallet and run `upgradetohd [mnemonic]`, the client would crash.

  ```
  dash@b9c6631a824d:/src/dash$ ./src/qt/dash-qt
  QStandardPaths: XDG_RUNTIME_DIR not set, defaulting to '/tmp/runtime-dash'
  dash-qt: wallet/scriptpubkeyman.cpp:399: void LegacyScriptPubKeyMan::GenerateNewCryptedHDChain(const SecureString &, const SecureString &, CKeyingMaterial): Assertion `res' failed.
  Posix Signal: Aborted
  No debug information available for stacktrace. You should add debug information and then run:
  dash-qt -printcrashinfo=bvcgc43iinzgc43ijfxgm3ybaadwiyltnawxc5avkbxxg2lyebjwsz3omfwduicbmjxxe5dfmqaaa===
  ```

  The expected set of operations when performing privileged operations is to first use `walletpassphrase [passphrase] [time]` to unlock the wallet and then perform the privileged operation. This routine that applies for almost all privileged RPCs doesn't apply here, the unlock state of the wallet has no bearing on constructing an encrypted HD chain as it needs to be encrypted with the master key stored in the wallet, which in turn is encrypted with a key derived from the passphrase (i.e., `upgradetohd` imports **always** need the passphrase, if encrypted).

  You might have noticed that I used `upgradetohd [mnemonic]` instead of the correct syntax, `upgradetohd [mnemonic] "" [passphrase]` that is supposed to be used when supplying a mnemonic to an encrypted wallet, because when you run the former, you don't get told to enter the passphrase into the RPC command, you're told.

  ```
  Error: Please enter the wallet passphrase with walletpassphrase first.
  ```

  Which tells you to treat it like any other routine privileged operation and follow the routine as mentioned above. This is where insufficient validation starts rearing its head, we only validate the passphrase if we're supplied one even though we should be demanding one if the wallet is encrypted and it isn't supplied. We didn't supply a passphrase because we're following the normal routine, we unlocked the wallet so `EnsureWalletIsUnlocked()` is happy, so now the following happens.

  ```
  upgradetohd()
    | Insufficient validation has allowed us to supply a blank passphrase
    | for an encrypted wallet
    |- CWallet::UpgradeToHD()
      |- CWallet::GenerateNewHDChainEncrypted()
       | We get our hands on vMasterKey by generating the key from our passphrase
       | and using it to unlock vCryptedMasterKey.
       |
       | There's one small problem, we don't know if the output of CCrypter::Decrypt
       | isn't just gibberish. Since we don't have a passphrase, whatever came from
       | CCrypter::SetKeyFromPassphrase isn't the decryption key, meaning, the
       | vMasterKey we just got is gibberish
       |- LegacyScriptPubKeyMan::GenerateNewCryptedHDChain()
         |- res = LegacyScriptPubKeyMan::EncryptHDChain()
         | |- EncryptSecret()
         |   |- CCrypter::SetKey()
         |      This is where everything unravels, the gibberish key's size doesn't
         |      match WALLET_CRYPTO_KEY_SIZE, it's no good for encryption. We bail out.
         |- assert(res)
            We assume are inputs are safe so there's no real reason we should crash.
            Except our inputs aren't safe, so we crash. Welp! :c
  ```

  This problem has existed for a while but didn't cause the client to crash, in v20.1.1 (19512988c6), trying to do the same thing would return you a vague error

  ```
  Failed to generate encrypted HD wallet (code -4)
  ```

  In the process of working on mitigating this crash, another edge case was discovered, where if the wallet was unlocked and an incorrect passphrase was provided to `upgradetohd`, the user would not receive any feedback that they entered the wrong passphrase and the client would similarly crash.

  ```
  upgradetohd()
   | We've been supplied a passphrase, so we can try and validate it by
   | trying to unlock the wallet with it. If it fails, we know we got the
   | wrong passphrase.
   |- CWallet::Unlock()
   | | Before we bother unlocking the wallet, we should check if we're
   | | already unlocked, if we are, we can just say "unlock successful".
   | |- CWallet::IsLocked()
   | |  Wallet is indeed unlocked.
   | |- return true;
   | The validation method we just tried to use has a bail-out mechanism
   | that we don't account for, the "unlock" succeded so I guess we have the
   | right passphrase.
   [...] (continue call chain as mentioned earlier)
         |- assert(res)
            Oh...
  ```

  This pull request aims to resolve crashes caused by the above two edge cases.

  ## Additional Information

  As this PR was required me to add additional guardrails on `GenerateNewCryptedHDChain()` and `GenerateNewHDChainEncrypted()`, it was taken as an opportunity to resolve a TODO ([source](9456d0761d/src/wallet/wallet.cpp (L5028-L5038))). The following mitigations have been implemented.

  * Validating `vMasterKey` size (any key not of `WALLET_CRYPTO_KEY_SIZE` size cannot be used for encryption and so, cannot be a valid key)
  * Validating `secureWalletPassphrase`'s presence to catch attempts at passing a blank value (an encrypted wallet cannot have a blank passphrase)
  * Using `Unlock()` to validate the correctness of `vMasterKey`. (the two other instances of iterating through `mapMasterKeys` use `Unlock()`, see [here](1394c41c8d/src/wallet/wallet.cpp (L5498-L5500)) and [here](1394c41c8d/src/wallet/wallet.cpp (L429-L431)))
    * `Lock()`'ing the wallet before `Unlock()`'ing the wallet to avoid the `IsLocked()` bail-out condition and then restoring to the previous lock state afterwards.
  * Add an `IsCrypted()` check to see if `upgradetohd`'s `walletpassphrase` is allowed to be empty.

  ## 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:
  knst:
    utACK 69c37f4ec2
  UdjinM6:
    utACK 69c37f4ec2
  PastaPastaPasta:
    utACK 69c37f4ec2

Tree-SHA512: 4bda1f7155511447d6672bbaa22b909f5e2fc7efd1fd8ae1c61e0cdbbf3f6c28f6e8c1a8fe2a270fdedff7279322c93bf0f8e01890aff556fb17288ef6907b3e
2024-07-19 11:33:58 -05:00
Kittywhiskers Van Gogh
69c37f4ec2
rpc: make sure upgradetohd always has the passphrase for UpgradeToHD
earlier it was possible to make it all the way to `EncryptSecret`
without actually having the passphrase in hand until being told off
by `CCrypter::SetKey`, we should avoid that.

also, let's get rid of checks that `UpgradeToHD` is now taking
responsibility for. no point in checking if the wallet is unlocked
as it has no bearing on your ability to upgrade the wallet.
2024-07-17 16:31:33 +00:00
pasta
f16025f735
Merge #6094: feat: support descriptor wallets for RPC governance votemany, votealias
c72ec70fdf feat: implement governance RPCs votealias and votemany for descriptor wallets (Konstantin Akimov)
490832959d refactor: new method to generate a signing message in CGovernanceVote (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented

  RPCs `governance votemany` and `governance votealias` use 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)
  ```
  See https://github.com/dashpay/dash-issues/issues/59 to track progress

  ## What was done?
  Use CWallet's interfaces instead LegacyScriptPubKeyMan

  ## How Has This Been Tested?
  Functional tests `feature_governance.py` and `feature_governance_cl.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:
  UdjinM6:
    utACK c72ec70fdf
  PastaPastaPasta:
    utACK c72ec70fdf

Tree-SHA512: 2c18f0d4acb1c4d57da81bf54f0d155682f558eeb7271df7e6fe75c126ef7f047562794a6730e3ca5351abc4e2daded06b874c2ab77f9c47b840c89d8a158c9f
2024-07-16 09:32:34 -05:00
Wladimir J. van der Laan
d55759fa79
Merge #20012: rpc: Remove duplicate name and argNames from CRPCCommand
fa04f9b4ddffc5ef23c2ee7f3cc72a7c2ae49204 rpc: Remove duplicate name and argNames from CRPCCommand (MarcoFalke)
fa92912b4bb4629addcbfdfb7cc000be701614af rpc: Use RPCHelpMan for check-rpc-mappings linter (MarcoFalke)
faf835680be39811827504f77005b6603165f53e rpc: [refactor] Use concise C++11 code in CRPCConvertTable constructor (MarcoFalke)

Pull request description:

  Currently, the RPC argument names are specified twice to simplify consistency linting. To avoid having to specify the argnames twice when adding new arguments, remove the linter and add an equivalent test based on RPCHelpMan.

ACKs for top commit:
  laanwj:
    ACK fa04f9b4ddffc5ef23c2ee7f3cc72a7c2ae49204

Tree-SHA512: 3f5f32f5a09b22d879f24aa67031639d2612cff481d6aebc6cfe6fd757cafb3e7bf72120b30466f59292a260747b71e57322c189d5478b668519b9f32fcde31a
2024-07-16 00:14:14 +07:00
MarcoFalke
1d87ce4e86
Merge #18531: rpc: remove deprecated CRPCCommand constructor
faaf9c58e4aa809019d4ca12747dd47411988e37 remove CRPCCommand constructor that takes rpcfn_type function pointer (MarcoFalke)
fa19bb2cd8c575593583138a84e6bb3444d6196d remove dead rpc code (MarcoFalke)

Pull request description:

  Remove the CRPCCommand arguments, now that they are asserted to be equal and thus redundant

  ### Future work

  > Here or follow up, makes sense to also assert type of returned UniValue?

  Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including:

  * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table
  * Auto-formatting and sanity checking the RPCExamples with RPCMan
  * Checking passed-in json in self-check. Removing redundant checks
  * Checking returned json against documentation to avoid regressions or false documentation
  * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static

  ### Bugs found

  * The assert identified issue #18607
  * The changes itself fixed bug #19250

ACKs for top commit:
  fjahr:
    tested ACK faaf9c58e4aa809019d4ca12747dd47411988e37
  promag:
    Tested ACK faaf9c58e4aa809019d4ca12747dd47411988e37.
  ryanofsky:
    Code review ACK faaf9c58e4aa809019d4ca12747dd47411988e37. Two obviously good simplifications.

Tree-SHA512: 5de3b440f7b2ed2c3e86655d4f0e2e5df9c67e8ce3c7817d5ea5311d1a38690f2f3e28fab41aad6936be9fc884326d037e5f19e85d4d2fe281474dada13911ee
2024-07-15 23:51:24 +07:00
pasta
1394c41c8d
Merge #6106: feat: create new composite quorum-command platformsign
2db69d7b81 chore: add release notes for "quorum platformsign" (Konstantin Akimov)
283c5f89a2 feat: create new composite command "quorum platformsign" (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  It splits from #6100
  With just whitelist it is impossible to limit the RPC `quorum sign` to use only one specific quorum type, this PR aim to provide ability for quorum signing for platform quorum only.

  ## What was done?
  Implemented a new composite command "quorum platformsign"

  This composite command let to limit quorum type for signing for case of whitelist.
  After that old way to limit platform commands can be deprecated - #6105

  ## How Has This Been Tested?
  Updated a functional tests to use platform signing for Asset Unlocks feature.

  ## 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 2db69d7b81
  PastaPastaPasta:
    utACK 2db69d7b81

Tree-SHA512: b0dff9934137c4faa85664058e1e77f85067cc8d931e6d76ee5b9e610164ac8b0609736d5f09475256cb78d65bf92466624d784f0b13d20136df7e75613662cb
2024-07-15 11:48:18 -05:00
pasta
ebd1d05103
Merge #6100: feat: make whitelist works with composite commands for platform needs
85abbb97b4 chore: add release notes for composite command for whitelist (Konstantin Akimov)
78ad778bb0 feat: test composite commands in functional test for whitelist (Konstantin Akimov)
a102a59787 feat: add support of composite commands in RPC'c whitelists (Konstantin Akimov)

Pull request description:

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

  ## What was done?
  Our composite commands such as "quorum list" have been refactored to make them truly compatible with other features, such as whitelist, see https://github.com/dashpay/dash/pull/6052 https://github.com/dashpay/dash/pull/6051 https://github.com/dashpay/dash/pull/6055 and other related PRs

  This PR makes whitelist feature to be compatible with composite commands.

  Instead implementing additional users such "dapi" better to provide universal way which do not require new build for every new API that has been used by platform, let's simplify things.

  Platform at their side can use config such as this one (created based on shumkov's example):
  ```
  rpc: {
            host: '127.0.0.1',
            port: 9998,
            users: [
              {
                user: 'dashmate',
                password: 'rpcpassword',
                whitelist: null,
                lowPriority: false,
              },
              {
                username: 'platform-dapi',
                password: 'rpcpassword',
                whitelist: [],
                lowPriority: true,
              },
              {
                username: 'platform-drive-consensus',
                password: 'rpcpassword',
                whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
                lowPriority: false,
              },
              {
                username: 'platform-drive-other',
                password: 'rpcpassword',
                whitelist: [getbestchainlock,getblockchaininfo,getrawtransaction,submitchainlock,verifychainlock,protx_listdiff,quorum_listextended,quorum_info,getassetunlockstatuses,sendrawtransaction,mnsync_status]
  ],
                lowPriority: true,
              },
            ],
            allowIps: ['127.0.0.1', '172.16.0.0/12', '192.168.0.0/16'],
          },
  ```

  ## How Has This Been Tested?
  Updated functional tests, see commits

  ## 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:
    LGTM, utACK 85abbb97b4
  PastaPastaPasta:
    utACK 85abbb97b4

Tree-SHA512: 88608179c347420269880c352cf9f3b46272f3fc62e8e7158042e53ad69dc460d5210a1f89e1e09081d090250c87fcececade88e2ddec09f73f1175836d7867b
2024-07-15 11:44:31 -05:00
pasta
dad9ff1108
Merge #6103: backport: bitcoin#18638
1840c9441a fix: drop extra pings - follow up for #18638 (UdjinM6)
264e7f9e62 Merge #18638: net: Use mockable time for ping/pong, add tests (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  Split from https://github.com/dashpay/dash/pull/6102

  ## What was done?
  So far as bitcoin#19499 is backported, bitcoin#18638 can be finished and removed related workarounds.

  ## 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 1840c9441a
  PastaPastaPasta:
    utACK 1840c9441a

Tree-SHA512: f34657c14514d6ee596175b3faf9ee44e58e8b0339939a0708d58ab2d119786830c183f9b236ed87c08ef8e1dbd031a38fc596b5aa4d38e10521658df4330e79
2024-07-15 10:57:09 -05:00
pasta
67b092255e
Merge #6102: fix: TODO related fixes for post-v21 release
d3e842f605 refactor: remove dead code which has no use since composite commands are refactored (Konstantin Akimov)
58c5d431fe fix: follow-up to #6017 - enable one more assert in wallet_descriptor test (Konstantin Akimov)
dbed4a31af fix: update comment for wallet_keypool_hd due to bitcoin#17681 DNM (Konstantin Akimov)
4741bcc5c3 chore: remove outdated todo - removed by bitcoin#16898 (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Multiple TODO is reviewed and fixes in this PR

  ## What was done?
  See commits

  ## 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 d3e842f605
  PastaPastaPasta:
    utACK d3e842f605

Tree-SHA512: 25080abcce8fa3dbb4e4c71d321b24cb9d23c073e6caf75366be58623023af50d28bc1cdac4098eb78a87b4fe0f3c33d39bb06257f0590b3e42e5fcad161ea2d
2024-07-15 10:52:50 -05:00
Konstantin Akimov
78ad778bb0
feat: test composite commands in functional test for whitelist 2024-07-12 00:07:54 +07:00
Konstantin Akimov
283c5f89a2
feat: create new composite command "quorum platformsign"
This composite command let to limit quorum type for signing for case of whitelist
After that old way to limit platform commands can be deprecated
2024-07-11 12:25:50 +07:00
UdjinM6
1840c9441a
fix: drop extra pings - follow up for #18638 2024-07-09 21:46:28 +07:00
pasta
38249a525c
Merge #6096: feat: split type of error in submitchainlock - return enum in CL verifying code
0133c9866d feat: add functional test for submitchainlock far ahead in future (Konstantin Akimov)
6004e06769 feat: return enum in RecoveredSig verifying code, apply for RPC submitchainlock (Konstantin Akimov)
130b6d1e96 refactor: replace static private member method to static method (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Currently by result of `submitchainlock` impossible to distinct a situation when a signature is invalid and when a core is far behind and just doesn't know about signing quorum yet.

  This PR aims to fix this issue, as requested by shumkov for needs of platform:

  > mailformed signature and can’t verify signature due to unknown quorum is the same error?
  > possible to distingush ?

  ## What was done?
  Return enum in CL verifying code `chainlock_handler.VerifyChainLock`.
  The RPC `submitchainlock` now returns error with code=-1 and message `no quorum found. Current tip height: {N} hash: {HASH}`

  ## How Has This Been Tested?
  Functional test `feature_llmq_chainlocks.py` is updated

  ## Breaking Changes
  `submitchainlock` return one more error code - not really a breaking change though, because v21 hasn't 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 0133c9866d
  PastaPastaPasta:
    utACK 0133c9866d

Tree-SHA512: 794ba410efa57aaa66c47a67914deed97c1d060326e5d11a722c9233a8447f5e9215aa4a5ca401cb2199b8fc445144b2b2a692fc35494bf3296a74e9e115bda7
2024-07-09 09:12:40 -05:00
Konstantin Akimov
0133c9866d
feat: add functional test for submitchainlock far ahead in future 2024-07-09 00:10:07 +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
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
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
c72ec70fdf
feat: implement governance RPCs votealias and votemany for descriptor wallets 2024-07-05 15:37:29 +07: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
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
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
Kittywhiskers Van Gogh
b23d94b14f
partial bitcoin#23706: getblockfrompeer followups
excludes:
- 8d1a3e6498de6087501969a9d243b0697ca3fe97
- 809d66bb65aa78048e27c2a878d6f7becaecfe11
- 60243cac7286e4c4bdda7094bef4cf6d1564b583
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
960e7687d4
merge bitcoin#19651: importdescriptors update existing 2024-06-27 19:27:37 +00:00
Kittywhiskers Van Gogh
69c5aa8947
merge bitcoin#21359: include_unsafe option for fundrawtransaction 2024-06-27 19:27:37 +00:00
Kittywhiskers Van Gogh
169dce7e50
merge bitcoin#20286: deprecate addresses and reqSigs from rpc outputs 2024-06-27 19:27:37 +00:00
Kittywhiskers Van Gogh
7cddf70c58
merge bitcoin#20867: Support up to 20 keys for multisig under Segwit context 2024-06-27 19:27:37 +00: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
fa20718b4f
refactor: drop usage of chainstate globals in asset locks logic 2024-06-26 13:50:48 +00: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
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
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
MarcoFalke
5ec99ff3b4
Merge #20715: util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet
fa61b9d1a68820758f9540653920deaeae6abe79 util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet (MarcoFalke)
7777105a24a36b62df35d12ecf6c6370671568c8 refactor: Move all command dependend checks to ExecuteWalletToolFunc (MarcoFalke)
fa06bce4ac17f93decd4ee38c956e7aa55983f0d test: Add tests (MarcoFalke)
fac05ccdade8b34c969b9cd9b37b355bc0aabf9c wallet: [refactor] Pass ArgsManager to WalletAppInit (MarcoFalke)

Pull request description:

  This not only moves the parsing responsibility out from the wallet tool, but it also makes it easier to implement bitcoin-util #19937

  Fixes: #20902

ACKs for top commit:
  ajtowns:
    ACK fa61b9d1a68820758f9540653920deaeae6abe79
  fjahr:
    Code review ACK fa61b9d1a68820758f9540653920deaeae6abe79

Tree-SHA512: 79622b806e8bf9dcd0dc24a8a6687345710df57720992e83a41cd8d6762a6dc112044ebc58fcf6e8fbf45de29a79b04873c5b8c2494a1eaaf902a2884703e47b
2024-06-20 12:23:02 +07:00
MarcoFalke
e5249fb307
Merge #21012: ci: Fuzz with integer sanitizer
faff3991a9be0ea7be31685fb46d94c212c5da34 ci: Fuzz with integer sanitizer (MarcoFalke)

Pull request description:

  Otherwise the suppressions file will go out of sync

ACKs for top commit:
  practicalswift:
    cr ACK faff3991a9be0ea7be31685fb46d94c212c5da34: patch looks correct

Tree-SHA512: 349216d071a2c5ccf24565fe0c52d7a570ec148d515d085616a284f1ab9992ce10ff82eb17962dddbcda765bbd3a9b15e8b25f34bdbed99fc36922d4161d307c
2024-06-20 12:19:21 +07:00
MarcoFalke
785f7310ed
Merge #20079: p2p: Treat handshake misbehavior like unknown message
faaad1bbac46cfeb22654b4c59f0aac7a680c03a p2p: Ignore version msgs after initial version msg (MarcoFalke)
fad68afcff731153d1c83f7f56c91ecbb264b59a p2p: Ignore non-version msgs before version msg (MarcoFalke)

Pull request description:

  Handshake misbehaviour doesn't cost us more than any other unknown message, so it seems odd to treat it differently

ACKs for top commit:
  jnewbery:
    utACK faaad1bbac46cfeb22654b4c59f0aac7a680c03a
  practicalswift:
    ACK faaad1bbac46cfeb22654b4c59f0aac7a680c03a: patch looks correct

Tree-SHA512: 9f30c3b5c1f6604fd02cff878f10999956152419a3dd9825f8267cbdeff7d06787418b41c7fde8a00a5e557fe89204546e05d5689042dbf7b07fbb7eb95cddff
2024-06-20 02:25:45 +07:00
pasta
77a025fb3b
Merge #6053: trivial: add a missing dash-specific files in util/ to linter lists
fbffe06dad fix: suppress lint warnings for edge due to both missing epoll and kpoll (Konstantin Akimov)
b799683d60 fix: pass reference instead copy of argument in util/edge (Konstantin Akimov)
9d941aacb9 fix: removed unused assigned (Konstantin Akimov)
d9e2e47685 fix: add a missing file util/wpipe to linter lists (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Some source files in src/util is missing to specify as dash specific for linters

  ## What was done?
  Added to list of dash's linters

  ## How Has This Been Tested?
  Run `test/lint/lint-all.sh`

  ## 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 fbffe06dad
  PastaPastaPasta:
    utACK fbffe06dad
  kwvg:
    utACK fbffe06dad

Tree-SHA512: 05bd40f987a93b47ca939f37d6e5b62e2044f2acce7a6ea6c361594eace808da17b17e0149095daecd0fb5418c6cbedbc70ad07ac3b7a1156a94188b04c2fd00
2024-06-17 21:56:53 -05:00
Konstantin Akimov
8299b3b369
feat: protxregistar implementation for descriptor wallets 2024-06-18 01:13:59 +07:00
pasta
30381acc76
Merge #6056: backport: trivial 2024 06 11
fb8a4db8f6 Merge bitcoin/bitcoin#26717: test: Improve `check-doc.py` pattern (MarcoFalke)
349cad2865 Merge bitcoin/bitcoin#26708: clang-tidy: Fix `modernize-use-nullptr` in headers (MarcoFalke)
6bf786d168 Merge bitcoin/bitcoin#25735: net: remove useless call to IsReachable() from CConnman::Bind() (fanquake)
012b0b7169 Merge bitcoin/bitcoin#24258: test: check localaddresses in getnetworkinfo for nodes with proxy (MarcoFalke)
c67f527b0b Merge bitcoin-core/gui#448: Add helper to load font (Hennadii Stepanov)
8e0abeb1c1 Merge bitcoin-core/gui#345: Connection Type Translator Comments (Hennadii Stepanov)
688b66e9d1 Merge bitcoin-core/gui#266: Doc: Copyright: Fix embedded font file location (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  Trivial backports

  ## What was done?

  ## How Has This Been Tested?
  Built and ran tests locally; p2p_addr_relay.py fails locally. Not sure why

  ## Breaking Changes

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

ACKs for top commit:
  UdjinM6:
    utACK fb8a4db8f6
  knst:
    utACK fb8a4db8f6

Tree-SHA512: abb9469f25c6d45acea01da6d2b9cb6df94822f61d06f80e3008b32d2522016370a1e0b0c9ff95b92df22c4f227fc40f7765d76f1987eac7603155fe2d894593
2024-06-15 12:02:51 -05:00
MarcoFalke
fb8a4db8f6
Merge bitcoin/bitcoin#26717: test: Improve check-doc.py pattern
2b77a33e5b91a2e54c5e99b11bd775807ade024d test: Improve `check-doc.py` pattern (Hennadii Stepanov)

Pull request description:

  On master (cb32328d1b80d0ccd6eb9532bd8fe4e0a4de385e):
  ```
  $ ./test/lint/check-doc.py
  Args used        : 158
  Args documented  : 219
  Args undocumented: 0
  set()
  Args unknown     : 61
  {'-stopatheight', '-maxtipage', '-maxreceivebuffer', '-txconfirmtarget', '-maxconnections', '-maxsigcachesize', '-peertimeout', '-limitancestorsize', '-output-csv', '-blockmaxweight', '-par', '-rpcclienttimeout', '-dbcrashratio', '-zmqpubsequence', '-zmqpubhashtxhwm', '-zmqpubrawblock', '-dbbatchsize', '-zmqpubrawtxhwm', '-includeconf', '-checkblocks', '-limitancestorcount', '-zmqpubrawtx', '-checklevel', '-checkmempool', '-rpcthreads', '-rpcworkqueue', '-zmqpubsequencehwm', '-zmqpubrawblockhwm', '-rpcservertimeout', '-testnet', '-zmqpubhashtx', '-signet', '-rpcwaittimeout', '-limitdescendantcount', '-output-json', '-maxmempool', '-mocktime', '-datacarriersize', '-rpcport', '-dbcache', '-zmqpubhashblockhwm', '-mempoolexpiry', '-settings', '-min-time', '-maxtimeadjustment', '-bytespersigop', '-blockversion', '-limitdescendantsize', '-maxorphantx', '-rpccookiefile', '-rpcserialversion', '-bantime', '-blockreconstructionextratxn', '-checkaddrman', '-debuglogfile', '-pid', '-dblogsize', '-timeout', '-zmqpubhashblock', '-maxsendbuffer', '-regtest'}
  ```

  With this PR:
  ```
  $ ./test/lint/check-doc.py
  Args used        : 208
  Args documented  : 219
  Args undocumented: 0
  set()
  Args unknown     : 11
  {'-zmqpubrawblock', '-zmqpubhashblockhwm', '-zmqpubsequencehwm', '-zmqpubrawtx', '-zmqpubhashblock', '-zmqpubhashtx', '-includeconf', '-zmqpubhashtxhwm', '-zmqpubrawblockhwm', '-zmqpubrawtxhwm', '-zmqpubsequence'}
  ```

ACKs for top commit:
  vincenzopalazzo:
    ACK 2b77a33e5b

Tree-SHA512: 6cf4ccc4e8319aad8006ae915f0d25637ac12974fbc1f81808f26b72fbe2649e2b6ff993bc2c1894f81bd6756bff77491b3d56382c034a84fd50325a3c807d8b
2024-06-13 10:06:12 -05:00
Kittywhiskers Van Gogh
a6aa3735be
merge bitcoin#20196: fix GetListenPort() to derive the proper port
continuation of 24205d94fe from dash#5982

includes:
- 0cfc0cd32239d3c08d2121e028b297022450b320
- 7d64ea4a01920bb55bc6de0de6766712ec792a11
2024-06-12 16:37:12 +00:00
Konstantin Akimov
fbffe06dad
fix: suppress lint warnings for edge due to both missing epoll and kpoll 2024-06-12 22:19:13 +07:00
Konstantin Akimov
d9e2e47685
fix: add a missing file util/wpipe to linter lists 2024-06-12 22:18:56 +07:00
Konstantin Akimov
241f073932
feat: rpc external users are comma separated list 2024-06-12 19:46:22 +07:00
MarcoFalke
012b0b7169
Merge bitcoin/bitcoin#24258: test: check localaddresses in getnetworkinfo for nodes with proxy
89bb25d22a0e1c700dba4e3b754984c9b2b14836 test: check localaddresses in getnetworkinfo for nodes with proxy (brunoerg)

Pull request description:

  This PR adds test coverage for the field `localaddresses` for `getnetworkinfo`. In this case, it verifies if this field is empty for all nodes since they are using proxy.

  Reference:
  515200298b/src/init.cpp (L449)

ACKs for top commit:
  jonatack:
    ACK 89bb25d22a0e1c700dba4e3b754984c9b2b14836

Tree-SHA512: 3c765c7060b6972c1ae5a1104734cd7669b650b5f6aa4f623f4299567732260da5083fef306a7c1e71c931f5d1396f24abad251d95c3d82b1f3ee0efee7fcd1f
2024-06-11 12:09:21 -05:00
pasta
2f93ee4a53
Merge #6048: backport: merge bitcoin#19776, #20599, #22147, #22340, #20799, #25147, #20764, bitcoin-core/gui#206 (BIP152 backports)
1cbf3b9a53 merge bitcoin-core/gui#206: Display fRelayTxes and bip152_highbandwidth_{to, from} in peer details (Kittywhiskers Van Gogh)
239062192e merge bitcoin#20764: cli -netinfo peer connections dashboard updates (Kittywhiskers Van Gogh)
06a6f8444c merge bitcoin#25147: follow ups to #20799 (removing support for v1 compact blocks) (Kittywhiskers Van Gogh)
6274a571b7 merge bitcoin#20799: Only support version 2 compact blocks (Kittywhiskers Van Gogh)
f4ce573538 merge bitcoin#22340: Use legacy relaying to download blocks in blocks-only mode (Kittywhiskers Van Gogh)
73b8f84fdb merge bitcoin#22147: p2p: Protect last outbound HB compact block peer (Kittywhiskers Van Gogh)
2ce481849a merge bitcoin#20599: Tolerate sendheaders and sendcmpct messages before verack (Kittywhiskers Van Gogh)
799214b2c8 merge bitcoin#19776: expose high bandwidth mode state via getpeerinfo (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Version 2 of BIP152 high-bandwidth mode/compact blocks implements SegWit support.

    As Dash does not implement SegWit, there has never been a need to implement v2 (and therefore, have all the code necessary to support both v1 and v2, that gets removed as part of making support v2 only).

    * Despite that, the changes surrounding removing support for both versions (that in our case, do not apply as we never have supported v2) refactor the code in other ways and influence their behaviour. In the interest of upstream alignment, those changes have been backported.

  * [bitcoin#19776](https://github.com/bitcoin/bitcoin/pull/19776) doesn't seem to work on its own without successive backports, specifically [bitcoin#20799](https://github.com/bitcoin/bitcoin/pull/20799), despite the latter being a later backport.

    <details>

    <summary>19776-only p2p_compactblocks.py run (9f2c868947cc254d021e1a9bd00eb7bc80061e81)</summary>

    ```
    dash@825a14c32b73:/src/dash$ ./test/functional/p2p_compactblocks.py
    2024-06-09T12:29:09.777000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_kb2nr5oe
    2024-06-09T12:29:16.341000Z TestFramework (INFO): Testing SENDCMPCT p2p message...
    2024-06-09T12:29:31.432000Z TestFramework (INFO): Testing compactblock construction...
    2024-06-09T12:29:40.068000Z TestFramework (INFO): Testing compactblock requests...
    2024-06-09T12:29:44.597000Z TestFramework (INFO): Testing getblocktxn handler...
    2024-06-09T12:29:59.808000Z TestFramework (INFO): Testing compactblock requests/announcements not at chain tip...
    2024-06-09T12:30:03.855000Z TestFramework (INFO): Testing handling of incorrect blocktxn responses...
    2024-06-09T12:30:05.868000Z TestFramework (INFO): Testing reconstructing compact blocks from all peers...
    2024-06-09T12:30:09.389000Z TestFramework (INFO): Testing end-to-end block relay...
    2024-06-09T12:30:10.404000Z TestFramework (INFO): Testing handling of invalid compact blocks...
    2024-06-09T12:30:12.418000Z TestFramework (INFO): Testing invalid index in cmpctblock message...
    2024-06-09T12:30:14.384000Z TestFramework (INFO): Testing high-bandwidth mode states via getpeerinfo...
    2024-06-09T12:30:16.893000Z TestFramework (ERROR): Assertion failed
    Traceback (most recent call last):
      File "/src/dash/test/functional/test_framework/test_framework.py", line 158, in main
        self.run_test()
      File "./test/functional/p2p_compactblocks.py", line 849, in run_test
        self.test_highbandwidth_mode_states_via_getpeerinfo()
      File "./test/functional/p2p_compactblocks.py", line 791, in test_highbandwidth_mode_states_via_getpeerinfo
        hb_test_node.send_and_ping(msg_block(block))
      File "/src/dash/test/functional/test_framework/p2p.py", line 579, in send_and_ping
        self.sync_with_ping(timeout=timeout)
      File "/src/dash/test/functional/test_framework/p2p.py", line 596, in sync_with_ping
        self.wait_until(test_function, timeout=timeout)
      File "/src/dash/test/functional/test_framework/p2p.py", line 487, in wait_until
        wait_until_helper(test_function, timeout=timeout, lock=p2p_lock, timeout_factor=self.timeout_factor)
      File "/src/dash/test/functional/test_framework/util.py", line 249, in wait_until_helper
        if predicate():
      File "/src/dash/test/functional/test_framework/p2p.py", line 484, in test_function
        assert self.is_connected
    AssertionError
    2024-06-09T12:30:17.396000Z TestFramework (INFO): Stopping nodes
    2024-06-09T12:30:18.400000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_kb2nr5oe
    2024-06-09T12:30:18.401000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_kb2nr5oe/test_framework.log
    2024-06-09T12:30:18.401000Z TestFramework (ERROR):
    2024-06-09T12:30:18.401000Z TestFramework (ERROR): Hint: Call /src/dash/test/functional/combine_logs.py '/tmp/dash_func_test_kb2nr5oe' to consolidate all logs
    2024-06-09T12:30:18.401000Z TestFramework (ERROR):
    2024-06-09T12:30:18.401000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
    2024-06-09T12:30:18.402000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
    2024-06-09T12:30:18.402000Z TestFramework (ERROR):
    ```

    </details>

    <details>

    <summary>20799-incl p2p_compactblocks.py run (aa116c4f0b4753b615f9483aa03adec5ee4fd655)</summary>

    ```
    dash@825a14c32b73:/src/dash$ ./test/functional/p2p_compactblocks.py
    2024-06-09T12:34:27.169000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_7d65lmhz
    2024-06-09T12:34:32.695000Z TestFramework (INFO): Testing SENDCMPCT p2p message...
    2024-06-09T12:34:51.288000Z TestFramework (INFO): Testing compactblock construction...
    2024-06-09T12:34:55.325000Z TestFramework (INFO): Testing compactblock requests...
    2024-06-09T12:34:59.861000Z TestFramework (INFO): Testing getblocktxn handler...
    2024-06-09T12:35:07.460000Z TestFramework (INFO): Testing compactblock requests/announcements not at chain tip...
    2024-06-09T12:35:09.503000Z TestFramework (INFO): Testing handling of incorrect blocktxn responses...
    2024-06-09T12:35:11.519000Z TestFramework (INFO): Testing reconstructing compact blocks from all peers...
    2024-06-09T12:35:15.039000Z TestFramework (INFO): Testing end-to-end block relay...
    2024-06-09T12:35:16.055000Z TestFramework (INFO): Testing handling of invalid compact blocks...
    2024-06-09T12:35:17.062000Z TestFramework (INFO): Testing invalid index in cmpctblock message...
    2024-06-09T12:35:19.139000Z TestFramework (INFO): Testing high-bandwidth mode states via getpeerinfo...
    2024-06-09T12:35:22.159000Z TestFramework (INFO): Stopping nodes
    2024-06-09T12:35:23.163000Z TestFramework (INFO): Cleaning up /tmp/dash_func_test_7d65lmhz on exit
    2024-06-09T12:35:23.163000Z TestFramework (INFO): Tests successful
    ```
    </details>

  * The backport of [bitcoin-core/gui#206](https://github.com/bitcoin-core/gui/pull/206) is a continuation of 3e8ba24c87 from [dash#5964](https://github.com/dashpay/dash/pull/5964)

  * The backport of [bitcoin#20764](https://github.com/bitcoin/bitcoin/pull/20764) is a continuation of bd934c71eb from [dash#6034](https://github.com/dashpay/dash/pull/6034)

  ## Breaking changes

  * The `getpeerinfo` RPC returns two new boolean fields, `bip152_hb_to` and `bip152_hb_from`, that respectively indicate whether we selected a peer to be in compact blocks high-bandwidth mode or whether a peer selected us as a compact blocks high-bandwidth peer.

    High-bandwidth peers send new block announcements via a `cmpctblock` message rather than the usual inv/headers announcements. See BIP 152 for more details.

  * Blocks-only mode will use legacy relaying instead of BIP152 high-bandwidth mode

  ## 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 1cbf3b9a53
  PastaPastaPasta:
    utACK 1cbf3b9a53
  knst:
    utACK 1cbf3b9a53

Tree-SHA512: 5947b622d8d57a1dc9445cd6e07d4ad690379416d0fcf04ed574269975d1beb704691a79ff081341f3c800cf11869d401f1ed90baa5449f371f9ce658f2d2e95
2024-06-11 08:42:46 -05:00
pasta
74fcd026db
Merge #6043: backport: merge bitcoin#22879, #22762, #23041, #22734, #22950, #23053, #22839, #23140, #23306, #23354, #23380 (addrman backports: part 2)
a93fec6f2d merge bitcoin#23380: Fix AddrMan::Add() return semantics and logging (Kittywhiskers Van Gogh)
d1a4b14b48 merge bitcoin#23354: Introduce new V4 format addrman (Kittywhiskers Van Gogh)
7a97aabfe0 test: restore pre-bitcoin#23306 tests to validate port distinguishment (Kittywhiskers Van Gogh)
1a050d6cb4 merge bitcoin#23306: Make AddrMan support multiple ports per IP (Kittywhiskers Van Gogh)
d56702aa0c merge bitcoin#23140: Make CAddrman::Select_ select buckets, not positions, first (Kittywhiskers Van Gogh)
19b0145379 merge bitcoin#22839: improve addrman logging (Kittywhiskers Van Gogh)
3910c68028 merge bitcoin#23053: Use public methods in addrman fuzz tests (Kittywhiskers Van Gogh)
b6ec8ab6df merge bitcoin#22950: Pimpl AddrMan to abstract implementation details (Kittywhiskers Van Gogh)
236cf36d88 merge bitcoin#22734: Avoid crash on corrupt data, Force Check after deserialize (Kittywhiskers Van Gogh)
2420ac9e53 merge bitcoin#23041: Add addrman deserialization error tests (Kittywhiskers Van Gogh)
8aefa9b93a merge bitcoin#22762: Raise InitError when peers.dat is invalid or corrupted (Kittywhiskers Van Gogh)
c9a645f814 merge bitcoin#22879: Fix format string in deserialize error (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependent on https://github.com/dashpay/dash/pull/6040.
  * Dash already introduced support for storage of address-port pairs (referred to as "port discrimination") and allowed the usage of non-default ports in P2P with [dash#2168](https://github.com/dashpay/dash/pull/2168).
    * Albeit this was only permitted for networks with `fAllowMultiplePorts` enabled (which at the time was `devnet` and as it stands currently, on every network except `mainnet`).
  * Keeping in line with the above policy (discussion on lifting such restrictions on `mainnet` is outside the scope of this PR), when backporting [bitcoin#23306](https://github.com/bitcoin/bitcoin/pull/23306), changes have been made to retain the effects of `discriminate_ports`.
    * This involves appending placing a `!m_discriminate_ports` condition to behaviour that otherwise would be _removed_ entirely.
    * Additionally, in line with upstream backports, changes have been made that render port distinguishment _enabled_ as the new default in `addrman_tests` (the old default was to keep it _disabled_, to mirror `mainnet` and pre-change upstream behaviour).
      * To ensure distinguishment _disabled_ works as expected, affected pre-backport tests were reintroduced with the `_nondiscriminate` suffix.

  ---

  I would propose at some point to rename the flag to `ignore_port`/`suppress_port` (if not remove it altogether should the `mainnet` restriction be lifted) as discriminate (or distinguish) isn't immediately clear with if address entries will be discriminated/distinguished _using_ ports (i.e. considered) or ports will be discriminated _against_ (i.e. ignored).

  ## Breaking Changes

  It's unclear if these backports result in serialization issues for older versions, as Dash Core technically supported address-port pairs since 0.12 and suppressed it on `mainnet` by setting zero-ing out the port ([source](19512988c6/src/addrman.cpp (L135-L138))), meaning even with port discrimination _disabled_, the serialization format should remain the same.

  Regardless, following upstream backports, a new version has been introduced (v4) that marks the AddrMan format incompatible with older versions of Dash Core.

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

Tree-SHA512: 49b35af3e4eb660249ce9a65d5a539205d852e9c728da22dc88779f6b3b15c13cf91522896a313bfe2a91889fedf3b6b2cebdea12cc2bbe865ec3b85b6a5dfa8
2024-06-10 23:35:43 -05:00
pasta
02549598b2
Merge #6050: backport: trivial 2024 06 07
6777ab73a2 Merge bitcoin-core/gui#682: Don't directly delete abandoned txes from GUI (Hennadii Stepanov)
6e1a8c1fdc Merge bitcoin/bitcoin#26628: RPC: Reject RPC requests with same named parameter specified multiple times (MarcoFalke)
7c28b01c78 Merge bitcoin/bitcoin#26666: refactor: Deleted unreachable code in httpserver.cpp (MarcoFalke)
478fe51ead Merge bitcoin/bitcoin#26100: doc: clarify that NetPermissionFlags::Implicit is only about whitelists (MarcoFalke)
69b19cbfc0 Merge bitcoin/bitcoin#26546: test: remove unused class `NodePongAdd1` (fanquake)
245df942c4 Merge bitcoin/bitcoin#26380: Revert "test: check importing wallets when blocks are pruned throw an error" (MacroFake)
3db3bd0d31 Merge bitcoin/bitcoin#24269: test: add functional test for `-discover` (Andrew Chow)
c72ef299da Merge bitcoin/bitcoin#25896: wallet: Log when Wallet::SetMinVersion sets a different minversion (Andrew Chow)
12b438cb46 Merge bitcoin/bitcoin#25925: doc: add `{import,list}descriptors` to list of descriptor RPCs (Andrew Chow)
73d64c48a9 Merge bitcoin/bitcoin#25738: depends: use a patch instead of sed in libxcb (fanquake)
1fae0c2bbb Merge bitcoin/bitcoin#25333: test: Fix out-of-range port collisions (MacroFake)
b75089667c Merge bitcoin/bitcoin#25231: ci: Install documented packages for "Win64" CI task (fanquake)

Pull request description:

  ## Issue being fixed or feature implemented
  Batch of trivial backports

  ## What was done?
  Trivial backports

  ## How Has This Been Tested?
  Built; ran tests locally

  ## Breaking Changes
  None

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

ACKs for top commit:
  UdjinM6:
    utACK 6777ab73a2

Tree-SHA512: 60d68c8d0fb9875d0b2421cad97b46a9d4deb79e03ca011d66cc0595ed68bb7ad207f2fd95973a3b7a0b9bca2c2f0deaf0e1116f0312a4d9d315f009228fe485
2024-06-10 17:35:10 -05:00
Kittywhiskers Van Gogh
6274a571b7
merge bitcoin#20799: Only support version 2 compact blocks 2024-06-10 17:31:24 +00:00
Kittywhiskers Van Gogh
f4ce573538
merge bitcoin#22340: Use legacy relaying to download blocks in blocks-only mode 2024-06-10 17:31:24 +00:00
Kittywhiskers Van Gogh
73b8f84fdb
merge bitcoin#22147: p2p: Protect last outbound HB compact block peer 2024-06-10 17:31:24 +00:00
Kittywhiskers Van Gogh
799214b2c8
merge bitcoin#19776: expose high bandwidth mode state via getpeerinfo 2024-06-10 17:31:23 +00:00
Kittywhiskers Van Gogh
a93fec6f2d
merge bitcoin#23380: Fix AddrMan::Add() return semantics and logging 2024-06-10 17:16:38 +00:00
Kittywhiskers Van Gogh
d1a4b14b48
merge bitcoin#23354: Introduce new V4 format addrman 2024-06-10 17:16:37 +00:00
Kittywhiskers Van Gogh
b6ec8ab6df
merge bitcoin#22950: Pimpl AddrMan to abstract implementation details 2024-06-10 17:15:04 +00:00
Kittywhiskers Van Gogh
236cf36d88
merge bitcoin#22734: Avoid crash on corrupt data, Force Check after deserialize 2024-06-10 17:15:04 +00:00
Kittywhiskers Van Gogh
2420ac9e53
merge bitcoin#23041: Add addrman deserialization error tests 2024-06-10 17:15:04 +00:00
Kittywhiskers Van Gogh
8aefa9b93a
merge bitcoin#22762: Raise InitError when peers.dat is invalid or corrupted 2024-06-10 17:15:04 +00:00
Kittywhiskers Van Gogh
c9a645f814
merge bitcoin#22879: Fix format string in deserialize error 2024-06-10 17:15:04 +00:00
pasta
21af5af6c5
Merge #6051: refactor: proper support for composite commands such as 'bls generate'
9413ecdc66 fix: disable linter 'check-rpc-mapping.py' for composite commands (Konstantin Akimov)
f4bc19fb99 refactor: proper support for composite commands such as 'bls generate' (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  We have composite commands such as 'bls generate' that do not exist in bitcoin's implementation of rpc.
  It doesn't let to backport yet bitcoin#18531 which enforced extra checks for arguments name (name of rpc and list arguments in rpc help and actual implementation must match).

  ## What was done?
  This PR improves support of composite commands in Dash Core. New style of composite commands are applied for `bls` composite commands: `bls generate` and `bls fromsecret` as proof of concept.
  Once this PR is merged, I will provide similar fixes for other "compose" rpc commands: `protx`, `masternode` (and everything else if any).

  Beside better validation of arguments and command names, it improves suggest menu in Qt app (see a screenshot) for composite commands.

  ![image](https://github.com/dashpay/dash/assets/545784/08dcc0b4-df92-4090-b163-af498bf200ef)

  ## How Has This Been Tested?
  Run unit and functional tests. Also extra tests to conduct in qt app:
   - check suggest for 'bls ....' and 'help bls ...'
   - check output of next commands:
  ```
  help bls
  help bls generate
  help bls from secret
  bls
  bls generate
  bls generate 1
  ```
   - also let's see that old-fashion composite commands are not broken:
  ```
  help protx
  help protx diff
  protx diff 00000021c7604b9992254f9f1ed91de5d65eaade33c773abea63a7b0e93293ee 000000e44f9894838ebf768b464177cfce8859dcf92b0509f5c2fba774315996
  ```

  ## 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
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK 9413ecdc66

Tree-SHA512: c8accd2f1ea1d2168d7a2bf25311c92b7839782fc300c587c7537880df5d3fbbf7e79cedb2c6fa88ab067b696994beaf7d92185ae3e64fe1a5f70ad9e8620bec
2024-06-10 11:46:14 -05:00
MarcoFalke
6e1a8c1fdc
Merge bitcoin/bitcoin#26628: RPC: Reject RPC requests with same named parameter specified multiple times
8c3ff7d52ae3314959e1e66da8718a3f0d30abaa test: Suggested cleanups for rpc_namedparams test (Ryan Ofsky)
d1ca56382512df3084fce7353bf1e8b66cae61bc bitcoin-cli: Make it an error to specify the "args" parameter two different ways (Ryan Ofsky)
6bd1d20b8cf27aa72ec2907342787e6fc9f94c50 rpc: Make it an error server-side to specify same named parameter multiple times (Ryan Ofsky)
e2c3b18e671e347e422d696d1cbdd9f82b2ce468 test: Add RPC tests for same named parameter specified more than once (Ryan Ofsky)

Pull request description:

  Make the JSON-RPC server reject requests with the same named parameter specified multiple times, instead of silently overwriting earlier parameter values with later ones.

  Generally JSON keys are supposed to unique, and their order isn't supposed to be significant, so having the server silently discard duplicate keys is error-prone. Most likely if an RPC client is sending a request with duplicate keys it means something is wrong with the request and there should be an error.

  After this change, named parameters are still allowed to specified multiple times on the `bitcoin-cli` command line, since `bitcoin-cli` automatically replaces earlier values with later values before sending the JSON-RPC request. This makes sense, since it's not unusual for the order of command line options to be significant or for later command line options to override earlier ones.

ACKs for top commit:
  MarcoFalke:
    review ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa 🗂
  kristapsk:
    ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa
  stickies-v:
    ACK 8c3ff7d52

Tree-SHA512: 2d1357dcc2c171da287aeefc7b333ba4e67babfb64fc14d7fa0940256e18010a2a65054f3bf7fa1571b144d2de8b82d53076111b5f97ba29320cfe84b6ed986f
2024-06-10 11:26:11 -05:00
pasta
825bba1312
Merge #6031: backport: merge bitcoin#23077, #22834, #24165, #24555, #24663, #24205, #24687, #25173, #24991, partial bitcoin#24468 (cjdns support)
32f8fda7d6 merge bitcoin#24991: allow startup with -onlynet=onion -listenonion=1 (Kittywhiskers Van Gogh)
e67ed92d3d merge bitcoin#25173: add coverage for unknown network in -onlynet (Kittywhiskers Van Gogh)
77efd36112 merge bitcoin#24687: Check an invalid -i2psam will raise an init error (Kittywhiskers Van Gogh)
fb1416f7cb merge bitcoin#24205: improve network reachability test coverage and safety (Kittywhiskers Van Gogh)
7cb7479829 merge bitcoin#24663: add links to doc/cjdns.md (Kittywhiskers Van Gogh)
c736ebf566 merge bitcoin#24555: create initial doc/cjdns.md for CJDNS how-to documentation (Kittywhiskers Van Gogh)
554bd24186 partial bitcoin#24468: improve -onlynet help and related tor/i2p documentation (Kittywhiskers Van Gogh)
5436b6a82d merge bitcoin#24165: extend inbound eviction protection by network to CJDNS peers (Kittywhiskers Van Gogh)
d52724d039 merge bitcoin#22834: respect -onlynet= when making outbound connections (Kittywhiskers Van Gogh)
f9d1a9a00d merge bitcoin#23077: Full CJDNS support (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Depends on https://github.com/dashpay/dash/pull/6034

  * Depends on https://github.com/dashpay/dash/pull/6035

  * If `-proxy=` is given together with `-noonion` then the provided proxy will not be set as a proxy for reaching the Tor network. So it will not be possible to open manual connections to the Tor network for example with the `addnode` RPC. To mimic the old behavior use `-proxy=` together with `-onlynet=` listing all relevant networks except `onion`.

  * [bitcoin#24165](https://github.com/bitcoin/bitcoin/pull/24165) has been backported _before_ [bitcoin#23758](https://github.com/bitcoin/bitcoin/pull/23758) and to account for this, minor changes were made in `src/test/net_peer_eviction_tests.cpp` (using `nTimeConnected` instead of `m_connected`). When backporting  [bitcoin#23758](https://github.com/bitcoin/bitcoin/pull/23758), these changes will have to be reversed as they won't be covered by the cherry-pick diff.

  * CJDNS support has been labelled as being introduced in Dash Core 21.0, in line with the milestone designation of the PR. Should `develop` be used for a new minor/patch release, `doc/cjdns.md` will have to be modified to reflect the correct version number.

  ## Breaking changes

  No expected protocol or consensus changes.

  ## Checklist:

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas **(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:
  PastaPastaPasta:
    utACK 32f8fda7d6

Tree-SHA512: e23b22ca5edbe4c4abeab0bc07780303e68e7c4cc46b7697300b0837c5acd3a98649b6b03bd07a23c827bd85f64210173027b0b0eea31872c031fa4ed04eeb0c
2024-06-10 11:11:11 -05:00
fanquake
69b19cbfc0
Merge bitcoin/bitcoin#26546: test: remove unused class NodePongAdd1
40bdc8a6e4dcf7faf90f63b0912c18d72436e37f test: remove unused class `NodePongAdd1` (Sebastian Falbesoner)

Pull request description:

  This class was introduced in commit fa3365430c5fb57d7c0b5f2bce9fbbe290be93c3 ("net: Use mockable time for ping/pong, add tests"), but actually never used.

ACKs for top commit:
  stickies-v:
    ACK 40bdc8a6e

Tree-SHA512: b5a6552e4f2e0b7e368a071cc53b9a8e6f5d1950565a9fda8eb1971a01d8be0541d066842723ef44174fe8189925fa36f2defb6d7bf8d104abc77de410cc4c13
2024-06-10 11:00:47 -05:00
MacroFake
245df942c4
Merge bitcoin/bitcoin#26380: Revert "test: check importing wallets when blocks are pruned throw an error"
e1eadaa72d6831d1d0a53ba97c215dc4cdb64436 Revert "test: check importing wallets when blocks are pruned throw an error" (Aurèle Oulès)

Pull request description:

  The test doesn't pass (not detected by the normal CI, because it is an extended test):

  ```
  Traceback (most recent call last):
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/test_framework/test_framework.py", line 133, in main
      self.run_test()
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/feature_pruning.py", line 480, in run_test
      self.wallet_test()
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/feature_pruning.py", line 361, in wallet_test
      assert_raises_rpc_error(-4, "Importing wallets is disabled when blocks are pruned", self.nodes[2].importwallet, "abc")
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/test_framework/util.py", line 130, in assert_raises_rpc_error
      assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
    File "/tmp/cirrus-ci-build/bitcoin-core/test/functional/test_framework/util.py", line 145, in try_rpc
      raise AssertionError(
  AssertionError: Expected substring not found in error message:
  substring: 'Importing wallets is disabled when blocks are pruned'
  error message: 'Only legacy wallets are supported by this command'.
  ```

  So revert it for now, which will be done anyway in https://github.com/bitcoin/bitcoin/pull/24865/commits. (This commit is taken from there)

ACKs for top commit:
  andrewtoth:
    ACK e1eadaa72d6831d1d0a53ba97c215dc4cdb64436

Tree-SHA512: 10f556ea1aa97dc9cb64f91055977eecb9534f658170aabb4909c3e85ca6c20588c7a021219356fab678e0e2bec999d347facd00054f07a9445ad393e6353b4c
2024-06-10 11:00:47 -05:00
Andrew Chow
3db3bd0d31
Merge bitcoin/bitcoin#24269: test: add functional test for -discover
bff05bd7456d3634b0c83539293a753db6d76376 test: add functional test for -discover (brunoerg)

Pull request description:

  This PR adds a functional test for `-discover`. It tests different scenarios where `localaddresses` should be empty or may contain the addresses. Obs: `localaddresses` is not always accurate, so it's not possible to ensure (100%) it will contain any addresses.

  515200298b/src/init.cpp (L449)

  Obs: See #24258  - It adds test coverage for this field but for nodes with proxy.

ACKs for top commit:
  mzumsande:
    Code review ACK bff05bd7456d3634b0c83539293a753db6d76376
  achow101:
    ACK bff05bd7456d3634b0c83539293a753db6d76376
  rajarshimaitra:
    tACK bff05bd7456d3634b0c83539293a753db6d76376

Tree-SHA512: 8782497c146bce1ba86fda6146f3847465d7069f2cb6b84f2afc8f3b43efa813442bffe7447e9ce02adee304100b60365409bf0e5d875dfb880038442feec2a6
2024-06-10 11:00:46 -05:00
Konstantin Akimov
9413ecdc66
fix: disable linter 'check-rpc-mapping.py' for composite commands
This linter is subject to remove in 20012 anyway, which is todo after 18531 is done
2024-06-10 17:29:13 +07:00
MacroFake
1fae0c2bbb
Merge bitcoin/bitcoin#25333: test: Fix out-of-range port collisions
fa7a711a30b707cbdee4435dd0a956bffb7aaccb test: Fix out-of-range port collisions (MacroFake)

Pull request description:

  Otherwise the test will fail if two tests running in parallel use the same port. See https://github.com/bitcoin/bitcoin/pull/25096#discussion_r892558783 and https://github.com/bitcoin/bitcoin/pull/25312

ACKs for top commit:
  dergoegge:
    ACK fa7a711a30b707cbdee4435dd0a956bffb7aaccb - This gets rid of some rather arbitrary choices for ports in some of our functional tests that can cause port collisions across test runs, resulting in intermittent failures.

Tree-SHA512: ac73da8a498230b992ab12e1ee3c4ff3d868cd63c00d2c71537d156cb7c8f8be8598ec574646b17c5a44ae3ac5bb54bf29d300f054a36cec6f6ce8054a0da0a4
2024-06-08 20:59:34 -05:00
pasta
d7413ffbf7
Merge #6047: backport: trivial 2024 06 05
76279c1a37 Merge bitcoin/bitcoin#25149: refactor: Add thread safety annotation to `BanMan::SweepBanned()` (MacroFake)
6269c6f1db Merge bitcoin/bitcoin#25053: Guard `#include <config/bitcoin-config.h>` (fanquake)
d50f0b016f Merge bitcoin/bitcoin#24977: rpc: Explain active and internal in listdescriptors (fanquake)
3c44399d55 Merge bitcoin/bitcoin#24856: lint: Converting lint-assertions.sh to lint-assertions.py (laanwj)
e9f5b4b735 Merge bitcoin/bitcoin#24213: refactor: use Span in random.* (laanwj)
7e0474ac1c Merge bitcoin/bitcoin#24632: add `(none)` in -getinfo `Warnings:` if no warning returned (laanwj)
57e9b56bad Merge bitcoin/bitcoin#24145: mempool: Clear vTxHashes when mapTx is cleared (laanwj)
fe56d9b994 Merge bitcoin/bitcoin#24698: test: -peerblockfilters without -blockfilterindex raises an error (MarcoFalke)
3cabce645e Merge bitcoin/bitcoin#24472: fuzz: execute each file in dir without fuzz engine (MarcoFalke)
f5116a7d31 Merge bitcoin-core/gui#549: refactor: use std::chrono for formatDurationStr() helper (Hennadii Stepanov)
3fa8158510 Merge bitcoin/bitcoin#22317: doc: Highlight DNS requests part in tor.md (Andrew Chow)
72b62edd5a Merge bitcoin/bitcoin#23834: wallettool: Check that the dumpfile checksum is the correct size (laanwj)
ee9b3cdb0e Merge bitcoin/bitcoin#23979: test: wait for rather than assert presence of file in startupnotify test (MarcoFalke)
2ec5940399 Merge bitcoin/bitcoin#23532: test: add functional test for -startupnotify (MarcoFalke)
5a31be9608 Merge bitcoin/bitcoin#23812: test: fix intermittent failures in p2p_timeouts.py (MarcoFalke)
10828f5b3d Merge bitcoin/bitcoin#23733: fuzz: Move ISO8601 to one place (MarcoFalke)
7f39b5af41 Merge bitcoin/bitcoin#23635: test: Bump shellcheck version to 0.8.0 (fanquake)

Pull request description:

  ## Issue being fixed or feature implemented
  Trivial batch of backports

  ## What was done?
  trivial backports

  ## How Has This Been Tested?
  Unit tests ran; waiting on CI

  ## Breaking Changes

  ## 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
  - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

Top commit has no ACKs.

Tree-SHA512: a3f97003e6441468951b827b2c3ea607740e5b9d36b96c2f93e45e7fb4088ecf4d0a2b7038de050ca0e7d61379c364969f4a8caff98ec1cc69016f4114e64c6a
2024-06-07 09:33:44 -05:00
Konstantin Akimov
c575a5808a
feat: change handler to '/' for external users, use only rpc user name to choose queue 2024-06-07 16:31:07 +07:00
pasta
d441cda50a
Merge #6040: backport: merge bitcoin#20233, #22627, #22725, #22697, #22740, #22849, #22791, #22848, #22915, #22911, #22974, #20234 (addrman backports)
f619f8ff80 merge bitcoin#20234: don't bind on 0.0.0.0 if binds are restricted to Tor (Kittywhiskers Van Gogh)
1698336fc8 merge bitcoin#22974: Improve performance of Good (Kittywhiskers Van Gogh)
29f4482e01 merge bitcoin#22911: Minor cleanups to asmap (Kittywhiskers Van Gogh)
5706edad5f merge bitcoin#22915: Remove confusing CAddrDB (Kittywhiskers Van Gogh)
3f69606827 merge bitcoin#22848: Expose BanMapToJson / BanMapFromJson (Kittywhiskers Van Gogh)
9065eed969 merge bitcoin#22791: Fix asmap/addrman initialization order bug (Kittywhiskers Van Gogh)
99b7812271 merge bitcoin#22849: Remove unused SERIALIZE_METHODS for CBanEntry (Kittywhiskers Van Gogh)
a30379c557 merge bitcoin#22740: Move serialization code to cpp (Kittywhiskers Van Gogh)
d4e79aa377 merge bitcoin#22697: Remove CAddrMan::Clear() function (Kittywhiskers Van Gogh)
77d8f6c918 merge bitcoin#22725: Move addrman ser/deser tests to addrman_tests.cpp (Kittywhiskers Van Gogh)
4ba3f49afc merge bitcoin#22627: De-duplicate Add() function (Kittywhiskers Van Gogh)
49af8185e0 merge bitcoin#20233: Make consistency checks a runtime option (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependency for https://github.com/dashpay/dash/pull/6043
  * [bitcoin#22915](https://github.com/bitcoin/bitcoin/pull/22915) is backported before [bitcoin#21850](https://github.com/bitcoin/bitcoin/pull/21850), which is the reason {`Dump`, `Read`}`PeerAddresses` takes in an `const ArgsManager&` but doesn't do anything with it.
    * This will need to be accounted for when backporting [bitcoin#21850](https://github.com/bitcoin/bitcoin/pull/21850)

  ## Breaking Changes

  None expected. No changes to serialization format.

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

Tree-SHA512: 19599f0322f049b123790a45530ecb2ec74b1463b47e0102921b489bad1d5cb671a94900b3b35dbd7c1d80bc0680f1ed01e6fac675ac0be81621878388be0bcd
2024-06-06 23:24:43 -05:00
laanwj
3c44399d55
Merge bitcoin/bitcoin#24856: lint: Converting lint-assertions.sh to lint-assertions.py
172c2333f03aecb4c347c791537e13c296adbde2 Porting lint-assertions.sh to lint-assertions.py (hiago)

Pull request description:

  This PR is converting `test/lint/lint-assertions.sh` to `test/lint/lint-assertions.py`. It's an item of #24783.

ACKs for top commit:
  laanwj:
    Tested ACK 172c2333f03aecb4c347c791537e13c296adbde2

Tree-SHA512: 94d5b03acfeaf2303fad95d489d6c3aa7bd655889ddaa807cc97e0613b8eb8f5ef094feee2a98d974606890deb554e76490a5c523d64eb5bc55afa6a43221aae
2024-06-06 22:58:31 -05:00
MarcoFalke
fe56d9b994
Merge bitcoin/bitcoin#24698: test: -peerblockfilters without -blockfilterindex raises an error
d6bc2322ed2e0674e027d39825fdadbb0db2c24a test: -peerblockfilters without -blockfilterindex raises an error (brunoerg)

Pull request description:

  This PR adds test coverage for the following init error:
  2a3e8fb359/src/init.cpp (L850)

  Setting -peerblockfilters without -blockfilterindex should raise an error when initializing.

ACKs for top commit:
  ccdle12:
    Tested ACK d6bc2322ed

Tree-SHA512: e740c2ccde6bb1bb8381bb676a6d01bd5746cf9ce0c8dadd62067a6b9b380027bfe8b8cdeae9846a0ab18385f3dc5dff607fe5274cb55107d47470db00015fb2
2024-06-06 22:58:29 -05:00
laanwj
72b62edd5a
Merge bitcoin/bitcoin#23834: wallettool: Check that the dumpfile checksum is the correct size
ac617cc141fe05bea0dc5e8f9df3da43c0945842 wallettool: Check that the dumpfile checksum is the correct size (Andrew Chow)

Pull request description:

  After parsing the checksum, make sure that it is the size that we expect it to be.

  This issue was reported by Pedro Baptista.

ACKs for top commit:
  laanwj:
    Code review ACK ac617cc141fe05bea0dc5e8f9df3da43c0945842

Tree-SHA512: 8135b3fb1f4f6b6c91cfbac7d1d3421f1f6c664a742c92940f68eae857f92ce49d042cc3aa5c2df6ef182825271483d65efc7543ec7a8ff047fd7c08666c8899
2024-06-06 22:57:57 -05:00
MarcoFalke
ee9b3cdb0e
Merge bitcoin/bitcoin#23979: test: wait for rather than assert presence of file in startupnotify test
96eb0093d07c58ad3b02c49a5a4385da37a8e337 test: wait rather than assert presence of file in startupnotify test (fanquake)

Pull request description:

  Should fix #23967.

ACKs for top commit:
  brunoerg:
    crACK 96eb0093d07c58ad3b02c49a5a4385da37a8e337
  kristapsk:
    utACK 96eb0093d07c58ad3b02c49a5a4385da37a8e337

Tree-SHA512: 9107970e45c027cfc6c6cbfcfd5a7d9f5956259bbbb11f5b180c3947126e42e62c0f8ffd69cf7b39b51c9c5b4fedbb753839d59aebe876be68c1484bb6065819
2024-06-06 22:57:57 -05:00
MarcoFalke
2ec5940399
Merge bitcoin/bitcoin#23532: test: add functional test for -startupnotify
126853214a490ee840e83ca17c717c40cfbe6837 test: add functional test for -startupnotify (Bruno Garcia)

Pull request description:

  This PR adds a functional test for -startupnotify. It basically starts the node passing a command on -startupnotify to create a file on tmp and then, we check if the file has been successfully created.

ACKs for top commit:
  theStack:
    Tested ACK 126853214a490ee840e83ca17c717c40cfbe6837
  kristapsk:
    re-ACK 126853214a490ee840e83ca17c717c40cfbe6837

Tree-SHA512: 5bf3e46124ee5c9d609c9993e6465d5a71a8d2275dcf07c8ce0549f013f7f8863d483b46b7164152f566468a689371ccb87f01cf118c3c9cac5b2be673b61a5c
2024-06-06 22:57:57 -05:00
MarcoFalke
5a31be9608
Merge bitcoin/bitcoin#23812: test: fix intermittent failures in p2p_timeouts.py
0a1b6fa5a18f3efb2ac3e28a23a4fd5e1cf9eaf0 test: fix intermittent timeouts in p2p_timeouts.py (Martin Zumsande)

Pull request description:

  Fixes  #23800 by making sure that all peers are connected (i.e. `m_connected` is set) before the mocktime is bumped.
  We can't wait for verack here, but we can wait for a debug log entry ("Added connection peer=2") instead.

  In the failed CI runs (e.g. https://cirrus-ci.com/task/5600553806856192?logs=ci#L7469)  different peers were added at different mocktimes.

ACKs for top commit:
  naumenkogs:
    ACK 0a1b6fa5a18f3efb2ac3e28a23a4fd5e1cf9eaf0
  theStack:
    Concept and approach ACK 0a1b6fa5a18f3efb2ac3e28a23a4fd5e1cf9eaf0

Tree-SHA512: 1a3c8a9a79339d4adc6ecb1731eb0d0eadb2e5024ad3c6779b4696691f85d6c3304ef8689746d0332150a4cf04489ca4b2ff3eeb0bb76feec28c1e4bb9dbca19
2024-06-06 22:57:56 -05:00
Konstantin Akimov
f1c1fd873e
feat: implementation for /external handler for RPC 2024-06-06 09:43:08 +07:00
MarcoFalke
a5e7b029f2
Merge #21124: test: remove unnecessary assignment in bdb
c9095b738fd4257ae5bdbb2ae38d3e7f41f51b64 test: remove unnecessary assignment in bdb (Bruno Garcia)

Pull request description:

  This PR removes the unnecessary assignment to page_info['entries'] on line 54 since there is another assignment for it in line 59.

  I think a lint (#21096) would detect cases like this one.

ACKs for top commit:
  achow101:
    ACK c9095b738fd4257ae5bdbb2ae38d3e7f41f51b64
  theStack:
    Code Review ACK c9095b738fd4257ae5bdbb2ae38d3e7f41f51b64

Tree-SHA512: 23377077c015b04361fd416b41bf6806ad0bdd4d264be6760f0fd3bc88d694d2cd52cae250519925c5d3b3c70715772714c3863f8fa181a2eb4883204ccdbf9d
2024-06-04 12:50:36 -05:00
UdjinM6
18328279ec
fix: force mnsync to skip gov obj sync on reconnection 2024-06-04 18:16:18 +03:00
UdjinM6
08331bb950
fix: apply suggestions 2024-06-04 18:16:01 +03:00
Kittywhiskers Van Gogh
f619f8ff80
merge bitcoin#20234: don't bind on 0.0.0.0 if binds are restricted to Tor 2024-06-04 13:32:27 +00:00
Kittywhiskers Van Gogh
32f8fda7d6
merge bitcoin#24991: allow startup with -onlynet=onion -listenonion=1 2024-05-29 18:07:46 +00:00
Kittywhiskers Van Gogh
e67ed92d3d
merge bitcoin#25173: add coverage for unknown network in -onlynet 2024-05-29 18:07:45 +00:00
Kittywhiskers Van Gogh
77efd36112
merge bitcoin#24687: Check an invalid -i2psam will raise an init error 2024-05-29 18:07:45 +00:00
Kittywhiskers Van Gogh
fb1416f7cb
merge bitcoin#24205: improve network reachability test coverage and safety 2024-05-29 18:07:45 +00:00
Kittywhiskers Van Gogh
d52724d039
merge bitcoin#22834: respect -onlynet= when making outbound connections 2024-05-29 18:07:45 +00:00
Kittywhiskers Van Gogh
f9d1a9a00d
merge bitcoin#23077: Full CJDNS support 2024-05-29 18:07:44 +00:00
pasta
7596a7320a
Merge #6033: backport: bitcoin#18202, #19202, #19501, #19725, #19770, #19877, #20043, partial #18878
34c80473a8 Merge #19877: [test] clarify rpc_net & p2p_disconnect_ban functional tests (Wladimir J. van der Laan)
e42412924f Merge #19770: RPC: getpeerinfo: Deprecate "whitelisted" field (replaced by "permissions") (MarcoFalke)
f96966b7ea Merge #20043: doc: Add 19501 release notes (fanquake)
6a164eaea9 Merge #19501: send* RPCs in the wallet returns the "fee reason" (MarcoFalke)
b6c8d852e3 Merge #19725: [RPC] Add connection type to getpeerinfo, improve logs (MarcoFalke)
f86263b180 Merge #18202: refactor: consolidate sendmany and sendtoaddress code (Samuel Dobson)
fab41fd3c5 partial Merge #18878: test: Add test for conflicted wallet tx notifications (Wladimir J. van der Laan)
db5bd34ee8 Merge #19202: log: remove deprecated `db` log category (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  Regular backports from bitcoin v21

  ## What was done?
   - bitcoin/bitcoin#19202
   - partial bitcoin/bitcoin#18878
   - bitcoin/bitcoin#18202
   - bitcoin/bitcoin#19725
   - bitcoin/bitcoin#19501
   - bitcoin/bitcoin#20043
   - bitcoin/bitcoin#19770
   - bitcoin/bitcoin#19877

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

  ## Breaking Changes
  - (RPC) The `getpeerinfo` RPC no longer returns the `addnode` field by default. This
    field will be fully removed in the next major release.  It can be accessed
    with the configuration option `-deprecatedrpc=getpeerinfo_addnode`. However,
    it is recommended to instead use the `connection_type` field (it will return
    `manual` when addnode is true)
  - (Settings) The `sendtoaddress` and `sendmany` RPCs accept an optional `verbose=True`
    argument to also return the fee reason about the sent tx.
  - (Settings) The `-debug=db` logging category, which was deprecated in v0.18 and replaced by
    `-debug=walletdb` to distinguish it from `coindb`, has been removed.
  - (RPC)  To make RPC `sendtoaddress` more consistent with `sendmany` the following error
      `sendtoaddress` codes were changed from `-4` to `-6`:
    - Insufficient funds
    - Fee estimation failed
    - Transaction has too long of a mempool chain

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

Tree-SHA512: 725a103e04c9c7d44a79da6f3f54e7745c7fb98ec906e7228ae16f7662d568e48c015c855902ff8485f2908f0f71815e769ca394cf6c3ca2e5fd920dd39cca74
2024-05-29 12:04:08 -05:00
Kittywhiskers Van Gogh
1dc50d05cb
merge bitcoin#26838: I2P documentation updates 2024-05-29 11:48:38 -05:00
Kittywhiskers Van Gogh
9bf3829558
merge bitcoin#25355: add support for transient addresses for outbound connections 2024-05-29 11:48:37 -05:00
Kittywhiskers Van Gogh
f04ce8bcda
partial bitcoin#22229: consolidate to f-strings (part 1)
includes:
- 86d958262dff43002820d58ccb8958e2dbfb9d5b
2024-05-29 11:48:36 -05:00
Wladimir J. van der Laan
34c80473a8
Merge #19877: [test] clarify rpc_net & p2p_disconnect_ban functional tests
47ff5098ad5ea2c20ea387f99940a7cde6c80789 [test] Clarify setup of node topology. (Amiti Uttarwar)
0672522aedd3760c30b8740c7e9487f00bf9dfeb [move-only, test]: Match test order with run order (Amiti Uttarwar)

Pull request description:

  small improvements to clarify logic in the functional tests
  1. have test logic in `rpc_net.py` match run order of the test
  2. remove `connect_nodes` calls that are redundant with the automatic test setup executed by the test framework

  Noticed when I was trying to debug a test for #19725. Small changes but imo very helpful, because they initially confused me.

ACKs for top commit:
  laanwj:
    ACK 47ff5098ad5ea2c20ea387f99940a7cde6c80789

Tree-SHA512: 2843da2c0b4f06b2600b3adb97900a62be7bb2228770abd67d86f2a65c58079af22c7c20957474a98c17da85f40a958a6f05cb8198aa0c56a58adc1c31100492
2024-05-29 14:03:57 +07:00
MarcoFalke
e42412924f
Merge #19770: RPC: getpeerinfo: Deprecate "whitelisted" field (replaced by "permissions")
5b57dc5458800e56b4dddfeb32a1813804a62b0f RPC: getpeerinfo: Wrap long help line for bytesrecv_per_msg (Luke Dashjr)
d681a28219d3876a2b6e3cd2fb0d92963674903e RPC: getpeerinfo: Deprecate "whitelisted" field (replaced by "permissions") (Luke Dashjr)

Pull request description:

  If we were going to continue support for "whitelisted", we should have probably made it true if any permission flag was set, rather than only if "default permissions" were used.

  This corrects the description, and deprecates it.

ACKs for top commit:
  laanwj:
    ACK 5b57dc5458800e56b4dddfeb32a1813804a62b0f

Tree-SHA512: a2e2137f8be8110357c1b2fef2c923fa8c7c4a49b0b2b3a2d78aedf12f8ed5cc7e140018a21b37e6ec7770ed4007542aeef7ad4558973901b107e8e0f81d6003
2024-05-29 14:03:56 +07:00
fanquake
f96966b7ea
Merge #20043: doc: Add 19501 release notes
fa710a6d67b2de64bde90def77c70d0a052f9030 doc: Add 19501 release notes (MarcoFalke)
faf60dee34ae3dbe8e103a2c1b0679f13df6a921 doc: Remove double-whitespace from help string, other whitespace fixups (MarcoFalke)

Pull request description:

  Adds release notes and fixes up some whitespace nits for the touched RPCs

ACKs for top commit:
  fanquake:
    ACK fa710a6d67b2de64bde90def77c70d0a052f9030
  laanwj:
    Code review ACK fa710a6d67b2de64bde90def77c70d0a052f9030

Tree-SHA512: b84a96386a9a8ed69f464c7dffdd600cf9a8b33a06120798b141b300991baed369ab91ae48df6446e89e1d62534ccd8ae721454e7a19b48900b317e9192afc47
2024-05-29 14:03:56 +07:00
MarcoFalke
6a164eaea9
Merge #19501: send* RPCs in the wallet returns the "fee reason"
69cf5d4eeb73f7d685e915fc17af64634d88a4a2 [test] Make sure send rpc returns fee reason (Sishir Giri)
d5863c0b3e20d56acf7246008b7832efde68ab21 [send] Make send RPCs return fee reason (Sishir Giri)

Pull request description:

  Whenever a wallet funds a transaction, the fee reason is reported to the user only if the verbose is set to true. I added an extra parameter to `CreateTransaction` function in wallet.cpp. Then I implemented the fee reason return logic in `SendMoney`  in rpcwallet.cpp, followed by verbose parameter in `sendtoaddress` and `sendmany` functions. I also added a fee reason test case in walletbasic.py.

  link to the issue: https://github.com/MarcoFalke/bitcoin-core/issues/22#issue-616251578

ACKs for top commit:
  instagibbs:
    ACK 69cf5d4eeb
  meshcollider:
    utACK 69cf5d4eeb73f7d685e915fc17af64634d88a4a2

Tree-SHA512: 2e3af32dcfbd5511ba95f8bc8edca7acfe709a8430ff03e43172e5d0af3dfa4b2f57906978e7f272d878043b9ed8c6004674cf47d7496b005d5f612e9a58aa0e
2024-05-29 14:03:56 +07:00
MarcoFalke
b6c8d852e3
Merge #19725: [RPC] Add connection type to getpeerinfo, improve logs
a512925e19a70d7f6b80ac530a169f45ffaafa1c [doc] Release notes (Amiti Uttarwar)
50f94b34a33c954f6e207f509c93d33267a5c3e2 [rpc] Deprecate getpeerinfo addnode field (Amiti Uttarwar)
df091b9b509f0b10e4315c0bfa2da0cc0c31c22f [refactor] Rename test file to allow any getpeerinfo deprecations. (Amiti Uttarwar)
395acfa83a5436790c1a722a5609ac9d48df235f [rpc] Add connection type to getpeerinfo RPC, update tests (Amiti Uttarwar)
49c10a9ca40967d28ae16dfea9cccc6f3a6624a1 [log] Add connection type to log statement (Amiti Uttarwar)

Pull request description:

  After #19316, we can more directly expose information about the connection type on the `getpeerinfo` RPC. Doing so also makes the existing addnode field redundant, so this PR begins the process of deprecating this field.

  This PR also includes one commit that improves a log message, as both use a shared function to return the connection type as a string.

  Suggested by sdaftuar- https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468001604 & https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468018093

ACKs for top commit:
  jnewbery:
    Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c.
  sipa:
    utACK a512925e19a70d7f6b80ac530a169f45ffaafa1c
  guggero:
    Tested and code review ACK a512925e.
  MarcoFalke:
    cr ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c 🌇
  promag:
    Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c.

Tree-SHA512: 601a7a38aee235ee59aca690784f886dc2ae4e418b2e6422c4b58cd597376c00f74910f66920b08a08a0bec28bf8022e71a1435785ff6ba8a188954261aba78e
2024-05-29 14:03:55 +07:00
Samuel Dobson
f86263b180
Merge #18202: refactor: consolidate sendmany and sendtoaddress code
08fc6f6cfc3b06fd170452a766696d7b833113fa [rpc] refactor: consolidate sendmany and sendtoaddress code (Sjors Provoost)

Pull request description:

  I consolidated code between these two RPC calls, since `sendtoaddress` is essentially `sendmany` with 1 destination.

  Unless I overlooked something, the only behaviour change is that some `sendtoaddress` error codes changed from `-4` to `-6`. The release note mentions this.

  Salvaged from #18201.

ACKs for top commit:
  fjahr:
    Code review ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
  jonatack:
    ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa
  meshcollider:
    Code review & functional test run ACK 08fc6f6cfc3b06fd170452a766696d7b833113fa

Tree-SHA512: 7b66c52fa0444a4d02fc3f81d9c2a386794d447616026a30111eda35fb46510475eea6506a9ceda00bb4e0230ebb758da5d236b3ac05c954c044fa68a1e3e909
2024-05-29 13:57:02 +07:00
pasta
3b3b1b8e00
Merge #6034: backport: merge bitcoin#21261, #20877, #21832, #22547, #22544, #22959, #23324, partial bitcoin#20764 (cli backports: part 2)
bde72a41fe merge bitcoin#23324: print peer counts for all reachable networks in -netinfo (Kittywhiskers Van Gogh)
4b245441a0 merge bitcoin#22959: Display all proxies in -getinfo (Kittywhiskers Van Gogh)
30b0fcf4a6 merge bitcoin#22544: drop torv2; torv3 becomes onion per GetNetworkName() (Kittywhiskers Van Gogh)
b6ca36edda merge bitcoin#22547: Add progress bar for -getinfo (Kittywhiskers Van Gogh)
1f89bfd176 merge bitcoin#21832: Implement human readable -getinfo (Kittywhiskers Van Gogh)
2200b78a15 merge bitcoin#20877: user help and argument parsing improvements (Kittywhiskers Van Gogh)
bd934c71eb partial bitcoin#20764: cli -netinfo peer connections dashboard updates (Kittywhiskers Van Gogh)
b2d865633f merge bitcoin#21261: update inbound eviction protection for multiple networks, add I2P peers (Kittywhiskers Van Gogh)
0b16b50fcb cli: fix loop counter comparison in `ProcessReply` (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * Dependency for https://github.com/dashpay/dash/pull/6035

  * Dependency for https://github.com/dashpay/dash/pull/6031

  * In [dash#5904](https://github.com/dashpay/dash/pull/5904) ([bitcoin#21595](https://github.com/bitcoin/bitcoin/pull/21595)), one of the loops in `ProcessReply` is supposed to iterate `rows.size()` times (which at the time was hardcoded to `3`), the backport erroneously set the value to `m_networks.size()` (which also evaluated to `3`) as part of increasing `m_networks.size()` usage.

    As this pull request includes [bitcoin#23324](https://github.com/bitcoin/bitcoin/pull/23324), which changes it over to  `rows.size()`, the above has been corrected in a separate commit for documentation purposes.

  * `-addrinfo` output

    ![dash-cli addrinfo output](https://github.com/dashpay/dash/assets/63189531/24db46be-729e-4fa8-a268-87f2497cff9a)

  * `-getinfo` output (diamonds are due to rendering limitations of my terminal and are not indicative of the symbols used)

    ![dash-cli getinfo output](https://github.com/dashpay/dash/assets/63189531/626fe67f-f505-4a04-931a-76e75146e5a0)

  * `-netinfo` output

    ![dash-cli netinfo output](https://github.com/dashpay/dash/assets/63189531/afbff3d0-7127-44e2-bfe7-81b08c0e214e)

  ## Breaking Changes

  * CLI `-addrinfo` now returns a single field for the number of `onion` addresses known to the node instead of separate `torv2` and `torv3` fields, as support for TorV2 addresses was removed from Dash Core in 18.0.

  * `-getinfo` has been updated to return data in a user-friendly format that also reduces vertical space.

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

ACKs for top commit:
  PastaPastaPasta:
    utACK bde72a41fe

Tree-SHA512: 921cb45b7e243a321a32c835eb23d5ba8df610ff234a548a9051436a2c21845ce70097fb9a9bb812b77b04373f9f0a9f90264168d97b08da1890be06bfd9f99c
2024-05-28 12:39:40 -05:00
fanquake
6d44f36afd
Merge bitcoin/bitcoin#22096: p2p: AddrFetch - don't disconnect on self-announcements
5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 test: Add functional test for AddrFetch connections (Martin Zumsande)
c34ad3309f93979b274a37de013502b05d25fad8 net, rpc: Enable AddrFetch connections for functional testing (Martin Zumsande)
533500d9072b7d5a36a6491784bdeb9247e91fb0 p2p: Add timeout for AddrFetch peers (Martin Zumsande)
b6c5d1e450dde6a54bd785504c923adfb45c7060 p2p: AddrFetch - don't disconnect on self-announcements (Martin Zumsande)

Pull request description:

  AddrFetch connections (old name: oneshots) are intended to be short-lived connections on which we ask a peer for addresses via `getaddr` and disconnect after receiving them.

  This is done by disconnecting after receiving the first `addr`. However, it is no longer working as intended, because nowadays, the first `addr` a typical bitcoin core node sends is its self-announcement.
  So we'll disconnect before the peer gets a chance to answer our `getaddr`.

  I checked that this affects both `-seednode` peers specified manually, and DNS seeds when AddrFetch is used as a fallback if DNS doesn't work for us.

  The current behavior of getting peers via AddrFetch when starting with an empty addrman would be to connect to the peer, receive its self-announcement and add it to addrman, disconnect, reconnect to the same peer again as a full outbound (no other addresses in addrman) and then receive more `addr`. This is silly and not in line with AddrFetch peer being intended to be short-lived peers. 

  Fix this by only disconnecting after receiving an `addr` message of size > 1.

  [Edit] As per review discussion, this PR now also adds a timeout after which we disconnect if we haven't received any suitable `addr`, and a functional test.

ACKs for top commit:
  amitiuttarwar:
    reACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6
  naumenkogs:
    ACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6
  jnewbery:
    ACK 5730a43703

Tree-SHA512: 8a81234f37e827705138eb254223f7f3b3bf44a06cb02126fc7990b0d231b9bd8f07d38d185cc30d55bf35548a6fdc286b69602498d875b937e7c58332158bf9
2024-05-27 12:18:23 -05:00
Wladimir J. van der Laan
fab41fd3c5
partial Merge #18878: test: Add test for conflicted wallet tx notifications
Backport notice:
we don't have bumpfee feature, so, only some part of code is backported

f963a680515eda66429b3d1537a7baf281ab9283 test: Add test for conflicted wallet tx notifications (Russell Yanofsky)

Pull request description:

  Add test coverage for conflicted wallet transaction notifications so we can improve current behavior and avoid future regressions

  https://github.com/bitcoin/bitcoin/pull/9240 - accidental break
  https://github.com/bitcoin/bitcoin/issues/9479 - bug report
  https://github.com/bitcoin/bitcoin/pull/9371 - fix
  https://github.com/bitcoin/bitcoin/pull/16624 - accidental break
  https://github.com/bitcoin/bitcoin/issues/18325 - bug report
  https://github.com/bitcoin/bitcoin/pull/18600 - potential fix

ACKs for top commit:
  laanwj:
    ACK f963a680515eda66429b3d1537a7baf281ab9283
  jonatack:
    re-ACK f963a680515eda66429b3d1537a7baf281ab9283
  MarcoFalke:
    ACK f963a680515eda66429b3d1537a7baf281ab9283

Tree-SHA512: d3a7952a2d3dc2ff0800ef857575ea4ef9759c0917d58a7fc91e2db0ca3cc3baf0dd0cf9af61683f691e5fefb11afe8120bb5810c7037ed9ecedee385dd4aa07

fixup dashify of feature_notifications
2024-05-27 21:55:54 +07:00
UdjinM6
3c3489d7a1
test: add test 2024-05-27 17:49:07 +03:00
Kittywhiskers Van Gogh
4b245441a0
merge bitcoin#22959: Display all proxies in -getinfo 2024-05-26 22:26:57 +00:00
Kittywhiskers Van Gogh
1f89bfd176
merge bitcoin#21832: Implement human readable -getinfo 2024-05-26 22:26:56 +00:00
Samuel Dobson
783a6bd842
Merge bitcoin/bitcoin#22541: Add a new RPC command: restorewallet
5fe8100ff36fed6d50c2a25b028f57b25af3504c Change the wallet_backup.py test to use the restorewallet RPC command instead of restoring wallets manually. (lsilva01)
ae23faba6fc5cabc896f1175456d1018576f912d Add a new RPC command: restorewallet (lsilva01)

Pull request description:

  As far as I know, there is no command to restore the wallet from a backup file.
  The only way to do this is to replace the `wallet.dat` of a newly created wallet with the backup file, which is hardly an intuitive way.

  This PR implements the `restorewallet` RPC command which restores the wallet from the backup file.

  To test:
  First create a backup file:
  `$ bitcoin-cli -rpcwallet="wallet-01" backupwallet /home/Backups/wallet-01.bak`

  Then restore it in another wallet:
  `$ bitcoin-cli  restorewallet "restored-wallet-01" /home/Backups/wallet-01.bak`

ACKs for top commit:
  achow101:
    re-ACK 5fe8100ff36fed6d50c2a25b028f57b25af3504c
  prayank23:
    tACK 5fe8100ff3
  meshcollider:
    utACK 5fe8100ff36fed6d50c2a25b028f57b25af3504c

Tree-SHA512: 9639df4d8ad32f255f5b868320dc69878bd9aceb3b471b49dfad500b67681e2d354292b5410982fbf18e25a44ed0c06fd4a0dd010e82807c2e00ff32e84047a1
2024-05-24 13:30:00 -05:00
MarcoFalke
00e556ac7a
Merge bitcoin/bitcoin#21872: net: Sanitize message type for logging
09205b33aa74e385caa2803aa6febc18ad1efa32 net: Clarify message header validation errors (W. J. van der Laan)
955eee76803c098978cf0bbc7f1f6d3c230544e2 net: Sanitize message type for logging (W. J. van der Laan)

Pull request description:

  - Use `SanitizeString` when logging message errors to make sure that the message type is sanitized. I have checked all logging in `net.cpp`.

  - For the `MESSAGESTART` error don't inspect and log header details at all: receiving invalid start bytes makes it likely that the packet isn't even formatted as valid P2P message. Logging the four unexpected start bytes (as hex) should be enough.

  - Update `p2p_invalid_messages.py` test to check this.

  - Improve error messages in a second commit.

  Issue reported by gmaxwell.

ACKs for top commit:
  MarcoFalke:
    re-ACK 09205b33aa74e385caa2803aa6febc18ad1efa32 only change is log message fixup 🔂
  practicalswift:
    re-ACK 09205b33aa74e385caa2803aa6febc18ad1efa32

Tree-SHA512: 8fe5326af135cfcf39ea953d9074a8c966b9b85a810b06a2c45b8a745cf115de4f321e72fc769709d6bbecfc5953aab83176db6735b04c0bc6796f59272cadce
2024-05-19 11:19:52 -05:00
fanquake
ea347e6a6e
Merge #21718: rpc: Improve error message for getblock invalid datatype.
a41149426168b8ea96099f10576022c6a09033d1 rpc: Improve getblock error message for invalid data type. (klementtan)

Pull request description:

  Improve error messages for getblock invalid datatype.

  fixes: #21717

ACKs for top commit:
  instagibbs:
    utACK a411494261
  theStack:
    ACK a41149426168b8ea96099f10576022c6a09033d1
  promag:
    Code review ACK a41149426168b8ea96099f10576022c6a09033d1.

Tree-SHA512: 6e7d8290681e8ab375629f81669d0f8e0c21f9eb7ed9e2455cd19ea013e69b2d95fa7a9ee795315b2d5c60c96035c6cefc3d6e1039a06fd88c1dc7fe275ee6a1
2024-05-19 11:19:51 -05:00
MarcoFalke
72d51a75e5
Merge bitcoin/bitcoin#22043: rpc, test: addpeeraddress test coverage, code simplify/constness
b36e0cd1b9d361ac6f9777c09328a13e9ee923be rpc: simplify addpeeraddress and improve code constness (Jon Atack)
6b1926cf1eac1ad1850599d2753dd22bc21fd327 test: addpeeraddress functional test coverage (Jon Atack)

Pull request description:

  - Add functional test coverage for rpc addpeeraddress
  - Simplify addpeeraddress and improve code constness

ACKs for top commit:
  klementtan:
    ACK [`b36e0cd`](b36e0cd1b9)
  MarcoFalke:
    review ACK b36e0cd1b9d361ac6f9777c09328a13e9ee923be 💭

Tree-SHA512: 01773fb70f23db5abf46806bb27804e48feff27272b2e6582bd5b886e9715088eb2d84755106bce2ad6f88e21582f7f071a30a89d5b17286d899c3dd8553b4fc
2024-05-19 11:13:42 -05:00
MarcoFalke
1ee01c801e
Merge bitcoin/bitcoin#22120: test: p2p_invalid_block: Check that a block rejected due to too-new tim…
754e802274e9373ad7e1dccb710acf74ded6e7fb test: check rejected future block later accepted (Luke Dashjr)

Pull request description:

  (Luke) was unsure if the code sufficiently avoided caching a
  time-too-new rejection, so wrote this test to check it.  It looks like
  despite only exempting BLOCK_MUTATED, it is still okay because header
  failures never cache block invalidity.  This test will help ensure that
  if this ever changes, BLOCK_TIME_FUTURE gets excluded at the same time.

  This PR re-opens https://github.com/bitcoin/bitcoin/pull/17872 which went stale and addresses the nits raised by reviewers there.

ACKs for top commit:
  MarcoFalke:
    review ACK 754e802274e9373ad7e1dccb710acf74ded6e7fb

Tree-SHA512: a2bbc8fffb523cf2831e1ecb05f20868e30106a38cc2e369e4973fa549cca06675a668df16f76c49cc4ce3a22925404255e5c53c4232d63ba1b9fca878509aa0
2024-05-19 11:11:34 -05:00
pasta
1fc62c81a0
Merge #6020: backport: bitcoin#21053, #22082, #22118, #22292, #22308, #22334, #22358, #22388, bitcoin-core/gui#271, #311
0bed7b4702 Merge bitcoin/bitcoin#22292: bench, doc: benchmarking updates and fixups (fanquake)
c95df68637 Merge bitcoin/bitcoin#22388: ci: use Ubuntu 20.04 as the default Docker container (MarcoFalke)
c586ca5b56 Merge bitcoin/bitcoin#22334: wallet: do not spam about non-existent spk managers (fanquake)
62f9394374 Merge bitcoin/bitcoin#22308: wallet: Add missing BlockUntilSyncedToCurrentChain (MarcoFalke)
240d8efb82 Merge bitcoin/bitcoin#22358: Remove unused wallet pointer from wallet signals (fanquake)
9a1500ab47 Merge bitcoin/bitcoin#22118: test: check anchors.dat when node starts for the first time (MarcoFalke)
262c8b6f44 Merge bitcoin/bitcoin#22082: test: update nanobench from release 4.0.0 to 4.3.4 (MarcoFalke)
3d2cea667b Merge bitcoin-core/gui#311: Peers Window rename 'Peer id' to 'Peer' (Hennadii Stepanov)
dc498be3be Merge bitcoin-core/gui#271: Don't clear console prompt when font resizing (W. J. van der Laan)
1ed2d2d891 Merge #21053: rpc, test: document {previous,next}blockhash as optional (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  Regular backports from bitcoin v22

  ## What was done?
   - bitcoin/bitcoin#21053
   - bitcoin-core/gui#271
   - bitcoin-core/gui#311
   - bitcoin/bitcoin#22082
   - bitcoin/bitcoin#22118
   - bitcoin/bitcoin#22358
   - bitcoin/bitcoin#22308
   - bitcoin/bitcoin#22334
   - bitcoin/bitcoin#22388
   - bitcoin/bitcoin#22292

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

Tree-SHA512: 8354297857516ddf94b242f2e50e1a28d999e613da2a7eb90c603e1fee7212e46d6e8a20ad42aa2945b48137e98fc7f589c9c77469c71cc01d032a33fa6da517
2024-05-18 21:08:06 -05:00
MarcoFalke
38649da984
Merge bitcoin/bitcoin#22520: test: improve rpc_blockchain.py tests and assert on time and mediantime
ef5e9304cd407adab1563f24215da1b582274c20 test: update logging and docstring in rpc_blockchain.py (Jon Atack)
d548dc71e4849f638fccaea6be86ac4fa5304f01 test: replace magic values by constants in rpc_blockchain.py (Jon Atack)
78c361086fc0bf27612e8142bd33e05e37a36af6 test: assert on mediantime in getblockheader and getblockchaininfo (Jon Atack)
0a9129c588ab016eb0453b40a0cae918ca4aa6a2 test: assert on the value of getblockchaininfo#time (Jon Atack)

Pull request description:

  Follow-up to #22407 improving test coverage per https://github.com/bitcoin/bitcoin/pull/22407#pullrequestreview-702077013.

ACKs for top commit:
  tryphe:
    untested ACK ef5e9304cd407adab1563f24215da1b582274c20

Tree-SHA512: f746d56f430331bc6a2ea7ecd27b21b06275927966aacf1f1127d8d5fdfd930583cabe72e23df3adb2e005da904fc05dc573b8e5eaa2f86e0e193b89a17a5734
2024-05-18 17:54:16 -05:00
MarcoFalke
61f9d96f38
Merge bitcoin/bitcoin#22423: test: wallet_listtransactions improvements (speedup, cleanup, logging)
a006d7d73019b8cf4d68626c019c3d69729dda69 test: add logging to wallet_listtransactions (Sebastian Falbesoner)
47915b118720c6e2b2ec9f599f25848041b42b99 test: remove unneeded/redundant code in wallet_listtransactions (Sebastian Falbesoner)
fb6c6a7938cb7c4808ad88d23bfc2b7408407b12 test: speedup wallet_listtransactions by whitelisting peers (immediate tx relay) (Sebastian Falbesoner)

Pull request description:

  This PR improves the test `wallet_listtransactions.py` in three ways:
  * speeds up runtime by a factor of 2-3x by using the good ol' immediate tx relay trick (`-whitelist=noban@127.0.0.1`)
  * removes unneeded/redundant code
  * adds log messages, mostly by turning comments into `self.log.info(...)` calls

ACKs for top commit:
  jonatack:
    ACK a006d7d73019b8cf4d68626c019c3d69729dda69
  kristapsk:
    ACK a006d7d73019b8cf4d68626c019c3d69729dda69

Tree-SHA512: a91a19f5ebc4d05f0b96c5419683c4c57ac0ef44b64eeb8dd550bd72296fd3a2857a3ba83f755fe4b0b3bd06439973f226070b5d0ce2dee58344dae78cb50290
2024-05-18 17:54:16 -05:00
Konstantin Akimov
e0ad143e08
chore: dashify file list exception for liner 2024-05-16 02:10:16 +07:00
Konstantin Akimov
98a2dad78c
fix: wrong permission for various files accordingly new linter 2024-05-16 02:09:48 +07:00
MacroFake
c8c58a1810
Merge bitcoin/bitcoin#25015: test: Use permissions from git in lint-files.py
908fb7e2ec37fe68675d38dbfee4df9f861bb2b5 test: Use permissions from git in `lint-files.py` (laanwj)
48d2e80a7479a44b0ab09e87542c8cb7a8f72223 test: Don't use shell=True in `lint-files.py` (laanwj)

Pull request description:

  Improvements to the `lint-files.py` script:

  - Avoid use of `shell=True`.
  - Check the permissions in git's metadata instead of in the filesystem. This stops the umask or filesystem from interfering. It's also more efficient as it only needs a single call to `git ls-files`.

  (what triggered this change was `File "..." contains a shebang line, but has the file permission 775 instead of the expected executable permission 755.` errors running the script locally).

ACKs for top commit:
  vincenzopalazzo:
    re-tACK 908fb7e2ec

Tree-SHA512: 2eaf868c55a9c3508b12658a5b3ac429963fd0551e645332d0ac54be56fefccee95115e4667386df24b46b545593cb0d0bf8c6cecab73f9cb19d37ddf704c614
2024-05-16 02:09:38 +07:00
MarcoFalke
f226e8dc1f
Merge bitcoin/bitcoin#24762: lint: Start to use py lint scripts
fae211c0ae0dd90876a3390eb21449b7b0bb45c4 lint: Start to use py lint scripts (MarcoFalke)
fa82e890e7950fe5ba6d4fa88fcd922cc929dc47 Move lint script and data file to avoid lint- prefix (MarcoFalke)

Pull request description:

ACKs for top commit:
  fjahr:
    tACK fae211c0ae0dd90876a3390eb21449b7b0bb45c4

Tree-SHA512: f8272a1bab9efb8203cac121710baae68f01f79e520ad71ff15aa516d19763d61c088b411b019de105a6a30e7ee3c274814d59963f6ac22ba1084560fb601f45
2024-05-16 02:09:38 +07:00
MarcoFalke
85013e99d6
Merge bitcoin/bitcoin#21873: test: minor fixes & improvements for files linter test
2227fc4e6203064b14e99bcf453601bd263a0196 test: minor fixes & improvements for files linter test (windsok)

Pull request description:

  Couple of minor fixes & improvements for files linter test added in #21740

  - Use a context manager when opening files, so that files are closed are we are done with them

  - Use the `-z` flag when shelling out to `git ls-files` so that we can catch newlines and other weird control characters in filenames.

  From the `git ls-files` manpage:
  ```
  -z \0 line termination on output and do not quote filenames. See OUTPUT below for more information.

  Without the -z option, pathnames with "unusual" characters are quoted as explained for the configuration variable
  core.quotePath (see git-config(1)). Using -z the filename is output verbatim and the line is terminated by a NUL byte.
  ```

ACKs for top commit:
  MarcoFalke:
    cr ACK 2227fc4e6203064b14e99bcf453601bd263a0196
  practicalswift:
    cr ACK 2227fc4e6203064b14e99bcf453601bd263a0196: patch looks correct

Tree-SHA512: af059a805f4a7614162de85dea856052a45ab531895cb0431087e7fc9e037513fa7501bb5eb2fe43238adf5f09e77712ebdbb15b1486983359ad3661a3da0c60
2024-05-16 02:09:38 +07:00
W. J. van der Laan
dce79f5c8e
Merge bitcoin/bitcoin#21740: test: add new python linter to check file names and permissions
46b025e00df40724175735eb5606ac73067cb3b8 test: add new python linter to check file names and permissions (windsok)
6f6bb3ebc7cb8e17a5dfc8ef55aa2d3f2dc6bdea test: fix file permissions on various scripts (windsok)

Pull request description:

  Adds a new python linter test which tests for correct filenames and file permissions in the repository.

  Replaces the existing tests in the `test/lint/lint-filenames.sh` and `test/lint/lint-shebang.sh` linter tests, as well as adding some new and increased testing. This increased coverage is intended to catch issues such as in #21728 and https://github.com/bitcoin/bitcoin/pull/16807/files#r345547050

  Summary of tests:
  * Checks every file in the repository against an allowed regexp to make sure only lowercase or uppercase alphanumerics (a-zA-Z0-9), underscores (_), hyphens (-), at (@) and dots (.) are used in repository filenames.

  * Checks only source files (*.cpp, *.h, *.py, *.sh) against a stricter allowed regexp to make sure only lowercase alphanumerics (a-z0-9), underscores (_), hyphens (-) and dots (.) are used in source code filenames. Additionally there is an exception regexp for directories or files which are excepted from matching this regexp (This should replicate the existing `test/lint/lint-filenames.sh` test)

  * Checks all files in the repository match an allowed executable or non-executable file permission octal. Additionally checks that for executable files, the file contains a shebang line.

  * Checks that for executable `.py` and `.sh` files, the shebang line used matches an allowable list of shebangs (This should replicate the existing `test/lint/lint-shebang.sh` test)

  * Checks every file that contains a shebang line to ensure it has an executable permission

  Additionally updates the permissions on various files to comply with the new tests.

  Fixes #21729

ACKs for top commit:
  practicalswift:
    cr re-ACK 46b025e00df40724175735eb5606ac73067cb3b8: patch still looks correct
  kiminuo:
    code review ACK 46b025e00df40724175735eb5606ac73067cb3b8 if `contrib/gitian-descriptors/assign_DISTNAME` permission change is deemed OK.
  laanwj:
    Code review ACK 46b025e00df40724175735eb5606ac73067cb3b8

Tree-SHA512: 1c8201a2cee0d9cbce15652b68cec9a6458a8b493fcd5392f98560aca0b1a12e668baab65a47100f116f626dadc3f591deb47f7368468c6a46c6c712c2533455
2024-05-16 02:09:37 +07:00
MarcoFalke
62f9394374
Merge bitcoin/bitcoin#22308: wallet: Add missing BlockUntilSyncedToCurrentChain
fa27baa9c8a13239625e5a7b6c472d236fe5b9fa Revert "test: Add temporary logging to debug #20975" (MarcoFalke)
fadb55085a02c9e355617bcb5f84b6335e4f8c9d wallet: Add missing BlockUntilSyncedToCurrentChain (MarcoFalke)

Pull request description:

  Fixes #20975

  Also replace the wallet pointer by a reference

ACKs for top commit:
  achow101:
    ACK fa27baa9c8a13239625e5a7b6c472d236fe5b9fa

Tree-SHA512: 79047a30998104a12c2ff84a8e3cc5207151410bbe92b74cfedbe1c1aca3ffa5909391607fc597f3a3cf0725fa827528a4c57edaeacc8360536b1965e166be6a
2024-05-15 03:03:17 +07:00
MarcoFalke
9a1500ab47
Merge bitcoin/bitcoin#22118: test: check anchors.dat when node starts for the first time
ef99d03c2bbb6b5fa5ff3d3d3cb9c5da7d471133 test: check anchors.dat when node starts for the first time (bruno)

Pull request description:

  See https://github.com/bitcoin/bitcoin/pull/21338#discussion_r598406712, https://github.com/bitcoin/bitcoin/pull/21338#discussion_r598406187, https://github.com/bitcoin/bitcoin/pull/21338#discussion_r598405613.

ACKs for top commit:
  laanwj:
    Code review ACK ef99d03c2bbb6b5fa5ff3d3d3cb9c5da7d471133

Tree-SHA512: 505f1f34fbc0c72a92968883be0f1c5f169a4ba3aa8a56e1ce8bc5e514f49e3a17ce51fd40be0073dc4bc06eaeda36dfe90ace843c181170e34643226afd78ef
2024-05-15 03:03:17 +07:00
MarcoFalke
1ed2d2d891
Merge #21053: rpc, test: document {previous,next}blockhash as optional
ba7e17e073f833eccd4c7c111ae9058c3f123371 rpc, test: document {previous,next}blockhash as optional (Sebastian Falbesoner)

Pull request description:

  This PR updates the result help of the following RPCs w.r.t. the `previousblockhash` and `nextblockhash` fields:
  - getblockheader
  - getblock

  Also adds trivial tests on genesis block (should not contain "previousblockhash") and best block (should not contain "nextblockhash").

Top commit has no ACKs.

Tree-SHA512: ef42c5c773fc436e1b4a67be14e2532e800e1e30e45e54a57431c6abb714d2c069c70d40ea4012d549293b823a1973b3f569484b3273679683b28ed40abf46bb
2024-05-15 03:02:45 +07:00
pasta
146be9f0d8
Merge #6003: feat: support rpc protx-register for descriptor wallets - part VI
a33dcb3283 fix: CheckWalletOwnsScript/CheckWalletOwnsKey to use wallet instead of SPK (Konstantin Akimov)
b2ede8bfee feat: update list of tests that still doesn't support descriptor wallets (Konstantin Akimov)
838d06f2fd feat: enable descriptor wallets for more tests (Konstantin Akimov)
5ab108c982 feat: implementation of RPC 'protx register' for descriptor wallets (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  Many rpc such as `protx register` 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)
  ```
  for all functional tests that uses Masternodes/evo nodes.

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

  ## What was done?
  Some direct usages of LegacyScriptPubKeyMan refactored to use CWallet's functionality.
  There are still 4 functional tests that doesn't work for descriptor wallets:
   - feature_dip3_deterministicmns.py (no rpc `protx updateregistar`)
   - feature_governance.py: no rpc for `governance votemany` and `governance votealias`
   - interface_zmq_dash.py (see governance)

  That's part I of changes, other changes are not PR-ready yet, WIP.

  ## How Has This Been Tested?
  Firstly, the flag `--legacy-wallets` are removed for many functional tests.
  Secondly, the flag `--descriptors` is inverted in default value:
  ```
  diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
  index 585a6a74d6..9ad5fd1daa 100755
  --- 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
               else:
                   # If neither are compiled, tests requiring a wallet will be skipped and the value of self.options.descriptors won't matter
                   # It still needs to exist and be None in order for tests to work however.
  ```

  ## Breaking Changes
  N/A, descriptor wallets have not been publicly 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
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  PastaPastaPasta:
    utACK a33dcb3283

Tree-SHA512: 24e22ac91e30a3804d1ac9af864eb9d073208bbb6d1f3326c7f0438f3ccce2b553aa46450989e48cb5c6e0d838ff1c88c6522f195e7aa2bd89342710f3ecef77
2024-05-14 09:17:00 -05:00
Samuel Dobson
ad25d54300
Merge bitcoin/bitcoin#21329: descriptor wallet: Cache last hardened xpub and use in normalized descriptors
e6cf0ed92de31a5ac35a271b0da8f0a8364d1175 wallet, rpc: listdescriptors does not need unlocked (Andrew Chow)
3280704886b60644d103a5eb310691c003a39328 Pass in DescriptorCache to ToNormalizedString (Andrew Chow)
7a26ff10c2f2e139fbc63e2f37fb33ea4efae088 Change DescriptorImpl::ToStringHelper to use an enum (Andrew Chow)
75530c93a83f3e94bcb78b6aa463c5570c1e737e Remove priv option for ToNormalizedString (Andrew Chow)
74fede3b8ba69e2cc82c617cdf406ab79df58825 wallet: Upgrade existing descriptor caches (Andrew Chow)
432ba9e5434da90d2cf680f23e8c7b7164c9f945 wallet: Store last hardened xpub cache (Andrew Chow)
d87b544b834077f102724415e0fada6ee8b2def2 descriptors: Cache last hardened xpub (Andrew Chow)
cacc3910989c4f3d7afa530dbab042461426abce Move DescriptorCache writing to WalletBatch (Andrew Chow)
0b4c8ef75cd03c8f0a8cfadb47e0fbcabe3c5e59 Refactor Cache merging and writing (Andrew Chow)
976b53b085d681645fd3a008fe382de85647e29f Revert "Cache parent xpub inside of BIP32PubkeyProvider" (Andrew Chow)

Pull request description:

  Currently fetching a normalized descriptor requires the wallet to be unlocked as it needs the private keys to derive the last hardened xpub. This is not very user friendly as normalized descriptors shouldn't require and don't involve the private keys except for derivation. We solve this problem by caching the last hardened xpub (which has to be derived at some point when generating the address pool).

  However the last hardened xpub was not already being cached. We only cached the immediate parent xpub and derived child keys. For example, with a descriptor derivation path of `/84'/0'/0'/0/*`, the parent xpub that is cached is `m/84'/0'/0'/0`, and the child keys of `m/84'/0'/0'/0/i` (note that child keys would not be cached in this case). This parent xpub is not suitable for the normalized descriptor form as we want the key at `m/84'/0'/0'`. So this PR adds another field to `DescriptorCache` to cache the last hardened xpub so that we can use them for normalized descriptors.

  Since `DescriptorCache` is changing, existing descriptor wallets need to be upgraded to use this new cache. The upgrade will occur in the background either at loading time (if the wallet is not encrypted) or at unlocking time in the same manner that `UpgradeKeyMetadata` operates. It will use a new wallet flag `WALLET_FLAG_LAST_HARDENED_XPUB_CACHED` to indicate whether the descriptor wallet has the last hardened xpub cache.

  Lastly `listdescriptors` will not require the wallet to be locked and `getaddressinfo`'s `parent_desc` will always be output (assuming the upgrade has occurred).

ACKs for top commit:
  fjahr:
    tACK e6cf0ed92de31a5ac35a271b0da8f0a8364d1175
  S3RK:
    reACK e6cf0ed
  jonatack:
    Semi ACK e6cf0ed92de31a5ac35a271b0da8f0a8364d1175 reviewed, debug-built and ran unit tests and some of the descriptor functional tests at each commit. I'm not very familiar with this code and it could be clearer to the uninitiated IMHO, so I'm not confident enough to give a full ACK. Various minor suggestions follow, most of them for readability, feel free to pick and choose.
  meshcollider:
    Code review + functional test run ACK e6cf0ed92de31a5ac35a271b0da8f0a8364d1175

Tree-SHA512: ac27aade8644525cd65bfcaf27ff32afb974085b1451faf4ff68c6671a690bd6a41d4f39a33cbf461ae0fbe85995c0a4c08dbd36171da1c1d2a1d00053ad298d
2024-05-10 14:02:01 +07:00
MarcoFalke
19b2b27785
Merge #20403: wallet: upgradewallet fixes, improvements, test coverage
3eb6f8b2e61c24a22ea9396d86672307845f35eb wallet (not for backport): improve upgradewallet error messages (Jon Atack)
ca8cd893bb56bf5d455154b0498b1f58f77d20ed wallet: fix and improve upgradewallet error responses (Jon Atack)
99d56e357159c7154f69f28cb5587c5ca20d6594 wallet: fix and improve upgradewallet result responses (Jon Atack)
2498b04ce88696a3216fc38b7d393906b733e8b1 Don't upgrade to HD split if it is already supported (Andrew Chow)
c46c18b788cb0862aafbb116fd37936cbed6a431 wallet: refactor GetClosestWalletFeature() (Jon Atack)

Pull request description:

  This follows up on #18836 and #20282 to fix and improve the as-yet unreleased `upgradewallet` feature and also implement review follow-up in https://github.com/bitcoin/bitcoin/pull/18836#discussion_r519328607.

  This PR fixes 4 upgradewallet issues:

  - this bug: https://github.com/bitcoin/bitcoin/pull/20403#discussion_r526063920
  - it returns nothing in the absence of an RPC error, which isn't reassuring for users
  - it returns the same thing both in the case of a successful upgrade and when no upgrade took place
  - the error message object is currently dead code

  This PR fixes the above and provides:

  ...user feedback to not silently return without upgrading
  ```
  {
    "wallet_name": "disable private keys",
    "previous_version": 169900,
    "current_version": 169900,
    "result": "Already at latest version. Wallet version unchanged."
  }
  ```
  ...better feedback after successfully upgrading
  ```
  {
    "wallet_name": "watch-only",
    "previous_version": 159900,
    "current_version": 169900,
    "result": "Wallet upgraded successfully from version 159900 to version 169900."
  }
  ```
  ...helpful error responses
  ```
  {
    "wallet_name": "blank",
    "previous_version": 169900,
    "current_version": 169900,
    "error": "Cannot downgrade wallet from version 169900 to version 159900. Wallet version unchanged."
  }
  {
    "wallet_name": "blank",
    "previous_version": 130000,
    "current_version": 130000,
    "error": "Cannot upgrade a non HD split wallet from version 130000 to version 169899 without upgrading to support pre-split keypool. Please use version 169900 or no version specified."
  }
  ```
  updated help:
  ```
  upgradewallet ( version )

  Upgrade the wallet. Upgrades to the latest version if no version number is specified.
  New keys may be generated and a new wallet backup will need to be made.
  Arguments:
  1. version    (numeric, optional, default=169900) The version number to upgrade to. Default is the latest wallet version.

  Result:
  {                            (json object)
    "wallet_name" : "str",     (string) Name of wallet this operation was performed on
    "previous_version" : n,    (numeric) Version of wallet before this operation
    "current_version" : n,     (numeric) Version of wallet after this operation
    "result" : "str",          (string, optional) Description of result, if no error
    "error" : "str"            (string, optional) Error message (if there is one)
  }
  ```

ACKs for top commit:
  achow101:
    ACK  3eb6f8b
  MarcoFalke:
    review ACK 3eb6f8b2e61c24a22ea9396d86672307845f35eb 🛡

Tree-SHA512: b767314069e26b5933b123acfea6aa40708507f504bdb22884da020a4ca1332af38a7072b061e36281533af9f4e236d94d3c129daf6fe5b55241127537038eed
2024-05-10 13:59:59 +07:00
Andrew Chow
708586c77e
Merge #18836: wallet: upgradewallet fixes and additional tests
5f9c0b6360215636cfa62a70d3a70f1feb3977ab wallet: Remove -upgradewallet from dummywallet (MarcoFalke)
a314271f08215feba53ead27096ac7fda34acb3c test: Remove unused wallet.dat (MarcoFalke)
bf7635963c03203e7189ddaa56c6b086a0108cbf tests: Test specific upgradewallet scenarios and that upgrades work (Andrew Chow)
4b418a9decc3e855ee4b0bbf9e61121c8e9904e5 test: Add test_framework/bdb.py module for inspecting bdb files (Andrew Chow)
092fc434854f881330771a93a1280ac67b1d3549 tests: Add a sha256sum_file function to util (Andrew Chow)
0bd995aa19be65b0dd23df1df571c71428c2bc32 wallet: upgrade the CHDChain version number when upgrading to split hd (Andrew Chow)
8e32e1c41c995e832e643f605d35a7aa112837e6 wallet: remove nWalletMaxVersion (Andrew Chow)
bd7398cc6258c258e9f4411c50630ec4a552341b wallet: have ScriptPubKeyMan::Upgrade check against the new version (Andrew Chow)
5f720544f34dedf75b063b962845fa8eca604514 wallet: Add GetClosestWalletFeature function (Andrew Chow)
842ae3842df489f1b8d68e67a234788966218184 wallet: Add utility method for CanSupportFeature (Andrew Chow)

Pull request description:

  This PR cleans up the wallet upgrade mechanism a bit, fixes some probably bugs, and adds more test cases.

  The `nWalletMaxVersion` member variable has been removed as it made `CanSupportFeature` unintuitive and was causing a couple of bugs. The reason this was introduced originally was to allow a wallet upgrade to only occur when the new feature is first used. While this makes sense for the old `-upgradewallet` option, for an RPC, this does not quite make sense. It's more intuitive for an upgrade to occur if possible if the `upgradewallet` RPC is used as that's an explicit request to upgrade a particular wallet to a newer version. `nWalletMaxVersion` was only relevant for upgrades to `FEATURE_WALLETCRYPT` and `FEATURE_COMPRPUBKEY` both of which are incredibly old features. So for such wallets, the behavior of `upgradewallet` will be that the feature is enabled immediately without the wallet needing to be encrypted at that time (note that `FEATURE_WALLETCRYPT` indicates support for encryption, not that the wallet is encrypted) or for a new key to be generated.

  `CanSupportFeature` would previously indicate whether we could upgrade to `nWalletMaxVersion` not just whether the current wallet version supported a feature. While this property was being used to determine whether we should upgrade to HD and HD chain split, it was also causing a few bugs. Determining whether we should upgrade to HD or HD chain split is resolved by passing into `ScriptPubKeyMan::Upgrade` the version we are upgrading to and checking against that. By removing `nWalletMaxVersion` we also fix a bug where you could upgrade to HD chain split without the pre-split keypool.

  `nWalletMaxVersion` was also the version that was being reported by `getwalletinfo` which meant that the version reported was not always consistent across restarts as it depended on whether `upgradewallet` was used. Additionally to make the wallet versions consistent with actually supported versions, instead of just setting the wallet version to whatever is given to `upgradewallet`, we normalize the version number to the closest supported version number. For example, if given 150000, we would store and report 139900.

  Another bug where CHDChain was not being upgraded to the version supporting HD chain split is also fixed by this PR.

  Lastly several more tests have been added. Some refactoring to the test was made to make these tests easier. These tests check specific upgrading scenarios, such as from non-HD (version 60000) to HD to pre-split keypool. Although not specifically related to `upgradewallet`, `UpgradeKeyMetadata` is now being tested too.

  Part of the new tests is checking that the wallet files are identical before and after failed upgrades. To facilitate this, a utility function `sha256sum_file` has been added. Another part of the tests is to examine the wallet file itself to ensure that the records in the wallet.dat file have been correctly modified. So a new `bdb.py` module has been added to deserialize the BDB db of the wallet.dat file. This format isn't explicitly documented anywhere, but the code and comments in BDB's source code in file `dbinc/db_page.h` describe it. This module just dumps all of the fields into a dict.

ACKs for top commit:
  MarcoFalke:
    approach ACK 5f9c0b6360
  laanwj:
    Code review ACK 5f9c0b6360215636cfa62a70d3a70f1feb3977ab
  jonatack:
    ACK 5f9c0b6360215636cfa62a70d3a70f1feb3977ab, approach seems fine, code review, only skimmed the test changes but they look well done, rebased on current master, debug built and verified the `wallet_upgradewallet.py` test runs green both before and after running `test/get_previous_releases.py -b v0.19.1 v0.18.1 v0.17.2 v0.16.3 v0.15.2`

Tree-SHA512: 7c4ebf420850d596a586cb6dd7f2ef39c6477847d12d105fcd362abb07f2a8aa4f7afc5bfd36cbc8b8c72fcdd1de8d2d3f16ad8e8ba736b6f4f31f133fe5feba
2024-05-10 13:59:59 +07:00
Konstantin Akimov
266aefc544
feat: sethdseed rpc added. Based on bitcoin#12560 and the newest related changes
The key difference between bitcoin's and dash's implementation that sethdseed
does not update existing seed for wallet. Seed can be set only once.
It behave similarly to `upgradetohd` rpc, but since v20.1 all wallets are HD and
the name `upgradetohd` is not relevant more.
2024-05-10 13:59:44 +07:00
Konstantin Akimov
b2ede8bfee
feat: update list of tests that still doesn't support descriptor wallets
That are:
 - feature_dip3_deterministicmns.py
 - interface_zmq_dash.py
 - feature_governance.py
 - wallet_upgradetohd.py (as expected to be implemented for legacy-only wallets)
 - p2p_timeouts.py (why? can not understand it)

This partially reverts commit b20f812674.
2024-05-07 00:17:18 +07:00
Konstantin Akimov
838d06f2fd
feat: enable descriptor wallets for more tests
Enables for rpc_quorum.py, feature_notifications.py

see #5981, it partial revert of b20f812674
2024-05-07 00:17:18 +07:00
Kittywhiskers Van Gogh
33098aefff
merge bitcoin#21160: Move tx inventory into net_processing 2024-04-26 20:25:55 +00:00
Kittywhiskers Van Gogh
24205d94fe
partial bitcoin#20196: fix GetListenPort() to derive the proper port
excludes:
- 0cfc0cd32239d3c08d2121e028b297022450b320
- 7d64ea4a01920bb55bc6de0de6766712ec792a11
2024-04-26 20:25:31 +00:00
Kittywhiskers Van Gogh
6f8c730f35
merge bitcoin#19499: Make timeout mockable and type safe, speed up test 2024-04-26 20:25:31 +00:00
Kittywhiskers Van Gogh
b0216ac8a6
refactor: remove fMasternodeMode usage in rpc logic 2024-04-24 18:46:34 +00:00
pasta
27b2e5cd5d
Merge #5985: backport: bitcoin#21373, #21377, #21681, #21714, #21749, #21792, #21814, #21874, #21884, bitcoin-core/gui#284
7cc77f3a30 Merge #21373: test: generate fewer blocks in feature_nulldummy to fix timeouts, speed up (MarcoFalke)
a933a60b1a feat: new command line argument -bip147height for bitcoin#21373 (Konstantin Akimov)
51911388f2 Merge #21377: Speedy trial support for versionbits (fanquake)
ecade9bc39 Merge bitcoin/bitcoin#21749: test: Bump shellcheck version (W. J. van der Laan)
eeec2f2799 Merge bitcoin/bitcoin#21884: fuzz: Remove unused --enable-danger-fuzz-link-all option (fanquake)
51633d70ea Merge bitcoin/bitcoin#21874: fuzz: Add WRITE_ALL_FUZZ_TARGETS_AND_ABORT (MarcoFalke)
a02a2c0322 Merge bitcoin/bitcoin#21681: validation: fix ActivateSnapshot to use hardcoded nChainTx (MarcoFalke)
71f23d6e33 Merge bitcoin/bitcoin#21814: test: Fix feature_config_args.py intermittent issue (MarcoFalke)
de4d2a839d Merge bitcoin/bitcoin#21714: refactor: Drop CCoinControl::SetNull (MarcoFalke)
a63f9c31cc Merge bitcoin-core/gui#284: refactor: Simplify SendCoinsDialog::updateCoinControlState (Hennadii Stepanov)
b2d889380c Merge bitcoin/bitcoin#21792: test: Fix intermittent issue in p2p_segwit.py (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  Regular batch of backports from bitcoin v22

  ## What was done?
  Implemented new commandline argument `-bip147height` for RegTest.
  Backports:

   - bitcoin/bitcoin#21377
   - bitcoin/bitcoin#21792
   - bitcoin-core/gui#284
   - bitcoin/bitcoin#21714
   - bitcoin/bitcoin#21373
   - bitcoin/bitcoin#21814
   - bitcoin/bitcoin#21681
   - bitcoin/bitcoin#21874
   - bitcoin/bitcoin#21884
   - bitcoin/bitcoin#21749

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

Tree-SHA512: d46218667430af19c445d67d6411b1e7f19920e85f86e6e74ae7b9062aeb3763637a8613587c203ba8d285ccc3ee755f936141010944cfae8627397e8b8584d3
2024-04-24 10:17:59 -05:00
Kittywhiskers Van Gogh
bfd33cd2b4
net: move CConnman::RelayInv{Filtered} into PeerManager 2024-04-23 16:08:10 +00:00
Kittywhiskers Van Gogh
313a7e9a50
trivial: cleanup unnecessary headers in context files 2024-04-23 16:06:41 +00:00
Kittywhiskers Van Gogh
c3f1ac2291
net: retire CConnman::RelayTransaction, use PeerManager::RelayTransaction 2024-04-23 16:06:41 +00:00