💬 maflcko commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2513459402)
https://github.com/bitcoin/bitcoin/actions/runs/19238750700/job/54998849583?pr=33842#step:12:208:
```
File "D:\a\bitcoin\bitcoin/test/functional/wallet_basic.py", line 550, in run_test
assert vout["ischange"]
~~~~^^^^^^^^^^^^
KeyError: 'ischange'
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2513459402)
https://github.com/bitcoin/bitcoin/actions/runs/19238750700/job/54998849583?pr=33842#step:12:208:
```
File "D:\a\bitcoin\bitcoin/test/functional/wallet_basic.py", line 550, in run_test
assert vout["ischange"]
~~~~^^^^^^^^^^^^
KeyError: 'ischange'
💬 fanquake commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2513507443)
There's also our `thread_local` clang-tidy plugin: https://github.com/bitcoin/bitcoin/tree/master/contrib/devtools/bitcoin-tidy.
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2513507443)
There's also our `thread_local` clang-tidy plugin: https://github.com/bitcoin/bitcoin/tree/master/contrib/devtools/bitcoin-tidy.
⚠️ fanquake opened an issue: "ci: failure in `wallet_basic.py` KeyError: 'ischange'"
(https://github.com/bitcoin/bitcoin/issues/33848)
https://github.com/bitcoin/bitcoin/actions/runs/19238750700/job/54998849583?pr=33842#step:12:208:
```bash
File "D:\a\bitcoin\bitcoin/test/functional/wallet_basic.py", line 550, in run_test
assert vout["ischange"]
~~~~^^^^^^^^^^^^
KeyError: 'ischange'
```
(https://github.com/bitcoin/bitcoin/issues/33848)
https://github.com/bitcoin/bitcoin/actions/runs/19238750700/job/54998849583?pr=33842#step:12:208:
```bash
File "D:\a\bitcoin\bitcoin/test/functional/wallet_basic.py", line 550, in run_test
assert vout["ischange"]
~~~~^^^^^^^^^^^^
KeyError: 'ischange'
```
💬 fanquake commented on issue "ci: failure in `wallet_basic.py` KeyError: 'ischange'":
(https://github.com/bitcoin/bitcoin/issues/33848#issuecomment-3515909671)
cc @pinheadmz @achow101 #32517
(https://github.com/bitcoin/bitcoin/issues/33848#issuecomment-3515909671)
cc @pinheadmz @achow101 #32517
💬 big14way commented on issue "Inconsistent CJDNS address handling in Local addresses and AddLocal logs":
(https://github.com/bitcoin/bitcoin/issues/33471#issuecomment-3515930062)
@willcl-ark thank you for clarifyingi will look through the CONTRIBUTING.md frist
i appreciate it
(https://github.com/bitcoin/bitcoin/issues/33471#issuecomment-3515930062)
@willcl-ark thank you for clarifyingi will look through the CONTRIBUTING.md frist
i appreciate it
🚀 fanquake merged a pull request: "kernel: trim Chain interface"
(https://github.com/bitcoin/bitcoin/pull/33820)
(https://github.com/bitcoin/bitcoin/pull/33820)
💬 Sjors commented on pull request "wallet: remove redundant sighash calculation in Musig2 signing flow":
(https://github.com/bitcoin/bitcoin/pull/33665#issuecomment-3516135207)
Each commit should leave the code in a valid state, so it would make sense to move 000c685bcdd395d61a58ad9698563cab41e2d7d0 up.
(https://github.com/bitcoin/bitcoin/pull/33665#issuecomment-3516135207)
Each commit should leave the code in a valid state, so it would make sense to move 000c685bcdd395d61a58ad9698563cab41e2d7d0 up.
💬 laanwj commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3516192532)
Concept ACK.
re "refactor: Use fixed size ints over (un)signed ints for serialized values"
i wonder if we can enforce this kind of thing? That only fixed-size values can be serialized, would make sense to avoid any kind of platform divergence. At least in theory, as we already make assumptions on these types it's not a pressing issue.
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3516192532)
Concept ACK.
re "refactor: Use fixed size ints over (un)signed ints for serialized values"
i wonder if we can enforce this kind of thing? That only fixed-size values can be serialized, would make sense to avoid any kind of platform divergence. At least in theory, as we already make assumptions on these types it's not a pressing issue.
📝 AminMemariani opened a pull request: "Gpt/asmap tests"
(https://github.com/bitcoin/bitcoin/pull/33849)
## Motivation
Add direct Boost unit tests for the asmap interpreter and validator so regressions in ASN bucketing are caught earlier. Existing coverage relied on indirect behavior through addrman tests; these new cases exercise `Interpret` and `SanityCheckASMap` without large fixtures.
## Changes
- Introduce `src/test/asmap_tests.cpp` with helpers to synthesize compact asmap programs and five focused tests:
- verifies match vs default routing
- rejects tables missing a `RETURN`
- r
...
(https://github.com/bitcoin/bitcoin/pull/33849)
## Motivation
Add direct Boost unit tests for the asmap interpreter and validator so regressions in ASN bucketing are caught earlier. Existing coverage relied on indirect behavior through addrman tests; these new cases exercise `Interpret` and `SanityCheckASMap` without large fixtures.
## Changes
- Introduce `src/test/asmap_tests.cpp` with helpers to synthesize compact asmap programs and five focused tests:
- verifies match vs default routing
- rejects tables missing a `RETURN`
- r
...
💬 Sjors commented on pull request "wallet: remove redundant sighash calculation in Musig2 signing flow":
(https://github.com/bitcoin/bitcoin/pull/33665#discussion_r2513732380)
A more thorough approach would be to introduce `typedef uint8_t HashType`, but it requires changes all over the codebase.
(https://github.com/bitcoin/bitcoin/pull/33665#discussion_r2513732380)
A more thorough approach would be to introduce `typedef uint8_t HashType`, but it requires changes all over the codebase.
👍 TheCharlatan approved a pull request: "build: Bump g++ minimum supported version to 12"
(https://github.com/bitcoin/bitcoin/pull/33842#pullrequestreview-3447427213)
ACK fabf8e51083120883162399293c620f2afc0c5e2
(https://github.com/bitcoin/bitcoin/pull/33842#pullrequestreview-3447427213)
ACK fabf8e51083120883162399293c620f2afc0c5e2
💬 Sjors commented on pull request "wallet: remove redundant sighash calculation in Musig2 signing flow":
(https://github.com/bitcoin/bitcoin/pull/33665#discussion_r2513754984)
I would also prefer not breaking up this loop.
(https://github.com/bitcoin/bitcoin/pull/33665#discussion_r2513754984)
I would also prefer not breaking up this loop.
💬 Sjors commented on pull request "wallet: remove redundant sighash calculation in Musig2 signing flow":
(https://github.com/bitcoin/bitcoin/pull/33665#discussion_r2513763886)
Since we do an early return here, it's nicer to make `sighash` (or `sig_hash`) not an optional, so below you don't need dereference pointers.
(https://github.com/bitcoin/bitcoin/pull/33665#discussion_r2513763886)
Since we do an early return here, it's nicer to make `sighash` (or `sig_hash`) not an optional, so below you don't need dereference pointers.
💬 Sjors commented on pull request "wallet: remove redundant sighash calculation in Musig2 signing flow":
(https://github.com/bitcoin/bitcoin/pull/33665#discussion_r2513769931)
Why is this added?
(https://github.com/bitcoin/bitcoin/pull/33665#discussion_r2513769931)
Why is this added?
💬 Sjors commented on pull request "Musig2 tests":
(https://github.com/bitcoin/bitcoin/pull/32724#issuecomment-3516337169)
ACK fa65a57a826192ffae48f62f10c216068e8236f4
I (lightly) checked that the vectors match BIP 328 and that the test fails if I sabotage the input data.
(https://github.com/bitcoin/bitcoin/pull/32724#issuecomment-3516337169)
ACK fa65a57a826192ffae48f62f10c216068e8236f4
I (lightly) checked that the vectors match BIP 328 and that the test fails if I sabotage the input data.
✅ fanquake closed a pull request: "Gpt/asmap tests"
(https://github.com/bitcoin/bitcoin/pull/33849)
(https://github.com/bitcoin/bitcoin/pull/33849)
🚀 fanquake merged a pull request: "guix: build for Linux HOSTS with `-static-libgcc`"
(https://github.com/bitcoin/bitcoin/pull/33181)
(https://github.com/bitcoin/bitcoin/pull/33181)
💬 laanwj commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2513829761)
Ideally we should not use `size_t` anywhere for on-disk sizes and offsets in the first place. It's meant for counters and in-memory sizes, at most. But it's probably not worth changing all of that in places where we don't actually expect to work with large files.
(https://github.com/bitcoin/bitcoin/pull/33724#discussion_r2513829761)
Ideally we should not use `size_t` anywhere for on-disk sizes and offsets in the first place. It's meant for counters and in-memory sizes, at most. But it's probably not worth changing all of that in places where we don't actually expect to work with large files.
🤔 Sjors reviewed a pull request: "doc: update interface, --stdin flag, IPC-command signtx (#31005)"
(https://github.com/bitcoin/bitcoin/pull/33765#pullrequestreview-3447533985)
Good idea to update these docs.
(https://github.com/bitcoin/bitcoin/pull/33765#pullrequestreview-3447533985)
Good idea to update these docs.
💬 Sjors commented on pull request "doc: update interface, --stdin flag, IPC-command signtx (#31005)":
(https://github.com/bitcoin/bitcoin/pull/33765#discussion_r2513826354)
Afaik the use of quotes depends on the shell environment, I don't think it's worth documenting.
More important is to point out that Bitcoin Core will use `--stdin` here, in order to support large PSBTs.
(https://github.com/bitcoin/bitcoin/pull/33765#discussion_r2513826354)
Afaik the use of quotes depends on the shell environment, I don't think it's worth documenting.
More important is to point out that Bitcoin Core will use `--stdin` here, in order to support large PSBTs.