Commit Graph

178 Commits

Author SHA1 Message Date
fanquake
c2df9366f0 Merge #16507: feefilter: Compute the absolute fee rather than stored rate
eb7b78165966f2c79da71b993c4c4d793e37297f modify p2p_feefilter test to catch rounding error (Gregory Sanders)
6a51f7951716d6d6fc0f9b56028f3a0dd02b61c8 Disallow implicit conversion for CFeeRate constructor (Gregory Sanders)
8e59af55aaf1b196575084bce2448af02d97d745 feefilter: Compute the absolute fee rather than stored rate to match mempool acceptance logic (Gregory Sanders)

Pull request description:

  This means we will use the rounding-down behavior in `GetFee` to match both mempool acceptance and wallet logic, with minimal changes.

  Fixes https://github.com/bitcoin/bitcoin/issues/16499

  Replacement PR for https://github.com/bitcoin/bitcoin/pull/16500

ACKs for top commit:
  ajtowns:
    ACK eb7b78165966f2c79da71b993c4c4d793e37297f code review only
  naumenkogs:
    utACK eb7b78165966f2c79da71b993c4c4d793e37297f
  achow101:
    re ACK eb7b78165966f2c79da71b993c4c4d793e37297f
  promag:
    ACK eb7b78165966f2c79da71b993c4c4d793e37297f.

Tree-SHA512: 484a11c8f0e825f0c983b1f7e71cf6252b1bba6858194abfe4c088da3bae8a418ec539ef6c4181bf30940e277a95c08d493595d59dfcc6ddf77c65b05563dd7e
2023-04-17 10:42:25 -05:00
Kittywhiskers Van Gogh
548e8704c5 merge bitcoin#21055: Prune remaining g_chainman usage in validation functions
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2023-04-04 12:41:45 -05:00
Kittywhiskers Van Gogh
30191be0b1 merge bitcoin#20750: Prune g_chainman usage in mempool-related validation functions 2023-04-04 12:41:45 -05:00
Kittywhiskers Van Gogh
228a7fb4a1 merge bitcoin#20972: Annotate CTxMemPool::check to require cs_main 2023-04-04 12:41:45 -05:00
MarcoFalke
be6a045b8c Merge #18807: [doc / test / mempool] unbroadcast follow-ups
9e1cb1adf1800efe429e348650931f2669b0d2c0 [trivial/doc] Fix comment type (Amiti Uttarwar)
8f30260a67166a6ab7c0f33f7ec1990d3c31761e [doc] Update unbroadcast description in RPC results (Amiti Uttarwar)
750456d6f29c63d57af05bfbdd6035bb9c965de2 [trivial] Remove misleading 'const' (Amiti Uttarwar)
fa32e676e5833a5c5fc735ef00c0a80f5fab7a2c [test] Manage node connections better in mempool persist test (Amiti Uttarwar)
1f94bb0c744a103b633c1051e8fbc01e612097dc [doc] Provide rationale for randomization in scheduling. (Amiti Uttarwar)
9c8a55d9cb0ec73f10b196e79b637aa601c0a6b7 [mempool] Don't throw expected error message when upgrading (Amiti Uttarwar)
ba5498318233ab81decbc585e9619d8ffe2df1b0 [test] Test that wallet transactions aren't rebroadcast before 12 hours (Amiti Uttarwar)
00d44a534b4e5ae249b8011360c6b0f7dc731581 [test] P2P connection behavior should meet expectations (Amiti Uttarwar)
bd093ca15de762fdaf0937a0877d17b0c2bce16e [test] updates to unbroadcast test (Amiti Uttarwar)
dab298d9ab5a5a41685f437db9081fa7b395fa73 [docs] add release notes (Amiti Uttarwar)

Pull request description:

  This PR is a follow up to #18038 which introduced the idea of an unbroadcast set & focuses mostly on documentation updates and test fixes. One small functionality update to not throw an expected error in `LoadMempool` when you upgrade software versions.

  #18895 is another follow up to that addresses other functionality updates.

  Background context:
  The unbroadcast set is a mechanism for the mempool to track locally submitted transactions (via wallet or RPC). The node does a best-effort of delivering the transactions to the network via retries every 10-15 minutes until either a `GETDATA` is received or the transaction is removed from the mempool.

ACKs for top commit:
  MarcoFalke:
    ACK 9e1cb1adf1 👁
  gzhao408:
    ACK [`9e1cb1a`](9e1cb1adf1)

Tree-SHA512: 0cd51c4ca368b9dce92d50d73ec6e9df278a259e609eef2858f24cb8595ad07acc3db781d9eb0c351715f18fca5a2b4526838981fdb34a522427e9dc868bdaa6
2023-02-27 23:12:41 -06:00
fanquake
f1885c2221 Merge #18895: p2p: unbroadcast followups: rpcs, nLastResend, mempool sanity check
651f1d816f054cb9c637f8a99c9360bba381ef58 [test] wait for inital broadcast before comparing mempool entries (gzhao408)
9d3f7eb9860254eb787ebe2734fd6a26bcf365c1 [mempool] sanity check that all unbroadcast txns are in mempool (gzhao408)
a7ebe48b94c5a9195c8eabd193204c499cb4bfdb [rpc] add unbroadcast info to mempool entries and getmempoolinfo (gzhao408)
d16006960443c2efe37c896e46edae9dca86c57d [wallet] remove nLastResend logic (gzhao408)

Pull request description:

  Followup to #18038 by amitiuttarwar which introduces the unbroadcast set: "a mechanism for the mempool to track locally submitted transactions" and decreases the frequency of rebroadcast from 10-15 minutes to 12-36 hours.

  This PR addresses some of the outstanding TODOs building on top of it:
  - remove `nLastResend` logic, which is used to ensure rebroadcast doesn't happen again if no new block has been mined (makes sense in 10-15 min period, but not necessary for 12-36 hour period). (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416826914))
  - expose unbroadcast info via RPCs, for more informative queries and testing (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416837980))
  - add sanity check to verify unbroadcast transactions still exist in mempool before rebroadcasting (#18038 [comment](https://github.com/bitcoin/bitcoin/pull/18038#discussion_r416861609))

ACKs for top commit:
  naumenkogs:
    Code review ACK 651f1d816f054cb9c637f8a99c9360bba381ef58
  amitiuttarwar:
    ACK 651f1d816f054cb9c637f8a99c9360bba381ef58 🎉
  MarcoFalke:
    Review ACK 651f1d816f054cb9c637f8a99c9360bba381ef58

Tree-SHA512: d5327e95ef39d44152b48df5c610502ae11c168f43dbbfb2885340c93d1ba9426eb3a5794573f5fc843502109cb3ffb63efa3f2db4f8f112efcde8f76d9a8845
2023-02-27 23:12:41 -06:00
Kittywhiskers Van Gogh
a5f20129fb merge bitcoin#20222: CTxMempool constructor clean up 2023-02-28 00:11:11 +03:00
Wladimir J. van der Laan
7dcdc95613 Merge #19854: Avoid locking CTxMemPool::cs recursively in simple cases
020f0519ec66d9626255b938e1c6c3f7f9aa4017 refactor: CTxMemPool::IsUnbroadcastTx() requires CTxMemPool::cs lock (Hennadii Stepanov)
7c4bd0387a01a0c3e2938d530dba3c882e4d8f2b refactor: CTxMemPool::GetTotalTxSize() requires CTxMemPool::cs lock (Hennadii Stepanov)
fa5fcb032b6ed04c49ee465235288b8059fa805e refactor: CTxMemPool::ClearPrioritisation() requires CTxMemPool::cs lock (Hennadii Stepanov)
7140b31b90cbd84d75eedb3e395d0d55f83b5b95 refactor: CTxMemPool::ApplyDelta() requires CTxMemPool::cs lock (Hennadii Stepanov)
66e47e5e506043fbb9b4e487b44bf992985709c9 refactor: CTxMemPool::UpdateChild() requires CTxMemPool::cs lock (Hennadii Stepanov)
939807768acd508932f2efabee660d56324a73df refactor: CTxMemPool::UpdateParent() requires CTxMemPool::cs lock (Hennadii Stepanov)

Pull request description:

  This is another step to transit `CTxMemPool::cs` from `RecursiveMutex` to `Mutex`.

  Split out from #19306.
  Only trivial thread safety annotations and lock assertions added. No new locks. No behavior change.

  Refactoring `const uint256` to `const uint256&` was [requested](https://github.com/bitcoin/bitcoin/pull/19647#discussion_r468471022) by **promag**.

  Please note that now, since #19668 has been merged, it is safe to apply `AssertLockHeld()` macros as they do not swallow compile time Thread Safety Analysis warnings.

ACKs for top commit:
  promag:
    Core review ACK 020f0519ec66d9626255b938e1c6c3f7f9aa4017.
  jnewbery:
    Code review ACK 020f0519ec66d9626255b938e1c6c3f7f9aa4017
  vasild:
    ACK 020f0519e

Tree-SHA512: a31e389142d5a19b25fef0aaf1072a337278564528b5cc9209df88ae548a31440e1b8dd9bae0169fd7aa59ea06e22fe5e0413955386512b83ef1f3e7d941e890
2023-02-15 00:07:39 -06:00
Wladimir J. van der Laan
97b7ecb256 Merge #17477: Remove the mempool's NotifyEntryAdded and NotifyEntryRemoved signals
e57980b4738c10344baf136de3e050a3cb958ca5 [mempool] Remove NotifyEntryAdded and NotifyEntryRemoved callbacks (John Newbery)
2dd561f36124972d2364f941de9c3417c65f05b6 [validation] Remove pool member from ConnectTrace (John Newbery)
969b65f3f527631ede1a31c7855151e5c5d91f8f [validation] Remove NotifyEntryRemoved callback from ConnectTrace (John Newbery)
5613f9842b4000fed088b8cf7b99674c328d15e1 [validation] Remove conflictedTxs from PerBlockConnectTrace (John Newbery)
cdb893443cc16edf974f099b8485e04b3db1b1d7 [validation interface] Remove vtxConflicted from BlockConnected (John Newbery)
1168394d759b13af68acec6d5bfa04aaa24561f8 [wallet] Notify conflicted transactions in TransactionRemovedFromMempool (John Newbery)

Pull request description:

  These boost signals were added in #9371, before we had a `TransactionRemovedFromMempool` method in the validation interface. The `NotifyEntryAdded` callback was used by validation to build a vector of conflicted transactions when connecting a block, which the wallet was notified of in the `BlockConnected` CValidationInterface callback.

  Now that we have a `TransactionRemovedFromMempool` callback, we can fire that signal directly from the mempool for conflicted transactions.

  Note that #9371 was implemented to ensure `-walletnotify` events were fired for these conflicted transaction. We inadvertently stopped sending these notifications in #16624 (Sep 2019 commit 7e89994). We should probably fix that, but in a different PR.

ACKs for top commit:
  jonatack:
    Re-ACK e57980b
  ryanofsky:
    Code review ACK e57980b4738c10344baf136de3e050a3cb958ca5, no code changes since previous review, but helpful new code comments have been added and the PR description is now more clear about where the old code came from

Tree-SHA512: 3bdbaf1ef2731e788462d4756e69c42a1efdcf168691ce1bbfdaa4b7b55ac3c5b1fd4ab7b90bcdec653703600501b4224d252cfc086aef28f9ce0da3b0563a69
2023-02-15 00:07:39 -06:00
Kittywhiskers Van Gogh
41eba6beef merge bitcoin#21415: remove Optional & nullopt 2022-10-20 16:08:45 -05:00
Wladimir J. van der Laan
9ea098413a Merge #16908: txmempool: Make entry time type-safe (std::chrono)
faec689bed7a5b66e2a7675853d10205b933cec8 txmempool: Make entry time type-safe (std::chrono) (MarcoFalke)
faaa1f01daba94b021ca77515266a16d27f0364e util: Add count_seconds time helper (MarcoFalke)
1111170f2f0141084b5b4ed565b2f07eba48599a test: mempool entry time is persisted (MarcoFalke)

Pull request description:

  This changes the type of the entry time of txs into the mempool from `int64_t` to `std::chrono::seconds`.

  The benefits:
  * Documents the type for developers
  * Type violations result in compile errors
  * After compilation, the two are equivalent (at no run time cost)

ACKs for top commit:
  ajtowns:
    utACK faec689bed7a5b66e2a7675853d10205b933cec8
  laanwj:
    ACK faec689bed7a5b66e2a7675853d10205b933cec8

Tree-SHA512: d958e058755d1a1d54cef536a8b30a11cc502b7df0d6ecf84a0ab1d38bc8105a67668a99cd5087a444f6de2421238111c5fca133cdf8e2e2273cb12cb6957845
2022-10-03 16:08:31 -04:00
Kittywhiskers Van Gogh
a250f2c977 merge bitcoin#14193: Add missing mempool locks
Co-authored-by: "UdjinM6 <UdjinM6@users.noreply.github.com>"
2022-05-18 20:49:34 +05:30
Kittywhiskers Van Gogh
6390cae926 merge bitcoin#18038: Mempool tracks locally submitted transactions to improve wallet privacy 2022-05-18 20:49:34 +05:30
fanquake
27727c7541
Merge #17316: refactor: Replace all uses of boost::optional with our own Optional type
d314e8a818d4c162b1c7201533e6b600dcab2d91 refactor: Replace all uses of boost::optional with our own Optional type (Wladimir J. van der Laan)

Pull request description:

  Replace all uses of boost::optional with our own Optional type. Luckily, there aren't so many.

  After this:

  - `boost::optional` is no longer used directly (only through `Optional` which is an alias for it)
  - `boost/optional.hpp` is only included in one place

ACKs for top commit:
  MarcoFalke:
    ACK d314e8a818d4c162b1c7201533e6b600dcab2d91
  practicalswift:
    ACK d314e8a818d4c162b1c7201533e6b600dcab2d91 -- diff looks correct + satisfying to see incremental progress towards the goal of a Boost free future :)
  jtimon:
    ACK d314e8a818d4c162b1c7201533e6b600dcab2d91
  fanquake:
    ACK d314e8a818d4c162b1c7201533e6b600dcab2d91

Tree-SHA512: b43e0017af81b07b5851377cd09624f114510ac5b9018d037664b58ad0fc8e893e30946b61f8f5e21e39125925bf9998a81f2226b468aab2df653ee57ed3213d
2022-04-07 07:22:08 +05:30
MarcoFalke
71e38b9ebc
Merge #15323: rpc: Expose g_is_mempool_loaded via getmempoolinfo
effe81f750 Move g_is_mempool_loaded into CTxMemPool::m_is_loaded (Ben Woosley)
bb8ae2c419 rpc: Expose g_is_mempool_loaded via getmempoolinfo and /rest/mempool/info.json (Ben Woosley)

Pull request description:

  And use it to fix a race condition in mempool_persist.py:
  https://travis-ci.org/Empact/bitcoin/jobs/487577243

  Since e.g. getrawmempool returns errors based on this status, this
  enables users to test it for readiness.

  Fixes #12863

ACKs for commit effe81:
  MarcoFalke:
    utACK effe81f750
  jnewbery:
    utACK effe81f7503d2ca3c88cfdea687f9f997f353e0d

Tree-SHA512: 74328b0c17a97efb8a000d4ee49b9a673c2b6dde7ea30c43a6a2eff961a233351c9471f9a42344412135786c02bdf2ee1b2526651bb8fed68bd94d2120c4ef86
2021-12-25 18:32:19 +05:30
Jonas Schnelli
551ce4e3e4
Merge #16433: txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWN
0000ff0aa763e3be524ac4537a41048a26529fb2 txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWN (MarcoFalke)

Pull request description:

  The `remove*` methods set the removal reason to `UNKNOWN` by default. This is nowhere used; Except in tests, where the value doesn't matter. Fix that by removing the confusing default.

ACKs for top commit:
  practicalswift:
    utACK 0000ff0aa763e3be524ac4537a41048a26529fb2
  promag:
    ACK 0000ff0aa763e3be524ac4537a41048a26529fb2.
  jonasschnelli:
    utACK 0000ff0aa763e3be524ac4537a41048a26529fb2

Tree-SHA512: ffc8b35dd3291a81225171577c743c8bb2645638cab02960b6361174cb68afd739aaab7ab8661d65de5750d37daf16bb7eee9338958d8609093a8d46c2ada1ab
2021-12-25 18:32:18 +05:30
MarcoFalke
3c5dcb036a Merge #16659: refactoring: Remove unused includes
084e17cebd424b8e8ced674bc810eef4e6ee5d3b Remove unused includes (practicalswift)

Pull request description:

  As requested by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/16273#issuecomment-521332089:

  This PR removes unused includes.

  Please note that in contrast to #16273 I'm limiting the scope to the trivial cases of pure removals (i.e. no includes added) to make reviewing easier.

  I'm seeking "Concept ACK":s for this obviously non-urgent minor cleanup.

  Rationale:
  * Avoids unnecessary re-compiles in case of header changes.
  * Makes reasoning about code dependencies easier.
  * Reduces compile-time memory usage.
  * Reduces compilation time.
  * Warm fuzzy feeling of being lean :-)

ACKs for top commit:
  ryanofsky:
    Code review ACK 084e17cebd424b8e8ced674bc810eef4e6ee5d3b. PR only removes include lines and it still compiles. In the worst case someone might have to explicitly add an include later for something now included implicitly. But maybe some effort was taken to avoid this, and it wouldn't be a tragedy anyway.

Tree-SHA512: 89de56edc6ceea4696e9579bccff10c80080821685b9fb4e8c5ef593b6e43cf662f358788701bb09f84867693f66b2e4db035b92b522a0a775f50b7ecffd6a6d
2021-12-12 16:44:54 -05:00
Kittywhiskers Van Gogh
0a760bb809
merge bitcoin#8365: Treat high-sigop transactions as larger rather than rejecting them (#4562) 2021-11-08 21:43:24 +03:00
Kittywhiskers Van Gogh
544823b272 merge bitcoin#15473: bench: Benchmark MempoolToJSON 2021-10-25 14:49:12 +05:30
Kittywhiskers Van Gogh
84339a4b84 merge bitcoin#16443: have CCoins* data managed under CChainState 2021-10-22 08:15:38 +05:30
Kittywhiskers Van Gogh
1f436f8aee merge bitcoin#15948: rename chainActive
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2021-10-22 08:15:22 +05:30
MarcoFalke
5a627762c1
Merge #15245: remove deprecated mentions of signrawtransaction from fundraw help
851380ce17 remove deprecated mentions of signrawtransaction from fundraw help (Gregory Sanders)

Pull request description:

  RPC call has been removed as of 0.17.99.

Tree-SHA512: a6a12a0e4572acd9b532c1719be85ed6f29d1c1a28f9ce691398528b8dde4fb4a3222b8f68632fcb1a8eddfe2d31e96d5efd5bc51c041af8e7cb99b61ca3a167
2021-09-06 18:01:57 +05:30
MarcoFalke
d5f50c3df8 Merge #14963: mempool, validation: Explain cs_main locking semantics
fa5e373365 validation: Add cs_main locking annotations (MarcoFalke)
fa5c346c5a doc: Add comment to cs_main and mempool::cs (MarcoFalke)
fafe941bdd test: Add missing validation locks (MarcoFalke)
fac4558462 sync: Add RecursiveMutex type alias (MarcoFalke)

Pull request description:

  Both the chain state and the transaction pool are validation specific, but access to them is protected by two locks. The two locks have the following semantics:

  * Writing to the chain state or adding transactions to the transaction pool -> Take both `cs_main` and `mempool::cs`
  * Reading either or removing transactions from the the transaction pool -> Take only the appropriate lock

Tree-SHA512: 6f6e612ffc391904c6434a79a4f3f8de1b928bf0a3e3434b73561037b395e2b40a70a5a4bd8472dd230e9eacc8e5d5374c904a3c509910cf3971dd7ff59a626c
2021-08-24 19:14:50 -04:00
MarcoFalke
e3c2fddf96 Merge #14331: doxygen: Fix member comments
fa69ac7614 doxygen: Fix member comments (MarcoFalke)

Pull request description:

  Trailing comments must be indicted with the caret `//!<`.

  Not all places do this right now, see for example https://dev.visucore.com/bitcoin/doxygen/txmempool_8h.html#a2bc6653552b5871101b6cbefdbaf251f, but they can be fixed with an almost-scripted-diff:

  ```
  sed -i --regexp-extended -e 's/((,|;) *\/\/!) /\1< /g' $(git grep --extended-regexp -l  '(,|;)\s*//!\s')
  ```

  (Same as  [doxygen] Fix member comments #7793)

Tree-SHA512: 451077008353ccc6fcc795f34094b2d022feb7a171b562a07ba4de0dcb0aebc137e12b03970764bd81e2da386751d042903db4c4831900f43c0cfde804c81b2b
2021-08-18 13:56:38 -05:00
5tefan
db302f912d Merge #13793: tx pool: Use class methods to hide raw map iterator impl details
faa1a749428a195af784633eb78e1df5d6a0e875 tx pool: Use class methods to hide raw map iterator impl details (MarcoFalke)

Pull request description:

  ATMP et al would often use map iterator implementation details such as
`end()` or `find()`, which is acceptable in current code.

  However, this not only makes it impossible to turn the maps into
private members in the future but also makes it harder to replace the
maps with different data structures.

  This is required for and split off of #13804

Tree-SHA512: 4f9017fd1d98d9df49d25bba92655a4a97755eea161fd1cbb565ceb81bbc2b4924129d214f8a29563a77e3d8eef85a67c81245ecdc9a9e5292d419922a93cb88
2021-08-12 12:17:41 -06:00
PastaPastaPasta
f87d62d677
refactor: misc bls, evo, include refactoring (#4305)
* refactor/bls: misc refactoring and spelling/grammar fixes in bls code

* refactor/evo: misc refactoring and spelling/grammar fixes in evo code

* refactor: some include changes

* refactor: remove redundant `public`

* fix linter

Signed-off-by: pasta <pasta@dashboost.org>

* Sort includes

* Move `class CTxDSIn;`

* Drop unused functions in CBLSWorker

Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2021-07-31 21:29:12 +03:00
UdjinM6
7ec01fd77a
Merge pull request #4278 from PastaPastaPasta/backports-0.18-pr9
Backports 0.18 pr9
2021-07-20 02:26:47 +03:00
pasta
832dc0b8a5
scripted-diff: Remove unused first argument to addUnchecked END 13792
-BEGIN VERIFY SCRIPT-
git grep -l addUnchecked | xargs sed --regexp-extended -i -e 's/addUnchecked\([^)][^,]+,\s*/addUnchecked(/g'
-END VERIFY SCRIPT-

Signed-off-by: pasta <pasta@dashboost.org>
2021-07-19 17:11:16 -05:00
MarcoFalke
221e8f6dc6
Begin Merge #13792
tx pool: Use the entry's hash instead of the one passed to addUnchecked

Mark CTxMemPoolEntry members that should not be modified const
2021-07-19 17:11:16 -05:00
UdjinM6
95f89eec56
Fix more spelling errors 2021-07-19 12:52:03 -05:00
UdjinM6
886024ba25
Merge pull request #4203 from UdjinM6/pr4196
ci: Add `--enable-werror` to arm and c++17 builds (and fix all issues found via these builds)
2021-07-17 02:37:07 +03:00
Wladimir J. van der Laan
a8a23d8dd6
Merge #17431: Remove unnecessary forward declaration
3d133482b20c0acc03cdbe5f9104ae8dedde8e4d Remove unnecessary forward declaration (Mark Erhardt)

Pull request description:

  This removes an unnecessary forward declaration.

ACKs for top commit:
  jnewbery:
    Tested ACK 3d133482b20c0acc03cdbe5f9104ae8dedde8e4d
  laanwj:
    ACK 3d133482b20c0acc03cdbe5f9104ae8dedde8e4d

Tree-SHA512: 9e5b14e861c2b9fa2d7707ed67c4667540e9812a762e00f5039691eeca82390eb7462d20ad781d4e8c9111517e989da7aef60b112ab33abb774e32d9845b5459
2021-07-14 18:44:42 -05:00
UdjinM6
d1ff298c8e
Add more compile time checking for cs_main runtime locking assertions 2021-07-13 19:43:33 +03:00
MarcoFalke
570d573786
Merge #13774: Return void instead of bool for functions that cannot fail
d78a8dc3e8 Return void instead of bool for functions that cannot fail (practicalswift)

Pull request description:

  Return `void` instead of `bool` for functions that cannot fail:
  * `CBlockTreeDB::ReadReindexing(...)`
  * `CChainState::ResetBlockFailureFlags(...)`
  * `CTxMemPool::addUnchecked(...)`
  * `CWallet::CommitTransaction(...)`
  * `CWallet::LoadDestData(...)`
  * `CWallet::LoadKeyMetadata(...)`
  * `CWallet::LoadScriptMetadata(...)`
  * `CWallet::LoadToWallet(...)`
  * `CWallet::SetHDChain(...)`
  * `CWallet::SetHDSeed(...)`
  * `PendingWalletTx::commit(...)`
  * `RemoveLocal(...)`
  * `SetMinVersion(...)`
  * `StartHTTPServer(...)`
  * `StartRPC(...)`
  * `TorControlConnection::Disconnect(...)`

  Some of the functions can fail by throwing.

  Found by manually inspecting the following candidate functions:

  ```
  $ git grep -E '(^((static|virtual|inline|friend)[^a-z])*[^a-z]*bool [^=]*\(|return true|return false)' -- "*.cpp" "*.h"
  ```

Tree-SHA512: c0014e045362dbcd1a0cc8f69844e7b8cbae4f538e7632028daeca3a797ac11d8d3d86ebc480bedcb8626df3e96779d592747d52a12556fc49921b114fa0ccc6
2021-07-02 12:59:28 +03:00
MarcoFalke
32b571bd13 Merge #14063: Move cs_main locking annotations from .cpp to .h
9e2de6b9d0 Move cs_main locking annotations from .cpp to .h (practicalswift)

Pull request description:

  Move `cs_main` locking annotations from `.cpp` to `.h`.

Tree-SHA512: 591bdc408a7a04c8208530fb6992b7535f30d2473e6c33fe39920330319a40e8dfb40407f2ea2d4c6c0d4624c24e34ffbdf57271b332e82f98339792372be84e
2021-07-01 16:33:23 -05:00
Kittywhiskers Van Gogh
680319643f merge bitcoin#14074: Use std::unordered_set instead of set in blockfilter interface 2021-06-27 12:03:15 +05:30
Kittywhiskers Van Gogh
f3e5ca703c bitcoin#17925: Improve UpdateTransactionsFromBlock with Epochs 2021-06-16 21:44:58 +05:30
Wladimir J. van der Laan
50607de7b2
Merge #11742: rpc: Add testmempoolaccept
b55555d rpc: Add testmempoolaccept (MarcoFalke)

Pull request description:

  To check if a single raw transaction makes it into the current transaction pool, one had to call `sendrawtransaction`. However, on success, this adds the transaction to the mempool with no easy way to undo.

  The call `testmempoolaccept` is introduced to provide a way to solely check the result without changing the mempool state.

Tree-SHA512: 5afd9311190135cee8fc1f229c7d39bf893f1028f29e28d34f70df820198ff97b4bf86b41cbbd6e6c36a5c30073cefa92d541c74a4939c7a2a6fa283dfd41b63
2021-05-23 01:19:31 +03:00
MarcoFalke
4a7387fba9 Merge #13786: refactor: Avoid locking tx pool cs thrice
fa5ed4f8d2 refactor: Avoid locking tx pool cs thrice (MarcoFalke)

Pull request description:

  `addUnchecked` is (outside the tests) only called by ATMP, which already takes the tx pool read lock. So locking it twice more in both `addUnchecked` methods seems redundant.

  Similarly `CalculateMemPoolAncestors` is (beside once in the wallet) only called in contexts, where the tx pool lock is already taken. So remove the lock there as well.

Tree-SHA512: fcf603b570da0fc529fe6db8add218663eae52845510732bee0d4611263d2429d3d3c9c8ae68493d67287d13504500ed51905ccbe711eb15a0af3b019edad543
2021-02-04 14:58:13 -06:00
Wladimir J. van der Laan
a8863dc3b5 Merge #12634: [refactor] Make TransactionWithinChainLimit more flexible
f77e1d34fd5f17304ce319b5f962b8005592501a test: Add MempoolAncestryTests (Karl-Johan Alm)
a08d76bcfee6f563a268933357931abe32060668 mempool: Calculate descendant maximum thoroughly (Karl-Johan Alm)
6d3568371eb2559d65a3e2334252d36a262319e8 wallet: Switch to using ancestor/descendant limits (Karl-Johan Alm)
6888195b062c8c58dd776fd10b44b25554eb1f15 wallet: Strictly greater than for ancestor caps (Karl-Johan Alm)
322b12ac4e0a8c892e81a760ff7225619248b74f Remove deprecated TransactionWithinChainLimit (Karl-Johan Alm)
47847515473b054929af0c8de3d54b6672500cab Switch to GetTransactionAncestry() in OutputEligibleForSpending (Karl-Johan Alm)
475a385a80198a46a6d99846f99b968f04e9b470 Add GetTransactionAncestry to CTxMemPool for general purpose chain limit checking (Karl-Johan Alm)
46847d69d2c1cc908fd779daac7884e365955dbd mempool: Fix max descendants check (Karl-Johan Alm)
b9ef21dd727dde33f5bd3c33226b05d07eb12aac mempool: Add explicit max_descendants (Karl-Johan Alm)

Pull request description:

  Currently, `TransactionWithinChainLimit` is restricted to single-output use, and needs to be called every time for different limits. If it is replaced with a chain limit value calculator, that can be called once and reused, and is generally more flexible (see e.g. #12257).

  Update: this PR now corrects usage of max ancestors / max descendants, including calculating the correct max descendant value, as advertised for the two limits.

  ~~This change also makes `nMaxAncestors` signed, as the replacement method will return `-1` for "not in the mempool", which is different from "0", which means "no ancestors/descendants in mempool".~~

  ~~This is a subset of #12257.~~

Tree-SHA512: aa59c849360542362b3126c0e29d44d3d58f11898e277d38c034dc4b86a5b4500f77ac61767599ce878c876b5c446fec9c02699797eb2fa41e530ec863a00cf9
2021-02-04 14:58:13 -06:00
Wladimir J. van der Laan
9909c3aca7
Merge #12717: [REST] Handle UTXO retrieval when ignoring the mempool
9cb9af8 [REST] Handle UTXO retrieval when ignoring the mempool (Roman Zeyde)
1fdc7c4 Make CTxMemPool::isSpent() const (Roman Zeyde)

Pull request description:

  Current REST API always returns empty UTXO when invoked without `/checkmempool/` URL part.

  After the fix:
  ```
  $ curl -s http://localhost:8332/rest/getutxos/0e3e2357e806b6cdb1f70b54c3a3a17b6714ee1f0e68bebb44a74b1efd512098-0.json | jq
  {
    "chainHeight": 514109,
    "chaintipHash": "0000000000000000001fe76d1445e8a6432fd2de04261dc9c5915311dc7ad6de",
    "bitmap": "1",
    "utxos": [
      {
        "height": 1,
        "value": 50,
        "scriptPubKey": {
          "asm": "0496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858ee OP_CHECKSIG",
          "hex": "410496b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52da7589379515d4e0a604f8141781e62294721166bf621e73a82cbf2342c858eeac",
          "reqSigs": 1,
          "type": "pubkey",
          "addresses": [
            "12c6DSiU4Rq3P4ZxziKxzrL5LmMBrzjrJX"
          ]
        }
      }
    ]
  }
  ```

  Before the fix:
  ```
  $ curl -s http://localhost:8332/rest/getutxos/0e3e2357e806b6cdb1f70b54c3a3a17b6714ee1f0e68bebb44a74b1efd512098-0.json | jq
  {
    "chainHeight": 514109,
    "chaintipHash": "0000000000000000001fe76d1445e8a6432fd2de04261dc9c5915311dc7ad6de",
    "bitmap": "0",
    "utxos": []
  }
  ```

Tree-SHA512: 994a350cb34a3c8f5a7afbc169c6b177c5be6cf223b2071c62d63644819d416d3e10d1c58b244d9d351bae7233d2974aa5e9ebadd1b5d6218f5245558675be0d
2020-10-14 13:27:31 -04:00
pasta
c49e506176 scripted-diff: Merge #13753: Remove trailing whitespaces
-BEGIN VERIFY SCRIPT-

sed --in-place'' --regexp-extended 's/[[:space:]]+$//g' $(git grep -I --files-with-matches --extended-regexp '[[:space:]]+$' -- src test  ':!*.svg' ':!src/crypto/sha256_sse4*' ':!src/leveldb' ':!src/qt/locale' ':!src/secp256k1' ':!src/univalue')

-END VERIFY SCRIPT-

Signed-off-by: pasta <pasta@dashboost.org>
2020-07-28 20:23:12 -05:00
MarcoFalke
cbfd59ad1f Merge #11762: Avoid locking mutexes that are already held by the same thread
01a06d6686 Avoid locking mutexes that are already held by the same thread (practicalswift)

Pull request description:

  Avoid locking mutexes that are already held by the same thread. These are reentrant mutexes, but still no need to lock them more than once per thread :-)

Tree-SHA512: e2fb85882e8800892fd8e8170f3c13128d6acfeb14d7b69fb9555f2b7ad0884fb201cf945b8144ffaf6fb1253c28af7c8c6c435319a7ae30ca003f28aa645a98
2020-07-28 20:11:57 -05:00
Wladimir J. van der Laan
ed5f046a99
Merge #12225: Mempool cleanups
669c943 Avoid leaking prioritization information when relaying transactions (Suhas Daftuar)
e868b22 fee estimator: avoid sorting mempool on shutdown (Suhas Daftuar)
0975406 Correct mempool mapTx comment (Suhas Daftuar)

Pull request description:

  Following up on #12127 and #12118, this cleans up a comment that was left incorrect in txmempool.h, and addresses a couple of the observations @TheBlueMatt made about an unnecessary use of `queryHashes()` and a small information leak when prioritizing transactions.

  Left undone is nuking queryHashes altogether; that would require changing the behavior of the `getrawmempool` rpc call, which I think I might be in favor of doing, but wanted to save for its own PR.

Tree-SHA512: c97d10b96dcd6520459287a4a2eda92774173757695100fcfe61e526aef86f394507c331d17f9e0c14b496c33ec46198a0f165a847762ca50f7c6780b993f162
2020-07-17 15:42:20 -05:00
MarcoFalke
e07bbeed91 Merge #13531: doc: Clarify that mempool txiter is const_iterator
faca0a8625 doc: Clarify that mempool txiter is const_iterator (MarcoFalke)

Pull request description:

  `iterator` and `const_iterator` are the same type for multi indexed transaction sets, but `const_iterator` should be preferred for documentation purposes.

Tree-SHA512: 83e8af36d15aa1e9fc59b3c2279504fd6f6ea3188dc43e36dec279ee0613ff07947d7143fd112bade7868b0dba59ecab3fd246cbde82e376ef965b646d9f8c4d
2020-07-08 18:32:04 -05:00
MarcoFalke
1d81e8dd52 Merge #11689: mempool: Fix missing locking in CTxMemPool::check(…) and CTxMemPool::setSanityCheck(…)
47782b49e6 Add Clang thread safety analysis annotations (practicalswift)
0e2dfa8a65 Fix missing locking in CTxMemPool::setSanityCheck(double dFrequency) (practicalswift)
6bc5b7100b Fix missing locking in CTxMemPool::check(const CCoinsViewCache *pcoins) (practicalswift)

Pull request description:

  Fix missing locking in `CTxMemPool::check(const CCoinsViewCache *pcoins)`:
  * reading variable `mapTx` requires holding mutex `cs`
  * reading variable `mapNextTx` requires holding mutex `cs`
  * reading variable `nCheckFrequency` requires holding mutex `cs`

  Fix missing locking in `CTxMemPool::setSanityCheck(double dFrequency)`:
  * writing variable `nCheckFrequency` requires holding mutex `cs`

Tree-SHA512: ce7c365ac89225223fb06e6f469451b121acaa499f35b21ad8a6d2a266c91194639b3703c5428871be033d4f5f7be790cc297bd8c25b2e0c59345ef09c3693d0
2020-07-01 14:08:47 -05:00
MarcoFalke
1ca8c77592 Merge #13080: mempool: Add compile time checking for ::mempool.cs runtime locking assertions
cbba1d2da4 Add compile time checking for all ::mempool.cs runtime locking assertions (practicalswift)

Pull request description:

  Add compile time checking for `::mempool.cs` runtime locking assertions.

  This PR is a subset of #12665. The PR was broken up to make reviewing easier.

  The intention is that literally all `EXCLUSIVE_LOCKS_REQUIRED`/`LOCKS_EXCLUDED`:s added in this PR should follow either directly or indirectly from `AssertLockHeld(…)`/`AssertLockNotHeld(…)`:s already existing in the repo.

  Consider the case where function `A(…)` contains `AssertLockHeld(cs_foo)` (without
  first locking `cs_foo` in `A`), and that `B(…)` calls `A(…)` (without first locking `cs_main`):
  * It _directly_ follows that: `A(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.
  * It _indirectly_ follows that: `B(…)` should have an `EXCLUSIVE_LOCKS_REQUIRED(cs_foo)` annotation.

Tree-SHA512: 1b5ec1cfca6be67edd1298fea1a52b5572ce833dd4ad05c4583f753c2d3229402663373675df87e950151d5c41aeb3ee02f0ad935ed83fe2f45ca8e4d55d901e
2020-06-27 10:43:32 -05:00
MarcoFalke
bb035972ee
Merge #12716: Fix typos and cleanup in various files
4d9b4256d8 Fix typos (Dimitris Apostolou)

Pull request description:

  Unfortunately I messed up my repo while trying to squash #12593 so I created a PR with just the correct fixes.

Tree-SHA512: 295d77b51bd2a9381f1802c263de7ffb2edd670d9647391e32f9a414705b3c8b483bb0e469a9b85ab6a70919ea13397fa8dfda2aea7a398b64b187f178fe6a06
Signed-off-by: pasta <pasta@dashboost.org>
2020-06-11 23:20:48 -05:00
PastaPastaPasta
8a1ec935a0
Backport 11651 (#3358)
* scripted-diff: Replace #include "" with #include <> (ryanofsky)

-BEGIN VERIFY SCRIPT-
for f in \
  src/*.cpp \
  src/*.h \
  src/bench/*.cpp \
  src/bench/*.h \
  src/compat/*.cpp \
  src/compat/*.h \
  src/consensus/*.cpp \
  src/consensus/*.h \
  src/crypto/*.cpp \
  src/crypto/*.h \
  src/crypto/ctaes/*.h \
  src/policy/*.cpp \
  src/policy/*.h \
  src/primitives/*.cpp \
  src/primitives/*.h \
  src/qt/*.cpp \
  src/qt/*.h \
  src/qt/test/*.cpp \
  src/qt/test/*.h \
  src/rpc/*.cpp \
  src/rpc/*.h \
  src/script/*.cpp \
  src/script/*.h \
  src/support/*.cpp \
  src/support/*.h \
  src/support/allocators/*.h \
  src/test/*.cpp \
  src/test/*.h \
  src/wallet/*.cpp \
  src/wallet/*.h \
  src/wallet/test/*.cpp \
  src/wallet/test/*.h \
  src/zmq/*.cpp \
  src/zmq/*.h
do
  base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-

Signed-off-by: Pasta <pasta@dashboost.org>

* scripted-diff: Replace #include "" with #include <> (Dash Specific)

-BEGIN VERIFY SCRIPT-
for f in \
  src/bls/*.cpp \
  src/bls/*.h \
  src/evo/*.cpp \
  src/evo/*.h \
  src/governance/*.cpp \
  src/governance/*.h \
  src/llmq/*.cpp \
  src/llmq/*.h \
  src/masternode/*.cpp \
  src/masternode/*.h \
  src/privatesend/*.cpp \
  src/privatesend/*.h
do
  base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f
done
-END VERIFY SCRIPT-

Signed-off-by: Pasta <pasta@dashboost.org>

* build: Remove -I for everything but project root

Remove -I from build system for everything but the project root,
and built-in dependencies.

Signed-off-by: Pasta <pasta@dashboost.org>

# Conflicts:
#	src/Makefile.test.include

* qt: refactor: Use absolute include paths in .ui files

* qt: refactor: Changes to make include paths absolute

This makes all include paths in the GUI absolute.

Many changes are involved as every single source file in
src/qt/ assumes to be able to use relative includes.

Signed-off-by: Pasta <pasta@dashboost.org>

# Conflicts:
#	src/qt/dash.cpp
#	src/qt/optionsmodel.cpp
#	src/qt/test/rpcnestedtests.cpp

* test: refactor: Use absolute include paths for test data files

* Recommend #include<> syntax in developer notes

* refactor: Include obj/build.h instead of build.h

* END BACKPORT #11651 Remove trailing whitespace causing travis failure

* fix backport 11651

Signed-off-by: Pasta <pasta@dashboost.org>

* More of 11651

* fix blockchain.cpp

Signed-off-by: pasta <pasta@dashboost.org>

* Add missing "qt/" in includes

* Add missing "test/" in includes

* Fix trailing whitespaces

Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>
Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2020-03-20 01:46:56 +03:00
UdjinM6
73258b363b
Merge pull request #3354 from PastaPastaPasta/backports-0.16-pr10
Backports 0.16 pr10
2020-03-10 14:57:35 +03:00