aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2023-09-13[refactor] split setup in mempool_limit testglozow
We want to be able to re-use fill_mempool so that none of the tests affect each other. Change the logs from info to debug because they are otherwise repeated many times in the test output.
2023-09-13[test framework] add ability to spend only confirmed utxosglozow
Useful to ensure that the topologies of packages/transactions are as expected, preventing bugs caused by having unexpected mempool ancestors.
2023-09-13[validation] don't LimitMempoolSize in any subpackage submissionsglozow
Don't do any mempool evictions until package validation is done, preventing the mempool minimum feerate from changing. Whether we submit transactions separately or as a package depends on whether they meet the mempool minimum feerate threshold, so it's best that the value not change while we are evaluating a package. This avoids a situation where we have a CPFP package in which the parents meet the mempool minimum feerate and are submitted by themselves, but they are evicted before we have submitted the child.
2023-09-13[validation] return correct result when already-in-mempool tx gets evictedglozow
Bug fix: a transaction may be in the mempool when package evaluation begins (so it is added to results_final with MEMPOOL_ENTRY or DIFFERENT_WITNESS), but get evicted due to another transaction submission.
2023-09-13[refactor] back-fill results in AcceptPackageglozow
Instead of populating the last PackageMempoolAcceptResult with stuff from results_final and individual_results_nonfinal, fill results_final and create a PackageMempoolAcceptResult using that one. A future commit will add LimitMempoolSize() which may change the status of each of these transactions from "already in mempool" or "submitted to mempool" to "no longer in mempool". We will change those transactions' results here. A future commit also gets rid of the last AcceptSubPackage outside of the loop. It makes more sense to use results_final as the place where all results end up.
2023-09-13[validation] make PackageMempoolAcceptResult members mutableglozow
After the PackageMempoolAcceptResult is returned from AcceptMultipleTransactions, leave room for results to change due to LimitMempool() eviction.
2023-09-13[validation] add AcceptSubPackage to delegate Accept* calls and clean up m_viewglozow
(1) Call AcceptSingleTransaction when there is only 1 transaction in the subpackage. This avoids calling PackageMempoolChecks() which enforces rules that don't need to be applied for a single transaction, i.e. disabling CPFP carve out. There is a slight change in the error type returned, as shown in the txpackage_tests change. When a transaction is the last one left in the package and its fee is too low, this returns a PCKG_TX instead of PCKG_POLICY. This interface is clearer; "package-fee-too-low" for 1 transaction would be a bit misleading. (2) Clean up m_view and m_viewmempool so that coins created in this sub-package evaluation are not available for other sub-package evaluations. The contents of the mempool may change, so coins that are available now might not be later.
2023-09-13[CCoinsViewMemPool] track non-base coins and allow Resetglozow
Temporary coins should not be available in separate subpackage submissions. Any mempool coins that are cached in m_view should be removed whenever mempool contents change, as they may be spent or no longer exist.
2023-09-13[policy] check for duplicate txids in packageglozow
Duplicates of normal transactions would be found by looking for conflicting inputs, but this doesn't catch identical empty transactions. These wouldn't be valid but exiting early is good and AcceptPackage's result sanity checks assume non-duplicate transactions.
2023-09-13fuzz: Rework addr fuzzingMarcoFalke
* Replace ConsumeDeserializationParams with V1, because V2 is unconditionally checked as well. * Also fuzz CAddress::Format::Disk in the address_deserialize fuzz target.
2023-09-13fuzz: Drop unused params from serialize helpersMarcoFalke
With the ser-type and ser-version going away, it seems unlikely that there is need for them in the future, so just remove them.
2023-09-13make DisconnectedBlockTransactions responsible for its own memory managementglozow
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
2023-09-13MOVEONLY: DisconnectedBlockTransactions to its own fileglozow
This struct is only used in validation + tests and has very little to do with txmempool.
2023-09-13rewrite DisconnectedBlockTransactions as a list + mapglozow
And encapsulate underlying data structures to avoid misuse. It's better to use stdlib instead of boost when we can achieve the same thing. Behavior change: the number returned by DynamicMemoryUsage for the same transactions is higher (due to change in usage or more accurate accounting), which effectively decreases the maximum amount of transactions kept for resubmission in a reorg. Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
2023-09-13ci: clang-17 for fuzz and tsanMarcoFalke
2023-09-13Do not use std::vector = {} to release memoryPieter Wuille
2023-09-13add std::list to memusageglozow
2023-09-13[bench] DisconnectedBlockTransactionsglozow
2023-09-12[refactor] Remove compat.h from kernel headersTheCharlatan
This commit makes compat.h no longer a required include for users of the libbitcoinkernel. Including compat.h imports a bunch of platform-specific definitions. This commit is part of the libbitcoinkernel project, namely its stage 1 step 3: Decouple most non-consensus headers from libbitcoinkernel.
2023-09-12[refactor] Remove netaddress.h from kernel headersTheCharlatan
Move functions requiring the netaddress.h include out of libbitcoinkernel source files. The netaddress.h file contains many non-consensus related definitions and should thus not be part of the libbitcoinkernel. This commit makes netaddress.h no longer a required include for users of the libbitcoinkernel. This commit is part of the libbitcoinkernel project, namely its stage 1 step 3: Decouple most non-consensus headers from libbitcoinkernel.
2023-09-12[refactor] Add CChainParams member to CConnmanTheCharlatan
This is done in preparation to the next commit, but has the nice effect of removing one further data structure relying on the global `Params()`.
2023-09-12[refactor] Add missing includes for next commitTheCharlatan
2023-09-12kernel: Move MessageStartChars to its own fileTheCharlatan
The protocol.h file contains many non-consensus related definitions and should thus not be part of the libbitcoinkernel. This commit makes protocol.h no longer a required include for users of the libbitcoinkernel. This commit is part of the libbitcoinkernel project, namely its stage 1 step 3: Decouple most non-consensus headers from libbitcoinkernel. Co-Authored-By: Cory Fields <cory-nospam-@coryfields.com>
2023-09-12[refactor] Define MessageStartChars as std::arrayTheCharlatan
2023-09-12[refactor] Allow std::array<std::byte, N> in serialize.hMarcoFalke
This is already possible for C-style arrays, so allow it for C++11 std::array as well.
2023-09-12gui: macOS, make appMenuBar part of the main app windowfurszy
By moving the appMenuBar destruction responsibility to the QT framework, we ensure the disconnection of the submenus signals prior to the destruction of the main app window. The standalone menu bar may have served a purpose in earlier versions when it didn't contain actions that directly open specific screens within the main application window. However, at present, all the actions within the appMenuBar lead to the opening of screens within the main app window. So, the absence of a main app window makes these actions essentially pointless.
2023-09-12Merge bitcoin/bitcoin#28101: doc, refactor: changing -torcontrol help to ↵Andrew Chow
specify that a default port is used 9a84200cfc994eebf38c46919b20e0c0261799ae doc, refactor: Changing -torcontrol help to specify that a default port is used (kevkevin) Pull request description: Right now when we get the help for -torcontrol it says that there is a default ip and port we dont specify if there is a specified ip that we would also use port 9051 as default Also I create a new const instead of using 9051 directly in the function linking this PR because this was discussed here https://github.com/bitcoin/bitcoin/pull/28018 ACKs for top commit: jonatack: re-ACK 9a84200cfc994eebf38c46919b20e0c0261799ae achow101: ACK 9a84200cfc994eebf38c46919b20e0c0261799ae MarnixCroes: utACK 9a84200cfc994eebf38c46919b20e0c0261799ae kristapsk: utACK 9a84200cfc994eebf38c46919b20e0c0261799ae Tree-SHA512: 21d9e65f3c280a2853a9cf60d4e93e8d72caccea106206d1862c19535bde7ea6ada7f55e6ea19a1fc0f59dbe791ec6fc4084fdbe7fa6d6991fa89c62070db637
2023-09-12Merge bitcoin/bitcoin#28414: wallet rpc: return final tx hex from ↵Andrew Chow
walletprocesspsbt if complete 2e249b922762f19d6ae61edaad062f31bc2849f3 doc: add release note for PR #28414 (Matthew Zipkin) 4614332fc4514f63fcbe9e6de507f7bb9b7e87e9 test: remove unnecessary finalizepsbt rpc calls (ismaelsadeeq) e3d484b603abff69c6ebfca5cfb78cf82743d090 wallet rpc: return final tx hex from walletprocesspsbt if complete (Matthew Zipkin) Pull request description: See https://github.com/bitcoin/bitcoin/pull/28363#discussion_r1315753887 `walletprocesspsbt` currently returns a base64-encoded PSBT and a boolean indicating if the tx is "complete". If it is complete, the base64 PSBT can be finalized with `finalizepsbt` which returns the hex-encoded transaction suitable for `sendrawtransaction`. With this patch, `walletprocesspsbt` return object will ALSO include the broadcast-able hex string if the tx is already final. This saves users the extra step of calling `finalizepsbt` assuming they have already inspected and approve the transaction from earlier steps. ACKs for top commit: ismaelsadeeq: re ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 BrandonOdiwuor: re ACK 2e249b9 Randy808: Tested ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 achow101: ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 ishaanam: ACK 2e249b922762f19d6ae61edaad062f31bc2849f3 Tree-SHA512: 229c1103265a9b4248f080935a7ad5607c3be3f9a096a9ab6554093b2cd8aa8b4d1fa55b1b97d3925ba208dbc3ccba4e4d37c40e1491db0d27ba3d9fe98f931e
2023-09-12wallet: Use CTxDestination in CRecipient rather than scriptPubKeyAndrew Chow
2023-09-12Add PubKeyDestination for P2PK scriptsAndrew Chow
P2PK scripts are not PKHash destinations, they should have their own type. This also results in no longer showing a p2pkh address for p2pk outputs. However for backwards compatibility, ListCoinst will still do this conversion.
2023-09-12Allow CNoDestination to represent a raw scriptAndrew Chow
Even if a script is not a standard destination type, it can still be useful to have a CTxDestination that stores the script.
2023-09-12Make WitnessUnknown members privateAndrew Chow
Make sure that nothing else can change WitnessUnknown's data members by making them private. Also change the program to use a vector rather than C-style array.
2023-09-12[fuzz] Use afl++ shared-memory fuzzingdergoegge
Using shared-memory is faster than reading from stdin, see https://github.com/AFLplusplus/AFLplusplus/blob/7d2122e0596132f9344a5d0896020ebc79cd33db/instrumentation/README.persistent_mode.md
2023-09-12Merge bitcoin/bitcoin#28105: doc: Clarify that -fstack-reuse=all bugs exist ↵fanquake
on all versions of GCC fabb419a3caafc00fbbc533fee14fab7a5d2a2c6 doc: Clarify that -fstack-reuse=all bugs exist on all versions of GCC (MarcoFalke) Pull request description: This is a follow-up to commit 7b850bc2a1cd8547a2dbb5a18173f53439601220. While the test case no longer reproduces, the general class of `-fstack-reuse` bugs still exists in all versions of GCC. The workaround can never be removed, unless the whole class of bugs is fixed. ACKs for top commit: fanquake: ACK fabb419a3caafc00fbbc533fee14fab7a5d2a2c6 Tree-SHA512: 566e14fe82d13dda4f7b8cca90c6de75006d14828906b936780716d5b5b31de9b36a904aa7cfc9820ccdfb4d3224a8437f502f25f7230da5abe87c927123f0c8
2023-09-12Merge bitcoin/bitcoin#28448: rpc: Deprecate rpcserialversion=0fanquake
971bae9174293b79f1f29822d662b31a2ba62234 rpc: Deprecate rpcserialversion=0 (Anthony Towns) Pull request description: This option was introduced in #9194 to ease the transition to segwit; now that most libraries and apps have been updated it should no longer be necessary. ACKs for top commit: MarcoFalke: review ACK 971bae9174293b79f1f29822d662b31a2ba62234 Randy808: Code Review ACK 971bae9174293b79f1f29822d662b31a2ba62234 glozow: ACK 971bae9174293b79f1f29822d662b31a2ba62234, seems appropriate to remove. Thanks for looking at usage in https://github.com/bitcoin/bitcoin/pull/28448#issuecomment-1714699556 Tree-SHA512: 6880314504281e9d7c288bd159f8cadefb3e653ac2dd148396810f7f5a27ba352ecfe720eb2dbc6172b57820cb9a2a254dcb2585881abae43811013505f0e09a
2023-09-12doc: Clarify that -fstack-reuse=all bugs exist on all versions of GCCMarcoFalke
2023-09-12scripted-diff: Rename CBufferedFile to BufferedFileMarcoFalke
While touching all constructors in the previous commit, the class name can be adjusted to comply with the style guide. -BEGIN VERIFY SCRIPT- sed -i 's/CBufferedFile/BufferedFile/g' $( git grep -l CBufferedFile ) -END VERIFY SCRIPT-
2023-09-12Remove unused GetType() from CBufferedFile and CAutoFileMarcoFalke
GetType() is only called in tests, so it is unused and can be removed.
2023-09-12dbwrapper: Use DataStream for batch operationsTheCharlatan
2023-09-12Merge bitcoin/bitcoin#28427: index: coinstats reorg, fail when block cannot ↵fanquake
be reversed c0bf667912064960df194ea94150976b34f7c267 index: add [nodiscard] attribute to functions writing to the db (furszy) eef595560e9ecf3a0d1db4d8ea7ecc33a49d839f index: coinstats reorg, fail when block cannot be reversed (furszy) Pull request description: Found it while reviewing https://github.com/bitcoin/bitcoin/pull/24230#discussion_r1310863359. During a reorg, continuing execution when a block cannot be reversed leaves the coinstats index in an inconsistent state. This was surely overlooked when 'CustomRewind' was implemented. ACKs for top commit: ryanofsky: Code review ACK c0bf667912064960df194ea94150976b34f7c267. Only change since last review is new commit adding [[nodiscard]] Tree-SHA512: f4fc8522508d23e4fff09a29c935971819b1bd3b2a260e08e2e2b72f9340980d74fbec742a58fe216baf61d27de057c7c8300e8fa075f8507cd1227f128af909
2023-09-11qt: Silence `-Wcast-function-type` warningHennadii Stepanov
Required for https://github.com/bitcoin/bitcoin/pull/25972. Picked from https://trac.nginx.org/nginx/ticket/1865.
2023-09-11Merge bitcoin/bitcoin#28433: Follow-up to BIP324 connection supportfanquake
64704386b28ce3a1ab607a946ec729286da8faa6 doc: fix typos and mistakes in BIP324 code comments (Pieter Wuille) 9bde93df2c84b6d5f333aa56cbd0b28b6ad337b0 net: do not use send buffer to store/cache garbage (Pieter Wuille) b6934fd03f080d437acb1fd2b665503c3d6de785 net: merge V2Transport constructors, move key gen (Pieter Wuille) Pull request description: This addresses a few remaining comments on #28196: * Deduplicate the `V2Transport` constructors (https://github.com/bitcoin/bitcoin/pull/28196#discussion_r1318573111) * Do not use the send buffer to store garbage (https://github.com/bitcoin/bitcoin/pull/28196#discussion_r1319134141) * Fix typo (https://github.com/bitcoin/bitcoin/pull/28196#discussion_r1315179378) In addition, also fix an incorrect description in `V2Transport::SendState` (it claimed garbage was sent in the `READY` state, but it's in the `AWAITING_KEY` state). ACKs for top commit: naumenkogs: ACK 64704386b28ce3a1ab607a946ec729286da8faa6 theStack: Code-review ACK 64704386b28ce3a1ab607a946ec729286da8faa6 Tree-SHA512: 4bf6d2fe73c8054502d0b60e9de1722f8b3dd269c2dd6bf67197c3fb6eabcf047b6360cdab3c1fd5504215c2ac4ac2890a022780efc30ff583776242c8112451
2023-09-11rpc: Deprecate rpcserialversion=0Anthony Towns
2023-09-10doc: fix typos and mistakes in BIP324 code commentsPieter Wuille
2023-09-10net: do not use send buffer to store/cache garbagePieter Wuille
Before this commit the V2Transport::m_send_buffer is used to store the garbage: * During MAYBE_V1 state, it's there despite not being sent. * During AWAITING_KEY state, while it is being sent. * At the end of the AWAITING_KEY state it cannot be wiped as it's still needed to compute the garbage authentication packet. Change this by introducing a separate m_send_garbage field, taking over the first and last role listed above. This means the garbage is only in the send buffer when it's actually being sent, removing a few special cases related to this.
2023-09-10net: merge V2Transport constructors, move key genPieter Wuille
This removes the ability for BIP324Cipher to generate its own key, moving that responsibility to the caller (mostly, V2Transport). This allows us to write the random-key V2Transport constructor by delegating to the explicit-key one.
2023-09-10Merge bitcoin/bitcoin#27944: test: various USDT functional test cleanups ↵fanquake
(27831 follow-ups) 9f55773a370a0d039e727445ccee6b84e05f562a test: refactor: usdt_mempool: store all events (stickies-v) bc432704505eb165dd86de39ea3434c6fb7a2514 test: refactor: remove unnecessary nonlocal (stickies-v) 326db63a6819813db55ba0d01ab4fe80f7a0d818 test: log sanity check assertion failures (stickies-v) f5525ad6808df6afc38e5c6e4767ab577e30629c test: store utxocache events (stickies-v) f1b99ac94fb77340c4d3a5b4bbc3df28009bc773 test: refactor: deduplicate handle_utxocache_* logic (stickies-v) ad90ba36bd930f00753643cd1fe0af72d1c828c2 test: refactor: rename inbound to is_inbound (stickies-v) afc0224cdbe73e326addf5fb98a3e95d941f2104 test: refactor: remove unnecessary blocks_checked counter (stickies-v) Pull request description: Various cleanups to the USDT functional tests, largely (but not exclusively) follow-ups to https://github.com/bitcoin/bitcoin/pull/27831#pullrequestreview-1491438045. Except for slightly different logging behaviour in "test: store utxocache events" and "test: log sanity check assertion failures", this is a refactor PR, removing unnecessary code and (imo) making it more readable and maintainable. The rationale for each change is in the corresponding commit message. Note: except for "test: store utxocache events" (which relies on its parent, and I separated into two commits because we may want the parent but not the child), all commits are stand-alone and I'm okay with dropping one/multiple commits if they turn out to be controversial or undesired. ACKs for top commit: 0xB10C: ACK 9f55773a370a0d039e727445ccee6b84e05f562a. Reviewed the code and ran the USDT interface tests. I stepped through the commits and think all changes are reasonable. Tree-SHA512: 6c37a0265b6c26d4f9552a056a690b8f86f7304bd33b4419febd8b17369cf6af799cb87c16df35d0c2a1b839ad31de24661d4384eafa88816c2051c522fd3bf5
2023-09-09Merge bitcoin/bitcoin#28431: Remove needless `GetTransactionOutputWeight` helperfanquake
8d6228fc1fe72db3ac38ab9c853be0256bed5f24 consensus/validation.h: remove needless GetTransactionOutputWeight helper (Antoine Poinsot) Pull request description: Introduced in #26567. My bad. Thanks AJ for noticing. ACKs for top commit: ajtowns: utACK 8d6228fc1fe72db3ac38ab9c853be0256bed5f24 Tree-SHA512: cf13647b4aac82fb6a54ae0338e3928e9bdf226ed4f5e91d529996328471744132db2bee9676e0b3f40a8bbe0e0ca51a9e5f91560a84e0f33597290551a1ee18
2023-09-09Merge bitcoin/bitcoin#28428: Hard-code version number value for ↵fanquake
CBlockLocator and CDiskBlockIndex e73d2a8018def940afadb5d699b18f39e882c1fc refactor: remove clientversion include from dbwrapper.h (Cory Fields) 4240a082b81d8ceb7615b1b4ca0d2857382f317b refactor: Use DataStream now that version/type are unused (Cory Fields) f15f790618d328abd207d55e6291229eb2a8643f Remove version/hashing options from CBlockLocator/CDiskBlockIndex (Cory Fields) Pull request description: This is also a much simpler replacement for #28327. There are version fields in `CBlockLocator` and `CDiskBlockIndex` that have always been written but discarded when read. I intended to convert them to use SerParams as introduced by #25284, which [ended up looking like this](https://github.com/theuni/bitcoin/commit/3e3af451652322c92e8e41cf918e69d608ec7c77). However because we don't currently have any definition of what a hash value would mean for either one of those, and we've never assigned the version field any meaning, I think it's better to just not worry about them. If we ever need to assign meaning in the future, we can introduce `SerParams` as was done for `CAddress`. As for the dummy values chosen: `CDiskBlockIndex::DUMMY_VERSION` was easy as the highest ever client version, and I don't expect any objection there. `CBlockLocator::DUMMY_VERSION` is hard-coded to the higest _PROTOCOL_ version ever used. This is to avoid a sudden bump that would be visible on the network if CLIENT_VERSION were used instead. In the future, if we ever need to use the value, we can discard anything in the CLIENT_VERSION range (for a few years as needed), as it's quite a bit higher. While reviewing, I suggest looking at the throwaway `SerParams` commit above as it shows where the call-sites are. I believe that should be enough to convince one's self that hashing is never used. ACKs for top commit: TheCharlatan: Re-ACK e73d2a8018def940afadb5d699b18f39e882c1fc ajtowns: reACK e73d2a8018def940afadb5d699b18f39e882c1fc Tree-SHA512: 45b0dd7c2e918493e2ee92a8e35320ad17991cb8908cb811150a96c5fd584ce177c775baeeb8675a602c90b9ba9203b8cefc0a2a0c6a71078b1d9c2b41e1f3ba
2023-09-08gui: macOS, do not process dock icon actions during shutdownfurszy
As the 'QMenuBar' is created without a parent window in MacOS, the app crashes when the user presses the shutdown button and, right after it, triggers any action in the menu bar. This happens because the QMenuBar is manually deleted in the BitcoinGUI destructor but the events attached to it children actions are not disconnected, so QActions events such us the 'QMenu::aboutToShow' could try to access null pointers. Instead of guarding every single QAction pointer inside the QMenu::aboutToShow slot, or manually disconnecting all registered events in the destructor, we can check if a shutdown was requested and discard the event. The 'node' field is a ref whose memory is held by the main application class, so it is safe to use here. Events are disconnected prior destructing the main application object. Furthermore, the 'MacDockIconHandler::dockIconClicked' signal can make the app crash during shutdown for the very same reason. The 'show()' call triggers the 'QApplication::focusWindowChanged' event, which is connected to the 'minimize_action' QAction, which is also part of the app menu bar, which could no longer exist.
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 13:38:35 +0000