wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict #29680

Eunovo · 2024-03-20T09:23:35Z

This PR implements a fix for the issue described in #29435.

The problem is that the wallet is unable to abandon transactions that have unrelated parent conflicts. The solution implemented here, augments the mempool transaction REPLACED signal with the double-spending transaction which the wallet stores and watches for in Block notifications. A map is added to the wallet to track conflicting tx ids and their child transactions. The entry is erased when the double-spending tx is removed from MemPool.

DrahtBot · 2024-03-20T09:23:38Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	josibake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30072 (refactor prep for package rbf by instagibbs)
#30019 (test: use assert_greater_than util by kevkevinpal)
#29415 (Broadcast own transactions only via short-lived Tor or I2P connections by vasild)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot · 2024-03-20T10:20:28Z

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

_{Debug: https://github.com/bitcoin/bitcoin/runs/22873809948}

Eunovo · 2024-03-26T15:15:38Z

Draft until I've gotten some more feedback on the approach.

glozow · 2024-03-27T12:12:48Z

cc @achow101?

achow101 · 2024-03-27T17:50:56Z

I'm actually wondering now if it would be sufficient to just have the MemPoolRejectReason include the replacement txid for replacements, and the conflicting block hash for conflicts. If our tx is in the mempool, we should get these notifications when it is removed which I think would be enough without having to be looking for the parents or looking to see if we have any descendants?

Eunovo · 2024-03-28T06:24:40Z

I'm actually wondering now if it would be sufficient to just have the MemPoolRejectReason include the replacement txid for replacements, and the conflicting block hash for conflicts. If our tx is in the mempool, we should get these notifications when it is removed which I think would be enough without having to be looking for the parents or looking to see if we have any descendants?

@achow101 I think this makes sense. Once we add conflicting block hash for conflicts then we can safely mark wallet tx as conflicted which should solve the issue. What would we still need the replacement txid for?

EDIT
On second thought, I have realized that the wallet might not get the Conflict MemPoolRemovalReason for its tx. The wallet tx may be kicked out of the Mempool early due to replacement so when CtxMemPool::removeForBlock is called, the wallet tx will no longer be in the Mempool. Therefore, the issue is not completely solved by adding conflicting block hash to Conflict MemPoolRemovalReason, But adding it is still useful for handling cases where a new block is received that conflicts with wallet tx.

When you say "replacement txid", are you referring to the txid of the tx causing the wallet tx to kicked out? if so, I believe the current fix implemented here does exactly what you're describing, See 635e2e4

achow101 · 2024-03-28T17:24:46Z

When you say "replacement txid", are you referring to the txid of the tx causing the wallet tx to kicked out?

Yes, we need the replacement txid for instances where the tx is removed by replacement rather than a block conflict.

if so, I believe the current fix implemented here does exactly what you're describing, See 635e2e4

It appears to also be watching for replacements of those replacements too, and I think that is unnecessary.

Eunovo · 2024-03-28T19:44:58Z

It appears to also be watching for replacements of those replacements too, and I think that is unnecessary.

Thanks @achow101. Yes, I added that when I realized that if the replacement is also replaced then the check in the BlockConnected callback will fail to mark the wallet tx as conflicted. Unless for some reason, the replacement cannot be replaced?

Eunovo · 2024-04-09T16:52:08Z

Rebased 532d25f to 8ee9629

It appears to also be watching for replacements of those replacements too, and I think that is unnecessary.

@achow101 I took this out because the new replacement is not guaranteed to conflict with the original wallet transaction

Added conflicting_block_hash and conflicting_block_height to ConflictReason in 8b5d3d7 and
used this information to mark wallet tx has conflicted in 8ee9629.

I had to use RecursiveUpdateTxState directly in 8ee9629 because CWallet::MarkConflicted checks that the conflicting block height is not more than the m_last_block_processed by the wallet but transactionRemovedFromMempool is triggered before blockConnected so the wallet hasn't had a chance to process the block causing the conflict notification. I had to force the wallet txs update. I wonder what the repercussions for doing this are.

Curious to see what others think.

EDIT

This PR now modifies AddToWalletIfInovlingMe to do this for all TxStateBlockConflicted transactions. See e868b2d

Now marking this PR as ready for review.

DrahtBot · 2024-04-09T18:43:27Z

🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the
documentation.

Possibly this is due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.

Leave a comment here, if you need help tracking down a confusing failure.

_{Debug: https://github.com/bitcoin/bitcoin/runs/23623413738}

Eunovo · 2024-04-21T03:43:30Z

Putting in draft while I fix falling test

josibake · 2024-05-13T13:05:24Z

Concept ACK

Seems like a reasonable approach to the problem described in #29435 , will dig in more. Using a variant as a replacement for the Enum seems a bit odd at first glance and requires a lot of duplicate code for each struct. Perhaps another approach could be a class for RemovalReason, which contains the Enum as a field and then uses a std::variant to hold the data needed by the wallet, if any. Would be more extensible, and then the to string method could be defined once on the class. Special logic per reason can also be handled all in the same place by switching on the Enum value and taking appropriate action per reason.

EDIT: this approach might also make it easier to do the first commit as a scripted-diff

Eunovo · 2024-05-13T14:25:00Z

Perhaps another approach could be a class for RemovalReason, which contains the Enum as a field and then uses a std::variant to hold the data needed by the wallet, if any. Would be more extensible, and then the to string method could be defined once on the class. Special logic per reason can also be handled all in the same place by switching on the Enum value and taking appropriate action per reason.

Thanks for the review @josibake. Will this approach still require the definition of structs to hold each data for each reason?

josibake · 2024-05-13T14:39:36Z

Will this approach still require the definition of structs to hold each data for each reason?

I don't think so: only the Conflict and Replace reasons need extra data, and from what I understand it can only be one reason at any given time. Given that, you could have a field on the new class for removal data which is a std::variant of the possible data types. For example, std::variant<CTransactionRef, BlockData>, where block data holds the block hash and block number.

Eunovo · 2024-05-13T15:11:30Z

I don't think so: only the Conflict and Replace reasons need extra data, and from what I understand it can only be one reason at any given time. Given that, you could have a field on the new class for removal data which is a std::variant of the possible data types. For example, std::variant<CTransactionRef, BlockData>, where block data holds the block hash and block number.

@josibake Makes sense but I'm skeptical about how this affects the removal reason usage. For example, you can just define SizeLimitReason() right now, but with this approach, you'd have to do RemovalReason(RemovalReasons::SIZE_LIMIT) and this new approach still requires the use of std::variant, so, I'm worried that the current code in 59ee08e will still be more clear.
WDYT?

josibake · 2024-05-13T15:54:23Z

The benefits I see are we can keep the existing Enum and code for converting the Enum to a string. This would also allow updating the function signatures with a scripted-diff, which makes larger refactors like this easier to verify as a reviewer.

It also feels like a more natural fit to me and my gut feeling is that having a class to encapsulate the removal reasons and necessary data will be more maintainable and extensible going forward.

Eunovo · 2024-05-13T18:47:49Z

The benefits I see are we can keep the existing Enum and code for converting the Enum to a string. This would also allow updating the function signatures with a scripted-diff, which makes larger refactors like this easier to verify as a reviewer.

It also feels like a more natural fit to me and my gut feeling is that having a class to encapsulate the removal reasons and necessary data will be more maintainable and extensible going forward.

@josibake Makes sense. I'll create a RemovalReason class

ryanofsky · 2024-05-13T20:10:29Z

Given that, you could have a field on the new class for removal data which is a std::variant of the possible data types. For example, std::variant<CTransactionRef, BlockData>, where block data holds the block hash and block number.

The drawback of a separate data field is that there is no longer a guarantee that data accompanying the reason is always present when the reason is set, and always absent when it is not set. So it is possible for code to only partially initialize the RemovalReason class or initialize it in an inconsistent state.

In general I like the idea of replacing enums with variants for more safety, and to be able to express rules about state in type definitions. But variants in c++ are more awkward than sum types in other languages, and I did not look into this specific case, so maybe the tradeoffs in this case are not worth it.

josibake · 2024-05-14T08:21:59Z

The drawback of a separate data field is that there is no longer a guarantee that data accompanying the reason is always present when the reason is set, and always absent when it is not set. So it is possible for code to only partially initialize the RemovalReason class or initialize it in an inconsistent state.

Seems easily addressed with a constructor, no? Something like:

class RemovedReason {
public:
    MemPoolRemovalReason m_reason;
    std::variant<std::monostate, CTxReference, BlockData> m_extra_data;

    // Constructor for reasons that don't require extra data
    RemovedReason(MemPoolRemovalReason r) : reason(r) {
        if (requiresExtraData(r)) {
            throw std::invalid_argument("reason X requires data Y etc");
        }
    }

    // Constructor needing CTxRef
    RemovedReason(RemovalReason r, const CTxRef& data) : reason(r), extra_data(data) {
        if (!IsA(r)) {
            throw std::invalid_argument("CTxRef is required for reason A, got bla");
        }
    }

    // Constructor needing BlockData
    RemovedReason(RemovalReason r, const BlockData& data) : reason(r), extra_data(data) {
        if (!IsB(r)) {
            throw std::invalid_argument("BlockData is required for reason B, got bla");
        }
    }

Maybe this is starting to be more complicated than just having a struct per each reason? But I'd still argue this is a better approach in that it keeps all the logic for mempool removal reasons in one place and avoids duplicating code on each struct.

Eunovo · 2024-05-14T14:26:12Z

Maybe this is starting to be more complicated than just having a struct per each reason? But I'd still argue this is a better approach in that it keeps all the logic for mempool removal reasons in one place and avoids duplicating code on each struct.

Same thing I was thinking. Using the class right now looks like it will make things more complicated. Maybe we should leave the class for a future change where it becomes necessary? @josibake @ryanofsky

ryanofsky · 2024-05-14T14:45:06Z

Seems easily addressed with a constructor, no? Something like:

Yes, but those are manual constraints that you are writing by hand rather than automatic constraints expressed implicitly in the data definition. Depending on the constructors it may also only provide runtime checking rather than compile-time checking like in your example. And if the struct is mutable could allow invalid representations of state after construction.

I don't know what is best in this particular case, I would just stand up for:

struct MyState1 { int data; };
struct MyState2 { bool flag; };
struct MyState3 {};
using MyState = std::variant<MyState1, MyState2, MyState3>;

as a good alternative to:

enum class MyState {
  STATE1,
  STATE2,
  STATE3,
};

class MyData {
  MyState m_state,
  // ... more data and methods...
};

in many cases.

Maybe we should leave the class for a future change where it becomes necessary?

I'm not sure the answer to this, but it is probably worth experimenting and choosing the approach that seems simplest.

josibake · 2024-05-14T14:57:41Z

Yes, but those are manual constraints that you are writing by hand rather than automatic constraints expressed implicitly in the data definition

Fair point, in that bugs could be introduced by someone not writing these pre-checks correctly / efficiently. I'll admit I'm not fully convinced that the struct per state approach isn't going to be harder to maintain / extend in the future, but given that I haven't convinced you guys on that point and you have convinced me there is an advantage per the struct per state approach, I'll retract my suggestion we change it 😄

This allows the mempool to send additional data with TransactionRemovedFromMempool event. Now, we can send conflicting_block_hash and conflicting_block_height for Conflicts and replacement_tx for Replacements.

Detect replacement of wallet txs and wait for confirmation of replacement tx before marking wallet tx as conflicted

DrahtBot · 2024-05-20T15:57:49Z

test/functional/wallet_conflicts.py

+            assert_equal(current_wallet.gettransaction(child_txid)["confirmations"], 0)
+
+            # Make a conflict spending parent
+            conflict_psbt = def_wallet.walletcreatefundedpsbt(inputs=[gp_utxo], outputs=[{def_wallet.getnewaddress(): 2}], fee_rate=parent_feerate*3)["psbt"]


node0 2024-05-20T11:47:13.485435Z [httpworker.2] [rpc/request.cpp:222] [parse] [rpc] ThreadRPCServer method=walletcreatefundedpsbt user=__cookie__ test 2024-05-20T11:47:13.486000Z TestFramework (ERROR): JSONRPC error Traceback (most recent call last): File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main self.run_test() File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_conflicts.py", line 41, in run_test self.test_unknown_parent_conflict() File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/wallet_conflicts.py", line 486, in test_unknown_parent_conflict conflict_psbt = def_wallet.walletcreatefundedpsbt(inputs=[gp_utxo], outputs=[{def_wallet.getnewaddress(): 2}], fee_rate=parent_feerate*3)["psbt"] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/coverage.py", line 50, in __call__ return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/ci_container_base/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/authproxy.py", line 129, in __call__ raise JSONRPCException(response['error'], status) test_framework.authproxy.JSONRPCException: Invalid amount (-3)

https://cirrus-ci.com/task/5869965947961344?logs=ci#L3151

Watch for wallet transaction conflicts triggered by adding conflicting blocks

josibake

Still reviewing the later commits, but had some initial feedback/questions for the first commit.

josibake · 2024-05-22T08:46:23Z

src/kernel/mempool_removal_reason.h


-/** Reason why a transaction was removed from the mempool,
+/** Reasons why a transaction was removed from the mempool,


in 7debac0 ("Change MemPoolRemovalReason to variant type"):

nit: I think it's more correct to leave this as "Reason." "Reasons" implies a single transaction can have multiple reasons for being removed at the same time.

josibake · 2024-05-22T08:54:43Z

src/validationinterface.cpp

@@ -19,7 +19,7 @@
 #include <unordered_map>
 #include <utility>

-std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept;
+// std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept;


in 7debac0 ("Change MemPoolRemovalReason to variant type"):

Can remove this line.

josibake · 2024-05-22T08:56:31Z

src/validationinterface.h

+class CValidationInterface;
+class CScheduler;


in 7debac0 ("Change MemPoolRemovalReason to variant type"):

AFAICT, these are unused (I was able to compile this commit fine without them). Maybe leftover from a different approach?

Same here. I'll take them out

josibake · 2024-05-22T09:05:06Z

src/wallet/wallet.h

@@ -56,7 +56,6 @@ class CKeyID;
 class CPubKey;
 class Coin;
 class SigningProvider;
-enum class MemPoolRemovalReason;


in 7debac0 ("Change MemPoolRemovalReason to variant type"):

Don't we also need to #include kernel/mempool_removal_reasons.h , for Include What You Use (IWYU)? I'll admit, I'm totally sure what our conventions are on this. cc @TheCharlatan or @maflcko

josibake · 2024-05-22T09:26:47Z

src/validation.cpp

@@ -3057,7 +3057,7 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew,
             Ticks<MillisecondsDouble>(time_chainstate) / num_blocks_total);
    // Remove conflicting transactions from the mempool.;
    if (m_mempool) {
-        m_mempool->removeForBlock(blockConnecting.vtx, pindexNew->nHeight);
+        m_mempool->removeForBlock(blockConnecting.vtx, pthisBlock->GetHash(), pindexNew->nHeight);


in 7debac0 ("Change MemPoolRemovalReason to variant type"):

Was a bit surprised that pthisBlock->GetHash() isn't returning a member variable and instead is calculating the hash each time its called. Worth mentioning we are adding an extra hash to ConnectTip. Probably negligible but wanted to mention it.

@josibake I looked around a bit and there might be something here. GetHash() is called before this point

when we read the block from disk

bitcoin/src/node/blockstorage.cpp

Line 1092 in e163d86

if (block.GetHash() != index.GetBlockHash()) {

when ActivateBestChain is called

bitcoin/src/validation.cpp

Line 3354 in e163d86

if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) {

There might be some gain in caching GetHash() but I think that has to be addressed on its own. I'll have to measure the runtimes and see its worth a PR.

I did also discover that pindexNew which is a CBlockIndex does store the blockhash, see

bitcoin/src/chain.h

Line 244 in e163d86

uint256 GetBlockHash() const

Looks like I can use that here to prevent recalculating the blockhash.

josibake · 2024-05-22T09:37:57Z

src/test/mempool_tests.cpp

@@ -15,7 +16,7 @@

 BOOST_FIXTURE_TEST_SUITE(mempool_tests, TestingSetup)

-static constexpr auto REMOVAL_REASON_DUMMY = MemPoolRemovalReason::REPLACED;
+static const auto REMOVAL_REASON_DUMMY = ReplacedReason(nullptr);


in 7debac0 ("Change MemPoolRemovalReason to variant type"):

I think it would be better to pass a dummy CTxRef here instead of nullptr.

josibake · 2024-05-22T09:46:07Z

src/kernel/mempool_removal_reason.h

+    explicit ConflictReason(const uint256& conflicting_block_hash, int conflicting_block_height) : conflicting_block_hash(conflicting_block_hash), conflicting_block_height(conflicting_block_height) {}
+    std::string toString() const noexcept {
+        return "conflict";
+    }
+};
+
+struct ReplacedReason {
+    CTransactionRef replacement_tx;
+
+    explicit ReplacedReason(const CTransactionRef replacement_tx) : replacement_tx(replacement_tx) {}


in 7debac0 ("Change MemPoolRemovalReason to variant type"):

Any reason why conflicting_block_hash is passed by reference but replacement_tx isn't?

Would also be nice if we prevented these objects from be constructed with nullptrs. I'm not sure if we have a convention in our codebase around this or other examples to point to, but would be worth looking into.

Any reason why conflicting_block_hash is passed by reference but replacement_tx isn't?

CTransactionRef is already a pointer

bitcoin/src/primitives/transaction.h

Lines 423 to 424 in e163d86

typedef std::shared_ptr<const CTransaction> CTransactionRef;

template <typename Tx> static inline CTransactionRef MakeTransactionRef(Tx&& txIn) { return std::make_shared<const CTransaction>(std::forward<Tx>(txIn)); }

Would also be nice if we prevented these objects from be constructed with nullptrs. I'm not sure if we have a convention in our codebase around this or other examples to point to, but would be worth looking into.

It looks like I may be able to do this by deleting the constructor

// Deleted constructor ReplacedReason(std::nullptr_t) = delete;

I'll test it

josibake · 2024-05-22T10:01:39Z

src/kernel/mempool_removal_reason.h

+    uint256 conflicting_block_hash;
+    unsigned int conflicting_block_height;


in 7debac0 ("Change MemPoolRemovalReason to variant type"):

I haven't finished reviewing the later commits yet, but seems odd to pass both conflicting_block_hash and conflicting_block_height. Seems like we should be able to only use conflicting_block_hash?

The TxStateBlockConflicted requires both that's why I added the both of them

DrahtBot · 2024-05-24T09:37:24Z

🐙 This pull request conflicts with the target branch and needs rebase.

DrahtBot added Wallet CI failed labels Mar 20, 2024

DrahtBot mentioned this pull request Mar 20, 2024

wallet: track mempool conflicts with wallet transactions #27307

Merged

Eunovo force-pushed the fix-unknown-parent-conflict branch from abaa8ca to 4853bb5 Compare March 22, 2024 21:13

DrahtBot removed the CI failed label Mar 22, 2024

Eunovo force-pushed the fix-unknown-parent-conflict branch 2 times, most recently from ba8c831 to 532d25f Compare March 26, 2024 10:55

DrahtBot mentioned this pull request Mar 27, 2024

Cluster size 2 package rbf #28984

Open

DrahtBot added the Needs rebase label Mar 27, 2024

Eunovo force-pushed the fix-unknown-parent-conflict branch from 532d25f to 47750d3 Compare April 9, 2024 16:35

DrahtBot removed the Needs rebase label Apr 9, 2024

Eunovo force-pushed the fix-unknown-parent-conflict branch from 47750d3 to 8ee9629 Compare April 9, 2024 18:43

DrahtBot added the CI failed label Apr 9, 2024

DrahtBot mentioned this pull request Apr 9, 2024

Broadcast own transactions only via short-lived Tor or I2P connections #29415

Open

Eunovo force-pushed the fix-unknown-parent-conflict branch from 8ee9629 to e868b2d Compare April 10, 2024 14:27

DrahtBot removed the CI failed label Apr 10, 2024

Eunovo marked this pull request as ready for review April 10, 2024 17:37

Eunovo marked this pull request as draft April 21, 2024 03:43

Eunovo force-pushed the fix-unknown-parent-conflict branch from e868b2d to e004ecf Compare April 22, 2024 10:20

Eunovo force-pushed the fix-unknown-parent-conflict branch from faea0a5 to 6bd69c3 Compare May 7, 2024 07:53

DrahtBot mentioned this pull request May 7, 2024

refactor: Simply include CTxMemPool::Options in CTxMemPool directly rather than duplicating definition #29086

Merged

Eunovo force-pushed the fix-unknown-parent-conflict branch from 6bd69c3 to ea66044 Compare May 9, 2024 15:04

DrahtBot removed the CI failed label May 11, 2024

DrahtBot added the Needs rebase label May 15, 2024

Eunovo and others added 3 commits May 20, 2024 11:04

Change MemPoolRemovalReason to variant type

7debac0

This allows the mempool to send additional data with TransactionRemovedFromMempool event. Now, we can send conflicting_block_hash and conflicting_block_height for Conflicts and replacement_tx for Replacements.

Handle double-spending of unrelated parents to wallet txs

9599f80

Detect replacement of wallet txs and wait for confirmation of replacement tx before marking wallet tx as conflicted

test: Conflict with unconfirmed parent not in wallet

10765cf

Eunovo force-pushed the fix-unknown-parent-conflict branch from ea66044 to 7da8f98 Compare May 20, 2024 11:06

DrahtBot added CI failed and removed Needs rebase labels May 20, 2024

DrahtBot reviewed May 20, 2024

View reviewed changes

Handle new blocks with unrelated parents conflicts

f7ec03e

Watch for wallet transaction conflicts triggered by adding conflicting blocks

Eunovo force-pushed the fix-unknown-parent-conflict branch from 7da8f98 to f7ec03e Compare May 21, 2024 15:51

DrahtBot removed the CI failed label May 22, 2024

josibake reviewed May 22, 2024

View reviewed changes

DrahtBot mentioned this pull request May 23, 2024

refactor prep for package rbf #30072

Merged

DrahtBot added the Needs rebase label May 24, 2024


		/** Reason why a transaction was removed from the mempool,
		/** Reasons why a transaction was removed from the mempool,

	typedef std::shared_ptr<const CTransaction> CTransactionRef;
	template <typename Tx> static inline CTransactionRef MakeTransactionRef(Tx&& txIn) { return std::make_shared<const CTransaction>(std::forward<Tx>(txIn)); }

		uint256 conflicting_block_hash;
		unsigned int conflicting_block_height;

wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict #29680

Are you sure you want to change the base?

wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict #29680

Conversation

Eunovo commented Mar 20, 2024

DrahtBot commented Mar 20, 2024 • edited

Code Coverage

Reviews

Conflicts

DrahtBot commented Mar 20, 2024

Eunovo commented Mar 26, 2024

glozow commented Mar 27, 2024

achow101 commented Mar 27, 2024

Eunovo commented Mar 28, 2024 • edited

achow101 commented Mar 28, 2024

Eunovo commented Mar 28, 2024

Eunovo commented Apr 9, 2024 • edited

DrahtBot commented Apr 9, 2024

Eunovo commented Apr 21, 2024

josibake commented May 13, 2024 • edited

Eunovo commented May 13, 2024 • edited

josibake commented May 13, 2024

Eunovo commented May 13, 2024

josibake commented May 13, 2024

Eunovo commented May 13, 2024

ryanofsky commented May 13, 2024

josibake commented May 14, 2024

Eunovo commented May 14, 2024

ryanofsky commented May 14, 2024 • edited

josibake commented May 14, 2024

Choose a reason for hiding this comment

josibake left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DrahtBot commented May 24, 2024

DrahtBot commented Mar 20, 2024 •

edited

Eunovo commented Mar 28, 2024 •

edited

Eunovo commented Apr 9, 2024 •

edited

josibake commented May 13, 2024 •

edited

Eunovo commented May 13, 2024 •

edited

ryanofsky commented May 14, 2024 •

edited