Commit Graph

25248 Commits

Author SHA1 Message Date
Wladimir J. van der Laan
b18351e415
Merge #20153: wallet: do not import a descriptor with hardened derivations into a watch-only wallet
538be4219ae7e65862e4aff540af88c9421e6061 wallet: fix importdescriptor silent fail (Ivan Metlushko)

Pull request description:

  Currently `importdescriptor` command will successfully import a descriptor with hardened derivations into a watch-only wallet while silently failing to expand the descriptor to fill the cache. This leads to a broken wallet state and failure to load such wallet due to missing cache on subsequent restart.

ACKs for top commit:
  laanwj:
    Code review ACK 538be4219ae7e65862e4aff540af88c9421e6061
  achow101:
    ACK 538be4219ae7e65862e4aff540af88c9421e6061
  meshcollider:
    utACK 538be4219ae7e65862e4aff540af88c9421e6061

Tree-SHA512: 4bdd0ab4437d55b3f1a79c3a300a0b186089155c020fe220a73d0cce274de47d90371d88918d39fd795f9fccf8db328f1e322d29a6062f9ce94a1c254398f004
2024-03-07 01:23:19 +07:00
Wladimir J. van der Laan
c995e5d957
Merge #20266: wallet: fix change detection of imported internal descriptors
bd93fc9945bfd4be117990c5d861f61ddd451f96 Fix change detection of imported internal descriptors (Andrew Chow)

Pull request description:

  Import internal descriptors were having address book entries added which meant they would be detected as non-change. Fix this and add a test for it.

ACKs for top commit:
  laanwj:
    Code review ACK bd93fc9945bfd4be117990c5d861f61ddd451f96
  meshcollider:
    utACK bd93fc9945bfd4be117990c5d861f61ddd451f96
  promag:
    Code review ACK bd93fc9945bfd4be117990c5d861f61ddd451f96.

Tree-SHA512: 8fa9e364be317627ec171eedffdb505976c0e7f1e55bc7e8cfdffa3aeea5db24d231f55166602cd0e97a5ba621acc871de0a765c75d0c65678f83e93c3b657c5
2024-03-07 01:23:18 +07:00
Samuel Dobson
c86458250c
Merge #18787: wallet: descriptor wallet release notes and cleanups
ca2a09640fe976b1e74a33d29d9381895e71b347 Change SetType to SetInternal and remove m_address_type (Andrew Chow)
89b1ce1140535b4c902a7c5999bed335b9ddfe7c Remove unimplemented SetCrypted from DescriptorScriptPubKeyMan (Andrew Chow)
b9073c8f13fb0ba94c2ec6365666343e19fd9ddf rpc: createwallet warning that descriptor wallets are experimental (Andrew Chow)
610030d95c60ea526440d801a98ac8bd370eac48 docs: Add release notes for descriptor wallets (Andrew Chow)

Pull request description:

  Some docs and cleanup following #16528.

  * Added release notes to explain a bit of motivation for descriptor wallets, what was changed, and how users will be effected by it. Also mentions the caveats regarding multsigs and watchonly that we have discussed on IRC.
  * Adds a warning to `createwallet` that descriptor wallets are experimental.
  * Removed unused `SetCrypted` as suggestioned: https://github.com/bitcoin/bitcoin/pull/16528#discussion_r415300916
  * Removed `m_address_type` as mentioned in https://github.com/bitcoin/bitcoin/pull/18782#issuecomment-620167077

ACKs for top commit:
  Sjors:
    tACK ca2a09640fe976b1e74a33d29d9381895e71b347
  instagibbs:
    utACK ca2a09640f
  meshcollider:
    utACK ca2a09640fe976b1e74a33d29d9381895e71b347

Tree-SHA512: 987188a912c191430e5d3f89bcef54ba6773692fc2d95b16a3ec11d9007ded210466ed980a3857e8b7196beef6422f07f9c85cc157f996c02d16f4dbde2e7b2a
2024-03-07 01:23:18 +07:00
Samuel Dobson
0949c08996
Merge #18782: wallet: Make sure no DescriptorScriptPubKeyMan or WalletDescriptor members are left uninitialized after construction
2a780980983f4b4aaae75817e57e7ed308713561 wallet: Make sure no WalletDescriptor members are uninitialized after construction (practicalswift)
ff046aeeba8d4f3ff210d37ba020616c12450ab3 wallet: Make sure no DescriptorScriptPubKeyMan members are uninitialized after construction (practicalswift)

Pull request description:

  This is a small folllow-up to #16528 ("Native Descriptor Wallets using DescriptorScriptPubKeyMan") which was merged in to `master` a couple of hours ago.

  Make sure no `DescriptorScriptPubKeyMan` or `WalletDescriptor` members are left uninitialized after construction.

  Before this change `bool m_internal` was left uninitialized when using the `DescriptorScriptPubKeyMan(WalletStorage&, WalletDescriptor&)` ctor.

  The same goes for the now initialized integers which were left uninitialized when using the `WalletDescriptor()` ctor.

ACKs for top commit:
  instagibbs:
    utACK  2a78098098
  fjahr:
    Code review ACK 2a780980983f4b4aaae75817e57e7ed308713561
  Sjors:
    utACK 2a78098
  achow101:
    ACK 2a780980983f4b4aaae75817e57e7ed308713561
  brakmic:
    Code review ACK 2a780980983f4b4aaae75817e57e7ed308713561
  meshcollider:
    utACK 2a780980983f4b4aaae75817e57e7ed308713561

Tree-SHA512: c98e035268fdc7f65a423b73ac0cf010b0ef7c5e679b3cf170c1813efac8ab5c657dcbaf43c746770bea59e4772bfefe4caa834f1175260c39c7f35d92946ba5
2024-03-07 01:23:18 +07:00
MarcoFalke
baa6959068
Merge #18805: tests: Add missing sync_all to wallet_importdescriptors.py
7b2b06dfe3061b5ab4a283245930e2f7773eb3ef tests: Add missing sync_all to wallet_importdescriptors.py (Andrew Chow)

Pull request description:

  node1 will sometimes do sendtoaddress before it has received a funding transaction which will cause the test to fail. sync_all to ensure it gets the transaction first.

  Fixes #18800

ACKs for top commit:
  instagibbs:
    ACK 7b2b06d The wallet endpoint right after is indeed node 1.

Tree-SHA512: b610a771d062b5f955cd70b34337577a1ab8dacbf4be20aa74e1e8234495b0be9faff138eb1713f29decb5574446e0583e221bc2c9a6eea13611b422ea3a296a
2024-03-07 01:23:17 +07:00
Samuel Dobson
76e08f9b3d
Merge #18027: "PSBT Operations" dialog
931dd4760855e036c176a23ec2de367c460e4243 Make lint-spelling.py happy (Glenn Willen)
11a0ffb29d1b4dcc55c8826873f340ab4196af21 [gui] Load PSBT from clipboard (Glenn Willen)
a6cb0b0c29d327d01aebb98b0504f317eb19c3dc [gui] PSBT Operations Dialog (sign & broadcast) (Glenn Willen)
5dd0c03ffa3aeaa69d8a3a716f902f450d5eaaec FillPSBT: report number of inputs signed (or would sign) (Glenn Willen)
9e7b23b73387600d175aff8bd5e6624dd51f86e7 Improve TransactionErrorString messages. (Glenn Willen)

Pull request description:

  Add a "PSBT Operations" dialog, reached from the "Load PSBT..." menu item, giving options to sign or broadcast the loaded PSBT as appropriate, as well as copying the result to the clipboard or saving it to a file.

  This is based on Sjors' #17509, and depends on that PR going in first. (It effectively replaces the small "load PSBT" dialog from that PR with a more feature-rich one.)

  Some notes:
  * The way I display status information is maybe unusual (a status bar, rather than messageboxes.) I think it's helpful to have the information in it be persistent rather than transitory. But if people dislike it, I would probably move the "current state of the transaction" info to the top line of the main label, and the "what action just happened, and did it succeed" info into a messagebox.
  * I don't really know much about the translation/localization stuff. I put tr() in all the places it seemed like it ought to go. I did not attempt to translate the result of TransactionErrorString (which is shared by GUI and non-GUI code); I don't know if that's correct, but it matches the "error messages in logs should be googleable in English" heuristic. I don't know whether there are things I should be doing to reduce translator effort (like minimizing the total number of distinct message strings I use, or something.)
  * I don't really know how (if?) automated testing is applied to GUI code. I can make a list of PSBTs exercising all the codepaths for manual testing, if that's the right approach. Input appreciated.

ACKs for top commit:
  instagibbs:
    tested ACK 931dd47608
  Sjors:
    re-tACK 931dd4760855e036c176a23ec2de367c460e4243
  jb55:
    ACK 931dd4760855e036c176a23ec2de367c460e4243
  achow101:
    ACK 931dd4760855e036c176a23ec2de367c460e4243

Tree-SHA512: ade52471a2242f839a8bd6a1fd231443cc4b43bb9c1de3fb5ace7c5eb59eca99b1f2e9f17dfdb4b08d84d91f5fd65677db1433dd03eef51c7774963ef4e2e74f
2024-03-07 01:23:17 +07:00
Konstantin Akimov
c1b94b6f52
fix: wallet should be unlocked before generating keys for Descriptor wallet 2024-03-07 01:23:17 +07:00
Andrew Chow
f293c046f4
Merge #16528: Native Descriptor Wallets using DescriptorScriptPubKeyMan
223588b1bbc63dc57098bbd0baa48635e0cc0b82 Add a --descriptors option to various tests (Andrew Chow)
869f7ab30aeb4d7fbd563c535b55467a8a0430cf tests: Add RPCOverloadWrapper which overloads some disabled RPCs (Andrew Chow)
cf060628590fab87d73f278e744d70ef2d5d81db Correctly check for default wallet (Andrew Chow)
886e0d75f5fea2421190aa4812777d89f68962cc Implement CWallet::IsSpentKey for non-LegacySPKMans (Andrew Chow)
3c19fdd2a2fd5394fcfa75b2ba84ab2277cbdabf Return error when no ScriptPubKeyMan is available for specified type (Andrew Chow)
388ba94231f2f10a0be751c562cdd4650510a90a Change wallet_encryption.py to use signmessage instead of dumpprivkey (Andrew Chow)
1346e14831489f9c8f53a08f9dfed61d55d53c6f Functional tests for descriptor wallets (Andrew Chow)
f193ea889ddb53d9a5c47647966681d525e38368 add importdescriptors RPC and tests for native descriptor wallets (Hugo Nguyen)
ce24a944940019185efebcc5d85eac458ed26016 Add IsLegacy to CWallet so that the GUI knows whether to show watchonly (Andrew Chow)
1cb42b22b11c27e64462afc25a94b2fc50bfa113 Generate new descriptors when encrypting (Andrew Chow)
82ae02b1656819f4bd5023b8955447e1d4ea8692 Be able to create new wallets with DescriptorScriptPubKeyMans as backing (Andrew Chow)
b713baa75a62335ab9c0eed9ef76a95bfec30668 Implement GetMetadata in DescriptorScriptPubKeyMan (Andrew Chow)
8b9603bd0b443e2f7984eb72bf2e21cf02af0bcb Change GetMetadata to use unique_ptr<CKeyMetadata> (Andrew Chow)
72a9540df96ffdb94f039b9c14eaacdc7d961196 Implement FillPSBT in DescriptorScriptPubKeyMan (Andrew Chow)
84b4978c02102171775c77a45f6ec198930f0a88 Implement SignMessage for descriptor wallets (Andrew Chow)
bde7c9fa38775a81d53ac0484fa9c98076a0c7d1 Implement SignTransaction in DescriptorScriptPubKeyMan (Andrew Chow)
d50c8ddd4190f20bf0debd410348b73408ec3143 Implement GetSolvingProvider for DescriptorScriptPubKeyMan (Andrew Chow)
f1ca5feb4ad668a3e1ae543d0addd5f483f1a88f Implement GetKeypoolOldestTime and only display it if greater than 0 (Andrew Chow)
586b57a9a6b4b12a78f792785b63a5a1743bce0c Implement ReturnDestination in DescriptorScriptPubKeyMan (Andrew Chow)
f866957979c23cefd41efa9dae9e53b9177818dc Implement GetReservedDestination in DescriptorScriptPubKeyMan (Andrew Chow)
a775f7c7fd0b9094fcbeee6ba92206d5bbb19164 Implement Unlock and Encrypt in DescriptorScriptPubKeyMan (Andrew Chow)
bfdd0734869a22217c15858d7a76d0dacc2ebc86 Implement GetNewDestination for DescriptorScriptPubKeyMan (Andrew Chow)
58c7651821b0eeff0a99dc61d78d2e9e07986580 Implement TopUp in DescriptorScriptPubKeyMan (Andrew Chow)
e014886a342508f7c8d80323eee9a5f314eaf94c Implement SetupGeneration for DescriptorScriptPubKeyMan (Andrew Chow)
46dfb99768e7d03a3cf552812d5b41ceaebc06be Implement writing descriptorkeys, descriptorckeys, and descriptors to wallet file (Andrew Chow)
4cb9b69be031e1dc65d8964794781b347fd948f5 Implement several simple functions in DescriptorScriptPubKeyMan (Andrew Chow)
d1ec3e4f19487b4b100f80ad02eac063c571777d Add IsSingleType to Descriptors (Andrew Chow)
953feb3d2724f5398dd48990c4957a19313d2c8c Implement loading of keys for DescriptorScriptPubKeyMan (Andrew Chow)
2363e9fcaa41b68bf11153f591b95f2d41ff9a1a Load the descriptor cache from the wallet file (Andrew Chow)
46c46aebb7943e1e2e96755e94dc6c197920bf75 Implement GetID for DescriptorScriptPubKeyMan (Andrew Chow)
ec2f9e1178c8e38c0a5ca063fe81adac8f916348 Implement IsHDEnabled in DescriptorScriptPubKeyMan (Andrew Chow)
741122d4c1a62ced3e96d16d67f4eeb3a6522d99 Implement MarkUnusedAddresses in DescriptorScriptPubKeyMan (Andrew Chow)
2db7ca765c8fb2c71dd6f7c4f29ad70e68ff1720 Implement IsMine for DescriptorScriptPubKeyMan (Andrew Chow)
db7177af8c159abbcc209f2caafcd45d54c181c5 Add LoadDescriptorScriptPubKeyMan and SetActiveScriptPubKeyMan to CWallet (Andrew Chow)
78f8a92910d34247fa5d04368338c598d9908267 Implement SetType in DescriptorScriptPubKeyMan (Andrew Chow)
834de0300cde57ca3f662fb7aa5b1bdaed68bc8f Store WalletDescriptor in DescriptorScriptPubKeyMan (Andrew Chow)
d8132669e10c1db9ae0c2ea0d3f822d7d2f01345 Add a lock cs_desc_man for DescriptorScriptPubKeyMan (Andrew Chow)
3194a7f88ac1a32997b390b4f188c4b6a4af04a5 Introduce WalletDescriptor class (Andrew Chow)
6b13cd3fa854dfaeb9e269bff3d67cacc0e5b5dc Create LegacyScriptPubKeyMan when not a descriptor wallet (Andrew Chow)
aeac157c9dc141546b45e06ba9c2e641ad86083f Return nullptr from GetLegacyScriptPubKeyMan if descriptor wallet (Andrew Chow)
96accc73f067c7c95946e9932645dd821ef67f63 Add WALLET_FLAG_DESCRIPTORS (Andrew Chow)
6b8119af53ee2fdb4c4b5b24b4e650c0dc3bd27c Introduce DescriptorScriptPubKeyMan as a dummy class (Andrew Chow)
06620302c713cae65ee8e4ff9302e4c88e2a1285 Introduce SetType function to tell ScriptPubKeyMans the type and internal-ness of it (Andrew Chow)

Pull request description:

  Introducing the wallet of the glorious future (again): native descriptor wallets. With native descriptor wallets, addresses are generated from descriptors. Instead of generating keys and deriving addresses from keys, addresses come from the scriptPubKeys produced by a descriptor. Native descriptor wallets will be optional for now and can only be created by using `createwallet`.

  Descriptor wallets will store descriptors, master keys from the descriptor, and descriptor cache entries. Keys are derived from descriptors on the fly. In order to allow choosing different address types, 6 descriptors are needed for normal use. There is a pair of primary and change descriptors for each of the 3 address types. With the default keypool size of 1000, each descriptor has 1000 scriptPubKeys and descriptor cache entries pregenerated. This has a side effect of making wallets large since 6000 pubkeys are written to the wallet by default, instead of the current 2000. scriptPubKeys are kept only in memory and are generated every time a descriptor is loaded. By default, we use the standard BIP 44, 49, 84 derivation paths with an external and internal derivation chain for each.

  Descriptors can also be imported with a new `importdescriptors` RPC.

  Native descriptor wallets use the `ScriptPubKeyMan` interface introduced in #16341 to add a `DescriptorScriptPubKeyMan`. This defines a different IsMine which uses the simpler model of "does this scriptPubKey exist in this wallet". Furthermore, `DescriptorScriptPubKeyMan` does not have watchonly, so with native descriptor wallets, it is not possible to have a wallet with both watchonly and non-watchonly things. Rather a wallet with `disable_private_keys` needs to be used for watchonly things.

  A `--descriptor` option was added to some tests (`wallet_basic.py`, `wallet_encryption.py`, `wallet_keypool.py`, `wallet_keypool_topup.py`, and `wallet_labels.py`) to allow for these tests to use descriptor wallets. Additionally, several RPCs are disabled for descriptor wallets (`importprivkey`, `importpubkey`, `importaddress`, `importmulti`, `addmultisigaddress`, `dumpprivkey`, `dumpwallet`, `importwallet`, and `sethdseed`).

ACKs for top commit:
  Sjors:
    utACK 223588b1bbc63dc57098bbd0baa48635e0cc0b82 (rebased, nits addressed)
  jonatack:
    Code review re-ACK 223588b1bbc63dc57098bbd0baa48635e0cc0b82.
  fjahr:
    re-ACK 223588b1bbc63dc57098bbd0baa48635e0cc0b82
  instagibbs:
    light re-ACK 223588b
  meshcollider:
    Code review ACK 223588b1bbc63dc57098bbd0baa48635e0cc0b82

Tree-SHA512: 59bc52aeddbb769ed5f420d5d240d8137847ac821b588eb616b34461253510c1717d6a70bab8765631738747336ae06f45ba39603ccd17f483843e5ed9a90986

Introduce SetType function to tell ScriptPubKeyMans the type and internal-ness of it

Introduce DescriptorScriptPubKeyMan as a dummy class

Add WALLET_FLAG_DESCRIPTORS

Return nullptr from GetLegacyScriptPubKeyMan if descriptor wallet

Create LegacyScriptPubKeyMan when not a descriptor wallet

Introduce WalletDescriptor class

WalletDescriptor is a Descriptor with other wallet metadata

Add a lock cs_desc_man for DescriptorScriptPubKeyMan

Store WalletDescriptor in DescriptorScriptPubKeyMan

Implement SetType in DescriptorScriptPubKeyMan

Add LoadDescriptorScriptPubKeyMan and SetActiveScriptPubKeyMan to CWallet

Implement IsMine for DescriptorScriptPubKeyMan

Adds a set of scriptPubKeys that DescriptorScriptPubKeyMan tracks.
If the given script is in that set, it is considered ISMINE_SPENDABLE

Implement MarkUnusedAddresses in DescriptorScriptPubKeyMan

Implement IsHDEnabled in DescriptorScriptPubKeyMan

Implement GetID for DescriptorScriptPubKeyMan

Load the descriptor cache from the wallet file

Implement loading of keys for DescriptorScriptPubKeyMan

Add IsSingleType to Descriptors

IsSingleType will return whether the descriptor will give one or multiple scriptPubKeys

Implement several simple functions in DescriptorScriptPubKeyMan

Implements a bunch of one liners: UpgradeKeyMetadata, IsFirstRun, HavePrivateKeys,
KeypoolCountExternalKeys, GetKeypoolSize, GetTimeFirstKey, CanGetAddresses,
RewriteDB

Implement writing descriptorkeys, descriptorckeys, and descriptors to wallet file

Implement SetupGeneration for DescriptorScriptPubKeyMan

Implement TopUp in DescriptorScriptPubKeyMan

Implement GetNewDestination for DescriptorScriptPubKeyMan

Implement Unlock and Encrypt in DescriptorScriptPubKeyMan

Implement GetReservedDestination in DescriptorScriptPubKeyMan

Implement ReturnDestination in DescriptorScriptPubKeyMan

Implement GetKeypoolOldestTime and only display it if greater than 0

Implement GetSolvingProvider for DescriptorScriptPubKeyMan

Internally, a GetSigningProvider function is introduced which allows for
some private keys to be optionally included. This can be called with a
script as the argument (i.e. a scriptPubKey from our wallet when we are
signing) or with a pubkey. In order to know what index to expand the
private keys for that pubkey, we need to also cache all of the pubkeys
involved when we expand the descriptor. So SetCache and TopUp are
updated to do this too.

Implement SignTransaction in DescriptorScriptPubKeyMan

Implement SignMessage for descriptor wallets

Implement FillPSBT in DescriptorScriptPubKeyMan

FillPSBT will add our own scripts to the PSBT if those inputs are ours.
If an input also lists pubkeys that we happen to know the private keys
for, we will sign those inputs too.

Change GetMetadata to use unique_ptr<CKeyMetadata>

Implement GetMetadata in DescriptorScriptPubKeyMan

Be able to create new wallets with DescriptorScriptPubKeyMans as backing

Generate new descriptors when encrypting

Add IsLegacy to CWallet so that the GUI knows whether to show watchonly

add importdescriptors RPC and tests for native descriptor wallets

Co-authored-by: Andrew Chow <achow101-github@achow101.com>

Functional tests for descriptor wallets

Change wallet_encryption.py to use signmessage instead of dumpprivkey

Return error when no ScriptPubKeyMan is available for specified type

When a CWallet doesn't have a ScriptPubKeyMan for the requested type
in GetNewDestination, give a meaningful error. Also handle this in
Qt which did not do anything with errors.

Implement CWallet::IsSpentKey for non-LegacySPKMans

tests: Add RPCOverloadWrapper which overloads some disabled RPCs

RPCOverloadWrapper overloads some deprecated or disabled RPCs with
an implementation using other RPCs to avoid having a ton of code churn
around replacing those RPCs.

Add a --descriptors option to various tests

Adds a --descriptors option globally to the test framework. This will
make the test create and use descriptor wallets. However some tests may
not work with this.

Some tests are modified to work with --descriptors and run with that
option in test_runer:
* wallet_basic.py
* wallet_encryption.py
* wallet_keypool.py <---- wallet_keypool_hd.py actually
* wallet_keypool_topup.py
* wallet_labels.py
* wallet_avoidreuse.py
2024-03-07 01:23:15 +07:00
Konstantin Akimov
4064334732
fix: get receiving address for Descriptor Wallets
Expected to use wallet's interface instead LegacyScriptPubKeyMan to get a new address
2024-03-07 01:22:37 +07:00
UdjinM6
bdbd0b14a7
chore: dashification of descriptor implementation in dash
It removes mentioning of wpkh
2024-03-07 01:22:37 +07:00
Konstantin Akimov
b02fc0b2ce
fix: counting calculation of internal keys for Descriptor Wallets
KeypoolCountInternalKeys is not working as expected if there is
more than one ScriptPubKeyManager
2024-03-07 01:22:37 +07:00
pasta
32d9e38157
Merge #5898: chore: bump cppcheck to 2.13 version
df554f396b test: add multiple suppression for cppcheck to make it finally quiet (Konstantin Akimov)
e2ac86b8f7 refactor: drop dependency of CJ to fee_estimator (Konstantin Akimov)
e7e355ba8b refactor: make SetNull in CJ classes virtual to prevent warning from compiler (Konstantin Akimov)
6bc14a35e5 chore: bump cpp check 2.10 to 2.13 (Konstantin Akimov)
b4ed65e15e refactor: add multiple missing `const` (Konstantin Akimov)
add99ea862 refactor: add more consts everywhere as required by cppcheck 2.13.0 (Konstantin Akimov)
61a6c407fe fix: re-order asserts in creditpool (Konstantin Akimov)
28f7ecc11f test: tidy up and reorder warnings in lint-cppcheck-dash (Konstantin Akimov)
3abeab16d3 refactor: replace multiple C-style casts to reinterpret_cast (Konstantin Akimov)
f36bb1f085 style: add semicolumn that the end to ENTER/LEAVE CRITICAL SECTION (Konstantin Akimov)
c23c25d00b test: supress any_of suggestions in cppcheck-dash (Konstantin Akimov)
7daa9bea9a refactor: use std::any_of (Konstantin Akimov)
3f1cb8d13d test: add suppression for state.Invalid() which is widely used by cppcheck is unhappy (Konstantin Akimov)

Pull request description:

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

  See also https://github.com/dashpay/dash/pull/5880 for prior work.
  There's too many false alarm warnings with new kubuntu 23.10 (cppcheck 2.11)

  ## What was done?
  Bump cppcheck to version 2.13 and fix related warnings or suppressing them if they are false-alarm.

  ## How Has This Been Tested?
  Run `test/lint/lint-cppcheck-dash.sh` and see no output.

  ## Breaking Changes
  N/A

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

Top commit has no ACKs.

Tree-SHA512: 352a36958102ee687a2f065691a5113f90a014816683335b11291a556ad09c7f8797c6ced98f0851f39f597c26493f2d5a2bf54c4e833a37d35487384b7102db
2024-03-06 12:07:33 -06:00
pasta
e84e3d9c85
Merge #5902: backport: merge bitcoin#27985, #27993, #28008, #28267, #28374, #28100 (p2p primitives)
b60c493265 merge bitcoin#28100: more Span<std::byte> modernization & follow-ups (Kittywhiskers Van Gogh)
c2aa01cf1d merge bitcoin#28374: python cryptography required for BIP 324 functional tests (Kittywhiskers Van Gogh)
7c5edf772a merge bitcoin#28267: BIP324 ciphersuite follow-up (Kittywhiskers Van Gogh)
1b1924e3d5 merge bitcoin#28008: BIP324 ciphersuite (Kittywhiskers Van Gogh)
ff542199bc merge bitcoin#27993: Make poly1305 support incremental computation + modernize (Kittywhiskers Van Gogh)
d7482eb8a6 merge bitcoin#27985: Add support for RFC8439 variant of ChaCha20 (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

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

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

  * Without modifications, tests introduced in [bitcoin#28008](https://github.com/bitcoin/bitcoin/pull/28008) will fail due to salt comprising of a fixed string (`bitcoin_v2_shared_secret`) and network bytes ([source](1c7582ead6/src/bip324.cpp (L39-L40))). Bitcoin uses [`{0xf9, 0xbe, 0xb4, 0xd9}`](1c7582ead6/src/kernel/chainparams.cpp (L114-L117)) for mainnet while Dash uses [`{0xbf, 0x0c, 0x6b, 0xbd}`](37f43e4e56/src/chainparams.cpp (L238-L241)).
    * The replacement parameters are generated by:
      * Cloning https://github.com/bitcoin/bips (as of this writing, at b3701faef2)
      * Editing `bip-0324/reference.py`
        * Changing `NETWORK_MAGIC` to Dash's network magic
      * Running `gen_test_vectors.py` to get a new `packet_encoding_test_vectors.csv`
      * Using [this python script](1c7582ead6/src/test/bip324_tests.cpp (L174-L196)) mentioned in a comment in `src/test/bip324_tests.cpp`, generate the values that will be used to replace the ones in `bip324_tests.cpp` (it will print to `stdout` so it's recommended to pipe it to a file)
      * Paste the new values over the old ones

  ## Breaking Changes

  None. Changes are restricted to BIP324 cryptosuite, tests and associated logic.

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

Top commit has no ACKs.

Tree-SHA512: bb056de8588026adae63e78d56878274ff3934a439e2eae81606fa9c0a37dab432a129315bb9c1b754400b5c883bf460eea3a0c857a3be0816c8fbf55c479843
2024-03-06 12:00:55 -06:00
Kittywhiskers Van Gogh
b60c493265
merge bitcoin#28100: more Span<std::byte> modernization & follow-ups 2024-03-05 21:43:22 +00:00
Kittywhiskers Van Gogh
c2aa01cf1d
merge bitcoin#28374: python cryptography required for BIP 324 functional tests 2024-03-05 21:43:22 +00:00
Kittywhiskers Van Gogh
7c5edf772a
merge bitcoin#28267: BIP324 ciphersuite follow-up 2024-03-05 21:43:22 +00:00
Kittywhiskers Van Gogh
1b1924e3d5
merge bitcoin#28008: BIP324 ciphersuite 2024-03-05 21:43:21 +00:00
Kittywhiskers Van Gogh
ff542199bc
merge bitcoin#27993: Make poly1305 support incremental computation + modernize 2024-03-05 21:42:49 +00:00
Kittywhiskers Van Gogh
d7482eb8a6
merge bitcoin#27985: Add support for RFC8439 variant of ChaCha20 2024-03-05 21:42:49 +00:00
Konstantin Akimov
df554f396b
test: add multiple suppression for cppcheck to make it finally quiet 2024-03-06 03:31:50 +07:00
Konstantin Akimov
e2ac86b8f7
refactor: drop dependency of CJ to fee_estimator
Coinjoin should not know about CBlockFeeEstimator
2024-03-06 03:31:50 +07:00
Konstantin Akimov
e7e355ba8b
refactor: make SetNull in CJ classes virtual to prevent warning from compiler 2024-03-06 03:31:49 +07:00
Konstantin Akimov
6bc14a35e5
chore: bump cpp check 2.10 to 2.13 2024-03-06 03:31:49 +07:00
Konstantin Akimov
b4ed65e15e
refactor: add multiple missing const 2024-03-06 03:31:48 +07:00
Konstantin Akimov
add99ea862
refactor: add more consts everywhere as required by cppcheck 2.13.0 2024-03-06 03:31:48 +07:00
Konstantin Akimov
61a6c407fe
fix: re-order asserts in creditpool 2024-03-06 03:31:47 +07:00
Konstantin Akimov
28f7ecc11f
test: tidy up and reorder warnings in lint-cppcheck-dash 2024-03-06 03:31:47 +07:00
Konstantin Akimov
3abeab16d3
refactor: replace multiple C-style casts to reinterpret_cast 2024-03-06 03:31:46 +07:00
Konstantin Akimov
f36bb1f085
style: add semicolumn that the end to ENTER/LEAVE CRITICAL SECTION
It is not required but it looks alien without it
2024-03-06 03:31:46 +07:00
Konstantin Akimov
c23c25d00b
test: supress any_of suggestions in cppcheck-dash
even it maybe useful lint message for some particular case, sometimes it asks
to make an refactoring that will be over-complex.

For example, it asks to refactor external loop to std::any_of:
```
for (const auto& inner_entry : vecEntries) {
    if (ranges::any_of(inner_entry.vecTxDSIn,
                    [&txin](const auto& txdsin){
                            return txdsin.prevout == txin.prevout;
                    })) {
        LogPrint(BCLog::COINJOIN, "CCoinJoinServer::%s -- ERROR: already have this txin in entries\n", __func__);
        nMessageIDRet = ERR_ALREADY_HAVE;
        // Two peers sent the same input? Can't really say who is the malicious one here,
        // could be that someone is picking someone else's inputs randomly trying to force
        // collateral consumption. Do not punish.
        return false;
    }
}
```
That's possible to refactor, but that's unreasonable complexity to have an
lambda inside an lambda... That's unreasonable.
Some other suggestion are also non-trivial.

One more suppression for any_of in llmq/commitment which is false-alarm:

There's used index but linter doesn't see it:
```
for (const auto i : irange::range(members.size(), size_t(llmq_params.size))) {
    if (validMembers[i]) {
        LogPrintfFinalCommitment("q[%s] invalid validMembers bitset. bit %d should not be set\n", quorumHash.ToString(), i);
        return false;
    }
    if (signers[i]) {
        LogPrintfFinalCommitment("q[%s] invalid signers bitset. bit %d should not be set\n", quorumHash.ToString(), i);
        return false;
    }
}
```
2024-03-06 03:31:46 +07:00
Konstantin Akimov
7daa9bea9a
refactor: use std::any_of 2024-03-06 03:31:45 +07:00
Konstantin Akimov
3f1cb8d13d
test: add suppression for state.Invalid() which is widely used by cppcheck is unhappy 2024-03-06 03:31:43 +07:00
pasta
27736114a1
Merge #5924: refactor: subsume CGovernanceTriggerManager into CGovernanceManager
a1636d429a refactor: subsume CGovernanceTriggerManager into CGovernanceManager (Kittywhiskers Van Gogh)

Pull request description:

  ## Issue being fixed or feature implemented
  Having a dedicated manager to manipulate one variable, that relies almost exclusively on another manager doesn't make much sense. Rather than converting it into a unique_ptr, creating an alias and deglob'ing it, it's much easier to subsume it into its dependent manager.

  ## What was done?
  See commit

  ## How Has This Been Tested?
  Built locally; kudos @kwvg. Good idea!

  ## Breaking Changes
  None

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

Top commit has no ACKs.

Tree-SHA512: 782aa7861e55675c77cd4ce3ebf5fdf8cfc953e535c6c3d5e98373a89611a86f14cdba0757df153b936ddb77b711472ec78c5b1a64fe596c85e6efcd49dec8fa
2024-03-05 13:16:14 -06:00
Kittywhiskers Van Gogh
a1636d429a
refactor: subsume CGovernanceTriggerManager into CGovernanceManager
Having a dedicated manager to manipulate one variable, that relies
almost exclusively on another manager doesn't make much sense. Rather
than converting it into a unique_ptr, creating an alias and deglob'ing
it, it's much easier to subsume it into its dependent manager.
2024-03-05 13:15:59 -06:00
pasta
8459820478
Merge #5854: refactor: move HD code from CWallet to LegacyScriptPubKeyMan
b5f4411d11 fix: extra logs to distinct WriteHDChain for encrypted/raw batches (Konstantin Akimov)
e8f84afd3e refactor: move BIP39 related code to wallet (Konstantin Akimov)
fa6519f320 refactor: move hdchain to wallet/ because it belongs there (Konstantin Akimov)
392b51b197 refactor: obsolete hdCryptedChain from ScriptPubKeyMan (Konstantin Akimov)
a0389e181f refactor: rename EncryptHDChain to EncryptAndSetHDChain (Konstantin Akimov)
b2143f9346 refactor: re-order public and private methods in scriptpubkeyman (Konstantin Akimov)
a219748f5e refactor: make SetHDChain private in ScriptPubKeyManager (Konstantin Akimov)
f418b9ac62 refactor: move Encrypt chain call inside ScriptPubKeyMan::Encrypt (Konstantin Akimov)
c3754d5183 refactor: move DecryptHDChain from public to private (Konstantin Akimov)
e08b64a1bb refactor: unify SetHDChainSingle and SetCryptedHDChainSingle (Konstantin Akimov)
59a998843b refactor: unify ScriptPubKeyMan's SetHDChain nad SetHDCryptedChain (Konstantin Akimov)
a180d73e5c refactor: unify WalletBatch's WriteHDChain and WriteCryptedHDChain (Konstantin Akimov)
9c8e4584ae refactor: unify SetHDChain and SetCryptedHDChain helpers (Konstantin Akimov)
7b72726b3e refactor: SetCryptedHDChain moved to scriptpubkeyman (Konstantin Akimov)
6993d112f3 feat: extra check failure case CWallet::GenerateNewHDChainEncrypted (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  During backport bitcoin#17261 significant part of HD chain has been forgotten in CWallet due to our own implementation.

  This PR do not change behaviour of HD wallets, it's just refactoring.

  ## What was done?
  This PR refactor HD wallets implementation:
   - key related stuff is moved from CWallet to LegacyScriptPubKeyMan (to follow-up bitcoin#17261)
   - refactored duplicated code between hdChain and hdCryptedChain
   - modules hdchain and bip39 related moved to wallet/ module

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

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [ ] 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

Top commit has no ACKs.

Tree-SHA512: 763ea6dd9af902dfc4dd99896c1cb16a75e3c0322e806e37ff05bc9229923057c544732ebf9fca705da870568643370867d3f81a46cab1e9d229b2949016c484
2024-03-05 13:10:56 -06:00
pasta
2441489c1a
Merge #5919: backport: bitcoin#18937, #19390, #19473, #19478, #19489, #19596, #19657, #19727, #19816, partial #18638
4e46e6101c Merge #19478: Remove CTxMempool::mapLinks data structure member (MarcoFalke)
be5662a785 Merge #19596: Deduplicate parent txid loop of requested transactions and missing parents of orphan transactions (Wladimir J. van der Laan)
9c3ecd167e Merge #19473: net: Add -networkactive option (MarcoFalke)
0593c1ffcb Merge #19390: doc/REST-interface: Remove stale info (fanquake)
8368bd795e Merge #19816: test: Rename wait until helper to wait_until_helper (fanquake)
063c9b744d Merge #19727: test: Remove unused classes from p2p_leak.py (fanquake)
941b39d9a9 Merge #18937: refactor: s/command/msg_type/ in CNetMsgMaker and CSerializedNetMsg (MarcoFalke)
a89bd562fd Merge #19657: test: Wait until is_connected in add_p2p_connection (Wladimir J. van der Laan)
a11743f009 Merge #19489: test: Fail wait_until early if connection is lost (MarcoFalke)
ad3f424b4d partial Merge #18638: net: Use mockable time for ping/pong, add tests (MarcoFalke)

Pull request description:

  ## Issue being fixed or feature implemented
  Just regular backports from bitcoin v21

  ## What was done?
   - partial bitcoin/bitcoin#18638
   - bitcoin/bitcoin#19489
   - bitcoin/bitcoin#19657
   - bitcoin/bitcoin#18937
   - bitcoin/bitcoin#19727
   - bitcoin/bitcoin#19816
   - bitcoin/bitcoin#19390
   - bitcoin/bitcoin#19473
   - bitcoin/bitcoin#19596
   - bitcoin/bitcoin#19478

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

Top commit has no ACKs.

Tree-SHA512: a656c7f8b39b0220250268e15e2c1d08272cb0253df555559f21ee142ce9ec9d4807ab3aafb0167c4ed79cc3ca73dc9c5aefd279b75b5901e1cc592f4df74bb0
2024-03-05 13:09:22 -06:00
Konstantin Akimov
b5f4411d11
fix: extra logs to distinct WriteHDChain for encrypted/raw batches 2024-03-06 02:04:25 +07:00
Konstantin Akimov
e8f84afd3e
refactor: move BIP39 related code to wallet 2024-03-06 02:04:25 +07:00
Konstantin Akimov
fa6519f320
refactor: move hdchain to wallet/ because it belongs there 2024-03-06 02:04:25 +07:00
Konstantin Akimov
392b51b197
refactor: obsolete hdCryptedChain from ScriptPubKeyMan 2024-03-06 02:04:24 +07:00
Konstantin Akimov
a0389e181f
refactor: rename EncryptHDChain to EncryptAndSetHDChain 2024-03-06 02:04:24 +07:00
Konstantin Akimov
b2143f9346
refactor: re-order public and private methods in scriptpubkeyman 2024-03-06 02:04:24 +07:00
Konstantin Akimov
a219748f5e
refactor: make SetHDChain private in ScriptPubKeyManager 2024-03-06 02:04:23 +07:00
Konstantin Akimov
f418b9ac62
refactor: move Encrypt chain call inside ScriptPubKeyMan::Encrypt 2024-03-06 02:04:23 +07:00
Konstantin Akimov
c3754d5183
refactor: move DecryptHDChain from public to private 2024-03-06 02:04:23 +07:00
Konstantin Akimov
e08b64a1bb
refactor: unify SetHDChainSingle and SetCryptedHDChainSingle 2024-03-06 02:04:22 +07:00
Konstantin Akimov
59a998843b
refactor: unify ScriptPubKeyMan's SetHDChain nad SetHDCryptedChain 2024-03-06 02:04:22 +07:00
Konstantin Akimov
a180d73e5c
refactor: unify WalletBatch's WriteHDChain and WriteCryptedHDChain 2024-03-06 02:04:21 +07:00
Konstantin Akimov
9c8e4584ae
refactor: unify SetHDChain and SetCryptedHDChain helpers 2024-03-06 02:04:21 +07:00
Konstantin Akimov
7b72726b3e
refactor: SetCryptedHDChain moved to scriptpubkeyman 2024-03-06 02:04:21 +07:00