aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-08-05Merge bitcoin/bitcoin#24662: addrman: Use system time instead of adjusted ↵fanquake
network time fadd8b2676f6d68ec87189871461c9a6a6aa3cac addrman: Use system time instead of adjusted network time (MarcoFalke) Pull request description: This changes addrman to use system time for address relay instead of the network adjusted time. This is an improvement, because network time has multiple issues: * It is non-monotonic, even if the system time is monotonic. * It may be wrong, even if the system time is correct. * It may be wrong, if the system time is wrong. For example, when the node has limited number of connections (`4`), or the system time is wrong by too much (more than +-70 minutes), or the system time only got wrong after timedata collected more than half of the entries while the time was correct, ...) This may slightly degrade addr relay for nodes where timedata successfully adjusted the time. Addr relay can already deal with minor offsets of up to 10 minutes. Offsets larger than this should still allow addr relay and not result in a DoS. ACKs for top commit: dergoegge: Code review ACK fadd8b2676f6d68ec87189871461c9a6a6aa3cac Tree-SHA512: b6c178fa01161544e5bc76c4cb23e11bcc30391f7b7a64accce864923766647bcfce2e8ae21d36fb1ffc1afa07bc46415aca612405bd8d4cc1f319c92a08498f
2022-08-04Merge bitcoin/bitcoin#24675: util: Use ArgsManager::GetPathArg more widelyfanquake
b01f336708019f8c8274ea701d3446e4123e7af2 util, refactor: Drop explicit conversion to fs::path (Hennadii Stepanov) 138c668e2b4d64279ddefbe07c1d9b7c3d3c537c util, refactor: Use GetPathArg to read "-rpccookiefile" value (Hennadii Stepanov) 1276090705060fcc97072481c2383bbaaa556194 util, refactor: Use GetPathArg to read "-conf" value (Hennadii Stepanov) Pull request description: This PR is a continuation of bitcoin/bitcoin#24265 and bitcoin/bitcoin#24306. Now the following command-line arguments / configure options been read with the `GetPathArg` method: - `-conf`, also `includeconf` values been normalized - `-rpccookiefile` ACKs for top commit: jarolrod: Code Review ACK b01f336708019f8c8274ea701d3446e4123e7af2 ryanofsky: Code review ACK b01f336708019f8c8274ea701d3446e4123e7af. Changes since last review: just dropping first commit (NormalizedPathFromString) as suggested Tree-SHA512: 2d26d50b73542acdbcc63a32068977b2a49a017d31ca337471a0446f964eb0a6e3e4e3bb1ebe6771566a260f2cae3bc2ebe93b4b523183cea0d51768daab85c9
2022-08-04Merge bitcoin/bitcoin#25023: Remove unused SetTip(nullptr) codefanquake
faab8dceb37a944d0763fcca390342111e6a9fcc Remove unused SetTip(nullptr) code (MacroFake) Pull request description: Now that this path is no longer used after commit b51e60f91472da5216116626afc032acd5616e85, we can remove it. Future code should reset `CChain` by simply discarding it and constructing a fresh one. ACKs for top commit: ryanofsky: Code review ACK faab8dceb37a944d0763fcca390342111e6a9fcc. Just moved an assert statement since last review Tree-SHA512: 7dc273b11133d85d32ca2a69c0c7c07b39cdd338141ef5b51496e7de334a809864d5459eb95535497866c8b1e468aae84ed8f91b543041e6ee20130d5622874e
2022-08-03validationcaches: Use size_t for sizesCarl Dong
...also move the 0-clamping logic to ApplyArgsManOptions, where it belongs.
2022-08-03validationcaches: Add and use ValidationCacheSizesCarl Dong
Also: - Make DEFAULT_MAX_SIG_CACHE_SIZE into constexpr DEFAULT_MAX_SIG_CACHE_BYTES to utilize the compile-time integer arithmetic overflow checking available to constexpr. - Fix comment (MiB instead of MB) for DEFAULT_MAX_SIG_CACHE_BYTES. - Pass in max_size_bytes parameter to InitS*Cache(), modify log line to no longer allude to maxsigcachesize being split evenly between the two validation caches. - Fix possible integer truncation and add a comment. [META] I've kept the integer types as int64_t in order to not introduce unintended behaviour changes, in the next commit we will make them size_t.
2022-08-03cuckoocache: Check for uint32 overflow in setup_bytesCarl Dong
This fixes an potential overflow which existed prior to this patchset. If CuckooCache::cache<Element, Hash>::setup_bytes is called with a `size_t bytes` which, when divided by sizeof(Element), does not fit into an uint32_t, the implicit conversion to uint32_t in the call to setup will result in an overflow. At least on x86_64, this overflow is possible: static_assert(std::numeric_limits<size_t>::max() / 32 <= std::numeric_limits<uint32_t>::max()); static_assert(std::numeric_limits<size_t>::max() / 4 <= std::numeric_limits<uint32_t>::max()); This commit detects such cases and signals to callers that the `size_t bytes` input is too large.
2022-08-03validationcaches: Abolish arbitrary limitCarl Dong
1. -maxsigcachesize is a DEBUG_ONLY option 2. Almost 7 years has passed since its semantics change in 830e3f3d027ba5c8121eed0f6a9ce99961352572 from "number of entries" to "number of mebibytes" 3. A std::new_handler was added to the codebase after the original PR which introduced this limit, which will terminate immediately instead of causing trouble by being caught somewhere unexpected.
2022-08-03cuckoocache: Return approximate memory sizeCarl Dong
Returning the approximate total size eliminates the need for InitS*Cache() to do nElems*sizeof(uint256). The cuckoocache has a better idea of this information.
2022-08-03tests: Reduce calls to InitS*Cache()Carl Dong
In src/test/fuzz/script_sigcache.cpp, we should really be setting up a full working BasicTestingSetup. The initialize_ function is only run once anyway. In src/test/txvalidationcache_tests.cpp, the Dersig100Setup inherits from BasicTestingSetup, which should have already set up a global script execution cache without the need to explicitly call InitScriptExecutionCache.
2022-08-03test: Add missing static to IsStandardTx helperMacroFake
2022-08-03Merge bitcoin/bitcoin#25648: refactor: Remove all policy globalsglozow
ddddd6913b1bdee1cad89a32d363306ea1f7b8d7 sort after scripted-diff (MacroFake) fac812ca835e0d843aba1d4db0e49d183018a29e scripted-diff: Move mempool_args to src/node (MacroFake) 66664384a6fec39ecb4d8d06db66a4f193a06e33 Remove ::g_max_datacarrier_bytes global (MacroFake) fad0b4fab849eb5f1f0aa54ebc290f85a473ec91 Pass datacarrier setting into IsStandard (MacroFake) fa2a6b8516b24d7e9ca11926a49cf2b07f661e81 Combine datacarrier globals into one (MacroFake) fa477d32eefcc3dd2f06b452066290d9936d8c5d Remove ::GetVirtualTransactionSize() alias (MacroFake) fa2f6c1a611dffe5a3f63fe1b453f1dd420371b1 Remove ::fIsBareMultisigStd global (MacroFake) fadc14e4f514e7167723285e0ac3d4a7149bbee6 Remove ::dustRelayFee (MacroFake) fa8a7f01fe1b6db98097021276ed5d929faadbec Remove ::IsStandardTx(tx, reason) alias (MacroFake) fa7a9114e59b81b50584311a4ab2b3e9a8d956bd test: Remove unused cs_main (MacroFake) fa9cba7afb73c01bd2c8fefd662dfc80dd98c5e8 Remove ::incrementalRelayFee and ::minRelayTxFee globals (MacroFake) fa148602e67fe035b1b21eff6c0b656919ac2d45 Remove ::fRequireStandard global (MacroFake) fa468bdfb62dec286cb977db78d3e47b64dafeba Return optional error from ApplyArgsManOptions (MacroFake) Pull request description: This change is good because: * It moves module-specific init-logic out of the bloated init.cpp * It removes a global from validation.cpp and places it into the data structure that needs it (mempool) ACKs for top commit: glozow: re ACK ddddd69 ryanofsky: Code review ACK ddddd6913b1bdee1cad89a32d363306ea1f7b8d7 ariard: Light Code Review ACK ddddd69 Tree-SHA512: 9de2ce601cfcaa4dfd7d1c92270568895ce8702ccdffb59829fbe9618eab0fd88d738afef33ed66988c66861115e0340e881056bfb71e2aed4af2440bd37eb1e
2022-08-03Remove unused SetTip(nullptr) codeMacroFake
2022-08-02Merge bitcoin/bitcoin#25272: wallet: guard and alert about a wallet invalid ↵Andrew Chow
state during chain sync 9e04cfaa76cf9dda27f10359dd43e78dd3268e09 test: add coverage for wallet inconsistent state during sync (furszy) 77de5c693ffe8dc0afa5e40126e9b0e9cc547e04 wallet: guard and alert about a wallet invalid state during chain sync (furszy) Pull request description: Follow-up work to my comment in #25239. Guarding and alerting the user about a wallet invalid state during chain synchronization. #### Explanation if the `AddToWallet` tx write fails, the method returns a wtx `nullptr` without removing the recently added transaction from the wallet's map. Which makes that `AddToWalletIfInvolvingMe` return false (even when the tx is on the wallet's map already), --> which makes `SyncTransaction` skip the `MarkInputsDirty` call --> which leads to a wallet invalid state where the inputs of this new transaction are not marked dirty, while the transaction that spends them still exist on the in-memory wallet tx map. Plus, as we only store the arriving transaction inside `AddToWalletIfInvolvingMe` when we synchronize/scan block/s from the chain and nowhere else, it makes sense to treat the transaction db write error as a runtime error to notify the user about the problem. Otherwise, the user will lose all the not stored transactions after a wallet shutdown (without be able to recover them automatically on the next startup because the chain sync would be above the block where the txs arrived). Note: On purpose, the first commit adds test coverage for it. Showing how the wallet can end up in an invalid state. The second commit corrects it with the proposed solution. ACKs for top commit: achow101: re-ACK 9e04cfaa76cf9dda27f10359dd43e78dd3268e09 jonatack: ACK 9e04cfaa76cf9dda27f10359dd43e78dd3268e09 Tree-SHA512: 81f765eca40547d7764833d8ccfae686b67c7728c84271bc00dc51272de643dafc270014079dcc9727b47577ba67b340aeb5f981588b54e69a06abea6958aa96
2022-08-02sort after scripted-diffMacroFake
2022-08-02scripted-diff: Move mempool_args to src/nodeMacroFake
It is part of the node library. Also, it won't be moved to the kernel lib, as it will be pruned of ArgsManager. -BEGIN VERIFY SCRIPT- # Move module git mv src/mempool_args.cpp src/node/ git mv src/mempool_args.h src/node/ # Replacements sed -i 's:mempool_args\.h:node/mempool_args.h:g' $(git grep -l mempool_args) sed -i 's:mempool_args\.cpp:node/mempool_args.cpp:g' $(git grep -l mempool_args) sed -i 's:MEMPOOL_ARGS_H:NODE_MEMPOOL_ARGS_H:g' $(git grep -l MEMPOOL_ARGS_H) -END VERIFY SCRIPT-
2022-08-02Remove ::g_max_datacarrier_bytes globalMacroFake
2022-08-02Pass datacarrier setting into IsStandardMacroFake
2022-08-02Combine datacarrier globals into oneMacroFake
2022-08-02Remove ::GetVirtualTransactionSize() aliasMacroFake
Each alias is only used in one place.
2022-08-02Remove ::fIsBareMultisigStd globalMacroFake
2022-08-02Remove ::dustRelayFeeMacroFake
2022-08-02Remove ::IsStandardTx(tx, reason) aliasMacroFake
Apart from tests, it is only used in one place, so there is no need for an alias.
2022-08-02test: Remove unused cs_mainMacroFake
2022-08-02Remove ::incrementalRelayFee and ::minRelayTxFee globalsMacroFake
2022-08-02Remove ::fRequireStandard globalMacroFake
2022-08-02Return optional error from ApplyArgsManOptionsMacroFake
Also pass in a (for now unused) reference to the params. Both changes are needed for the next commit.
2022-08-02Merge bitcoin/bitcoin#25736: univalue: Remove unused and confusing set*() ↵MacroFake
return value fa7bef2e80c9c290b3e97114cfa7afdea5cbd53c univalue: Remove unused and confusing set*() return value (MacroFake) Pull request description: The value is: * currently unused, and useless without `[[nodiscard]]` * confusing, because it is always `true`, unless a num-string is set Instead of adding `[[nodiscard]]`, throw when setting is not possible. ACKs for top commit: shaavan: ACK fa7bef2e80c9c290b3e97114cfa7afdea5cbd53c aureleoules: ACK fa7bef2e80c9c290b3e97114cfa7afdea5cbd53c. Tree-SHA512: 0d74f96f34cb93b66019ab75e12334c964630cc83434f22e58cc7a4fff2ee96a5767e42ab37f08acb67aeacba6811b09c75f1edc68d5e903ccfc59b1c82de891
2022-08-01Merge bitcoin/bitcoin#25651: refactor: make all ↵MacroFake
NodeImpl/ChainImpl/ExternalSignerImpl members public, rm temporaries, simplify 4bedfd702ad878645c51bea6ee8ce40d8c0bd3da refactor: remove unneeded temporaries in node/interfaces, simplify code (Jon Atack) b27ba169ebd4a8e4ec29be590f03a4d0da61a0cc refactor: make all NodeImpl/ChainImpl/ExternalSignerImpl members public (Jon Atack) Pull request description: - Make all `NodeImpl`, `ChainImpl` and `ExternalSignerImpl` class members `public` (and document why), to be consistent in all the `*Impl` classes in `src/node/interfaces.cpp` and `src/wallet/interfaces.cpp` and to help future reviewers and contributors. - Remove unneeded temporaries in `NodeImpl` and `ChainImpl` methods in `src/node/interfaces.cpp` and simplify, to make the code easier to read and understand and to improve performance by avoiding unnecessary move operations. ACKs for top commit: ryanofsky: Code review ACK 4bedfd702ad878645c51bea6ee8ce40d8c0bd3da. Changes since last review, applying suggested style & simplifiying first commit. Also avoiding another lock in second commit. Tree-SHA512: 112f7cad5e2838c94c5b79d61328f42fe75fdb97f401ab49eccf696fc2c6a8a0c0ee55ec974c0602acf7423f78bb82e90eb8a0cc531e1d3347f73b7c83685504
2022-08-01Merge bitcoin/bitcoin#25663: tracing: do not use `coin` after move in ↵MacroFake
`CCoinsViewCache::AddCoin` f8e228476f54b36beacc3fd09038dfeebdac6b3c tracing: do not use `coin` after move in `CCoinsViewCache::AddCoin` (Seibart Nedor) Pull request description: This is fix for https://github.com/bitcoin/bitcoin/issues/25640. ACKs for top commit: 0xB10C: ACK f8e228476f54b36beacc3fd09038dfeebdac6b3c Tree-SHA512: e7643ac8e6b6247aaf250f44572c4b458da4aea030ac0268227564e6857200e9c23efe325cfc535f46498cbeccaf46301551efeeb54b062f71d2dcf1ffe71fb8
2022-08-01Merge bitcoin/bitcoin#25610: wallet, rpc: Opt in to RBF by defaultMacroFake
ab3c06db1aed979847158505f3df1dcea9fd6c2b doc: Release notes for default RBF (Andrew Chow) 61d9149e7804e2cec8fecf4150837344322eb301 rpc: Default rbf enabled (Andrew Chow) e3c33637bac7db8ae56ab497df10911fad773981 wallet: Enable -walletrbf by default (Andrew Chow) Pull request description: The GUI currently opts in to RBF by default, but RPCs do not, and `-walletrbf` is default disabled. This PR makes the default in those two places to also opt in. The last time this was proposed (#9527), the primary objections were the novelty at the time, the inability to bump transactions, and the gui not having the option to disable rbf. In the 5 years since, RBF usage has steadily grown, with ~27% of txs opting in. The GUI has the option to enable/disable RBF, and is also defaulted to having it enabled. And we have the ability to bump RBF'd transactions in both the RPC and the GUI. So I think it makes sense to finally change the default to always opt in to RBF. ACKs for top commit: darosior: reACK ab3c06db1aed979847158505f3df1dcea9fd6c2b aureleoules: ACK ab3c06db1aed979847158505f3df1dcea9fd6c2b. glozow: utACK ab3c06db1a Tree-SHA512: 81b012c5033e270f86a87a6a196ccc549eb54b158eebf88e917cc6621d40d7bdcd1566b602688907dd5d364b95a557b29f97dce869cea512e339588262c027b6
2022-08-01Merge bitcoin/bitcoin#25739: Update leveldb subtreefanquake
f608f25313d2867a6abdfc38abdb86da40924cfc Squashed 'src/leveldb/' changes from 330dd6235f..22f1e4a02f (fanquake) Pull request description: Replaces #25463 (for master). Includes: - https://github.com/bitcoin-core/leveldb-subtree/pull/32 Guix Build (arm64): ```bash ed41ae2555ae3b638b65d870cef385805e621481831ae992e84645f5c234af63 guix-build-bec911e37ac8/output/arm-linux-gnueabihf/SHA256SUMS.part 7d8237026bfccedee0e56e14d7b89cf2dcb52195b94070dc4b6c3c6970fdc774 guix-build-bec911e37ac8/output/arm-linux-gnueabihf/bitcoin-bec911e37ac8-arm-linux-gnueabihf-debug.tar.gz 1afeff9d70864be66f7ac48d31d1977c7844e2cc117d3f0438fb2f9c6f6a56a4 guix-build-bec911e37ac8/output/arm-linux-gnueabihf/bitcoin-bec911e37ac8-arm-linux-gnueabihf.tar.gz ab2df265897a0142093b582d3c61e2b17a713d93e478b47c7aa2b0a677295007 guix-build-bec911e37ac8/output/arm64-apple-darwin/SHA256SUMS.part 025a52babdee479800801e951f6fe1fe490e1f5fe8361104eb85e7b0319cdb37 guix-build-bec911e37ac8/output/arm64-apple-darwin/bitcoin-bec911e37ac8-arm64-apple-darwin-unsigned.dmg 7b2992bf5543891b1e6ef4f48c52fc5febc58cc31ccf4307edd27d4d630aa54a guix-build-bec911e37ac8/output/arm64-apple-darwin/bitcoin-bec911e37ac8-arm64-apple-darwin-unsigned.tar.gz 3d93eb009ab6459cdca5fe767795f94ba5e00e5969e44bac19c7a5f344d42030 guix-build-bec911e37ac8/output/arm64-apple-darwin/bitcoin-bec911e37ac8-arm64-apple-darwin.tar.gz b8c30c5c561c96bc4e280ececc0dd1cd673bc6194591b848903aa6c54c9d37cf guix-build-bec911e37ac8/output/dist-archive/bitcoin-bec911e37ac8.tar.gz cb2632b9b5ff473e504d3d6244191eb5a852718f8ac8bb1032ba1c65e07d6b3f guix-build-bec911e37ac8/output/powerpc64-linux-gnu/SHA256SUMS.part 26ecc2f42ce37f8bd7ba24bf2f80f493cd1fd45b58409de71c44e2445c291c8c guix-build-bec911e37ac8/output/powerpc64-linux-gnu/bitcoin-bec911e37ac8-powerpc64-linux-gnu-debug.tar.gz 4a83381ea472cc71b4a1c6569483a7e85a5f53065c1633828bf7a75d357b0297 guix-build-bec911e37ac8/output/powerpc64-linux-gnu/bitcoin-bec911e37ac8-powerpc64-linux-gnu.tar.gz 455a9af5e7ee1c2857d87abec29284ae7bb447cf7cb2b3befa2f8e0a0a8cbec6 guix-build-bec911e37ac8/output/powerpc64le-linux-gnu/SHA256SUMS.part 3445f53fd150032ec3c3f324e001b4ecf72728900961a7a7789c32ceb7616617 guix-build-bec911e37ac8/output/powerpc64le-linux-gnu/bitcoin-bec911e37ac8-powerpc64le-linux-gnu-debug.tar.gz baefeaac88bb4fbf8662c8e1150b043aa2534535a82e828c13e971d2c5fa5cbd guix-build-bec911e37ac8/output/powerpc64le-linux-gnu/bitcoin-bec911e37ac8-powerpc64le-linux-gnu.tar.gz 543484396a47def1636d4e54d4a105c7093265c8896165a4140edec10aeef880 guix-build-bec911e37ac8/output/riscv64-linux-gnu/SHA256SUMS.part ac1f6e57016703f1319a3ef80014581aee96053e56525b8cd11ada2395496b86 guix-build-bec911e37ac8/output/riscv64-linux-gnu/bitcoin-bec911e37ac8-riscv64-linux-gnu-debug.tar.gz f9a2b65ed21f777524b078046c84b98239b0fbb92eae8d840bc7a25cab0eec6d guix-build-bec911e37ac8/output/riscv64-linux-gnu/bitcoin-bec911e37ac8-riscv64-linux-gnu.tar.gz a08ed0ee78ab1c4c815ba8368f1a21d3bd4327ce1104cb3793d63edd2bde1ef4 guix-build-bec911e37ac8/output/x86_64-apple-darwin/SHA256SUMS.part 74059397c6c8f0e899b60415d1aae04f2f7df18b8f39cea9f314e5e0c2e0ede6 guix-build-bec911e37ac8/output/x86_64-apple-darwin/bitcoin-bec911e37ac8-x86_64-apple-darwin-unsigned.dmg 6909ff6f59b78059d505f2b98e6fad63a4e3deb843566061c4cd6e94be1de066 guix-build-bec911e37ac8/output/x86_64-apple-darwin/bitcoin-bec911e37ac8-x86_64-apple-darwin-unsigned.tar.gz 21b45719d927422acc69662108e7255d8cd0b1d832493e70c622c1b1b3a3a314 guix-build-bec911e37ac8/output/x86_64-apple-darwin/bitcoin-bec911e37ac8-x86_64-apple-darwin.tar.gz b5fdee6fd2dbcdaa6302f388c7fcaa6d130ff91fd5760e19facf6e24c7216ef6 guix-build-bec911e37ac8/output/x86_64-linux-gnu/SHA256SUMS.part 56db24b0b0d2f463c8085a12502977c6d4f4ee5b85484e52522361f54ab3a6aa guix-build-bec911e37ac8/output/x86_64-linux-gnu/bitcoin-bec911e37ac8-x86_64-linux-gnu-debug.tar.gz 96dbe08e2ed0f68fe734dd6f0280c2d22af7b56c84debde424367054f118173f guix-build-bec911e37ac8/output/x86_64-linux-gnu/bitcoin-bec911e37ac8-x86_64-linux-gnu.tar.gz 70ac424b229befb2834a8a02ce27551e3ddde2d438497e8c11a3cedf0cea5d3c guix-build-bec911e37ac8/output/x86_64-w64-mingw32/SHA256SUMS.part bad144a599b28e8dc0018cc2fd1754543e79df39d651e58f565c197241f2b8cb guix-build-bec911e37ac8/output/x86_64-w64-mingw32/bitcoin-bec911e37ac8-win64-debug.zip 7ce2e72621eb070a8d23b7edbaaccc9f06257b82b9851c1cad4c61f08d2c7451 guix-build-bec911e37ac8/output/x86_64-w64-mingw32/bitcoin-bec911e37ac8-win64-setup-unsigned.exe 5f726ef8b478e3ac90b93cd3ae3c38a0e7bffa5f80306c46a7535518a73251c7 guix-build-bec911e37ac8/output/x86_64-w64-mingw32/bitcoin-bec911e37ac8-win64-unsigned.tar.gz 87cf1a23e948e471ed35c6a518813505c907c58788b55665829e7f12f33bd312 guix-build-bec911e37ac8/output/x86_64-w64-mingw32/bitcoin-bec911e37ac8-win64.zip ``` ACKs for top commit: hebasto: ACK bec911e37ac826d55b789428bc07280abab76443, I have reviewed the code and it looks OK, I agree it can be merged. jarolrod: ACK bec911e37ac826d55b789428bc07280abab76443 Tree-SHA512: 190381d9489ec6cc52bb9557750925c8574f1344eb6893095e9e31e66a579bd1bc283e8cbfcba52cec3fb072985895f929103b6f5351a23f908bdd0a04b474f1
2022-07-30Merge bitcoin/bitcoin#25709: script: actually trigger the optimization in ↵MacroFake
BuildScript 00897d0677c2cb6609b90d52b89907c8b50d6de0 script: actually trigger the optimization in BuildScript (Antoine Poinsot) Pull request description: The counter is an optimization over calling `ret.empty()`. It was suggested that the compiler would realize `cnt` is only `0` on the first iteration, and not actually emit the check and conditional. This optimization was actually not triggered at all, since we incremented `cnt` at the beginning of the first iteration. Fix it by incrementing at the end instead. This was reported by Github user "Janus". Fixes #25682. Note this does *not* change semantics. It only allows the optimization of moving instead of copying on first `CScript` element to actually be reachable. ACKs for top commit: sipa: utACK 00897d0677c2cb6609b90d52b89907c8b50d6de0 MarcoFalke: review ACK 00897d0677c2cb6609b90d52b89907c8b50d6de0 Tree-SHA512: b575bd444b0cd2fe754ec5f3e2f3f53d2696d5dcebedcace1e38be372c82365e75938dfe185429ed5a83efe1a395e204bfb33efe56c10defc5811eaee50580e3
2022-07-30addrman: Use system time instead of adjusted network timeMarcoFalke
2022-07-29refactor: remove unneeded temporaries in node/interfaces, simplify codeJon Atack
- make the code easier to read and understand - improve performance by avoiding unnecessary move operations - the cleaner, simpler, and easier to read the code is, the better chance the compiler has at implementing it well
2022-07-29refactor: make all NodeImpl/ChainImpl/ExternalSignerImpl members publicJon Atack
as the classes themselves are private, and to be consistent within all the *Impl classes in src/node/interfaces.cpp and src/wallet/interfaces.cpp following this order: public: // ... virtual methods ... // ... nonvirtual helper methods ... // ... data members ... and add documentation in src/node/interfaces.cpp and src/wallet/interfaces.cpp to help future reviewers and contributors.
2022-07-29Merge bitcoin/bitcoin#25571: refactor: Make mapBlocksUnknownParent local, ↵fanquake
and rename it dd065dae9fcebd6806ff67703ffa8128e80b97cc refactor: Make mapBlocksUnknownParent local, and rename it (Hennadii Stepanov) Pull request description: This PR is a second attempt at #19594. This PR has two motivations: - Improve code hygiene by eliminating a global variable, `mapBlocksUnknownParent` - Fix fuzz test OOM when running too long ([see #19594 comment](https://github.com/bitcoin/bitcoin/pull/19594#issuecomment-958801638)) A minor added advantage is to release `mapBlocksUnknownParent` memory when the reindexing phase is done. The current situation is somewhat similar to a memory leak because this map exists unused for the remaining lifetime of the process. It's true that this map should be empty of data elements after use, but its internal metadata (indexing structures, etc.) can have non-trivial size because there can be many thousands of simultaneous elements in this map. This PR helps our efforts to reduce the use of global variables. This variable isn't just global, it's hidden inside a function (it looks like a local variable but has the `static` attribute). This global variable exists because the `-reindex` processing code calls `LoadExternalBlockFile()` multiple times (once for each block file), but that function must preserve some state between calls (the `mapBlocksUnknownParent` map). This PR fixes this by allocating this map as a local variable in the caller's scope and passing it in on each call. When reindexing completes, the map goes out of scope and is deallocated. I tested this manually by reindexing on mainnet and signet. Also, the existing `feature_reindex.py` functional test passes. ACKs for top commit: mzumsande: re-ACK dd065dae9fcebd6806ff67703ffa8128e80b97cc theStack: re-ACK dd065dae9fcebd6806ff67703ffa8128e80b97cc shaavan: reACK dd065dae9fcebd6806ff67703ffa8128e80b97cc Tree-SHA512: 9cd20e44d2fa1096dd405bc107bc065ea8f904f5b3f63080341b08d8cf57b790df565f58815c2f331377d044d5306708b4bf6bdfc5ef8d0ed85d8e97d744732c
2022-07-29Merge bitcoin/bitcoin#25683: refactor: log `nEvicted` message in ↵MacroFake
`LimitOrphans` then return void b4b657ba57a2ce31b3c21ea9245aad26d5b06a57 refactor: log `nEvicted` message in `LimitOrphans` then return void (chinggg) Pull request description: Fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=49347 LimitOrphans() can log expired tx and it should log evicted tx as well instead of returning the `nEvicted` number for caller to print the message. Since `LimitOrphans()` now returns void, the redundant assertion check in fuzz test is also removed. Top commit has no ACKs. Tree-SHA512: 18c41702321b0e59812590cd389f3163831d431f4ebdc3b3e1e0698496a6bdbac52288f28f779237a58813c6717da1a35e8933d509822978ff726c1b13cfc778
2022-07-29Update leveldb-subtree subtree to latest upstreamfanquake
2022-07-29univalue: Remove unused and confusing set*() return valueMacroFake
2022-07-28Merge bitcoin/bitcoin#24584: wallet: avoid mixing different `OutputTypes` ↵Andrew Chow
during coin selection 71d1d13627ccd27319f347e2d8167c8fe8a433f4 test: add unit test for AvailableCoins (josibake) da03cb41a4ce15ebceee7fa4a4fdd2d3602fe284 test: functional test for new coin selection logic (josibake) 438e04845bf3302b7f459a50e88a1b772527f1e6 wallet: run coin selection by `OutputType` (josibake) 77b07072061c59f50c69be29fbcddf0d433e1077 refactor: use CoinsResult struct in SelectCoins (josibake) 2e67291ca3ab2d8f498fa910738ca655fde11c5e refactor: store by OutputType in CoinsResult (josibake) Pull request description: # Concept Following https://github.com/bitcoin/bitcoin/pull/23789, Bitcoin Core wallet will now generate a change address that matches the payment address type. This improves privacy by not revealing which of the outputs is the change at the time of the transaction in scenarios where the input address types differ from the payment address type. However, information about the change can be leaked in a later transaction. This proposal attempts to address that concern. ## Leaking information in a later transaction Consider the following scenario: ![mix input types(1)](https://user-images.githubusercontent.com/7444140/158597086-788339b0-c698-4b60-bd45-9ede4cd3a483.png) 1. Alice has a wallet with bech32 type UTXOs and pays Bob, who gives her a P2SH address 2. Alice's wallet generates a P2SH change output, preserving her privacy in `txid: a` 3. Alice then pays Carol, who gives her a bech32 address 4. Alice's wallet combines the P2SH UTXO with a bech32 UTXO and `txid: b` has two bech32 outputs From a chain analysis perspective, it is reasonable to infer that the P2SH input in `txid: b` was the change from `txid: a`. To avoid leaking information in this scenario, Alice's wallet should avoid picking the P2SH output and instead fund the transaction with only bech32 Outputs. If the payment to Carol can be funded with just the P2SH output, it should be preferred over the bech32 outputs as this will convert the P2SH UTXO to bech32 UTXOs via the payment and change outputs of the new transaction. **TLDR;** Avoid mixing output types, spend non-default `OutputTypes` when it is economical to do so. # Approach `AvailableCoins` now populates a struct, which makes it easier to access coins by `OutputType`. Coin selection tries to find a funding solution by each output type and chooses the most economical by waste metric. If a solution can't be found without mixing, coin selection runs over the entire wallet, allowing mixing, which is the same as the current behavior. I've also added a functional test (`test/functional/wallet_avoid_mixing_output_types.py`) and unit test (`src/wallet/test/availablecoins_tests.cpp`. ACKs for top commit: achow101: re-ACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4 aureleoules: ACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4. Xekyo: reACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4 via `git range-diff master 6530d19 71d1d13` LarryRuane: ACK 71d1d13627ccd27319f347e2d8167c8fe8a433f4 Tree-SHA512: 2e0716efdae5adf5479446fabc731ae81d595131d3b8bade98b64ba323d0e0c6d964a67f8c14c89c428998bda47993fa924f3cfca1529e2bd49eaa4e31b7e426
2022-07-28Merge bitcoin/bitcoin#25674: add unit tests for RBF rules in isolationglozow
c320cddb1b57a9c9911054fc440f7a12aaea61b5 [unit tests] individual RBF Rules in isolation (glozow) Pull request description: Test each RBF rule more thoroughly and in isolation so we're not relying on things like overall mempool acceptance logic, ordering of mempool checks, RPC results, etc. RBF was pretty recently refactored out, so there isn't much unit test coverage. From https://marcofalke.github.io/btc_cov/test_bitcoin.coverage/src/policy/rbf.cpp.gcov.html: ![image](https://user-images.githubusercontent.com/25183001/180783280-6777f4b4-ef95-462a-b414-1a9e268836a6.png) ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/25674/commits/c320cddb1b57a9c9911054fc440f7a12aaea61b5 jonatack: ACK c320cddb1b57a9c9911054fc440f7a12aaea61b5 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25674/commits/c320cddb1b57a9c9911054fc440f7a12aaea61b5 Tree-SHA512: dab555214496255801b9ea92b7bf708bba1ff23edf055c85e29be5eab7d7a863440ee19588aacdce54b2c03feaa4b5963eb159ed89473560bd228737cbfec160
2022-07-28[unit tests] individual RBF Rules in isolationglozow
Test each component of the RBF policy in isolation. Unlike the RBF functional tests, these do not rely on things like RPC results, mempool submission, etc.
2022-07-28refactor: log `nEvicted` message in `LimitOrphans` then return voidchinggg
`LimitOrphans()` can log expired tx and it should log evicted tx as well instead of returning the number for caller to print the message. Since `LimitOrphans()` now return void, the redundant assertion check in fuzz test is also removed.
2022-07-27test: Drop unused boost workaroundHennadii Stepanov
2022-07-27Merge bitcoin/bitcoin#24697: refactor address relay timefanquake
fa64dd6673767992eb4e0e775fb0afdfd298610d refactor: Use type-safe std::chrono for addrman time (MarcoFalke) fa2ae373f33fa76dc4e435e7cb4778055aa6afd5 Add type-safe AdjustedTime() getter to timedata (MarcoFalke) fa5103a9f5f8559ab005c0b012d3d3a8057d81fb Add ChronoFormatter to serialize (MarcoFalke) fa253d385f9201ea10beacecf3e0e80ff69f3138 util: Add HoursDouble (MarcoFalke) fa21fc60c292ab947b2200e54201440f16230566 scripted-diff: Rename addrman time symbols (MarcoFalke) fa9284c3e9acec4b44b2560256f27b3d78c753e2 refactor: Remove not needed std::max (MacroFake) Pull request description: Those refactors are overlapping with, but otherwise largely unrelated to #24662. ACKs for top commit: naumenkogs: utACK fa64dd6673767992eb4e0e775fb0afdfd298610d dergoegge: Code review ACK fa64dd6673767992eb4e0e775fb0afdfd298610d Tree-SHA512: a50625e78036e7220a11997e6d9b6c6b317cb38ce02b1835fb41cbee2d8bfb1faf29b29d8990be78d6b5e15e9a9d8dec33bf25fa439b47610ef708950969724b
2022-07-26Merge bitcoin/bitcoin#25705: tidy: enable readability-redundant-string-initMacroFake
49168df073d465450b1da4a506ac7ea24fbbb877 tidy: enable readability-redundant-string-init (fanquake) 4ddd746bf9714a209b2f82918a70c4fe81d895c9 refactor: remove unnecessary string initializations (fanquake) Pull request description: Remove unnecessary `std::string` = "" initializations. Enable `readability-redundant-string-init`. See: https://releases.llvm.org/14.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-redundant-string-init.html ACKs for top commit: shaavan: ACK 49168df073d465450b1da4a506ac7ea24fbbb87 Tree-SHA512: 69e72a434908c9166d407551657b310361ae2ef0170f8289cb1c2b8e96a4632be718c0d55cb12af03a3c3d621d9583eced88e5e9d924abb0a8b1a9b36c903d66
2022-07-26Merge bitcoin/bitcoin#24974: refactor: Make FEELER_SLEEP_WINDOW type safe ↵fanquake
(std::chrono) fa74e726c414f5f7a1e63126a69463491f66e0ec refactor: Make FEELER_SLEEP_WINDOW type safe (std::chrono) (MacroFake) fa3b3cb9b5d944d34b1d5ac3e102ac333482a475 Expose underlying clock in CThreadInterrupt (MacroFake) Pull request description: This gets rid of the `value*1000` manual conversion. ACKs for top commit: naumenkogs: utACK fa74e726c414f5f7a1e63126a69463491f66e0ec dergoegge: Code review ACK fa74e726c414f5f7a1e63126a69463491f66e0ec Tree-SHA512: 90409c05c25f0dd2f1c4dead78f707ebfd78b7d84ea4db9fcefd9c4958a1a3338ac657cd9e99eb8b47d52d4485fa3c947dce4ee1559fb56ae65878685e1ed9a3
2022-07-26Merge bitcoin/bitcoin#25694: refactor: Make CTransaction constructor explicitMacroFake
fa2247a9f9754d90ea60f254f6c0ed881c55772b refactor: Make CTransaction constructor explicit (MacroFake) Pull request description: It involves calculating two hashes, so the performance impact should be made explicit. Also, add the module to iwyu. ACKs for top commit: aureleoules: ACK fa2247a9f9754d90ea60f254f6c0ed881c55772b. hebasto: ACK fa2247a9f9754d90ea60f254f6c0ed881c55772b, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: e236c352a472c7edfd4f0319a5a16a59f627b0ab7eb8531b53c75d730a3fa3e990a939978dcd952cd73e647925fc79bfa6d9fd87624bbc3ef180f40f95acef19
2022-07-26script: actually trigger the optimization in BuildScriptAntoine Poinsot
The counter is an optimization over calling `ret.empty()`. It was suggested that the compiler would realize `cnt` is only `0` on the first iteration, and not actually emit the check and conditional. This optimization was actually not triggered at all, since we incremented `cnt` at the beginning of the first iteration. Fix it by incrementing at the end instead. This was reported by Github user "Janus".
2022-07-26Merge bitcoin/bitcoin#25689: fuzz: Remove no-op SetMempoolConstraintsglozow
fa57c449cf45a4f1df195970c711bba8f02f3cc6 fuzz: Remove no-op SetMempoolConstraints (MacroFake) Pull request description: Now that the mempool no longer uses the args manager (after commit e4e201dfd9a9dbd8e22cac688dbbde16234cd937), there is no point setting the mempool limits after it is constructed. Fix that by setting them once right before the mempool is constructed. ACKs for top commit: dongcarl: utACK fa57c449cf45a4f1df195970c711bba8f02f3cc6 glozow: utACK fa57c449cf45a4f1df195970c711bba8f02f3cc6 Tree-SHA512: d236f9cdcee8c2076272b82c97f8a5942f1ecf119ab36edafd42088ef97554592348a61e1fbe504fd52b30301ef0177813042599ad12e8cb95b4a20586c85bb0
generated by cgit v1.2.3 (git 2.26.2) at 2024-09-01 20:41:06 +0000