💬 stickies-v commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595950409)
```suggestion
- #29776 Fix #29767, set m_synced = true after Commit()
```
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595950409)
```suggestion
- #29776 Fix #29767, set m_synced = true after Commit()
```
💬 stickies-v commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595965787)
Also, this is the commit message and not the PR title but this is a bit more descriptive so makes sense.
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1595965787)
Also, this is the commit message and not the PR title but this is a bit more descriptive so makes sense.
🚀 achow101 merged a pull request: "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`"
(https://github.com/bitcoin/bitcoin/pull/29939)
(https://github.com/bitcoin/bitcoin/pull/29939)
💬 mzumsande commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1596018428)
The current logic seems really brittle to me, I suspected that there might be a bug hiding somewhere, but it barely works out in all scenarios I could think of.
With blocks being save on disk out of order, it is possible that at the end of a reindex, the cursor points to an older block file. Yet, it appears that nothing really bad happens as a result: When a new blocks arrives, `FindBlockPos` will still find the correct position in [this while loop](https://github.com/bitcoin/bitcoin/blob/24572
...
(https://github.com/bitcoin/bitcoin/pull/29975#discussion_r1596018428)
The current logic seems really brittle to me, I suspected that there might be a bug hiding somewhere, but it barely works out in all scenarios I could think of.
With blocks being save on disk out of order, it is possible that at the end of a reindex, the cursor points to an older block file. Yet, it appears that nothing really bad happens as a result: When a new blocks arrives, `FindBlockPos` will still find the correct position in [this while loop](https://github.com/bitcoin/bitcoin/blob/24572
...
💬 achow101 commented on pull request "test: use sleepy wait-for-log in reindex readonly":
(https://github.com/bitcoin/bitcoin/pull/30006#issuecomment-2103505565)
ACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c
(https://github.com/bitcoin/bitcoin/pull/30006#issuecomment-2103505565)
ACK fd6a7d3a13d89d74e161095b0e9bd3570210a40c
🚀 achow101 merged a pull request: "test: use sleepy wait-for-log in reindex readonly"
(https://github.com/bitcoin/bitcoin/pull/30006)
(https://github.com/bitcoin/bitcoin/pull/30006)
📝 theStack opened a pull request: "test: fix MiniWallet script-path spend (missing parity bit in leaf version)"
(https://github.com/bitcoin/bitcoin/pull/30076)
This PR fixes a dormant bug in MiniWallet that exists since support for P2TR was initially added in #23371 (see commit 041abfebe49ae5e3e882c00cc5caea1365a27a49).
In the course of spending the output, the leaf version byte of the control block in the witness stack doesn't set the parity bit, i.e. we were so far just lucky that the used combinations of relevant data (internal pubkey, leaf script / version) didn't result in a tweaked pubkey with odd y-parity. If that was the case, we'd get t
...
(https://github.com/bitcoin/bitcoin/pull/30076)
This PR fixes a dormant bug in MiniWallet that exists since support for P2TR was initially added in #23371 (see commit 041abfebe49ae5e3e882c00cc5caea1365a27a49).
In the course of spending the output, the leaf version byte of the control block in the witness stack doesn't set the parity bit, i.e. we were so far just lucky that the used combinations of relevant data (internal pubkey, leaf script / version) didn't result in a tweaked pubkey with odd y-parity. If that was the case, we'd get t
...
💬 tdb3 commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-2103628952)
> I wonder if a sub-object would be a good idea before we start adding more settings here. There's at least also `m_expiry` that would make sense to add too.
Implementing as a sub-object makes sense to me as well. If it is decided to adjust this RPC, then I'm thinking we'd also need a `-deprecatedrpc` option to retain the existing behavior, so we don't break RPC for downstream clients.
See also: https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1571053657
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-2103628952)
> I wonder if a sub-object would be a good idea before we start adding more settings here. There's at least also `m_expiry` that would make sense to add too.
Implementing as a sub-object makes sense to me as well. If it is decided to adjust this RPC, then I'm thinking we'd also need a `-deprecatedrpc` option to retain the existing behavior, so we don't break RPC for downstream clients.
See also: https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1571053657
💬 tdb3 commented on pull request "RPC: Return `permitbaremultisig` and `maxdatacarriersize` in `getmempoolinfo`":
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-2103631437)
> Actually, since these don't change on their own... maybe a separate RPC altogether? (later extended to allow some changes?)
This option also makes sense to me. Maybe something like `getmempoolpolicy` or similar?
(https://github.com/bitcoin/bitcoin/pull/29954#issuecomment-2103631437)
> Actually, since these don't change on their own... maybe a separate RPC altogether? (later extended to allow some changes?)
This option also makes sense to me. Maybe something like `getmempoolpolicy` or similar?
🤔 ajtowns reviewed a pull request: "test, subprocess: Improve coverage report correctness"
(https://github.com/bitcoin/bitcoin/pull/30075#pullrequestreview-2049083052)
Approach ACK
(https://github.com/bitcoin/bitcoin/pull/30075#pullrequestreview-2049083052)
Approach ACK
💬 ajtowns commented on pull request "test, subprocess: Improve coverage report correctness":
(https://github.com/bitcoin/bitcoin/pull/30075#discussion_r1596149065)
The code in https://inbox.sourceware.org/gcc-cvs/20200505141638.8B852388F43D@sourceware.org/T/ suggests that `__gcov_dump()` should be called before exec() because otherwise a successful exec will cause the information to be discarded, and `__gcov_reset()` should be called after exec() returns, because that means exec failed and the information wasn't discarded, so coverage stats should continue to be gathered without duplicating the info that's already been dumped. Doing the reset unconditional
...
(https://github.com/bitcoin/bitcoin/pull/30075#discussion_r1596149065)
The code in https://inbox.sourceware.org/gcc-cvs/20200505141638.8B852388F43D@sourceware.org/T/ suggests that `__gcov_dump()` should be called before exec() because otherwise a successful exec will cause the information to be discarded, and `__gcov_reset()` should be called after exec() returns, because that means exec failed and the information wasn't discarded, so coverage stats should continue to be gathered without duplicating the info that's already been dumped. Doing the reset unconditional
...
💬 ajtowns commented on pull request "test, subprocess: Improve coverage report correctness":
(https://github.com/bitcoin/bitcoin/pull/30075#discussion_r1596146092)
These lines are indented too much compared to surrounding code.
Are they necessary at all?
(https://github.com/bitcoin/bitcoin/pull/30075#discussion_r1596146092)
These lines are indented too much compared to surrounding code.
Are they necessary at all?
⚠️ grizznaut opened an issue: "RPC: Internal bug in `walletprocesspsbt` when non_witness_utxo is not provided and a witness signature is invalid"
(https://github.com/bitcoin/bitcoin/issues/30077)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Calling `walletprocesspsbt` on a signed PSBT that only includes a `witness_utxo` (no `non_witness_utxo`) with an invalid signature produces an internal bug (`CHECK_NONFATAL`):
```
❯ bitcoin-cli walletprocesspsbt "cHNidP8BAJoCAAAAAq3g8bgaDs84DkYQ6urdGcuevx4mTtc5ImnEuw/RKiZ2AAAAAAD9////O+qpBnMQ44R69kLMIMPrviyL4ml4zSC1iA8Ec7ux/8YBAAAAAP3///8ClnsOAAAAAAAWABQ1vvbTc5DqIMQ7qVHMKjCDEWY5m+DhDQA
...
(https://github.com/bitcoin/bitcoin/issues/30077)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Calling `walletprocesspsbt` on a signed PSBT that only includes a `witness_utxo` (no `non_witness_utxo`) with an invalid signature produces an internal bug (`CHECK_NONFATAL`):
```
❯ bitcoin-cli walletprocesspsbt "cHNidP8BAJoCAAAAAq3g8bgaDs84DkYQ6urdGcuevx4mTtc5ImnEuw/RKiZ2AAAAAAD9////O+qpBnMQ44R69kLMIMPrviyL4ml4zSC1iA8Ec7ux/8YBAAAAAP3///8ClnsOAAAAAAAWABQ1vvbTc5DqIMQ7qVHMKjCDEWY5m+DhDQA
...
💬 fanquake commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1596208350)
Thanks, addressed both.
(https://github.com/bitcoin/bitcoin/pull/29888#discussion_r1596208350)
Thanks, addressed both.
📝 fanquake opened a pull request: "depends: set AR & RANLIB for CMake"
(https://github.com/bitcoin/bitcoin/pull/30078)
Needed for #21778. Should be more correct in any case.
(https://github.com/bitcoin/bitcoin/pull/30078)
Needed for #21778. Should be more correct in any case.
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1596235623)
Yea. Split into #30078.
(https://github.com/bitcoin/bitcoin/pull/21778#discussion_r1596235623)
Yea. Split into #30078.
💬 fanquake commented on pull request "build: LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2103855244)
Darwin Guix build (aarch64):
```bash
1de4299604e1711c90134a94a7ba6bc2685cc43e4acb283a7b3689afba883205 guix-build-c5f2df00a26c/output/arm64-apple-darwin/SHA256SUMS.part
7dcb77afb8dfa36c39be686827bec06bd39d197faed179b6e17d62492567ce9a guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin-unsigned.tar.gz
2f2bcf41e7b1f7ac28258e6db4600b0230db515f9549dae1c714948c9986c433 guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin
...
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-2103855244)
Darwin Guix build (aarch64):
```bash
1de4299604e1711c90134a94a7ba6bc2685cc43e4acb283a7b3689afba883205 guix-build-c5f2df00a26c/output/arm64-apple-darwin/SHA256SUMS.part
7dcb77afb8dfa36c39be686827bec06bd39d197faed179b6e17d62492567ce9a guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin-unsigned.tar.gz
2f2bcf41e7b1f7ac28258e6db4600b0230db515f9549dae1c714948c9986c433 guix-build-c5f2df00a26c/output/arm64-apple-darwin/bitcoin-c5f2df00a26c-arm64-apple-darwin
...
💬 fanquake commented on pull request "build: swap cctools otool for llvm-objdump":
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2103926877)
> There is a mismatch on the windows debug zip :/
I've rebuilt, and see matching now (you and Sjors):
```bash
67f4db93b4f985590948dc344306042841ab747b40ad73bf97ba62d77eb7bfd4 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/SHA256SUMS.part
62e019f164d3c8cf8a1a649b2caa500dd85b00a319910feaf97658020b8cd1e3 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/bitcoin-7f5ac4520d15-aarch64-linux-gnu-debug.tar.gz
c2227a058cf91c0bc3eb244cb254c3c6fcecddc21ba619bca8839d9d79bb1961 guix-build-7f5ac452
...
(https://github.com/bitcoin/bitcoin/pull/29739#issuecomment-2103926877)
> There is a mismatch on the windows debug zip :/
I've rebuilt, and see matching now (you and Sjors):
```bash
67f4db93b4f985590948dc344306042841ab747b40ad73bf97ba62d77eb7bfd4 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/SHA256SUMS.part
62e019f164d3c8cf8a1a649b2caa500dd85b00a319910feaf97658020b8cd1e3 guix-build-7f5ac4520d15/output/aarch64-linux-gnu/bitcoin-7f5ac4520d15-aarch64-linux-gnu-debug.tar.gz
c2227a058cf91c0bc3eb244cb254c3c6fcecddc21ba619bca8839d9d79bb1961 guix-build-7f5ac452
...
💬 Sjors commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2104027691)
@wiz the v28 release isn't going to happen sooner, so I assume you'd like this merged so people can more easily test on the master branch? That's probably fine - worst case people need to delete their `~/.bitcoin/testnet4` directory when the release comes out.
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2104027691)
@wiz the v28 release isn't going to happen sooner, so I assume you'd like this merged so people can more easily test on the master branch? That's probably fine - worst case people need to delete their `~/.bitcoin/testnet4` directory when the release comes out.
💬 Sjors commented on issue "BIP352 tracking issue":
(https://github.com/bitcoin/bitcoin/issues/28536#issuecomment-2104063959)
You can point to the actual BIP now in the description 🎉
(https://github.com/bitcoin/bitcoin/issues/28536#issuecomment-2104063959)
You can point to the actual BIP now in the description 🎉