Skip to content

Conversation

@theStack
Copy link
Contributor

@theStack theStack commented Oct 7, 2021

These helpers are exclusively used in txmempool.cpp, hence they should also be moved there. The PR also contains a commit which fixes const-correctness for parents in CTxMemPool::UpdateAncestorsOf and declares them as reference to avoid a copy.

These helpers are exclusively used in txmempool.cpp, hence they
should also be moved there.

Can be reviewed with "--color-moved=dimmed-zebra".
@practicalswift
Copy link
Contributor

Concept ACK

@DrahtBot
Copy link
Contributor

DrahtBot commented Oct 7, 2021

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #23418 (Fix signed integer overflow in prioritisetransaction RPC by MarcoFalke)

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.

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 65aaf94. Verified move-only commit locally.

CTxMemPoolEntry::Parents parents = it->GetMemPoolParents();
const CTxMemPoolEntry::Parents& parents = it->GetMemPoolParentsConst();
// add or remove this tx as a child of each parent
for (const CTxMemPoolEntry& parent : parents) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

9947ce6

Note that parent can be non-const. Maybe GetMemPoolParentsConst should return std::set<const CTxMemPoolEntryRef, CompareIteratorByHash> instead.

@maflcko maflcko merged commit e2b5192 into bitcoin:master Nov 3, 2021
@theStack theStack deleted the 202110-refactor-various_mempool_cleanups branch November 3, 2021 09:32
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Nov 3, 2021
….h to .cpp file

65aaf94 refactor: move `update_*` structs from txmempool.h to .cpp file (Sebastian Falbesoner)
9947ce6 refactor: use const reference for parents in `CTxMemPool::UpdateAncestorsOf` (Sebastian Falbesoner)

Pull request description:

  These helpers are exclusively used in txmempool.cpp, hence they should also be moved there. The PR also contains a commit which fixes const-correctness for parents in `CTxMemPool::UpdateAncestorsOf` and declares them as reference to avoid a copy.

ACKs for top commit:
  promag:
    Code review ACK 65aaf94. Verified move-only commit locally.

Tree-SHA512: 7ce29f3ba0e68b5355001f27725b00f6d54cc993015356eb40b61b8cdd17db49b980f4c3d798c8e0c940d245dc3a72c474bb9ff3c0ee971ead450786076812c2
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 19, 2022
Summary:
> refactor: use const reference for parents in `CTxMemPool::UpdateAncestorsOf`

> refactor: move update_* structs from txmempool.h to .cpp file
>
> These helpers are exclusively used in txmempool.cpp, hence they
> should also be moved there.
>
> Can be reviewed with "--color-moved=dimmed-zebra".

This is a backport of [[bitcoin/bitcoin#23211 | core#23211]]

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, sdulfari

Reviewed By: #bitcoin_abc, sdulfari

Subscribers: sdulfari

Differential Revision: https://reviews.bitcoinabc.org/D12287
@bitcoin bitcoin locked and limited conversation to collaborators Nov 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants