💬 brunoerg commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#issuecomment-1453928410)
> The commit message of https://github.com/bitcoin/bitcoin/commit/287bb3f4ce41341aa0aea97b8176dc790889830d "validation: add CChainState::m_disabled and ChainMan::isUsable" still mentions m_stop_use rather than m_disabled
In the pull description, it still mention `m_stop_use`.
(https://github.com/bitcoin/bitcoin/pull/25740#issuecomment-1453928410)
> The commit message of https://github.com/bitcoin/bitcoin/commit/287bb3f4ce41341aa0aea97b8176dc790889830d "validation: add CChainState::m_disabled and ChainMan::isUsable" still mentions m_stop_use rather than m_disabled
In the pull description, it still mention `m_stop_use`.
💬 stratospher commented on pull request "p2p: Prevent block index fingerprinting by sending additional getheaders messages":
(https://github.com/bitcoin/bitcoin/pull/24571#discussion_r1124917812)
did i understand this correctly? - `UpdateChainTipSet` updates chain tip set based on:
1. the whole block/valid headers we received and were told of from other nodes on the same network
2. when we switch networks, chain tip set initially would only contain [tip of active chain.](https://github.com/bitcoin/bitcoin/blob/86f19bdab78c09a124b15e2d4a72439f13281999/src/net_processing.cpp#L2653)
3. inbound and outbound connections on same network would have different chain tip set. is th
...
(https://github.com/bitcoin/bitcoin/pull/24571#discussion_r1124917812)
did i understand this correctly? - `UpdateChainTipSet` updates chain tip set based on:
1. the whole block/valid headers we received and were told of from other nodes on the same network
2. when we switch networks, chain tip set initially would only contain [tip of active chain.](https://github.com/bitcoin/bitcoin/blob/86f19bdab78c09a124b15e2d4a72439f13281999/src/net_processing.cpp#L2653)
3. inbound and outbound connections on same network would have different chain tip set. is th
...
💬 stratospher commented on pull request "p2p: Prevent block index fingerprinting by sending additional getheaders messages":
(https://github.com/bitcoin/bitcoin/pull/24571#discussion_r1124920540)
`IsAncestorOfBestHeaderOrTip` would already by contained in `IsBlockInChainTipSet` right? do we include `IsAncestorOfBestHeaderOrTip` check for performance gain instead of iterating over all the chain tips? or is there some other reason?
(https://github.com/bitcoin/bitcoin/pull/24571#discussion_r1124920540)
`IsAncestorOfBestHeaderOrTip` would already by contained in `IsBlockInChainTipSet` right? do we include `IsAncestorOfBestHeaderOrTip` check for performance gain instead of iterating over all the chain tips? or is there some other reason?
💬 adamjonas commented on issue "estimatesmartfee returns 50 sat/vB on an empty mempool for inclusion in 1008 blocks":
(https://github.com/bitcoin/bitcoin/issues/26304#issuecomment-1454129097)
> I don't think anything can be done here
This appears to be something that isn't going to be picked up in the near future.
(https://github.com/bitcoin/bitcoin/issues/26304#issuecomment-1454129097)
> I don't think anything can be done here
This appears to be something that isn't going to be picked up in the near future.
✅ adamjonas closed an issue: "estimatesmartfee returns 50 sat/vB on an empty mempool for inclusion in 1008 blocks"
(https://github.com/bitcoin/bitcoin/issues/26304)
(https://github.com/bitcoin/bitcoin/issues/26304)
💬 pinheadmz commented on issue "Versionbits should have an ignore button":
(https://github.com/bitcoin/bitcoin/issues/8266#issuecomment-1454132480)
I think this issue can be closed since the warnings were removed in https://github.com/bitcoin/bitcoin/pull/15471
(https://github.com/bitcoin/bitcoin/issues/8266#issuecomment-1454132480)
I think this issue can be closed since the warnings were removed in https://github.com/bitcoin/bitcoin/pull/15471
👍 brunoerg approved a pull request: "assumeutxo: background validation completion"
(https://github.com/bitcoin/bitcoin/pull/25740)
crACK a00d0e78510d79bc9fc57ec622aec98a65efa8c0
In 5ee22cdafd2562bcb8bf0ae6025e4b53c826382d, it creates `ChainstateManager::GetSnapshotBaseBlock` and `ChainstateManager::GetSnapshotBaseHeight`. `GetSnapshotBaseHeight` is used (not in this commit) in `MaybeCompleteSnapshotValidation` to check if IBD (_in background_) has been completed.
In c29f26b47b8ef978d8689dc0222aa663361ee6cb, it creates `CChainState::m_disabled` and I could check it's been used to point that a chainstate should no long
...
(https://github.com/bitcoin/bitcoin/pull/25740)
crACK a00d0e78510d79bc9fc57ec622aec98a65efa8c0
In 5ee22cdafd2562bcb8bf0ae6025e4b53c826382d, it creates `ChainstateManager::GetSnapshotBaseBlock` and `ChainstateManager::GetSnapshotBaseHeight`. `GetSnapshotBaseHeight` is used (not in this commit) in `MaybeCompleteSnapshotValidation` to check if IBD (_in background_) has been completed.
In c29f26b47b8ef978d8689dc0222aa663361ee6cb, it creates `CChainState::m_disabled` and I could check it's been used to point that a chainstate should no long
...
💬 pinheadmz commented on issue "Adoption of BIP39/44/49/84, and classification of (extended) pub/priv keys, addresses, mnemonics, etc":
(https://github.com/bitcoin/bitcoin/issues/17748#issuecomment-1454140876)
I think this issue has been solved by descriptor wallets. We can keep open #19151 for more discussion about using BIP39 in Bitcoin Core
(https://github.com/bitcoin/bitcoin/issues/17748#issuecomment-1454140876)
I think this issue has been solved by descriptor wallets. We can keep open #19151 for more discussion about using BIP39 in Bitcoin Core
💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#issuecomment-1454159231)
Rebased it due silent merge conflicts with #26889.
Ready to go now.
(https://github.com/bitcoin/bitcoin/pull/25806#issuecomment-1454159231)
Rebased it due silent merge conflicts with #26889.
Ready to go now.
📝 hernanmarino opened a pull request: "Fixed typos in comments to make linter happy"
(https://github.com/bitcoin/bitcoin/pull/27197)
While working on a different PR, I stumbled upon and fixed a couple of typos being reported by the linter.
(https://github.com/bitcoin/bitcoin/pull/27197)
While working on a different PR, I stumbled upon and fixed a couple of typos being reported by the linter.
👋 hernanmarino's pull request is ready for review: "Fixed typos in comments to make linter happy"
(https://github.com/bitcoin/bitcoin/pull/27197)
(https://github.com/bitcoin/bitcoin/pull/27197)
💬 theuni commented on pull request "util: add missing include and fix function signature":
(https://github.com/bitcoin/bitcoin/pull/27192#discussion_r1125077264)
Reordered. I think this change is simple enough for manual review :)
(https://github.com/bitcoin/bitcoin/pull/27192#discussion_r1125077264)
Reordered. I think this change is simple enough for manual review :)
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123812481)
I’d probably prefer using `.value()` for new code I’d write, but the asterisk-variant seems prevalent throughout this code.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123812481)
I’d probably prefer using `.value()` for new code I’d write, but the asterisk-variant seems prevalent throughout this code.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123811383)
Yes, agreed using an optional would be more explicit, however handling the optional return value would touch a bunch of lines here, and given that the result should never be empty unless something went wrong or the function got called with an empty `txids` input, I feel it’s a bit of a cosmetic improvement here.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123811383)
Yes, agreed using an optional would be more explicit, however handling the optional return value would touch a bunch of lines here, and given that the result should never be empty unless something went wrong or the function got called with an empty `txids` input, I feel it’s a bit of a cosmetic improvement here.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123823270)
Thanks, will consider, change but feel it’s okay at this time.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123823270)
Thanks, will consider, change but feel it’s okay at this time.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123754749)
I’ve removed the `reserve(…)` call here.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123754749)
I’ve removed the `reserve(…)` call here.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123819758)
As above with `GetIterVec()`, an empty vector is not a valid outcome for a call, so I tend to leave as is.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1123819758)
As above with `GetIterVec()`, an empty vector is not a valid outcome for a call, so I tend to leave as is.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1124908789)
Updated
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1124908789)
Updated
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1125071450)
Thanks, I’m not going to interfere here, because outpoints are tiny anyway, and it seems more readable without the move or constructor.
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1125071450)
Thanks, I’m not going to interfere here, because outpoints are tiny anyway, and it seems more readable without the move or constructor.
💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1124956187)
Thanks that’s much nicer
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1124956187)
Thanks that’s much nicer