💬 joostjager commented on pull request "Make non-standard tx acceptance a peer option.":
(https://github.com/bitcoin/bitcoin/pull/27772#discussion_r1209312388)
Are you suggesting to add `-acceptnonstdtxnrpc`?
(https://github.com/bitcoin/bitcoin/pull/27772#discussion_r1209312388)
Are you suggesting to add `-acceptnonstdtxnrpc`?
💬 fanquake commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1567147860)
> I end up with three files / folders:
This is the same as the current build process, where you end up with:
1. Bitcoin-Qt.app (which Finder displays as "Bitcoin Core")
2. Bitcoin-Core.dmg
3. dist/Bitcoin-Qt.app
Happy to refine/rework this further in a follow up, but don't really want to start changing this now, as it's not new behaviour.
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1567147860)
> I end up with three files / folders:
This is the same as the current build process, where you end up with:
1. Bitcoin-Qt.app (which Finder displays as "Bitcoin Core")
2. Bitcoin-Core.dmg
3. dist/Bitcoin-Qt.app
Happy to refine/rework this further in a follow up, but don't really want to start changing this now, as it's not new behaviour.
💬 fanquake commented on pull request "Make non-standard tx acceptance a peer option.":
(https://github.com/bitcoin/bitcoin/pull/27772#issuecomment-1567155749)
> I will reply to your other feedback points within the context of https://github.com/bitcoin/bitcoin/issues/27768, to avoid spreading out the discussion across multiple places.
I think it'd be better to close this PR for now, (I agree with suhas's comments above), and continue discussion in #27768 (I see [you've responded there](https://github.com/bitcoin/bitcoin/issues/27768#issuecomment-1567115600) with further details of what you are interested in).
I don't think there's a need to keep
...
(https://github.com/bitcoin/bitcoin/pull/27772#issuecomment-1567155749)
> I will reply to your other feedback points within the context of https://github.com/bitcoin/bitcoin/issues/27768, to avoid spreading out the discussion across multiple places.
I think it'd be better to close this PR for now, (I agree with suhas's comments above), and continue discussion in #27768 (I see [you've responded there](https://github.com/bitcoin/bitcoin/issues/27768#issuecomment-1567115600) with further details of what you are interested in).
I don't think there's a need to keep
...
👍 TheCharlatan approved a pull request: "refactor: Add [[nodiscard]] where ignoring a Result return type is an error"
(https://github.com/bitcoin/bitcoin/pull/27774#pullrequestreview-1449518089)
ACK fa5680b7520c340952239c4e25ebe505d16c7c39
(https://github.com/bitcoin/bitcoin/pull/27774#pullrequestreview-1449518089)
ACK fa5680b7520c340952239c4e25ebe505d16c7c39
💬 furszy commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567161622)
> > I started there but.. do we really care?
>
> We definitely care enough to investigate further, before adding a new RPC command to fix an intermittent issue in a functional test.
>
> Looks like @theStack has done that investigation here: [#27749 (comment)](https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566354933), and @mzumsande has suggested a potential alternative fix: [#27749 (comment)](https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566554075):
>
> >
...
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567161622)
> > I started there but.. do we really care?
>
> We definitely care enough to investigate further, before adding a new RPC command to fix an intermittent issue in a functional test.
>
> Looks like @theStack has done that investigation here: [#27749 (comment)](https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566354933), and @mzumsande has suggested a potential alternative fix: [#27749 (comment)](https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1566554075):
>
> >
...
✅ joostjager closed a pull request: "Make non-standard tx acceptance a peer option."
(https://github.com/bitcoin/bitcoin/pull/27772)
(https://github.com/bitcoin/bitcoin/pull/27772)
💬 ajtowns commented on issue "Allow accepting non-standard transactions on mainnet via local rpc":
(https://github.com/bitcoin/bitcoin/issues/27768#issuecomment-1567174457)
> The reason that I am interested in non-standard transactions is the taproot annex. There seem to be many ways to benefit from this field, and it might also reduce chain space requirements for certain applications.
The way to fix that isn't to standardise uses of the annex, not to change the node to accept anything into the mempool / relay anything. See also https://github.com/bitcoin/bips/pull/1381 and https://github.com/bitcoin-inquisition/bitcoin/pull/22
(https://github.com/bitcoin/bitcoin/issues/27768#issuecomment-1567174457)
> The reason that I am interested in non-standard transactions is the taproot annex. There seem to be many ways to benefit from this field, and it might also reduce chain space requirements for certain applications.
The way to fix that isn't to standardise uses of the annex, not to change the node to accept anything into the mempool / relay anything. See also https://github.com/bitcoin/bips/pull/1381 and https://github.com/bitcoin-inquisition/bitcoin/pull/22
💬 MarcoFalke commented on pull request "Fix `#include`s in `src/wallet`":
(https://github.com/bitcoin/bitcoin/pull/27759#issuecomment-1567205314)
lgtm ACK 1f97572b9c0d339a8497340e7066050aba9d7694
(https://github.com/bitcoin/bitcoin/pull/27759#issuecomment-1567205314)
lgtm ACK 1f97572b9c0d339a8497340e7066050aba9d7694
💬 furszy commented on issue "rpc_getblockfrompeer.py intermittent failure: assert_equal(pruneheight, 248); not(249 == 248)":
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1567211356)
Awesome investigation! It feels good when all dots are connected.
Apart from mzumzande's patch to enforce sequential block storage, which looks promising at first glance. I would like to refer to a portion of what I just wrote in https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567161622:
I believe the main conclusion of the analysis (at least mine) is that the accuracy of the `pruneblockchain` result is questionable. The real issue lies in the fact that the RPC command's result
...
(https://github.com/bitcoin/bitcoin/issues/27749#issuecomment-1567211356)
Awesome investigation! It feels good when all dots are connected.
Apart from mzumzande's patch to enforce sequential block storage, which looks promising at first glance. I would like to refer to a portion of what I just wrote in https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567161622:
I believe the main conclusion of the analysis (at least mine) is that the accuracy of the `pruneblockchain` result is questionable. The real issue lies in the fact that the RPC command's result
...
🤔 mzumsande reviewed a pull request: "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command"
(https://github.com/bitcoin/bitcoin/pull/27770#pullrequestreview-1449612463)
> Furthermore, what theStack analysis really says is that pruneblockchain result is not accurate. Which is the real issue for me. The RPC command result description says that it returns the "height of the last block pruned", which could not be true as blocks could have been stored out of order.
So retrieving block height 249 as "last block pruned" doesn't mean that block 248 has been pruned from disk.
But is it really incorrect that the "last block pruned" is 249? Yes, this doesn't imply tha
...
(https://github.com/bitcoin/bitcoin/pull/27770#pullrequestreview-1449612463)
> Furthermore, what theStack analysis really says is that pruneblockchain result is not accurate. Which is the real issue for me. The RPC command result description says that it returns the "height of the last block pruned", which could not be true as blocks could have been stored out of order.
So retrieving block height 249 as "last block pruned" doesn't mean that block 248 has been pruned from disk.
But is it really incorrect that the "last block pruned" is 249? Yes, this doesn't imply tha
...
🚀 fanquake merged a pull request: "Fix `#include`s in `src/wallet`"
(https://github.com/bitcoin/bitcoin/pull/27759)
(https://github.com/bitcoin/bitcoin/pull/27759)
👍 fanquake approved a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1449677075)
ACK 59c89447499bd9d6202269879555b8bc37373aa2
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1449677075)
ACK 59c89447499bd9d6202269879555b8bc37373aa2
💬 furszy commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567299154)
> But is it really incorrect that the "last block pruned" is 249? Yes, this doesn't imply that all blocks before have been pruned as well - but the RPC description isn't claiming that.
That would be the devil's lawyer argument. It works for us, but not for our users.
Let's agree that while it's not clearly stated, anyone reading the RPC result description would understand that all blocks prior the returned height were pruned.
> But I think that introducing any new RPC should primarily hav
...
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567299154)
> But is it really incorrect that the "last block pruned" is 249? Yes, this doesn't imply that all blocks before have been pruned as well - but the RPC description isn't claiming that.
That would be the devil's lawyer argument. It works for us, but not for our users.
Let's agree that while it's not clearly stated, anyone reading the RPC result description would understand that all blocks prior the returned height were pruned.
> But I think that introducing any new RPC should primarily hav
...
💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1209434809)
> Looks like it is only passing CI because Cirrus seems to be ignoring the 2CPU/8GB limit and just uses 4/15 unconditionally for ci image builds?
@fkorotkov is this what should be happening, and can we rely on this behaviour into the future?
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1209434809)
> Looks like it is only passing CI because Cirrus seems to be ignoring the 2CPU/8GB limit and just uses 4/15 unconditionally for ci image builds?
@fkorotkov is this what should be happening, and can we rely on this behaviour into the future?
👍 TheCharlatan approved a pull request: "lint: stop ignoring LIEF imports"
(https://github.com/bitcoin/bitcoin/pull/27507#pullrequestreview-1449687958)
ACK 015cc5e588fa25f65f6ea2ed03701def8dfd5444
(https://github.com/bitcoin/bitcoin/pull/27507#pullrequestreview-1449687958)
ACK 015cc5e588fa25f65f6ea2ed03701def8dfd5444
✅ fanquake closed an issue: "CPU DoS on mainnet in debug mode"
(https://github.com/bitcoin/bitcoin/issues/27586)
(https://github.com/bitcoin/bitcoin/issues/27586)
🚀 fanquake merged a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724)
(https://github.com/bitcoin/bitcoin/pull/27724)
🚀 fanquake merged a pull request: "lint: stop ignoring LIEF imports"
(https://github.com/bitcoin/bitcoin/pull/27507)
(https://github.com/bitcoin/bitcoin/pull/27507)
📝 fanquake opened a pull request: "[25.x] build: disable boost multi index safe mode"
(https://github.com/bitcoin/bitcoin/pull/27775)
Backports https://github.com/bitcoin/bitcoin/pull/27724 to `25.x`.
(https://github.com/bitcoin/bitcoin/pull/27775)
Backports https://github.com/bitcoin/bitcoin/pull/27724 to `25.x`.
💬 fanquake commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1567320438)
Backported to 25.x in #27775.
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1567320438)
Backported to 25.x in #27775.