π¬ achow101 commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#issuecomment-1894553295)
ACK 3ba815b42db74804e341ce15f648c2b297af55ca
(https://github.com/bitcoin/bitcoin/pull/29239#issuecomment-1894553295)
ACK 3ba815b42db74804e341ce15f648c2b297af55ca
π¬ Xaspr commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-1894553540)
I removed 26.0, installed 24.2 and kept the documents folder. It works well up to now, for about 2 hours already.
I'll post an update if this goes up to the tip. If it crashes again, I'll try 26.0 with antivirus software disabled.
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-1894553540)
I removed 26.0, installed 24.2 and kept the documents folder. It works well up to now, for about 2 hours already.
I'll post an update if this goes up to the tip. If it crashes again, I'll try 26.0 with antivirus software disabled.
π achow101 merged a pull request: "rpc: Make v2transport default for addnode RPC when enabled"
(https://github.com/bitcoin/bitcoin/pull/29239)
(https://github.com/bitcoin/bitcoin/pull/29239)
π ryanofsky opened a pull request: "Improve new LogDebug/Trace/Info/Warning/Error Macros"
(https://github.com/bitcoin/bitcoin/pull/29256)
Improve new LogDebug(), LogTrace(), LogInfo(), LogWarning(), LogError() macros introduced in #28318:
- Make them always accept log categories to make it possible to only log messages from a particular component.
- Make them compatible with wallet logging, which includes the wallet name in log messages
- Make them not rely on a global LogInstance, to provide better control of logging in controlled environments, such as unit tests that want to selectively capture log output, or libbitcoinker
...
(https://github.com/bitcoin/bitcoin/pull/29256)
Improve new LogDebug(), LogTrace(), LogInfo(), LogWarning(), LogError() macros introduced in #28318:
- Make them always accept log categories to make it possible to only log messages from a particular component.
- Make them compatible with wallet logging, which includes the wallet name in log messages
- Make them not rely on a global LogInstance, to provide better control of logging in controlled environments, such as unit tests that want to selectively capture log output, or libbitcoinker
...
π¬ TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-1894600399)
Updated 2cb38d93a8fea16bf84b072a90ac023ef345134d -> a885a166cec6d84d08600f12b25d912bd28af80e ([kernelRmKey_1](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_1) -> [kernelRmKey_2](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmKey_1..kernelRmKey_2))
* Addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453627000), re-added explicit context destructor.
* Addressed @ma
...
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-1894600399)
Updated 2cb38d93a8fea16bf84b072a90ac023ef345134d -> a885a166cec6d84d08600f12b25d912bd28af80e ([kernelRmKey_1](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_1) -> [kernelRmKey_2](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmKey_1..kernelRmKey_2))
* Addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1453627000), re-added explicit context destructor.
* Addressed @ma
...
π¬ ryanofsky commented on pull request "logging: Simplify API for level based logging":
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1454120255)
> Will see how this looks and probably open a PR
Opened #29256 with these changes. Code changes should be complete but there are some test failures so it is a draft.
(https://github.com/bitcoin/bitcoin/pull/28318#discussion_r1454120255)
> Will see how this looks and probably open a PR
Opened #29256 with these changes. Code changes should be complete but there are some test failures so it is a draft.
β
aureleoules closed a pull request: "refactor(tidy): Use C++20 contains method"
(https://github.com/bitcoin/bitcoin/pull/29191)
(https://github.com/bitcoin/bitcoin/pull/29191)
π¬ aureleoules commented on pull request "refactor(tidy): Use C++20 contains method":
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1894612555)
> FWIW I would approve this PR if it would only contain the changes that are automatically picked up by tidy and applied by running with -fix, making resolving potential conflicts purely manual.
Sure, I agree. Closing this pull.
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1894612555)
> FWIW I would approve this PR if it would only contain the changes that are automatically picked up by tidy and applied by running with -fix, making resolving potential conflicts purely manual.
Sure, I agree. Closing this pull.
π¬ ajtowns commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-1894630511)
Concept NACK from me, this seems much uglier and trickier to work with. I've already written [in depth](https://github.com/bitcoin/bitcoin/pull/28318#issuecomment-1702318968) about why the current approach makes sense, so I'll be brief here.
> Make them always accept log categories to make it possible to only log messages from a particular component.
Being able to avoid logging critical messages is adding a bug, not a feature.
> Make them less verbose by not requiring BCLog category con
...
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-1894630511)
Concept NACK from me, this seems much uglier and trickier to work with. I've already written [in depth](https://github.com/bitcoin/bitcoin/pull/28318#issuecomment-1702318968) about why the current approach makes sense, so I'll be brief here.
> Make them always accept log categories to make it possible to only log messages from a particular component.
Being able to avoid logging critical messages is adding a bug, not a feature.
> Make them less verbose by not requiring BCLog category con
...
π¬ chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1894632119)
Concept ACK, what will it take to get this merged? Stamps are absolutely ravaging the UTXO set.
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1894632119)
Concept ACK, what will it take to get this merged? Stamps are absolutely ravaging the UTXO set.
π€ mzumsande reviewed a pull request: "p2p: adaptive connections services flags"
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1825332930)
If I understand it correctly, the logic deciding whether to try and accept outbound connections, using `HasAllDesirableServiceFlags()`, relies on `IsInitialBlockDownload()` / `DEFAULT_MAX_TIP_AGE` (24h) on master, but is changed to rely on `NODE_NETWORK_LIMITED_MIN_BLOCKS` (244 blocks, ~48h) in this PR.
I think this means that the safety buffer of 144 blocks proposed in [BIP159](https://github.com/bitcoin/bips/blob/master/bip-0159.mediawiki) would have been removed. I'm not sure about that, t
...
(https://github.com/bitcoin/bitcoin/pull/28170#pullrequestreview-1825332930)
If I understand it correctly, the logic deciding whether to try and accept outbound connections, using `HasAllDesirableServiceFlags()`, relies on `IsInitialBlockDownload()` / `DEFAULT_MAX_TIP_AGE` (24h) on master, but is changed to rely on `NODE_NETWORK_LIMITED_MIN_BLOCKS` (244 blocks, ~48h) in this PR.
I think this means that the safety buffer of 144 blocks proposed in [BIP159](https://github.com/bitcoin/bips/blob/master/bip-0159.mediawiki) would have been removed. I'm not sure about that, t
...
π¬ mzumsande commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1454112543)
I don't think that `CheckForStaleTipAndEvictPeers` triggers any status updates with the current approach, only `UpdatedBlockTip` does. Maybe this is left from an earlier version? In any case, if I remove all calls to it, the test still succeeds.
(https://github.com/bitcoin/bitcoin/pull/28170#discussion_r1454112543)
I don't think that `CheckForStaleTipAndEvictPeers` triggers any status updates with the current approach, only `UpdatedBlockTip` does. Maybe this is left from an earlier version? In any case, if I remove all calls to it, the test still succeeds.
β οΈ mrdomino opened an issue: "bitcoin.org still lists laanwj's now-revoked PGP key"
(https://github.com/bitcoin/bitcoin/issues/29257)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The instructions at https://bitcoin.org/en/full-node#linux-instructions still say:
> The 0.11 and later releases are signed by [Wladimir J. van der Laanβs releases key](https://bitcoin.org/laanwj-releases.asc) with the fingerprint:
>
> `01EA 5486 DE18 A882 D4C2 6845 90C8 019E 36C2 E964`
But the key expired, and was revoked on 2023-02-11, and is no longer used to sign `SHA256SUM.a
...
(https://github.com/bitcoin/bitcoin/issues/29257)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
The instructions at https://bitcoin.org/en/full-node#linux-instructions still say:
> The 0.11 and later releases are signed by [Wladimir J. van der Laanβs releases key](https://bitcoin.org/laanwj-releases.asc) with the fingerprint:
>
> `01EA 5486 DE18 A882 D4C2 6845 90C8 019E 36C2 E964`
But the key expired, and was revoked on 2023-02-11, and is no longer used to sign `SHA256SUM.a
...
β
mrdomino closed an issue: "bitcoin.org still lists laanwj's now-revoked PGP key"
(https://github.com/bitcoin/bitcoin/issues/29257)
(https://github.com/bitcoin/bitcoin/issues/29257)
π¬ mrdomino commented on issue "bitcoin.org still lists laanwj's now-revoked PGP key":
(https://github.com/bitcoin/bitcoin/issues/29257#issuecomment-1894791931)
I see now that you guys may not own bitcoin.org.
(https://github.com/bitcoin/bitcoin/issues/29257#issuecomment-1894791931)
I see now that you guys may not own bitcoin.org.
π¬ ajtowns commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1894895307)
> Concept ACK, what will it take to get this merged? Stamps are absolutely ravaging the UTXO set.
Nobody's given anything more than a concept ack, would be unusual to merge without some actual acks. Also the code should be rebased onto master (`git rebase origin/master`), at present there's a merge commit, which would also be unusual to have merged.
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1894895307)
> Concept ACK, what will it take to get this merged? Stamps are absolutely ravaging the UTXO set.
Nobody's given anything more than a concept ack, would be unusual to merge without some actual acks. Also the code should be rebased onto master (`git rebase origin/master`), at present there's a merge commit, which would also be unusual to have merged.
π¬ luke-jr commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1894900263)
It needs a rebase, unless we're going to be okay with merge commits now (I'm fine with it, but historically it's been a blocker)
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1894900263)
It needs a rebase, unless we're going to be okay with merge commits now (I'm fine with it, but historically it's been a blocker)
π¬ ryanofsky commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-1894928988)
Hi AJ, this is a draft, and there will be some more changes which may address your concerns.
> > Make them always accept log categories to make it possible to only log messages from a particular component.
> Being able to avoid logging critical messages is adding a bug, not a feature.
Agree, but the idea here is not to discard log messages, the idea just is to attach meaningful metadata to log messages so they can be filtered by component.
> > Make them less verbose by not requiring BC
...
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-1894928988)
Hi AJ, this is a draft, and there will be some more changes which may address your concerns.
> > Make them always accept log categories to make it possible to only log messages from a particular component.
> Being able to avoid logging critical messages is adding a bug, not a feature.
Agree, but the idea here is not to discard log messages, the idea just is to attach meaningful metadata to log messages so they can be filtered by component.
> > Make them less verbose by not requiring BC
...
β οΈ ajtowns opened an issue: "Update nChainTx to 64bit type"
(https://github.com/bitcoin/bitcoin/issues/29258)
https://github.com/bitcoin/bitcoin/blob/8106b268cde8e97a7c330afdda39b6bb55e5574a/src/chain.h#L178-L186
Hey, it's 2024! `nChainTx` seems to currently be 953368191 (height 826100), which is hex `38d3 3e7f`, so we're only 22% of the way to an overflow, and would still need another 200k blocks (~ four years) full of 60 byte transactions to get there.
(https://github.com/bitcoin/bitcoin/issues/29258)
https://github.com/bitcoin/bitcoin/blob/8106b268cde8e97a7c330afdda39b6bb55e5574a/src/chain.h#L178-L186
Hey, it's 2024! `nChainTx` seems to currently be 953368191 (height 826100), which is hex `38d3 3e7f`, so we're only 22% of the way to an overflow, and would still need another 200k blocks (~ four years) full of 60 byte transactions to get there.
π shaavan approved a pull request: "refactor: Allow std::span construction from CKey"
(https://github.com/bitcoin/bitcoin/pull/29133#pullrequestreview-1826678191)
ReACK fa96d937116682f32613d31a3ae7d6f652e8146d
Changes since my last [review](https://github.com/bitcoin/bitcoin/pull/29133#pullrequestreview-1800426197)
- Removed the last commit, which was temporarily introduced to test the workings of changes in this commit.
- Tested that the above patch leads to failure in master, while successful compilation with this PR.
(https://github.com/bitcoin/bitcoin/pull/29133#pullrequestreview-1826678191)
ReACK fa96d937116682f32613d31a3ae7d6f652e8146d
Changes since my last [review](https://github.com/bitcoin/bitcoin/pull/29133#pullrequestreview-1800426197)
- Removed the last commit, which was temporarily introduced to test the workings of changes in this commit.
- Tested that the above patch leads to failure in master, while successful compilation with this PR.