Commit Graph

16723 Commits

Author SHA1 Message Date
Konstantin Akimov
bacaa805ea fix: use proper pindex/pindex->pprev in credit pool code during v20/mn_rr activations 2023-12-21 23:02:31 -06:00
Konstantin Akimov
f3d2f2da26 refactor: drop dependency unit test block_reward_reallocation_tests on llmq/utils 2023-12-21 23:02:31 -06:00
Konstantin Akimov
c25d336e95 refactor: drop dependency masternode/payments on llmq/utils 2023-12-21 23:02:31 -06:00
Konstantin Akimov
8f3d3db9a4 refactor: drop dependency governance/classes on llmq/utils 2023-12-21 23:02:31 -06:00
Konstantin Akimov
c329615584 refactor: use value_or in std::optional 2023-12-21 23:02:31 -06:00
Konstantin Akimov
a7ad399824 refactor: drop IsDIP0024Active and its usage of llmq_vbc 2023-12-21 23:02:31 -06:00
Konstantin Akimov
68442e8dfe chore: add TODO for llmq/utils.h refactoring 2023-12-21 23:02:31 -06:00
PastaPastaPasta
acd2acd592
refactor: pass large structure by const reference on every RPC call (#5780) 2023-12-21 12:01:39 -06:00
UdjinM6
6fe36cc1cb
fix: Improve quorum caching (again) (#5761)
## Issue being fixed or feature implemented
1. `scanQuorumsCache` is a special one and we use it incorrectly.
2. Platform doesn't really use anything that calls `ScanQuorums()`
directly, they specify the exact quorum hash in RPCs so it's
`GetQuorum()` that is used instead. The only place `ScanQuorums()` is
used for Platform related stuff is `StartCleanupOldQuorumDataThread()`
because we want to preserve quorum data used by `GetQuorum()`. But this
can be optimised with its own (much more compact) cache.
3. RPCs that use `ScanQuorums()` should in most cases be ok with smaller
cache, for other use cases there is a note in help text now.

## What was done?
pls see individual commits

## How Has This Been Tested?
run tests, run a node (~in progress~ looks stable)

## 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
2023-12-20 09:54:00 -06:00
UdjinM6
9a99a4abdc
fix(rpc): pass blockhash into TxToJSON so that getspecialtxes could show correct instantlock/chainlock values (#5774)
## Issue being fixed or feature implemented
`instantlock` and `chainlock` are broken in `getspecialtxes`

kudos to @thephez for finding the issue

## What was done?
pass the hash and also rename the variable to self-describing

## How Has This Been Tested?
run `getspecialtxes` on a node with and without the patch

## Breaking Changes
`instantlock` and `chainlock` will show actual values and not just
`false` all the time now (not sure if that qualifies for "breaking"
though)

## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository
code-owners and collaborators only)_
2023-12-19 07:43:36 -06:00
Odysseas Gabrielides
3bc77a6e1d
feat(rpc): submit chainlock signature if needed RPC (#5765)
## Issue being fixed or feature implemented
Once Platform is live, there could be an edge case where the CL could
arrive to an EvoNode faster through Platform quorum than regular P2P
propagation.

## What was done?
This PR introduces a new RPC `submitchainlock` with the following 3
mandatory parameters:
- `blockHash`, `signature` and `height`.

Besides some basic tests:
- If the block is unknown then the RPC returns an error (could happen if
the node is stucked)
- If the signature is not verified then the RPC return an error.
- If the node already has this CL, the RPC returns true.
- If the node doesn't have this CL, it inserts it, broadcast it through
the inv system and return true.

## How Has This Been Tested?
`feature_llmq_chainlocks.py` was modified with the following scenario:

1. node0 is isolated from the rest of the network
2. node1 mines a new block and waits for CL
3. Make sure node0 doesn't know the new block/CL (by checking
`getbestchainlock()`)
4. CL is submitted via the new RPC on node0
5. checking `getbestchainlock()` and make sure the CL was processed +
'known_block' is false
6. reconnect node0

## Breaking Changes
no

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

---------

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: thephez <thephez@users.noreply.github.com>
2023-12-18 22:27:19 -06:00
Konstantin Akimov
e3d9327d17 refactor: replace usage of deterministicMNManager to new interface listMNCollaterials 2023-12-17 12:42:45 -06:00
Konstantin Akimov
2c61d842a9 refactor: new method listMNCollaterials() for chain interface 2023-12-17 12:42:45 -06:00
Konstantin Akimov
4966bf7582 refactor: use static method CDeterministicMNManager without touching global variable in wallet's code 2023-12-17 12:42:45 -06:00
Konstantin Akimov
2310e353ff refactor: use std::all_of in isGroupISLocked 2023-12-17 12:42:45 -06:00
Konstantin Akimov
41b71dc0f5 refactor: removed usage of llmq::chainLocksHandler from wallet 2023-12-17 12:42:45 -06:00
Konstantin Akimov
0fea98b65f refactor: replace llmq::quorumInstantSendManager->IsLocked() to new chain's interface 2023-12-17 12:42:45 -06:00
Konstantin Akimov
808e631a08 fix: dirty removal of dependency of scriptpubkeyman on ShutdownRequested introduced in #5456 2023-12-17 12:42:45 -06:00
Konstantin Akimov
d95d7f63c9 refactor: remove dependency scriptpubkeyman on uiInterface 2023-12-17 12:42:45 -06:00
Konstantin Akimov
aa48c222ab follow-up bitcoin#15288 - missing fPrune usage 2023-12-17 12:42:45 -06:00
Vijay Das Manikpuri
38b9074275 (partial) Merge #24203: doc: Fix typos pointed out by lint-spelling 2023-12-11 15:48:44 -06:00
furszy
d7bae33f57
partial merge bitcoin#26532: wallet: bugfix, invalid crypted key "checksum_valid" set
backports cc5a5e81217506ec6f9fff34056290f8f40a7396 only

```
wallet: bugfix, invalid crypted key "checksum_valid" set

At wallet load time, we set the crypted key "checksum_valid" variable always to false.
Which, on every wallet decryption call, forces the process to re-write the entire ckeys to db when
it's not needed.
```
2023-12-10 20:04:48 +03:00
MarcoFalke
09973f2bee Merge #20932: refactor: Replace fs::absolute calls with AbsPathJoin calls
da9caa1cedd69702aea44cb44b2fd0a2d6d56916 Replace fs::absolute calls with AbsPathJoin calls (Kiminuo)
66576c4fd532ac18b8b355ea93d25581a2c15654 test: Clear forced -walletdir setting after wallet init_tests (Kiminuo)

Pull request description:

  This adds better test coverage and will make it easier in #20744 to remove our dependency on the two-argument boost::filesystem::absolute() function which does not have a direct equivalent in C++17.

  This PR doesn't change behavior aside from adding an assert and fixing a test bug.

ACKs for top commit:
  jonatack:
    Code review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916 only doxygen improvements since my last review per `git diff d867d7a da9caa1`
  MarcoFalke:
    review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916 📯
  ryanofsky:
    Code review ACK da9caa1cedd69702aea44cb44b2fd0a2d6d56916. Just comment and test tweaks since previous review.

Tree-SHA512: c940ee60f3ba374d4927cf34cf12d27c4c735c94af591fbc0ca408c641b30f8f8fbcfe521d66bfbddf9877a1fc8cd99bd8a47ebcd2fa59789de6bd87a7b9cf4d
2023-12-08 21:16:00 +03:00
MarcoFalke
d6d3e0905c Merge bitcoin/bitcoin#19362: rpc/blockchain: Reset scantxoutset progress before inferring descriptors
8c4129b4540f4f739413ed9a6fbfc78afc252f42 rpc: reset scantxoutset progress on finish (Pavol Rusnak)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/19361 by moving resetting the `g_scan_progress` variable **before** inferring the descriptors

ACKs for top commit:
  achow101:
    Code review ACK 8c4129b4540f4f739413ed9a6fbfc78afc252f42

Tree-SHA512: 2b81e5c930b9c201a1b9cc742c37e0617582326acd9adbd6b14991d33c05d51b1d5ff6a26f3accab7d274dc8c6a48bc8cbccf0811b60c1f76dd805f9b31482c0
2023-12-08 21:16:00 +03:00
Wladimir J. van der Laan
99c1307ae3 Merge #18017: txmempool: split epoch logic into class
fd6580e405699ccb051fd2a34525e48d3253673d [refactor] txmempool: split epoch logic into class (Anthony Towns)

Pull request description:

  Splits the epoch logic introduced in #17925 into a separate class.

  Uses clang's thread safety annotations and encapsulates the data more strongly to reduce chances of bugs from API misuse.

ACKs for top commit:
  jonatack:
    ACK fd6580e405699ccb051fd2a34525e48d3253673d using clang thread safety annotations looks like a very good idea, and the encapsulation this change adds should improve robustness (and possible unit test-ability) of the code. Verified that changing some of the locking duly provoked build-time warnings with Clang 9 on Debian and that small changes in the new `Epoch` class were covered by failing functional test assertions in `mempool_updatefromblock.py`, `mempool_resurrect.py`, and `mempool_reorg.py`
  hebasto:
    re-ACK fd6580e405699ccb051fd2a34525e48d3253673d, since my [previous](https://github.com/bitcoin/bitcoin/pull/18017#pullrequestreview-569619362) review:

Tree-SHA512: 7004623faa02b56639aa05ab7a078320a6d8d54ec62d8022876221e33f350f47df51ddff056c0de5be798f8eb39b5c03c2d3f035698555d70abc218e950f2f8c
2023-12-08 21:16:00 +03:00
Wladimir J. van der Laan
da20789983 Merge bitcoin-core/gui#162: Add network to peers window and peer details
e262a19b0ba07d8d2334551f49ca1577ab2999fc gui: display network in peer details (Jon Atack)
913695307360d3582f74ffe225cb122b2e48023d gui: rename peer tab column headers, initialize in .h (Hennadii Stepanov)
05c08c696a1bc45ab28abe6f82bebbdbd6239c23 gui: add network column in peers tab/window (Jon Atack)
e0e55060bf697fe8b9ed43ff62383a0451e9c0ce gui: fix broken doxygen formatting in src/qt/guiutil.h (Jon Atack)
0d5613f9ded846aa20a95c757672e9560c664c4c gui: create GUIUtil::NetworkToQString() utility function (Jon Atack)
af9103cc792e17f35249f1d4cb30f0d6958ceb75 net, rpc: change CNodeStats::m_network from string to Network (Jon Atack)

Pull request description:

  and rename peers window column headers from NodeId and Node/Service to Peer Id and Address.

  ![Screenshot from 2020-12-27 14-45-31](https://user-images.githubusercontent.com/2415484/103172228-efec8600-4849-11eb-8cee-04a3d2ab1273.png)

ACKs for top commit:
  laanwj:
    ACK e262a19b0ba07d8d2334551f49ca1577ab2999fc

Tree-SHA512: 709c2a805c109c2dd033aca7b6b6dc94ebe2ce7a0168c71249e1e661c9c57d1f1c781a5b9ccf3b776bedeb83ae2fb5c505637337c45b1eb9a418cb1693a89761
2023-12-08 21:16:00 +03:00
MarcoFalke
cff1c7b2c3 Merge #21043: net: Avoid UBSan warning in ProcessMessage(...)
3ddbf22ed179a2db733af4b521bec5d2b13ebf4b util: Disallow negative mocktime (MarcoFalke)
f5f2f9716885e7548809e77f46b493c896a019bf net: Avoid UBSan warning in ProcessMessage(...) (practicalswift)

Pull request description:

  Avoid UBSan warning in `ProcessMessage(...)`.

  Context: https://github.com/bitcoin/bitcoin/pull/20380#issuecomment-770427182 (thanks Crypt-iQ!)

ACKs for top commit:
  MarcoFalke:
    re-ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b only change is adding patch written by me
  ajtowns:
    ACK 3ddbf22ed179a2db733af4b521bec5d2b13ebf4b -- code review only

Tree-SHA512: e8d7af0457ca86872b75a4e406c0a93aafd841c2962e244e147e748cc7ca118c56be0fdafe53765f4b291410030b2c3cc8f76f733b37a955d34fc885ab6037b9
2023-12-08 21:16:00 +03:00
fanquake
c7a6f378f5 Merge #21162: Net Processing: Move RelayTransaction() into PeerManager
680eb56d828ce358b4e000c140f5b247ff5e6179 [net processing] Don't pass CConnman to RelayTransactions (John Newbery)
a38a4e8f039dfabfd9435f3a63f1a9b56de086d6 [net processing] Move RelayTransaction into PeerManager (John Newbery)

Pull request description:

  This is the first part of #21160. It moves the RelayTransaction() function to be a member function of the PeerManager class. This is required in order to move the transaction inventory data into the Peer object, since Peer objects are only accessible from within PeerManager.

ACKs for top commit:
  ajtowns:
    ACK 680eb56d828ce358b4e000c140f5b247ff5e6179

Tree-SHA512: 8c93491a4392b6369bb7f090de326a63cd62a088de59026e202f226f64ded50a0cf1a95ed703328860f02a9d2f64d3a87ca1bca9a6075b978bd111d384766235
2023-12-08 21:16:00 +03:00
MarcoFalke
787ae682da Merge #18842: wallet: Mark replaced tx to not be in the mempool anymore
fa4e088cbac035b8029a10b492849540150d0622 wallet: Mark replaced tx to not be in the mempool anymore (MarcoFalke)

Pull request description:

  The wallet does not mark the replaced tx as out-of-mempool. This causes failures in user scripts, because later RPCs may depend on this state change from `bumpfee`.

  For example, the following might fail on current master:

  ```
  txid = sendtoaddress(...)
  bumpfee(txid)
  abandontransaction(txid)  # fails because txid is still marked as "in mempool"
  ```

  Fixes #18831

ACKs for top commit:
  meshcollider:
    utACK fa4e088cbac035b8029a10b492849540150d0622
  ryanofsky:
    Code review ACK fa4e088cbac035b8029a10b492849540150d0622, and previous ACK faeedff5c87091fd83d2fb2b29eb49c948363f29 is also still valid in case there's a preference for the original fix

Tree-SHA512: 9858f40f5fb5a43a7b584b5c4268b6befa82e6a84583be5206fe721bcb6c255e8d35479d347d0b9aed72703df49887c02b14ab680e8efdd28b90dd6b93d9439a
2023-12-08 21:16:00 +03:00
fanquake
c6ff51a030
Merge bitcoin/bitcoin#24461: build: Minor leveldb subtree update
1b20109b04061304eab84cf921a030300b4f9fe3 Squashed 'src/leveldb/' changes from f8ae182c1e..330dd6235f (MarcoFalke)

Pull request description:

  A minor change to:

  * Consistently use the same symbol names in the whole project.
  * Fix compiling with C++20.

ACKs for top commit:
  fanquake:
    ACK fa0c32eb74f448c67d0ff9d91dfdd66e2f1e4c48

Tree-SHA512: b5d4540dd621cf4aa8caac811bae03bb74e502a31dbdda9354182e4caa39905550e62ad3cf8ea7d7f9bfc3e5120d119d34ab0f1e633716ec8089876037cbf192
2023-12-07 09:14:57 -06:00
Wladimir J. van der Laan
3f77d2312f Merge #16525: Dump transaction version as an unsigned integer in RPC/TxToUniv
e80259f1976545e4f1ab6a420644be0c32261773 Additionally treat Tx.nVersion as unsigned in joinpsbts (Matt Corallo)
970de70bdd3542e75b73c79b06f143168c361494 Dump transaction version as an unsigned integer in RPC/TxToUniv (Matt Corallo)

Pull request description:

  Consensus-wise we already treat it as an unsigned integer (the
  only rules around it are in CSV/locktime handling), but changing
  the underlying data type means touching consensus code for a
  simple cleanup change, which isn't really worth it.

  See-also, https://github.com/rust-bitcoin/rust-bitcoin/pull/299

ACKs for top commit:
  sipa:
    ACK e80259f1976545e4f1ab6a420644be0c32261773
  practicalswift:
    ACK e80259f1976545e4f1ab6a420644be0c32261773
  ajtowns:
    ACK e80259f1976545e4f1ab6a420644be0c32261773 code review -- checked all other uses of tx.nVersion treat it as unsigned (except for policy.cpp:IsStandard anyway), so looks good.
  naumenkogs:
    ACK e80259f

Tree-SHA512: 6760a2c77e24e9e1f79a336ca925f9bbca3a827ce02003c71d7f214b82ed3dea13fa7d9f87df9b9445cd58dff8b44a15571d821c876f22f8e5a372a014c9976b
2023-12-06 12:33:15 -06:00
Wladimir J. van der Laan
e29a35a997 Merge #18309: zmq: Add support to listen on multiple interfaces
e66870c5a4c2adbd30dca67d409fd5cd98697587 zmq: Append address to notify log output (nthumann)
241803da211265444e65f254f24dd184f2457fa9 test: Add zmq test to support multiple interfaces (nthumann)
a0b2e5cb6aa8db0563fac7d67a949b9baefe3a25 doc: Add release notes to support multiple interfaces (nthumann)
b1c3f180ecb63f3960506d202feebaa4271058ae doc: Adjust ZMQ usage to support multiple interfaces (nthumann)
347c94f551c3f144c44e00373e4dd61ff6d908b7 zmq: Add support to listen on multiple interfaces (Nicolas Thumann)

Pull request description:

  This PR adds support for ZeroMQ to listen on multiple interfaces, just like the RPC server.
  Currently, if you specify more than one e.g. `zmqpubhashblock` paramter, only the first one will be used. Therefore a user may be forced to listen on all interfaces (e.g. `zmqpubhashblock=0.0.0.0:28332`), which can result in an increased attack surface.
  With this PR a user can specify multiple interfaces to listen on, e.g.
  `-zmqpubhashblock=tcp://127.0.0.1:28332 -zmqpubhashblock=tcp://192.168.1.123:28332`.

ACKs for top commit:
  laanwj:
    Code review ACK e66870c5a4c2adbd30dca67d409fd5cd98697587
  instagibbs:
    reACK e66870c5a4

Tree-SHA512: f38ab4a6ff00dc821e5f4842508cefadb701e70bb3893992c1b32049be20247c8aa9476a1f886050c5f17fe7f2ce99ee30193ce2c81a7482a5a51f8fc22300c7
2023-12-06 12:33:15 -06:00
MarcoFalke
08349ee1da Merge #15367: feature: Added ability for users to add a startup command
090530cc24054d6b4658752bb88f75a3b73eab5d feature: Added ability for users to add a startup command (Ben Carman)

Pull request description:

  Thoughts for adding the feature is for users to be able to add things like electrum-personal-server or lnd to run whenever Bitcoin Core is running.  Open to feedback about the feature.

ACKs for top commit:
  MarcoFalke:
    re-ACK 090530cc24
  dongcarl:
    tACK 090530c

Tree-SHA512: ba514d2fc8b4fb12b781c1a9c89845a25fce0b80ba7c907761cde4abb81edd03fa643682edc895986dc20b273ac3b95769508806db7fbd99ec28623f85c41e67
2023-12-06 12:33:15 -06:00
Konstantin Akimov
da212ad425 follow-up Merge #17381: LegacyScriptPubKeyMan code cleanups - now it's possible to remove workaround 2023-12-06 11:46:53 -06:00
Konstantin Akimov
e88bf528af follow-up bitcoin#17926 - add missing changes 2023-12-06 11:46:53 -06:00
Samuel Dobson
94643ca47d Merge #20230: wallet: Fix bug when just created encrypted wallet cannot get address
bf6855a9096b25aa75bba61b57ee1b2433d49707 wallet: Fix bug when just created encrypted wallet cannot get address (Hennadii Stepanov)

Pull request description:

  Fix https://github.com/bitcoin-core/gui/issues/105

ACKs for top commit:
  achow101:
    Tested ACK bf6855a9096b25aa75bba61b57ee1b2433d49707
  kristapsk:
    ACK bf6855a9096b25aa75bba61b57ee1b2433d49707
  meshcollider:
    Tested ACK bf6855a9096b25aa75bba61b57ee1b2433d49707

Tree-SHA512: eca0ab306d7206f2e5db568e83217bd854caac104379f4d8fb261db832d4d6310cbb1eab44ce9b05a5ac2eb5879a623b729752a88810f8370c24518a8d81292d
2023-12-06 11:46:53 -06:00
Samuel Dobson
d11a933c5b Merge #9381: Remove CWalletTx merging logic from AddToWallet
28b112e9bd3fd1181c0720306051ba7efca8b436 Get rid of BindWallet (Russell Yanofsky)
d002f9d15d938e78360ad906f2d74a249c7e923e Disable CWalletTx copy constructor (Russell Yanofsky)
65b9d8f8ddb5a838454efc8bdd6576f0deb65f6d Avoid copying CWalletTx in LoadToWallet (Russell Yanofsky)
bd2fbc7cdbec46400341209f4cb7e69e5b2cee19 Get rid of unneeded CWalletTx::Init parameter (Russell Yanofsky)
2b9cba206594bfbcefcef0c88a0bf793819643bd Remove CWalletTx merging logic from AddToWallet (Russell Yanofsky)

Pull request description:

  This is a pure refactoring, no behavior is changing.

  Instead of AddToWallet taking a temporary CWalletTx object and then potentially merging it with a pre-existing CWalletTx, have it take a callback so callers can update the pre-existing CWalletTx directly.

  This makes AddToWallet simpler because now it is only has to be concerned with saving CWalletTx objects and not merging them.

  This makes AddToWallet calls clearer because they can now make direct updates to CWalletTx entries without having to make temporary objects and then worry about how they will be merged.

  Motivation for this change came from the bumpfee PR #8456 where we wanted to be able to call AddToWallet to make a simple update to an existing transaction, but were reluctant to, because the existing CWalletTx merging logic did not apply and seemed dangerous try to update as part of that PR. After this refactoring, the bumpfee PR could call AddToWallet safely instead of implementing a duplicate AddToWallet function.

  This also allows getting rid of the CWalletTx copy constructor to prevent unintentional copying.

ACKs for top commit:
  MarcoFalke:
    Anyway, re-ACK 28b112e9bd3fd1181c0720306051ba7efca8b436

Tree-SHA512: 528dd088714472a237500b200f4433db850bdb7fc29c5e5d81cae48072061dfb967f7c37edd90b33f24901239f9be982988547c1f8c80abc25fb243fbf7330ef
2023-12-06 11:46:53 -06:00
Wladimir J. van der Laan
849b5ed2a4 Merge #18853: wallet: Fix typo in assert that is compile-time true
fa47cf9d95dc2c2822fc96df16f179176935bf96 wallet: Fix typo in assert that is compile-time true (MarcoFalke)

Pull request description:

  Commit 92bcd70808b9cac56b184903aa6d37baf9641b37 presumably added a check that a `dest` of type `CNoDestination` implies an empty `scriptChange`.

  However, it accidentally checked for `boost::variant::empty`, which always returns false: https://www.boost.org/doc/libs/1_72_0/doc/html/boost/variant.html#id-1_3_46_5_4_1_1_16_2-bb

ACKs for top commit:
  Sjors:
    utACK fa47cf9d95dc2c2822fc96df16f179176935bf96

Tree-SHA512: 9626b1e2947039853703932a362c2ee204e002d3344856eb93eef0e0f833401336f2dfa80fd43b83c8ec6eac624e6302aee771fb67aec436ba6483be02b8d615
2023-12-06 11:46:53 -06:00
Samuel Dobson
f6f9b9851f Merge #17219: wallet: allow transaction without change if keypool is empty
92bcd70808b9cac56b184903aa6d37baf9641b37 [wallet] allow transaction without change if keypool is empty (Sjors Provoost)
709f8685ac37510aa145ac259753583c82280038 [wallet] CreateTransaction: simplify change address check (Sjors Provoost)
5efc25f9638866941028454cfa9bae27f1519cb4 [wallet] translate "Keypool ran out" message (Sjors Provoost)

Pull request description:

  Extracted from #16944

  First this PR simplifies the check when generating a change address, by dropping `CanGetAddresses` and just letting `reservedest.GetReservedDestination` do this check.

  Second, when the keypool is empty, instead of immediately giving up, we create a dummy change address and pass that to coin selection. If we didn't need the change address (e.g. when spending the entire balance), then it's all good. If we did need a change address, we throw the original error.

ACKs for top commit:
  fjahr:
    Code review ACK 92bcd70808b9cac56b184903aa6d37baf9641b37
  jonasschnelli:
    utACK 92bcd70808b9cac56b184903aa6d37baf9641b37
  achow101:
    ACK 92bcd70808b9cac56b184903aa6d37baf9641b37
  meshcollider:
    Code review ACK 92bcd70808b9cac56b184903aa6d37baf9641b37

Tree-SHA512: 07b8c8251f57061c58a85ebf0359be63583c23bac7a2c4cefdc14820c0cdebcc90a2bb218e5ede0db11d1e204cda149e056dfd18614642070b3d56efe2735006
2023-12-06 11:46:53 -06:00
Samuel Dobson
0065d7fefd Merge #18671: wallet: Add BlockUntilSyncedToCurrentChain to dumpwallet
fa60afc4fb957875bab1c8982d9d9e4999a3814c wallet: Add BlockUntilSyncedToCurrentChain to dumpwallet (MarcoFalke)

Pull request description:

  dumpwallet includes the block hash in the output, so this method depends on the chainstate. According to the developer notes e84a5f0004/doc/developer-notes.md (L1095) it must include a `BlockUntilSyncedToCurrentChain`.

  This is a minor fix and does not need backport, I think.

  It fixes test failures such as https://travis-ci.org/github/bitcoin/bitcoin/jobs/675487097#L2657 , which can only happen in master because the test was not backported.

ACKs for top commit:
  promag:
    Code review ACK fa60afc4fb957875bab1c8982d9d9e4999a3814c.
  ryanofsky:
    Code review ACK fa60afc4fb957875bab1c8982d9d9e4999a3814c
  meshcollider:
    utACK fa60afc4fb957875bab1c8982d9d9e4999a3814c

Tree-SHA512: 8df70b06b226b2cdf880dec9264adb72d66fd81b09b404fd1665a79e5f5236d26122eebf15df00fe71ee292b5c91b2dc23a0a42b2aa50a8d690604b23832723f
2023-12-06 11:46:53 -06:00
Konstantin Akimov
6183bd1085 follow-up Merge #17260: Split some CWallet functions into new LegacyScriptPubKeyMan
Some changes are missing or incorrectly backported during CWallet refactoring in #17260, #17261 such as:
 - Missing changes for CWallet::GetOldestKeyPoolTime
 - useless check of spk_man existance in getnewaddress
 - GetHDChain is used assuming it exists only legacy keymanager
 - using internal spk_man API instead wallet's in getwalletinfo
2023-12-06 11:46:53 -06:00
Konstantin Akimov
fa23a64471 partial (missing function GetKeyForDestination) Merge #11403: SegWit wallet support 2023-12-06 11:46:53 -06:00
Konstantin Akimov
7631118e04 fix: correct parsing pubkey in merge bitcoin#18204: improve descriptor cache and cache xpubs 2023-12-06 11:46:53 -06:00
fanquake
0eba155aa7 Merge #19072: doc: Expand section on Getting Started
facef3d4131f9980a4516282f11731361559509c doc: Explain that anyone can work on good first issues, move text to CONTRIBUTING.md (MarcoFalke)
fae2fb2a196ee864e9a13fffc24a0279cd5d17e6 doc: Expand section on Getting Started (MarcoFalke)
100000d1b2c2e38d7a14a31b0af79e0e4316b04c doc: Add headings to CONTRIBUTING.md (MarcoFalke)
fab893e0caf510d4836a20194892ef9c71426c51 doc: Fix unrelated typos reported by codespell (MarcoFalke)

Pull request description:

  Some random doc changes:

  * Add sections to docs, so that they can be linked to
  * Explain that anyone (even maintainers) are allowed to work on good first issues
  * Expand section on Getting Started slightly

ACKs for top commit:
  hebasto:
    ACK facef3d4131f9980a4516282f11731361559509c
  fanquake:
    ACK facef3d4131f9980a4516282f11731361559509c

Tree-SHA512: 8998e273a76dbf4ca77e79374c14efe4dfcc5c6df6b7d801e1e1e436711dbe6f76b436f9cbc6cacb45a56827babdd6396f3bd376a9426ee7be3bb9b8a3b8e383
2023-12-06 11:40:14 -06:00
fanquake
2da9982e55 Merge #17829: scripted-diff: Bump copyright of files changed in 2019
aaaaad6ac95b402fe18d019d67897ced6b316ee0 scripted-diff: Bump copyright of files changed in 2019 (MarcoFalke)

Pull request description:

ACKs for top commit:
  practicalswift:
    ACK aaaaad6ac95b402fe18d019d67897ced6b316ee0
  promag:
    ACK aaaaad6ac95b402fe18d019d67897ced6b316ee0 🎉
  fanquake:
    ACK aaaaad6ac95b402fe18d019d67897ced6b316ee0 - going to merge this now because the year is over and conflicts are minimal.

Tree-SHA512: 58cb1f53bc4c1395b2766f36fabc7e2332e213780a802762fff0afd59468dad0c3265f553714d761c7a2c44ff90f7dc250f04458f4b2eb8eef8b94f8c9891321
2023-12-06 11:40:14 -06:00
fanquake
ab36852267 Merge #16286: refactoring: wallet: Fix GCC 7.4.0 warning
d8bd97d5ee766a75dec016203bd4d4aafcc24c4f Fix GCC 7.4.0 warning (Hennadii Stepanov)

Pull request description:

  Having #13756 and #16239 merged cause GCC warning:
  ```
  $ gcc --version
  gcc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
  ...
  $ make -j 4 > /dev/null
  wallet/wallet.cpp: In member function ‘CWallet::Balance CWallet::GetBalance(int, bool) const’:
  wallet/wallet.cpp:2269:45: warning: enumeral and non-enumeral type in conditional expression [-Wextra]
       isminefilter reuse_filter = avoid_reuse ? 0 : ISMINE_USED;
                                   ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
  ```

  Fixed with this PR.

ACKs for top commit:
  practicalswift:
    utACK d8bd97d5ee766a75dec016203bd4d4aafcc24c4f
  promag:
    ACK d8bd97d.
  kallewoof:
    utACK d8bd97d5ee766a75dec016203bd4d4aafcc24c4f
  meshcollider:
    Trivial utACK d8bd97d5ee

Tree-SHA512: 2fb315ac82f290c8b9f4e48d1b4526b9babe0717c68593c7bc55cd6c289e64b6322aba72984f39291a9254b57d3f6ba8dbfe03799f510c0c1dc108b21b286732
2023-12-06 11:40:14 -06:00
MarcoFalke
2dc520be52 Merge #15054: Update copyright headers to 2018
1a49a0e310 Bump manpages (DrahtBot)
06ba77973e Update copyright headers to 2018 (DrahtBot)

Pull request description:

  * `./contrib/devtools/copyright_header.py update ./`
  * `./contrib/devtools/gen-manpages.sh`

Tree-SHA512: ca0dc5e97f4c33814d4ccd17769bbf2d23a99a71d62534fe1064fedfe47de3b5c30caf9b6deb0d70bf125e08c7ae6335ac4fcded918049d6b63b13b319d798e3
2023-12-06 11:40:14 -06:00
MarcoFalke
5ebb66db18 Merge #18781: Add templated GetRandDuration<>
0000ea32656833efa3d2ffd9bab66c88c83334f0 test: Add test for GetRandMillis and GetRandMicros (MarcoFalke)
fa0e5b89cf742df56c6c8f49fe9b3c54d2970a66 Add templated GetRandomDuration<> (MarcoFalke)

Pull request description:

  A naive implementation of this template is dangerous, because the call site might accidentally omit the template parameter:

  ```cpp
  template <typename D>
  D GetRandDur(const D& duration_max)
  {
      return D{GetRand(duration_max.count())};
  }

  BOOST_AUTO_TEST_CASE(util_time_GetRandTime)
  {
      std::chrono::seconds rand_hour = GetRandDur(std::chrono::hours{1});
      // Want seconds to be in range [0..1hour), but always get zero :((((
      BOOST_CHECK_EQUAL(rand_hour.count(), 0);
  }
  ```

  Luckily `std::common_type` is already specialised in the standard lib for `std::chrono::duration` (https://en.cppreference.com/w/cpp/chrono/duration/common_type). And its effect seem to be that the call site must always specify the template argument explicitly.

  So instead of implementing the function for each duration type by hand, replace it with a templated version that is safe to use.

ACKs for top commit:
  laanwj:
    Code review ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0
  promag:
    Code review ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0.
  jonatack:
    ACK 0000ea3 thanks for the improved documentation. Code review, built, ran `src/test/test_bitcoin -t random_tests -l test_suite` for the new unit tests, `git diff fa05a4c 0000ea3` since previous review:
  hebasto:
    ACK 0000ea32656833efa3d2ffd9bab66c88c83334f0 with non-blocking [nit](https://github.com/bitcoin/bitcoin/pull/18781#discussion_r424924671).

Tree-SHA512: e89d46e31452be6ea14269ecbbb2cdd9ae83b4412cd14dff7d1084283092722a2f847cb501e8054394e4a3eff852f9c87f6d694fd008b3f7e8458cb5a3068af7
2023-12-05 12:47:47 -06:00
fanquake
da7ca00f03 Merge #18814: rpc: Relock wallet only if most recent callback
9f59dde9740d065118bdddde75ef9f4e4603a7b1 rpc: Relock wallet only if most recent callback (João Barbosa)
a2e6db5c4f1bb52a8814102b628e51652493d06a rpc: Add mutex to guard deadlineTimers (João Barbosa)

Pull request description:

  This PR fixes an early relocking race condition from #18811 where old relock callback runs after new wallet unlock code and nRelockTime update but before rpcRunLater call, causing early relock and incorrect nRelockTime time

  Issue introduced in #18487.
  Fixes #18811.

ACKs for top commit:
  MarcoFalke:
    ACK 9f59dde9740d065118bdddde75ef9f4e4603a7b1
  ryanofsky:
    Code review ACK 9f59dde9740d065118bdddde75ef9f4e4603a7b1. No changes since last review except squashing commits.
  jonatack:
    ACK 9f59dde9740d065118bdddde75ef

Tree-SHA512: 2f7fc03e5ab6037337f2d82dfad432495cc337c77d07c968ee2355105db6292f24543c03456f5402e0e759577a4327758f9372f7ea29de6d56dc3695fda9b379
2023-12-05 12:47:47 -06:00
UdjinM6
e1630d8fc3
fix: Start LLMQContext early to let VerifyDB() check ChainLock signatures in coinbase (#5752)
## Issue being fixed or feature implemented
Now that we have ChainLock sigs in coinbase `VerifyDB()` have to process
them. It works most of the time because usually we simply read
contributions from quorum db
https://github.com/dashpay/dash/blob/develop/src/llmq/quorums.cpp#L385.
However, sometimes these contributions aren't available so we try to
re-build them
https://github.com/dashpay/dash/blob/develop/src/llmq/quorums.cpp#L388.
But by the time we call `VerifyDB()` bls worker threads aren't started
yet, so we keep pushing jobs into worker's queue but it can't do
anything and it halts everything.

backtrace:
```
  * frame #0: 0x00007fdd85a2873d libc.so.6`syscall at syscall.S:38
    frame #1: 0x0000555c41152921 dashd_testnet`std::__atomic_futex_unsigned_base::_M_futex_wait_until(unsigned int*, unsigned int, bool, std::chrono::duration<long, std::ratio<1l, 1l> >, std::chrono::duration<long, std::ratio<1l, 1000000000l> >) + 225
    frame #2: 0x0000555c40e22bd2 dashd_testnet`CBLSWorker::BuildQuorumVerificationVector(Span<std::shared_ptr<std::vector<CBLSPublicKey, std::allocator<CBLSPublicKey> > > >, bool) at atomic_futex.h:102:36
    frame #3: 0x0000555c40d35567 dashd_testnet`llmq::CQuorumManager::BuildQuorumContributions(std::unique_ptr<llmq::CFinalCommitment, std::default_delete<llmq::CFinalCommitment> > const&, std::shared_ptr<llmq::CQuorum> const&) const at quorums.cpp:419:65
    frame #4: 0x0000555c40d3b9d1 dashd_testnet`llmq::CQuorumManager::BuildQuorumFromCommitment(Consensus::LLMQType, gsl::not_null<CBlockIndex const*>) const at quorums.cpp:388:37
    frame #5: 0x0000555c40d3c415 dashd_testnet`llmq::CQuorumManager::GetQuorum(Consensus::LLMQType, gsl::not_null<CBlockIndex const*>) const at quorums.cpp:588:37
    frame #6: 0x0000555c40d406a9 dashd_testnet`llmq::CQuorumManager::ScanQuorums(Consensus::LLMQType, CBlockIndex const*, unsigned long) const at quorums.cpp:545:64
    frame #7: 0x0000555c40937629 dashd_testnet`llmq::CSigningManager::SelectQuorumForSigning(Consensus::LLMQParams const&, llmq::CQuorumManager const&, uint256 const&, int, int) at signing.cpp:1038:90
    frame #8: 0x0000555c40937d34 dashd_testnet`llmq::CSigningManager::VerifyRecoveredSig(Consensus::LLMQType, llmq::CQuorumManager const&, int, uint256 const&, uint256 const&, CBLSSignature const&, int) at signing.cpp:1061:113
    frame #9: 0x0000555c408e2d43 dashd_testnet`llmq::CChainLocksHandler::VerifyChainLock(llmq::CChainLockSig const&) const at chainlocks.cpp:559:53
    frame #10: 0x0000555c40c8b09e dashd_testnet`CheckCbTxBestChainlock(CBlock const&, CBlockIndex const*, llmq::CChainLocksHandler const&, BlockValidationState&) at cbtx.cpp:368:47
    frame #11: 0x0000555c40cf75db dashd_testnet`ProcessSpecialTxsInBlock(CBlock const&, CBlockIndex const*, CMNHFManager&, llmq::CQuorumBlockProcessor&, llmq::CChainLocksHandler const&, Consensus::Params const&, CCoinsViewCache const&, bool, bool, BlockValidationState&, std::optional<MNListUpdates>&) at specialtxman.cpp:202:60
    frame #12: 0x0000555c40c00a47 dashd_testnet`CChainState::ConnectBlock(CBlock const&, BlockValidationState&, CBlockIndex*, CCoinsViewCache&, bool) at validation.cpp:2179:34
    frame #13: 0x0000555c40c0e593 dashd_testnet`CVerifyDB::VerifyDB(CChainState&, CChainParams const&, CCoinsView&, CEvoDB&, int, int) at validation.cpp:4789:41
    frame #14: 0x0000555c40851627 dashd_testnet`AppInitMain(std::variant<std::nullopt_t, std::reference_wrapper<NodeContext>, std::reference_wrapper<WalletContext>, std::reference_wrapper<CTxMemPool>, std::reference_wrapper<ChainstateManager>, std::reference_wrapper<CBlockPolicyEstimator>, std::reference_wrapper<LLMQContext> > const&, NodeContext&, interfaces::BlockAndHeaderTipInfo*) at init.cpp:2098:50
    frame #15: 0x0000555c4082fe11 dashd_testnet`AppInit(int, char**) at bitcoind.cpp:145:54
    frame #16: 0x0000555c40823c64 dashd_testnet`main at bitcoind.cpp:173:20
    frame #17: 0x00007fdd85934083 libc.so.6`__libc_start_main(main=(dashd_testnet`main at bitcoind.cpp:160:1), argc=3, argv=0x00007ffcb8ca5b88, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007ffcb8ca5b78) at libc-start.c:308:16
    frame #18: 0x0000555c4082f27e dashd_testnet`_start + 46
```

Fixes #5741

## What was done?
Start LLMQContext early. Alternative solution could be moving bls worker
Start/Stop into llmq context ctor/dtor.

## How Has This Been Tested?
I had a node with that issue. This patch fixed it.

## Breaking Changes
Not sure, hopefully none.

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