Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Dec 3, 2018

nChainTx is an implementation detail that shouldn't be exposed without a wrapper that comes with appropriate documentation.

@maflcko maflcko force-pushed the Mf1812-docNchainTx branch 2 times, most recently from fa92430 to fac019a Compare December 3, 2018 23:29
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.

Concept ACK

Could also update these?:

if (pindex->nChainTx)

if (pindex->nChainTx && !pindex->IsValid(BLOCK_VALID_SCRIPTS) &&

} else if (block->nChainTx == 0) {

@Empact
Copy link
Contributor

Empact commented Dec 4, 2018

Concept ACK

  • would make sense as a method on CBlockIndex
  • maybe try for privatizing nChainTx altogether?

@maflcko maflcko force-pushed the Mf1812-docNchainTx branch 2 times, most recently from fab51e1 to fa55fa9 Compare December 4, 2018 15:49
@maflcko maflcko force-pushed the Mf1812-docNchainTx branch from fa55fa9 to fa4fc88 Compare December 4, 2018 15:52
@promag
Copy link
Contributor

promag commented Dec 5, 2018

utACK fa4fc88.

@Empact
Copy link
Contributor

Empact commented Dec 7, 2018

utACK fa4fc88

@laanwj
Copy link
Member

laanwj commented Dec 7, 2018

utACK fa4fc88

@laanwj laanwj merged commit fa4fc88 into bitcoin:master Dec 7, 2018
laanwj added a commit that referenced this pull request Dec 7, 2018
…iate

fa4fc88 validation: Add and use HaveTxsDownloaded where appropriate (MarcoFalke)

Pull request description:

  `nChainTx` is an implementation detail that shouldn't be exposed without a wrapper that comes with appropriate documentation.

Tree-SHA512: 56ab7378c2ce97794498724c271f861de982de69099e90ec09632a26230ae6fded3c59668adb378bd64dcb8ef714769b970210977b88a53fc7550774ddba3d59
@maflcko maflcko deleted the Mf1812-docNchainTx branch December 7, 2018 16:30
laanwj added a commit that referenced this pull request Oct 28, 2019
…yHeaderTip

fa39809 Avoid unused call to GuessVerificationProgress in NotifyHeaderTip (MarcoFalke)

Pull request description:

  `GuessVerificationProgress` for a header (not a block) is always 0 because the number of txs in the block can not be determined from the header alone. Anyway, this result was never used, so we can optimize this call by hardcoding 0.

  This is the next commit in a series of changes toward removing nChainTx (see #14863, #13875)

ACKs for top commit:
  promag:
    Code review ACK fa39809, missed that.
  laanwj:
    ACK fa39809

Tree-SHA512: 11016f8dbb1af1cf75241948d1ad35eac0c79d1311cd0db8c6ec806df2a9e3dc5f998dbd66ccbad5d84564e6cec7fe21ce7a2a13c2b34c746e2d3b31aa1db53a
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Dec 6, 2019
Summary:
`nChainTx` is an implementation detail that shouldn't be exposed without a wrapper that comes with appropriate documentation.

Backport of Bitcoin Core PR14863
bitcoin/bitcoin#14863

Test Plan:
```
make check
```

Reviewers: Fabien, #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D4552
jonspock pushed a commit to jonspock/devault that referenced this pull request Oct 2, 2020
Summary:
`nChainTx` is an implementation detail that shouldn't be exposed without a wrapper that comes with appropriate documentation.

Backport of Bitcoin Core PR14863
bitcoin/bitcoin#14863

Test Plan:
```
make check
```

Reviewers: Fabien, #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D4552
jonspock pushed a commit to jonspock/devault that referenced this pull request Oct 5, 2020
Summary:
`nChainTx` is an implementation detail that shouldn't be exposed without a wrapper that comes with appropriate documentation.

Backport of Bitcoin Core PR14863
bitcoin/bitcoin#14863

Test Plan:
```
make check
```

Reviewers: Fabien, #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D4552
jonspock pushed a commit to devaultcrypto/devault that referenced this pull request Oct 10, 2020
Summary:
`nChainTx` is an implementation detail that shouldn't be exposed without a wrapper that comes with appropriate documentation.

Backport of Bitcoin Core PR14863
bitcoin/bitcoin#14863

Test Plan:
```
make check
```

Reviewers: Fabien, #bitcoin_abc, deadalnix

Reviewed By: #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D4552
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 11, 2021
…appropriate

fa4fc88 validation: Add and use HaveTxsDownloaded where appropriate (MarcoFalke)

Pull request description:

  `nChainTx` is an implementation detail that shouldn't be exposed without a wrapper that comes with appropriate documentation.

Tree-SHA512: 56ab7378c2ce97794498724c271f861de982de69099e90ec09632a26230ae6fded3c59668adb378bd64dcb8ef714769b970210977b88a53fc7550774ddba3d59
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 15, 2021
…appropriate

fa4fc88 validation: Add and use HaveTxsDownloaded where appropriate (MarcoFalke)

Pull request description:

  `nChainTx` is an implementation detail that shouldn't be exposed without a wrapper that comes with appropriate documentation.

Tree-SHA512: 56ab7378c2ce97794498724c271f861de982de69099e90ec09632a26230ae6fded3c59668adb378bd64dcb8ef714769b970210977b88a53fc7550774ddba3d59
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
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.

4 participants