💬 sipa commented on issue "Documentation on 'services' parsing in RPC response for getnodeaddresses":
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568645276)
Bitcoin nodes have no knowledge about what other functionality may run on top, like LN or mining. The "services" the P2P node network knows about only relate to functionality offered over that P2P network.
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568645276)
Bitcoin nodes have no knowledge about what other functionality may run on top, like LN or mining. The "services" the P2P node network knows about only relate to functionality offered over that P2P network.
💬 pinheadmz commented on issue "how to syn testnet4 data":
(https://github.com/bitcoin/bitcoin/issues/33931#issuecomment-3568647252)
use `testnet4=1`
https://github.com/bitcoin/bitcoin/blob/a14e7b9dee9145920f93eab0254ce92942bd1e5e/share/examples/bitcoin.conf#L568-L570
(https://github.com/bitcoin/bitcoin/issues/33931#issuecomment-3568647252)
use `testnet4=1`
https://github.com/bitcoin/bitcoin/blob/a14e7b9dee9145920f93eab0254ce92942bd1e5e/share/examples/bitcoin.conf#L568-L570
✅ taoteh1221 closed an issue: "Documentation on 'services' parsing in RPC response for getnodeaddresses"
(https://github.com/bitcoin/bitcoin/issues/33930)
(https://github.com/bitcoin/bitcoin/issues/33930)
💬 taoteh1221 commented on issue "Documentation on 'services' parsing in RPC response for getnodeaddresses":
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568664817)
Thanks @sipa, I appreciate the quick response. 👍
(https://github.com/bitcoin/bitcoin/issues/33930#issuecomment-3568664817)
Thanks @sipa, I appreciate the quick response. 👍
💬 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568675720)
There was another issue in the `feature_taproot.py` test where the "compat/nocsa" test case incorrectly assumed bare NONSTANDARD scripts with `OP_CHECKSIGADD` would pass `AreInputsStandard`. Since `OP_CHECKSIGADD` (0xba) exceeds `MAX_OPCODE` (0xb9), `HasValidOps()` returns false, so `AreInputsStandard` rejects it regardless of sigop count. I restored the original standardness logic requiring wrapping (p2sh or witv0) for this case.
There's also a fuzzer failure in `process_messages` with an as
...
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568675720)
There was another issue in the `feature_taproot.py` test where the "compat/nocsa" test case incorrectly assumed bare NONSTANDARD scripts with `OP_CHECKSIGADD` would pass `AreInputsStandard`. Since `OP_CHECKSIGADD` (0xba) exceeds `MAX_OPCODE` (0xb9), `HasValidOps()` returns false, so `AreInputsStandard` rejects it regardless of sigop count. I restored the original standardness logic requiring wrapping (p2sh or witv0) for this case.
There's also a fuzzer failure in `process_messages` with an as
...
💬 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568706803)
It looks like the fuzzer failure in `process_messages` came from the mempool consistency check. When checking inputs, if an input references an output from another mempool transaction, that output must be in `mempoolDuplicate` before the assertion. Transactions are sorted by ancestor count, but there can be cases where a parent's output isn't in `mempoolDuplicate` when a child is checked. The fix adds the parent transaction's output to `mempoolDuplicate` before the assertion, making the check mo
...
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568706803)
It looks like the fuzzer failure in `process_messages` came from the mempool consistency check. When checking inputs, if an input references an output from another mempool transaction, that output must be in `mempoolDuplicate` before the assertion. Transactions are sorted by ancestor count, but there can be cases where a parent's output isn't in `mempoolDuplicate` when a child is checked. The fix adds the parent transaction's output to `mempoolDuplicate` before the assertion, making the check mo
...
💬 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568779013)
I think the mempool consistency check was failing because input coins weren't always available in mempoolDuplicate when the assertion checked for them. The fix in [73399e8](https://github.com/bitcoin/bitcoin/pull/33926/commits/73399e80a20c57cb6ba3a8e766be9166d840e526) ensures both mempool transaction outputs and UTXO set coins are available in mempoolDuplicate before the assertion, which should resolve the fuzzer failure.
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568779013)
I think the mempool consistency check was failing because input coins weren't always available in mempoolDuplicate when the assertion checked for them. The fix in [73399e8](https://github.com/bitcoin/bitcoin/pull/33926/commits/73399e80a20c57cb6ba3a8e766be9166d840e526) ensures both mempool transaction outputs and UTXO set coins are available in mempoolDuplicate before the assertion, which should resolve the fuzzer failure.
💬 plebhash commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3568788623)
we are getting reports of crashes on https://github.com/stratum-mining/sv2-apps/pull/59#issuecomment-3568252007
originally I suspected there could be thread-related issues similar to what I reported on https://github.com/bitcoin/bitcoin/issues/33923, but it turns out it was the (relatively weak) VPS running out of memory, which only happened after long (12h+) running sessions
so I leveraged [`psrecord`](https://github.com/astrofrog/psrecord) to observe how Bitcoin Core (a14e7b9dee9145920f93eab
...
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3568788623)
we are getting reports of crashes on https://github.com/stratum-mining/sv2-apps/pull/59#issuecomment-3568252007
originally I suspected there could be thread-related issues similar to what I reported on https://github.com/bitcoin/bitcoin/issues/33923, but it turns out it was the (relatively weak) VPS running out of memory, which only happened after long (12h+) running sessions
so I leveraged [`psrecord`](https://github.com/astrofrog/psrecord) to observe how Bitcoin Core (a14e7b9dee9145920f93eab
...
💬 151henry151 commented on pull request "Align legacy script policy with P2SH policy in AreInputsStandard":
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568915804)
After finding the fuzz tests were still failing, I've amended to add all transaction outputs to mempoolDuplicate after processing inputs.
(https://github.com/bitcoin/bitcoin/pull/33926#issuecomment-3568915804)
After finding the fuzz tests were still failing, I've amended to add all transaction outputs to mempoolDuplicate after processing inputs.
✅ maflcko closed an issue: "`bitcoin-node` is unkillable after mining IPC connection is established"
(https://github.com/bitcoin/bitcoin/issues/33463)
(https://github.com/bitcoin/bitcoin/issues/33463)
✅ maflcko closed an issue: "how to syn testnet4 data"
(https://github.com/bitcoin/bitcoin/issues/33931)
(https://github.com/bitcoin/bitcoin/issues/33931)
💬 hodlinator commented on pull request "validation: fix assumevalid is ignored during reindex":
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3569453173)
> > Would it be acceptable to emit an error and halt, saying that `-reindex` for cases where nodes don't have sufficient block data also requires decreasing `-minimumchainwork=` to whatever level we have in the block data? Could also explain that this is in order to keep logic consistent regardless of `-reindex`, and also say that running with lowered `-minimumchainwork` long-term is not advisable.
>
> It is possible to do this, but will this be considered a better solution than automatically
...
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3569453173)
> > Would it be acceptable to emit an error and halt, saying that `-reindex` for cases where nodes don't have sufficient block data also requires decreasing `-minimumchainwork=` to whatever level we have in the block data? Could also explain that this is in order to keep logic consistent regardless of `-reindex`, and also say that running with lowered `-minimumchainwork` long-term is not advisable.
>
> It is possible to do this, but will this be considered a better solution than automatically
...
📝 maflcko opened a pull request: "ci: Use latest Xcode that the minimum macOS version allows"
(https://github.com/bitcoin/bitcoin/pull/33932)
Changing the CI policy to use the *latest* Xcode (instead of the *earliest*), allowed by the Bitcoin Core minimum supported macOS version, makes sense: While this may require the developer or user to install a later security point-release on macOS, this should generally be fine and it is even expected that users run the latest supported security release of their operating system. Also, in practise, this often doesn't result in a visible change anyway: This specific change from Xcode 16.0 to 16.2
...
(https://github.com/bitcoin/bitcoin/pull/33932)
Changing the CI policy to use the *latest* Xcode (instead of the *earliest*), allowed by the Bitcoin Core minimum supported macOS version, makes sense: While this may require the developer or user to install a later security point-release on macOS, this should generally be fine and it is even expected that users run the latest supported security release of their operating system. Also, in practise, this often doesn't result in a visible change anyway: This specific change from Xcode 16.0 to 16.2
...
💬 maflcko commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2555155096)
> @fanquake / @maflcko - are there plans to bump to a more recent version of xcode to have proper C++20 support?
Yeah, it should be possible to bump to the latest version. Though, it may be best to first make it deterministic: https://github.com/bitcoin/bitcoin/pull/32009. Otherwise, it will be harder to re-create.
> For proper spaceship operator support we would need: https://developer.apple.com/documentation/xcode-release-notes/xcode-16_3-release-notes:
That is a bit odd, because
...
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2555155096)
> @fanquake / @maflcko - are there plans to bump to a more recent version of xcode to have proper C++20 support?
Yeah, it should be possible to bump to the latest version. Though, it may be best to first make it deterministic: https://github.com/bitcoin/bitcoin/pull/32009. Otherwise, it will be harder to re-create.
> For proper spaceship operator support we would need: https://developer.apple.com/documentation/xcode-release-notes/xcode-16_3-release-notes:
That is a bit odd, because
...
💬 Eunovo commented on pull request "validation: fix assumevalid is ignored during reindex":
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3569687541)
> This suggestion was meant for the case where we can't do headers sync due to being offline (using zero peers after a certain timeout or something like that, I'm not sure how to best check whether we are offline). I guess the error message could also point out that going online should resolve the situation.
I see. A message that indicates that the node is waiting for headers sync to finish reindex will be valuable. We probably don't need to check that we are offline at all.
(https://github.com/bitcoin/bitcoin/pull/33854#issuecomment-3569687541)
> This suggestion was meant for the case where we can't do headers sync due to being offline (using zero peers after a certain timeout or something like that, I'm not sure how to best check whether we are offline). I guess the error message could also point out that going online should resolve the situation.
I see. A message that indicates that the node is waiting for headers sync to finish reindex will be valuable. We probably don't need to check that we are offline at all.
💬 vasild commented on issue "Issues with new create wallet dialogue":
(https://github.com/bitcoin-core/gui/issues/151#issuecomment-3569746495)
> Checking `Disable Private Keys` checks `Make Blank Wallet` (and disables `Encrypt Wallet`). This is correct.
This is no more in the latest version (as of 17072f7005), I guess it has changed in the mean time:
Initial state:
field | state | is clickable
---|---|---
`Encrypt Wallet` | ☐ | yes
`Disable Private Keys` | ☐ | yes
`Make Blank Wallet` | ☐ | yes
When I click on `Disable Private Keys`:
field | state | is clickable
---|---|---
`Encrypt Wallet` | ☐ | no
`Disable Private Keys` | ☑ | yes
`
...
(https://github.com/bitcoin-core/gui/issues/151#issuecomment-3569746495)
> Checking `Disable Private Keys` checks `Make Blank Wallet` (and disables `Encrypt Wallet`). This is correct.
This is no more in the latest version (as of 17072f7005), I guess it has changed in the mean time:
Initial state:
field | state | is clickable
---|---|---
`Encrypt Wallet` | ☐ | yes
`Disable Private Keys` | ☐ | yes
`Make Blank Wallet` | ☐ | yes
When I click on `Disable Private Keys`:
field | state | is clickable
---|---|---
`Encrypt Wallet` | ☐ | no
`Disable Private Keys` | ☑ | yes
`
...
💬 maflcko commented on pull request "Wallet: Add `maxfeerate` wallet startup option":
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2555503278)
// Also the wallet prevents creating transaction with base fee above maxtxfee. -> // Also the wallet prevents creating transactions with base fee above maxtxfee. [“creating transaction” is ungrammatical; plural or an article is needed for clarity]
(https://github.com/bitcoin/bitcoin/pull/29278#discussion_r2555503278)
// Also the wallet prevents creating transaction with base fee above maxtxfee. -> // Also the wallet prevents creating transactions with base fee above maxtxfee. [“creating transaction” is ungrammatical; plural or an article is needed for clarity]
💬 Sjors commented on issue "should concurrent IPC requests directed to the same thread cause a crash?":
(https://github.com/bitcoin/bitcoin/issues/33923#issuecomment-3569919239)
> to be honest, we could have gotten away with only 2 by forcing `getHeader`, `getCoinbaseTx`, `getCoinbaseMerklePath` and waitNext to be done sequentially under the same server thread
I think you should bench this before deciding if it needs optimising and also check whether it's _actually_ faster and not e.g. slowing the node responses down with overhead.
If `getHeader`, `getCoinbase(Tx)` and `getCoinbaseMerklePath` really need to be called in parallel, then I think it would be better if the
...
(https://github.com/bitcoin/bitcoin/issues/33923#issuecomment-3569919239)
> to be honest, we could have gotten away with only 2 by forcing `getHeader`, `getCoinbaseTx`, `getCoinbaseMerklePath` and waitNext to be done sequentially under the same server thread
I think you should bench this before deciding if it needs optimising and also check whether it's _actually_ faster and not e.g. slowing the node responses down with overhead.
If `getHeader`, `getCoinbase(Tx)` and `getCoinbaseMerklePath` really need to be called in parallel, then I think it would be better if the
...
💬 Sjors commented on issue "Block template memory management (for IPC clients)":
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3569959628)
I'm planning to make similar plots for #33922 (without CPU and ideally with marks to indicate where blocks were found).
If you're measuring the process memory instead of only the template memory (which #33922 enables), you'll want to hold the mempool itself constant. E.g. by picking some value for `-maxmempool` and waiting for it to fill before starting the measurement. You also want to set `-dbcache` to its minimum, because that's also accuring
(https://github.com/bitcoin/bitcoin/issues/33899#issuecomment-3569959628)
I'm planning to make similar plots for #33922 (without CPU and ideally with marks to indicate where blocks were found).
If you're measuring the process memory instead of only the template memory (which #33922 enables), you'll want to hold the mempool itself constant. E.g. by picking some value for `-maxmempool` and waiting for it to fill before starting the measurement. You also want to set `-dbcache` to its minimum, because that's also accuring
🤔 rkrux reviewed a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3499650048)
ACK 6e37b3b4861733ca97ec5d27d0bf52d187b6a2c9
I do remember facing issues on Mac some time back that prompted me to create issue #33667.
Like mentioned in earlier couple comments, it'd be helpful to mention that Linux is recommended in the beginning "Quickstart guide" section.
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3499650048)
ACK 6e37b3b4861733ca97ec5d27d0bf52d187b6a2c9
I do remember facing issues on Mac some time back that prompted me to create issue #33667.
Like mentioned in earlier couple comments, it'd be helpful to mention that Linux is recommended in the beginning "Quickstart guide" section.