💬 fjahr commented on pull request "wallet, assumeutxo: Don't Assume m_chain_tx_count, Improve wallet RPC errors":
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2457711117)
Rebased and addressed nit from @maflcko
(https://github.com/bitcoin/bitcoin/pull/30909#issuecomment-2457711117)
Rebased and addressed nit from @maflcko
📝 dergoegge converted_to_draft a pull request: "ci: Split out native fuzz jobs for macOS and windows (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31221)
Split out two new CI jobs (for native macOS and windows) that run the fuzz tests on the qa-assets input corpora.
In both jobs the fuzz binary is built with -DBUILD_FOR_FUZZING to enable Assume assertions as well as FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION.
(https://github.com/bitcoin/bitcoin/pull/31221)
Split out two new CI jobs (for native macOS and windows) that run the fuzz tests on the qa-assets input corpora.
In both jobs the fuzz binary is built with -DBUILD_FOR_FUZZING to enable Assume assertions as well as FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION.
💬 Christewart commented on pull request "consensus: Consistently encode and decode `OP_1NEGATE` similar to other small ints in Script":
(https://github.com/bitcoin/bitcoin/pull/29589#discussion_r1829738339)
This seems like a reasonable thing to do. However it seems like the `script.py` implementation should still handle `OP_1NEGATE` imo.
IIUC the `solver.cpp`'s `IsSmallInteger()` and `script.py`'s `is_small_int()` serve 2 different purposes, and renaming the solver.cpp function name would help differentiate them.
(https://github.com/bitcoin/bitcoin/pull/29589#discussion_r1829738339)
This seems like a reasonable thing to do. However it seems like the `script.py` implementation should still handle `OP_1NEGATE` imo.
IIUC the `solver.cpp`'s `IsSmallInteger()` and `script.py`'s `is_small_int()` serve 2 different purposes, and renaming the solver.cpp function name would help differentiate them.
💬 stickies-v commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2457771782)
re-ACK 5e4df9f67179e9cc284cb2ad2264de6b8bb6c606, no changes except for addressing merge conflict from #31139
(https://github.com/bitcoin/bitcoin/pull/31096#issuecomment-2457771782)
re-ACK 5e4df9f67179e9cc284cb2ad2264de6b8bb6c606, no changes except for addressing merge conflict from #31139
💬 achow101 commented on issue "intermittent issue in wallet_upgradewallet.py: AssertionError: bdb magic does not match bdb btree magic":
(https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2457791648)
I don't think this issue has to do with ZFS. I'm debugging the test on the problematic system and it seems that BDB decided to use a page size of 16384 for some reason, rather than our expected and hardcoded 4096. Changing it to 16384 resolves this, but we should instead be using the pagesize given by the wallet file rather than a fixed pagesize. This is a test only issue as the C++ parser does not use a fixed pagesize.
(https://github.com/bitcoin/bitcoin/issues/31210#issuecomment-2457791648)
I don't think this issue has to do with ZFS. I'm debugging the test on the problematic system and it seems that BDB decided to use a page size of 16384 for some reason, rather than our expected and hardcoded 4096. Changing it to 16384 resolves this, but we should instead be using the pagesize given by the wallet file rather than a fixed pagesize. This is a test only issue as the C++ parser does not use a fixed pagesize.
💬 marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829772701)
I should also add, I used the expression in `GetBlockProof` https://github.com/bitcoin/bitcoin/blob/65b194193661e27cf2d9c0e0d7e3b627a379513a/src/chain.cpp#L143 to calculate the work on a single header.
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829772701)
I should also add, I used the expression in `GetBlockProof` https://github.com/bitcoin/bitcoin/blob/65b194193661e27cf2d9c0e0d7e3b627a379513a/src/chain.cpp#L143 to calculate the work on a single header.
📝 achow101 opened a pull request: "tests: Handle BDB dynamic pagesize"
(https://github.com/bitcoin/bitcoin/pull/31222)
BDB may choose to use a pagesize other than 4096. As such, the parser should use the pagesize given by the BDB file.
Fixes #31210
(https://github.com/bitcoin/bitcoin/pull/31222)
BDB may choose to use a pagesize other than 4096. As such, the parser should use the pagesize given by the BDB file.
Fixes #31210
📝 mzumsande opened a pull request: "net, init: derive default onion port if a user specified a -port"
(https://github.com/bitcoin/bitcoin/pull/31223)
This resolves #31133 (setups with multiple local nodes each using a different `-port` no longer working with v28.0, see the issue description for more details) by deriving the default onion listening port to be the value specified by `-port` incremented by 1 (idea by vasild / laanwj).
Note that with this fix, the chosen `-port` values of two local nodes cannot be adjacent, otherwise there will be port collisions again.
From the discussion in the linked issue, this was the most popular option
...
(https://github.com/bitcoin/bitcoin/pull/31223)
This resolves #31133 (setups with multiple local nodes each using a different `-port` no longer working with v28.0, see the issue description for more details) by deriving the default onion listening port to be the value specified by `-port` incremented by 1 (idea by vasild / laanwj).
Note that with this fix, the chosen `-port` values of two local nodes cannot be adjacent, otherwise there will be port collisions again.
From the discussion in the linked issue, this was the most popular option
...
💬 mzumsande commented on issue "net: Tor service target port collides when running multiple nodes, making bitcoind error out":
(https://github.com/bitcoin/bitcoin/issues/31133#issuecomment-2457841759)
I opened #31223 to move this forward (it seems to be consensus that we should fix this either now or not at all).
(https://github.com/bitcoin/bitcoin/issues/31133#issuecomment-2457841759)
I opened #31223 to move this forward (it seems to be consensus that we should fix this either now or not at all).
💬 bigspider commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r1829804219)
These three constants are `0x1a` , `0x1b`, `0x1c` in the final version of [BIP-373](https://github.com/bitcoin/bips/blob/master/bip-0373.mediawiki).
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r1829804219)
These three constants are `0x1a` , `0x1b`, `0x1c` in the final version of [BIP-373](https://github.com/bitcoin/bips/blob/master/bip-0373.mediawiki).
💬 hebasto commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1829817574)
@theuni
> Arguably the issue here is that we have debug symbols for secp at all.
What are your suggestions?
(https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1829817574)
@theuni
> Arguably the issue here is that we have debug symbols for secp at all.
What are your suggestions?
💬 Christewart commented on pull request "net, init: derive default onion port if a user specified a -port":
(https://github.com/bitcoin/bitcoin/pull/31223#issuecomment-2457898255)
concept ACK
(https://github.com/bitcoin/bitcoin/pull/31223#issuecomment-2457898255)
concept ACK
💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2457975437)
Sorry for the delay.
@maflcko, the issue seems to be that the exception occurs in the test base class constructor. Which is not part of the actual test code and runs prior to it. So it might not be captured by the framework, throwing the "unknown location error".
Just dropped the fix and pushed a commit to verify it. Let's see what the CI prints now.
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2457975437)
Sorry for the delay.
@maflcko, the issue seems to be that the exception occurs in the test base class constructor. Which is not part of the actual test code and runs prior to it. So it might not be captured by the framework, throwing the "unknown location error".
Just dropped the fix and pushed a commit to verify it. Let's see what the CI prints now.
⚠️ Sandra-Amina-Boss opened an issue: "Sm"
(https://github.com/bitcoin/bitcoin/issues/31224)
https://github.com/bnb-chain/greenfield-whitepaper/tree/main
(https://github.com/bitcoin/bitcoin/issues/31224)
https://github.com/bnb-chain/greenfield-whitepaper/tree/main
✅ fanquake closed an issue: "Sm"
(https://github.com/bitcoin/bitcoin/issues/31224)
(https://github.com/bitcoin/bitcoin/issues/31224)
:lock: fanquake locked an issue: "Sm"
(https://github.com/bitcoin/bitcoin/issues/31224)
(https://github.com/bitcoin/bitcoin/issues/31224)
💬 marcofleon commented on pull request "fuzz: Fix difficulty target generation in `p2p_headers_presync`":
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829926285)
If I wanted to do a lower target based on `MinimumChainWork`, I think I could do
```
target = (~work + 1) / work
```
where work would be `MinimumChainWork / 1600` or maybe a bit less to be safe.
(https://github.com/bitcoin/bitcoin/pull/31213#discussion_r1829926285)
If I wanted to do a lower target based on `MinimumChainWork`, I think I could do
```
target = (~work + 1) / work
```
where work would be `MinimumChainWork / 1600` or maybe a bit less to be safe.
👋 glozow's pull request is ready for review: "TxDownloadManager followups"
(https://github.com/bitcoin/bitcoin/pull/31190)
(https://github.com/bitcoin/bitcoin/pull/31190)
💬 achow101 commented on pull request "Update secp256k1 subtree to v0.6.0":
(https://github.com/bitcoin/bitcoin/pull/31216#issuecomment-2458061763)
> Should we disable the musig module in this PR and enable it only when it is needed?
Done
(https://github.com/bitcoin/bitcoin/pull/31216#issuecomment-2458061763)
> Should we disable the musig module in this PR and enable it only when it is needed?
Done
💬 glozow commented on pull request "BlockAssembler: return selected packages vsize and feerate":
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1829943882)
Nonblocking, I can understand it's a larger change to use it.
(https://github.com/bitcoin/bitcoin/pull/30391#discussion_r1829943882)
Nonblocking, I can understand it's a larger change to use it.