Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Jan 19, 2022

Follow-up to commit 64e4963 and #23976 (comment)

@maflcko maflcko changed the title Run coin.IsSpent only once in a row mempool: Run coin.IsSpent only once in a row Jan 19, 2022
Copy link
Contributor

@theStack theStack 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 fa2bcc4

The condition false || expression is logically equivalent to just expression.

Copy link
Contributor

@w0xlt w0xlt left a comment

Choose a reason for hiding this comment

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

crACK fa2bcc4

There is no need for coin.IsSpent(), given the previous assertion.

Copy link
Contributor

@shaavan shaavan 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 fa2bcc4

The reason is nicely explained by @theStack.

The condition false || expression is logically equivalent to just expression.

Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

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

crACK fa2bcc4

@glozow
Copy link
Member

glozow commented Jan 20, 2022

utACK fa2bcc4, agree the assertion is sufficient

@maflcko maflcko merged commit e3de7cb into bitcoin:master Jan 24, 2022
@maflcko maflcko deleted the 2201-mempoolCoinOnce branch January 24, 2022 12:34
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 28, 2022
fa2bcc4 Run coin.IsSpent only once in a row (MarcoFalke)

Pull request description:

  Follow-up to commit 64e4963 and bitcoin#23976 (comment)

ACKs for top commit:
  glozow:
    utACK fa2bcc4, agree the assertion is sufficient
  theStack:
    Code-review ACK fa2bcc4
  w0xlt:
    crACK bitcoin@fa2bcc4
  shaavan:
    Code Review ACK fa2bcc4
  brunoerg:
    crACK fa2bcc4

Tree-SHA512: 3be9d6b313bf6bb835f031826c81777b4659118d839001d084e72462391cb64ba81d06a5e07fd21fcfb709a71b08892b23212a98604ce8481da489476b72f072
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Nov 10, 2022
Summary:
This is an extremely cheap function (just checks that the coin CTxOut isn't null) that doesn't need to be gated on check_ratio.

Run coin.IsSpent only once in a row

This is a partial backport of [[bitcoin/bitcoin#22677 | core#22677]] and [[bitcoin/bitcoin#24102 | core#24102]]
bitcoin/bitcoin@64e4963
bitcoin/bitcoin@fa2bcc4

Depends on D12443

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D12446
@bitcoin bitcoin locked and limited conversation to collaborators Jan 24, 2023
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