circular dependency followups #23649
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
CC @MarcoFalke, comments from #22677 (review) |
maflcko
approved these changes
Dec 2, 2021
Member
maflcko
left a comment
maflcko
left a comment
There was a problem hiding this comment.
Nice. ACK
Left one nit in case you retouch
Member
|
Thoughts on this suggested diff? diff --git a/src/txmempool.cpp b/src/txmempool.cpp
index 27fbb8acac..2cc0aa1e90 100644
--- a/src/txmempool.cpp
+++ b/src/txmempool.cpp
@@ -5,6 +5,7 @@
#include <txmempool.h>
+#include <chain.h>
#include <coins.h>
#include <consensus/consensus.h>
#include <consensus/tx_verify.h>
diff --git a/src/txmempool.h b/src/txmempool.h
index c6e08a3ca5..15a57de922 100644
--- a/src/txmempool.h
+++ b/src/txmempool.h
@@ -14,7 +14,6 @@
#include <utility>
#include <vector>
-#include <chain.h>
#include <coins.h>
#include <consensus/amount.h>
#include <indirectmap.h>
@@ -26,12 +25,13 @@
#include <util/epochguard.h>
#include <util/hasher.h>
-#include <boost/multi_index_container.hpp>
#include <boost/multi_index/hashed_index.hpp>
#include <boost/multi_index/ordered_index.hpp>
#include <boost/multi_index/sequenced_index.hpp>
+#include <boost/multi_index_container.hpp>
class CBlockIndex;
+class CChain;
class CChainState;
extern RecursiveMutex cs_main;
|
The lockpoints are not changed in this function. There is no reason to pass a pointer.
glozow
force-pushed
the
22677-followups
branch
from
c1ba9e8 to
e611763
Compare
glozow
force-pushed
the
22677-followups
branch
from
e611763 to
ddd74ff
Compare
Member
Author
|
Oops forgot the txmempool.cpp include. Added suggestions, thanks @MarcoFalke |
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Dec 3, 2021
RandyMcMillan
pushed a commit
to RandyMcMillan/mempool-tab
that referenced
this pull request
Dec 23, 2021
e55807c clean up txmempool includes (glozow) f486144 change TestLockPointValidity to take a const reference (glozow) 378643e remove unnecessary casts and use braced initialization (glozow) Pull request description: Followups from #22677 + clean up `TestLockPointValidity` ACKs for top commit: theStack: Code-review ACK e55807c Tree-SHA512: 0f7f26535b7301e2fb379e676310bdc7cfb2c5e232a6657f41dc6d3bc91583ec452eb2359ad2f2416ea12dd856f7fab3fa507a391ccf80f14de96da989281d96
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Nov 10, 2022
Summary: This is a partial backport of [[bitcoin/bitcoin#22677 | core#22677]] and [[bitcoin/bitcoin#23649 | core#23649]] bitcoin/bitcoin@1b3a11e bitcoin/bitcoin@ddd74ff Depends on D12442 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D12440
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Nov 10, 2022
Summary: This is a partial backport of [[bitcoin/bitcoin#22677 | core#22677]], [[bitcoin/bitcoin#23649 | core#23649]] and [[bitcoin/bitcoin#23683 | core#23683]] bitcoin/bitcoin@bedf246 bitcoin/bitcoin@b01784f bitcoin/bitcoin@b4adc5a Note that [[bitcoin/bitcoin#23683 | core#23683]] mostly reverts the commit from [[bitcoin/bitcoin#22677 | core#22677]]. The remaining diff after applying both commits consists of just an additional assertion and comment. Also note that core#23683 was applied out of sequence to avoid introducing a bug. In the source material, the assertion is added to the code after a piece of it was moved to validation.cpp. Depends on D12440 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D12441
Fabcien
pushed a commit
to Bitcoin-ABC/bitcoin-abc
that referenced
this pull request
Nov 10, 2022
Summary: The lockpoints are not changed in this function. There is no reason to pass a pointer. Note for review: this is backported out of order after [[bitcoin/bitcoin#23683 | core#23683]] This concludes backport of [[bitcoin/bitcoin#23649 | core#23649]] bitcoin/bitcoin@c4efc4d Depends on D12448 Test Plan: `ninja all check-all` Reviewers: #bitcoin_abc, Fabien Reviewed By: #bitcoin_abc, Fabien Differential Revision: https://reviews.bitcoinabc.org/D12453
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Followups from #22677 + clean up
TestLockPointValidity