š¬ fanquake commented on pull request "ci: Retry image building once on failure":
(https://github.com/bitcoin/bitcoin/pull/33677#issuecomment-3443215955)
cc @willcl-ark
(https://github.com/bitcoin/bitcoin/pull/33677#issuecomment-3443215955)
cc @willcl-ark
š¬ BenWestgate commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#issuecomment-3443322429)
> utACK [de7c358](https://github.com/bitcoin/bitcoin/commit/de7c3587cd4586bbed94a4ea6eae4a252301daee)
This commit is documentation only so our tutorial text matches what we do in shell snippets.
Tests have not changed since 2a46e94a1600a4f28e01db23a89f039acaa2c45e on Oct 6.
(https://github.com/bitcoin/bitcoin/pull/33286#issuecomment-3443322429)
> utACK [de7c358](https://github.com/bitcoin/bitcoin/commit/de7c3587cd4586bbed94a4ea6eae4a252301daee)
This commit is documentation only so our tutorial text matches what we do in shell snippets.
Tests have not changed since 2a46e94a1600a4f28e01db23a89f039acaa2c45e on Oct 6.
š¬ maflcko commented on issue "Internal bug detected: FinalizeAndExtractPSBT(psbtx_copy, mtx)":
(https://github.com/bitcoin/bitcoin/issues/32849#issuecomment-3443358096)
> Is this bug still there? I am not able to reproduce it.
I can, and oss-fuzz can as well.
(https://github.com/bitcoin/bitcoin/issues/32849#issuecomment-3443358096)
> Is this bug still there? I am not able to reproduce it.
I can, and oss-fuzz can as well.
š brunoerg approved a pull request: "wallet: warn against accidental unsafe older() import"
(https://github.com/bitcoin/bitcoin/pull/33135#pullrequestreview-3377054460)
reACK fb72cc33be570d12fc6b76146fc89047e58f5aaf
(https://github.com/bitcoin/bitcoin/pull/33135#pullrequestreview-3377054460)
reACK fb72cc33be570d12fc6b76146fc89047e58f5aaf
š¬ l0rinc commented on pull request "rpc: Fix internal bug in descriptorprocesspsbt when encountering invalid signatures":
(https://github.com/bitcoin/bitcoin/pull/33014#discussion_r2460749140)
this is likely a change in behavior since && short circuits
(https://github.com/bitcoin/bitcoin/pull/33014#discussion_r2460749140)
this is likely a change in behavior since && short circuits
ā
achow101 closed a pull request: "Mining: Avoid copying template CBlocks"
(https://github.com/bitcoin/bitcoin/pull/32547)
(https://github.com/bitcoin/bitcoin/pull/32547)
š¬ achow101 commented on pull request "Mining: Avoid copying template CBlocks":
(https://github.com/bitcoin/bitcoin/pull/32547#issuecomment-3443473022)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
(https://github.com/bitcoin/bitcoin/pull/32547#issuecomment-3443473022)
This PR does not seem to have attracted much attention from reviewers. As such, it does not seem important enough right now to keep it sitting idle in the list of open PRs.
Closing due to lack of interest.
ā
achow101 closed a pull request: "crypto: optimize SipHash `Write()` method with chunked processing"
(https://github.com/bitcoin/bitcoin/pull/33696)
(https://github.com/bitcoin/bitcoin/pull/33696)
š¬ rkrux commented on issue "Internal bug detected: FinalizeAndExtractPSBT(psbtx_copy, mtx)":
(https://github.com/bitcoin/bitcoin/issues/32849#issuecomment-3443528573)
I see, this commit https://github.com/bitcoin/bitcoin/commit/ee045b61efc1479c1866b786661ef39a863677d0 returns the above error while signing an unsigned PSBT. This bug, however, is when the PSBT is already signed during the flow.
I will try to reproduce the bug again.
(https://github.com/bitcoin/bitcoin/issues/32849#issuecomment-3443528573)
I see, this commit https://github.com/bitcoin/bitcoin/commit/ee045b61efc1479c1866b786661ef39a863677d0 returns the above error while signing an unsigned PSBT. This bug, however, is when the PSBT is already signed during the flow.
I will try to reproduce the bug again.
š¬ brunoerg commented on pull request "addrman, net: filter during address selection via AddrPolicy to avoid underfill":
(https://github.com/bitcoin/bitcoin/pull/33663#discussion_r2460863763)
You should add a functional/unit test for this. See uncovered new code: https://corecheck.dev/bitcoin/bitcoin/pulls/33663
(https://github.com/bitcoin/bitcoin/pull/33663#discussion_r2460863763)
You should add a functional/unit test for this. See uncovered new code: https://corecheck.dev/bitcoin/bitcoin/pulls/33663
š¬ waketraindev commented on pull request "addrman, net: filter during address selection via AddrPolicy to avoid underfill":
(https://github.com/bitcoin/bitcoin/pull/33663#discussion_r2460878690)
Will do, looking into coverege right now
(https://github.com/bitcoin/bitcoin/pull/33663#discussion_r2460878690)
Will do, looking into coverege right now
š maflcko opened a pull request: "test: Use same rpc timeout for authproxy and cli"
(https://github.com/bitcoin/bitcoin/pull/33698)
It seems odd to use different timeouts (and timeout factors) depending on whether the Python RPC proxy is used, or the bitcoin rpc command line interface.
Fix it by using the same timeout.
This can be tested by reducing the timeout until a failure occurs for the both equally. For example, `./bld-cmake/test/functional/p2p_headers_sync_with_minchainwork.py --timeout-factor 0.2 --valgrind -l DEBUG --tracerpc`, with and without `--usecli`.
(https://github.com/bitcoin/bitcoin/pull/33698)
It seems odd to use different timeouts (and timeout factors) depending on whether the Python RPC proxy is used, or the bitcoin rpc command line interface.
Fix it by using the same timeout.
This can be tested by reducing the timeout until a failure occurs for the both equally. For example, `./bld-cmake/test/functional/p2p_headers_sync_with_minchainwork.py --timeout-factor 0.2 --valgrind -l DEBUG --tracerpc`, with and without `--usecli`.
š¬ l0rinc commented on pull request "refactor: optimize: avoid allocations in script & policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3443675381)
Please only push after you're actually done, everyone gets a notification for each push, and people will just simply unsubscribe from this thread otherwise...
Also, please revert unrelated refactorings, the change is just getting more complicated after the simplification requests.
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3443675381)
Please only push after you're actually done, everyone gets a notification for each push, and people will just simply unsubscribe from this thread otherwise...
Also, please revert unrelated refactorings, the change is just getting more complicated after the simplification requests.
š¬ cedwies commented on pull request "refactor: optimize: avoid allocations in script & policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3443676998)
I strongly suggest splitting this draft into smaller, more focused PRs (or separate drafts), each exploring one idea at a time.
The current draft mixes several independent changes (policy loop optimizations, Solver/IsWitnessProgram API changes, and stylistic updates), which makes it harder to benchmark and review effectively.
Iām happy to follow along and review, but it would be great to have clear, targeted benchmarks for future iterations (ones that measure the actual code (paths) being
...
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3443676998)
I strongly suggest splitting this draft into smaller, more focused PRs (or separate drafts), each exploring one idea at a time.
The current draft mixes several independent changes (policy loop optimizations, Solver/IsWitnessProgram API changes, and stylistic updates), which makes it harder to benchmark and review effectively.
Iām happy to follow along and review, but it would be great to have clear, targeted benchmarks for future iterations (ones that measure the actual code (paths) being
...
š¬ diegoviola commented on pull request "Fix bitcoin-qt visual glitches on Wayland":
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3444009050)
> tested [095f920](https://github.com/bitcoin-core/gui/commit/095f920629207b5ec4c50de7b454dfced0eafefb) and it fixes the problem.
>
Great! Thanks for confirming that.
> I still need to check if this is the right approach, need to check against `xcb` and `macOS`.
Sure.
> Also there are other places where `bringToFront` is being used, the `rpcconsole` has the same issue as the main window in `wayland`, so perhaps the fix has to be within `bringToFront`, need more time to test.
I a
...
(https://github.com/bitcoin-core/gui/pull/904#issuecomment-3444009050)
> tested [095f920](https://github.com/bitcoin-core/gui/commit/095f920629207b5ec4c50de7b454dfced0eafefb) and it fixes the problem.
>
Great! Thanks for confirming that.
> I still need to check if this is the right approach, need to check against `xcb` and `macOS`.
Sure.
> Also there are other places where `bringToFront` is being used, the `rpcconsole` has the same issue as the main window in `wayland`, so perhaps the fix has to be within `bringToFront`, need more time to test.
I a
...
š¬ musaHaruna commented on pull request "refactor/doc: Add blockman param to GetTransaction doc comment":
(https://github.com/bitcoin/bitcoin/pull/33683#discussion_r2461258996)
Done. Thanks for the suggestion
(https://github.com/bitcoin/bitcoin/pull/33683#discussion_r2461258996)
Done. Thanks for the suggestion
š¬ l0rinc commented on pull request "refactor/doc: Add blockman param to GetTransaction doc comment":
(https://github.com/bitcoin/bitcoin/pull/33683#issuecomment-3444074180)
Recreated the change locally, matches the version in the PR.
ACK 1a1f46c2285994908df9c11991c1f363c9733087
(https://github.com/bitcoin/bitcoin/pull/33683#issuecomment-3444074180)
Recreated the change locally, matches the version in the PR.
ACK 1a1f46c2285994908df9c11991c1f363c9733087
š 0xB10C opened a pull request: "p2p: reduce false-positives in addr rate-limiting"
(https://github.com/bitcoin/bitcoin/pull/33699)
Start the rate-limiting with 5 tokens instead of 1 token.
---
On some occasions, a peer will send us addresses a few seconds after the connection is established. As peers start with token=1 and only get an additional token only every 10 seconds, some of these addresses might be rate-limited. The following log snippet contains an example:
```
04:12:02 [net] Added connection to [redacted] peer=1234
...
04:12:03 [net] Received addr: 1 addresses (1 processed, 0 rate-limited) from peer=12
...
(https://github.com/bitcoin/bitcoin/pull/33699)
Start the rate-limiting with 5 tokens instead of 1 token.
---
On some occasions, a peer will send us addresses a few seconds after the connection is established. As peers start with token=1 and only get an additional token only every 10 seconds, some of these addresses might be rate-limited. The following log snippet contains an example:
```
04:12:02 [net] Added connection to [redacted] peer=1234
...
04:12:03 [net] Received addr: 1 addresses (1 processed, 0 rate-limited) from peer=12
...
š¬ real-or-random commented on pull request "test: Update BIP324 test vectors":
(https://github.com/bitcoin/bitcoin/pull/33688#issuecomment-3444131265)
Ready for review.
I think what is necessary to review this PR is to run the Python script in the code comment on the updated `packet_encoding_test_vectors.csv` file from the BIP and check that its output matches the changes here.
https://github.com/bitcoin/bitcoin/blob/f6ba97cea1d36683fa8c1ebde78af3772d00ec12/src/test/bip324_tests.cpp#L171-L195
(https://github.com/bitcoin/bitcoin/pull/33688#issuecomment-3444131265)
Ready for review.
I think what is necessary to review this PR is to run the Python script in the code comment on the updated `packet_encoding_test_vectors.csv` file from the BIP and check that its output matches the changes here.
https://github.com/bitcoin/bitcoin/blob/f6ba97cea1d36683fa8c1ebde78af3772d00ec12/src/test/bip324_tests.cpp#L171-L195
š real-or-random's pull request is ready for review: "test: Update BIP324 test vectors"
(https://github.com/bitcoin/bitcoin/pull/33688)
(https://github.com/bitcoin/bitcoin/pull/33688)