Bitcoin Core Github
44 subscribers
120K links
Download Telegram
:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/29201)
💬 maflcko commented on pull request "test: Treat msg_version.relay as unsigned, Remove `struct` packing in messages.py":
(https://github.com/bitcoin/bitcoin/pull/29067#issuecomment-1880721233)
> over int.from_bytes(bs, "little")

In python3.11, one could also write `int.from_bytes(bs)` for single-byte conversions.
💬 glozow commented on pull request "mempool / rpc: followup to getprioritisedtransactions and delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/28885#issuecomment-1880722797)
Still working on this? I'll be available to re-review quickly after comments are addressed.
💬 maflcko commented on pull request "ci: Switch native macOS CI job to Xcode 15.0":
(https://github.com/bitcoin/bitcoin/pull/29195#issuecomment-1880726651)
pull title should be "build: Fix -Xclang -internal-isystem option"? Otherwise lgtm
💬 hebasto commented on pull request "build: Fix `-Xclang -internal-isystem` option":
(https://github.com/bitcoin/bitcoin/pull/29195#issuecomment-1880728913)
> pull title should be "build: Fix -Xclang -internal-isystem option"? Otherwise lgtm

Done.
💬 fanquake commented on pull request "build: Fix `-Xclang -internal-isystem` option":
(https://github.com/bitcoin/bitcoin/pull/29195#issuecomment-1880736065)
> The same Xcode version is used for the release binaries.

Can probably drop this, or be more specific, because we don't use Xcode to build release binaries (only the SDK, which is versioned differently).
💬 fanquake commented on pull request "ci: Do not set inane value for `LD_LIBRARY_PATH`":
(https://github.com/bitcoin/bitcoin/pull/29196#issuecomment-1880740854)
> For example, in the native macOS CI job,

Unless there's a downside to having this set, I don't think it's worth the extra code / shellcheck exceptions. i.e in this example, ld64 will never read this ENV var?
💬 hebasto commented on pull request "build: Fix `-Xclang -internal-isystem` option":
(https://github.com/bitcoin/bitcoin/pull/29195#issuecomment-1880740932)
> > The same Xcode version is used for the release binaries.
>
> Can probably drop this, or be more specific, because we don't use Xcode to build release binaries (only the SDK, which is versioned differently).

Dropped.
🚀 glozow merged a pull request: "RPC/Blockchain: scanblocks: Accept named param for filter_false_positives"
(https://github.com/bitcoin/bitcoin/pull/29184)
💬 ismaelsadeeq commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444456650)
Should enable passing `maxfeerate` and `maxburnamount` as a config variables since it is used by various RPC, downstream might prefer this?
💬 ismaelsadeeq commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444397171)
Shoulbe we testi `maxfeerate` and `maxburnamount` for `testmempoolaccept` with a package also?
💬 ismaelsadeeq commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444464488)
To turn off for maxfeerate you should pass 0 fee rate, you cant turn off the restriction of `maxburnamount`
nit: maybe reword to relax?
```suggestion
# Relax the restrictions and send it; parent gets through as own subpackage
```
💬 ismaelsadeeq commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444454903)
My understanding of package submission is that a child with unconfirmed parents is bundled together as a package for incentive compatibility. Should this instead check the package fee rate against `maxfeerate`?

If I understand correctly, currently, we are checking the package transaction fee rate individually against `maxfeerate`. The child transactions might be greater than the `maxfeerate`, but as a package might not be. In this case, we might reject the child transactions and accept the su
...
💬 ismaelsadeeq commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#discussion_r1444461653)
We did not document this anywhere but the default behavior if `maxburnamount` is not passed it we are rejecting the package if there is any transactions with unspendable output greater than 0.
Maybe indicate it in the `RPCHelpMan` of `maxburnamount`?

---

Its also not indicated for `sendrawtransaction`.
🤔 BrandonOdiwuor reviewed a pull request: "RFC: Deprecate libconsensus"
(https://github.com/bitcoin/bitcoin/pull/29189#pullrequestreview-1808889544)
Concept ACK
💬 TheCharlatan commented on pull request "refactor(tidy): Use C++20 contains method":
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1880837705)
Can the full diff be reproduced with `run-clang-tidy -fix`? If so, can you post the full commands you are using? I am getting a slightly different diff if I run it myself.
💬 aureleoules commented on pull request "refactor(tidy): Use C++20 contains method":
(https://github.com/bitcoin/bitcoin/pull/29191#issuecomment-1880843285)
> Can the full diff be reproduced with run-clang-tidy -fix? If so, can you post the full commands you are using? I am getting a slightly different diff if I run it myself.

Sorry, my initial push did not pass the tidy check because it seems the `-fix` option does not fix code in macros (all the asserts, Assume, etc.) so I manually fixed these in a second push. This may be the reason why you are not getting the same diff as me.

I don't have the specific command I used on hand but I used the
...
💬 fanquake commented on pull request "Weaken serfloat tests":
(https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1880865337)
Pushed up the patches into [my PR](https://github.com/Homebrew/homebrew-core/pull/156745), it has fixed some of the test failures, but not all: https://github.com/Homebrew/homebrew-core/actions/runs/7446626418/job/20257419440?pr=156745#step:4:82.
```bash
==> /home/linuxbrew/.linuxbrew/Cellar/bitcoin/26.0/bin/test_bitcoin
Running 585 test cases...
test/serfloat_tests.cpp(39): error: in "serfloat_tests/double_serfloat_tests": check repr == i has failed [1 != 0]
test/serfloat_tests.cpp
...
💬 fanquake commented on issue "brew: serfloat_tests tests fail on Linux":
(https://github.com/bitcoin/bitcoin/issues/28941#issuecomment-1880865889)
> Again, it would be good to have steps to reproduce locally.

See https://github.com/bitcoin/bitcoin/pull/29192#issuecomment-1880865337.