💬 pinheadmz commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907817185)
2fb833e69627a8f9b1c4b14b60d1df9903adf210
You might want to mention the old default of `3996000` here. I was staring at `blockmaxweight=4000000` on the following line for a minute trying to figure out what actually changed.
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907817185)
2fb833e69627a8f9b1c4b14b60d1df9903adf210
You might want to mention the old default of `3996000` here. I was staring at `blockmaxweight=4000000` on the following line for a minute trying to figure out what actually changed.
💬 pinheadmz commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907760169)
b55bb95d8d80175f30d2d4da5c6cfd72e5d8eedd
nit: could be `LARGE_TXS_COUNT + 1` to make it clear the difference from previous template
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907760169)
b55bb95d8d80175f30d2d4da5c6cfd72e5d8eedd
nit: could be `LARGE_TXS_COUNT + 1` to make it clear the difference from previous template
💬 pinheadmz commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907807887)
a45af6af4e7a59cd14ef6e16b8e5e180fa9d564d
Question about this phrasing: what is "maximum" about it? If a user sets `-maxcoinbaseweight=1000` then 1000 weight will always be reserved in block templates, and if the actual coinbase ends up being 50 weight instead of 1000 that extra weight will just be unused and the final block will be smaller than maximum size.
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907807887)
a45af6af4e7a59cd14ef6e16b8e5e180fa9d564d
Question about this phrasing: what is "maximum" about it? If a user sets `-maxcoinbaseweight=1000` then 1000 weight will always be reserved in block templates, and if the actual coinbase ends up being 50 weight instead of 1000 that extra weight will just be unused and the final block will be smaller than maximum size.
💬 pinheadmz commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907812521)
a45af6af4e7a59cd14ef6e16b8e5e180fa9d564d
Should this test against `args.GetIntArg("-blockmaxweight", MAX_BLOCK_WEIGHT)` ? Or, what happens if a user sets `-blockmaxweight=1000 -maxcoinbaseweight=4000` ?
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907812521)
a45af6af4e7a59cd14ef6e16b8e5e180fa9d564d
Should this test against `args.GetIntArg("-blockmaxweight", MAX_BLOCK_WEIGHT)` ? Or, what happens if a user sets `-blockmaxweight=1000 -maxcoinbaseweight=4000` ?
💬 luke-jr commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2578616268)
>i'm not aware of a straightforward way to "log network traffic of this process and subproceses only".
Not quite that, but iptables has the ability to match and log uid
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2578616268)
>i'm not aware of a straightforward way to "log network traffic of this process and subproceses only".
Not quite that, but iptables has the ability to match and log uid
🤔 furszy reviewed a pull request: "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors"
(https://github.com/bitcoin/bitcoin/pull/30909#pullrequestreview-2538306680)
Code review ACK 9d2d9f7ce29636f08322df70cf6abec8e0ca3727
(https://github.com/bitcoin/bitcoin/pull/30909#pullrequestreview-2538306680)
Code review ACK 9d2d9f7ce29636f08322df70cf6abec8e0ca3727
💬 furszy commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1907872493)
tiny nit: the `validation` category is a bit odd in the wallet scan context or GUI updates.
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1907872493)
tiny nit: the `validation` category is a bit odd in the wallet scan context or GUI updates.
💬 luke-jr commented on pull request "wallet: fix crash during watch-only wallet migration":
(https://github.com/bitcoin/bitcoin/pull/31374#issuecomment-2578726192)
> this isn't being backported.
Why not?
(https://github.com/bitcoin/bitcoin/pull/31374#issuecomment-2578726192)
> this isn't being backported.
Why not?
💬 luke-jr commented on pull request "Miner: never create a template which exploits the timewarp bug":
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2578764484)
To have the intended effect, "mintime" needs to be adjusted too, like in #31600
(https://github.com/bitcoin/bitcoin/pull/31376#issuecomment-2578764484)
To have the intended effect, "mintime" needs to be adjusted too, like in #31600
💬 achow101 commented on pull request "wallet: fix crash during watch-only wallet migration":
(https://github.com/bitcoin/bitcoin/pull/31374#issuecomment-2578773379)
> > this isn't being backported.
>
> Why not?
It was unclean, and making it a clean backport seemed to be a somewhat large change.
(https://github.com/bitcoin/bitcoin/pull/31374#issuecomment-2578773379)
> > this isn't being backported.
>
> Why not?
It was unclean, and making it a clean backport seemed to be a somewhat large change.
💬 luke-jr commented on pull request "wallet: fix crash during migration due to invalid multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-2578782712)
> we are enforcing this rules within the descriptors parsing logic.
That sounds dumb. Why are we?
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-2578782712)
> we are enforcing this rules within the descriptors parsing logic.
That sounds dumb. Why are we?
💬 pinheadmz commented on pull request "wallet: fix crash during migration due to invalid multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-2578808220)
@luke-jr can you rephrase your comment in a friendlier way please
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-2578808220)
@luke-jr can you rephrase your comment in a friendlier way please
💬 mzumsande commented on pull request "kernel: Move block tree db open to block manager":
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1907957838)
I agree that it should be possible to drop the check.
This is a off-topic here, but maybe the entire `NeedsRedownload()` check could be removed at some point, or simplified to just check block one after segwit, instead over iterating over 400k indexes each startup? Segwit activation is now more than 8 years in the past after all...
(https://github.com/bitcoin/bitcoin/pull/30965#discussion_r1907957838)
I agree that it should be possible to drop the check.
This is a off-topic here, but maybe the entire `NeedsRedownload()` check could be removed at some point, or simplified to just check block one after segwit, instead over iterating over 400k indexes each startup? Segwit activation is now more than 8 years in the past after all...
💬 sipa commented on pull request "cluster mempool: add txgraph diagrams/mining/eviction":
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2578874634)
I have simplified the eviction interface. Instead of getting a `TxGraph::Evictor` object through `TxGraph::GetEvictor()`, there is now a simpler `TxGraph::GetWorstMainChunk()` function, which is a pure inspector.
(https://github.com/bitcoin/bitcoin/pull/31444#issuecomment-2578874634)
I have simplified the eviction interface. Instead of getting a `TxGraph::Evictor` object through `TxGraph::GetEvictor()`, there is now a simpler `TxGraph::GetWorstMainChunk()` function, which is a pure inspector.
💬 davidgumberg commented on pull request "util: detect and warn when using exFAT on MacOS":
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2578881602)
Concept ACK
I second the suggestion by @tdb3 [above](https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1885462353) to also check `-walletdir`, although I have looked and have not been able to find similar reports of corruption with SQLite and exFAT drives on macOS.
To add a little data pointing in the direction of this bug only existing on macOS, I have synced nodes with datadirs on exFAT drives over both SATA and USB on Fedora 40 and have a long-running bitcoind node that has been
...
(https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2578881602)
Concept ACK
I second the suggestion by @tdb3 [above](https://github.com/bitcoin/bitcoin/pull/31453#discussion_r1885462353) to also check `-walletdir`, although I have looked and have not been able to find similar reports of corruption with SQLite and exFAT drives on macOS.
To add a little data pointing in the direction of this bug only existing on macOS, I have synced nodes with datadirs on exFAT drives over both SATA and USB on Fedora 40 and have a long-running bitcoind node that has been
...
💬 luke-jr commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907984056)
This should remain logically separate from consensus constant.
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907984056)
This should remain logically separate from consensus constant.
💬 luke-jr commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907983031)
This changes the meaning of `getmininginfo` fields unexpectedly, and forces us to add in the coinbase reserved size over and over. Seems like it would be better to include it here once.
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1907983031)
This changes the meaning of `getmininginfo` fields unexpectedly, and forces us to add in the coinbase reserved size over and over. Seems like it would be better to include it here once.
💬 0xB10C commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2578892122)
> I tried testing the asan job, via `time MAKEJOBS="-j17" FILE_ENV="./ci/test/00_setup_env_native_asan.sh" ./ci/test_run_all.sh`, on a Fedora dev box, and it fails with the following output.
I guess checking that were in a docker container (that hopefully doesn't have other services that does anything network related) is the only option here. Only fail in docker, otherwise just print, if possible.
Possibly by checking that `/proc/1/cgroup` exists and this is not empty with `cat /proc/1/cgroup
...
(https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2578892122)
> I tried testing the asan job, via `time MAKEJOBS="-j17" FILE_ENV="./ci/test/00_setup_env_native_asan.sh" ./ci/test_run_all.sh`, on a Fedora dev box, and it fails with the following output.
I guess checking that were in a docker container (that hopefully doesn't have other services that does anything network related) is the only option here. Only fail in docker, otherwise just print, if possible.
Possibly by checking that `/proc/1/cgroup` exists and this is not empty with `cat /proc/1/cgroup
...
💬 davidgumberg commented on issue "Data corruption on MacOS when using exFAT datadir or blocksdir":
(https://github.com/bitcoin/bitcoin/issues/31454#issuecomment-2578940904)
https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2578881602 (by me):
> To add a little data pointing in the direction of this bug only existing on macOS, I have synced nodes with datadirs on exFAT drives over both SATA and USB on Fedora 40 and have a long-running bitcoind node that has been running off of an exFAT SATA SSD on Fedora 40 for the past ~6 months and have never experienced any of the symptoms described in https://github.com/bitcoin/bitcoin/issues/28552 or https://github.co
...
(https://github.com/bitcoin/bitcoin/issues/31454#issuecomment-2578940904)
https://github.com/bitcoin/bitcoin/pull/31453#issuecomment-2578881602 (by me):
> To add a little data pointing in the direction of this bug only existing on macOS, I have synced nodes with datadirs on exFAT drives over both SATA and USB on Fedora 40 and have a long-running bitcoind node that has been running off of an exFAT SATA SSD on Fedora 40 for the past ~6 months and have never experienced any of the symptoms described in https://github.com/bitcoin/bitcoin/issues/28552 or https://github.co
...
📝 adlai opened a pull request: "doc: Update dependency installation for Debian/Ubuntu"
(https://github.com/bitcoin/bitcoin/pull/31621)
This is similar to the recently-pushed 8d20348 and results in slightly cleaner systems for future Debian/Ubuntu builds.
According to the description for pkg-config, "pkgconf is a replacement for pkg-config, providing additional functionality while also maintaining compatibility. This package only provides a dependency link to the pkgconf package to help with package upgrades. It can be safely removed."
Thus the relevant section of doc/build-unix.md is updated.
(https://github.com/bitcoin/bitcoin/pull/31621)
This is similar to the recently-pushed 8d20348 and results in slightly cleaner systems for future Debian/Ubuntu builds.
According to the description for pkg-config, "pkgconf is a replacement for pkg-config, providing additional functionality while also maintaining compatibility. This package only provides a dependency link to the pkgconf package to help with package upgrades. It can be safely removed."
Thus the relevant section of doc/build-unix.md is updated.