Bitcoin Core Github
44 subscribers
122K links
Download Telegram
🚀 fanquake merged a pull request: "doc: update windows `-fstack-clash-protection` doc"
(https://github.com/bitcoin/bitcoin/pull/28084)
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1265268347)
Resolved!
🚀 fanquake merged a pull request: "fuzz: Flatten all FUZZ_TARGET macros into one"
(https://github.com/bitcoin/bitcoin/pull/28065)
🚀 fanquake merged a pull request: "guix: Remove librt usage from release binaries"
(https://github.com/bitcoin/bitcoin/pull/28069)
💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1265311577)
Needs rebase
👍 0xB10C approved a pull request: "test: Disable known broken USDT test"
(https://github.com/bitcoin/bitcoin/pull/28088#pullrequestreview-1532737766)
ACK on disabling the test for now - I'll revisit #27380 when I have more bandwith it.
🤔 MarcoFalke reviewed a pull request: "Make poly1305 support incremental computation + modernize"
(https://github.com/bitcoin/bitcoin/pull/27993#pullrequestreview-1527628367)
Sure, happy to leave comments, but I think it may be better to fix them in a follow-up, unless you really want to here.
💬 MarcoFalke commented on pull request "Make poly1305 support incremental computation + modernize":
(https://github.com/bitcoin/bitcoin/pull/27993#discussion_r1262023316)
```suggestion
BOOST_CHECK_EQUAL(HexStr(total_tag), "64afe2e8d6ad7bbdd287f97c44623d39");
```

style nit, if you retouch.
💬 MarcoFalke commented on pull request "Make poly1305 support incremental computation + modernize":
(https://github.com/bitcoin/bitcoin/pull/27993#discussion_r1265337111)
nit: If you want to avoid the cast here, you could make `ConsumeRandomLengthByteVector` a template on `B` and then directly consume into a `std::vector<std::byte>` and use that.
💬 MarcoFalke commented on pull request "Make poly1305 support incremental computation + modernize":
(https://github.com/bitcoin/bitcoin/pull/27993#discussion_r1265338188)
```suggestion
std::vector<std::byte> msg(i, std::byte{uint8_t(i)});
```

(nit, just personal preference)
💬 furszy commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1265351007)
Guess that you are referring to the last two args?

I think that it isn't much of a problem because it is an internal static helper function.

They are needed for Knapsack min change target calculation. What maybe could do is to pass them as one: `mean_amount = std::floor(recipients_sum / recipients_size)` and document the function properly.

And also, in a follow-up, we could go further and move the else branch of the `scriptChange` block of code inside `ComputeChangeParams` too. That par
...
📝 MarcoFalke opened a pull request: "test: Bump walletpassphrase timeout to avoid intermittent issue"
(https://github.com/bitcoin/bitcoin/pull/28089)
There is no risk increasing the timeout, but it avoids intermittent issues (when running in valgrind) of the form:

```
node1 2023-07-15T06:17:37.827609Z [http] [httpserver.cpp:255] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:45950
node1 2023-07-15T06:17:37.833168Z [httpworker.3] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=signrawtransactionwithwallet user=__cookie__
test 2023-07-15T06:17:38.037000Z TestFramework (ERROR): JSONRPC error

...
📝 fanquake opened a pull request: "validation: use noexcept instead of deprecated throw()"
(https://github.com/bitcoin/bitcoin/pull/28090)
We fixed this once before in https://github.com/bitcoin/bitcoin/pull/10965.
Turn on https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-noexcept.html#modernize-use-noexcept.
💬 fanquake commented on pull request "test: Disable known broken USDT test":
(https://github.com/bitcoin/bitcoin/pull/28088#issuecomment-1638210191)
Seems fine to disable this for now.
Also cc @virtu.
🚀 fanquake merged a pull request: "test: Disable known broken USDT test"
(https://github.com/bitcoin/bitcoin/pull/28088)
💬 furszy commented on pull request "test: Bump walletpassphrase timeout to avoid intermittent issue":
(https://github.com/bitcoin/bitcoin/pull/28089#discussion_r1265432942)
This looks fishy. It seems that we unlock the wallet inside a specific test case, and use the unlocked period for other test cases.

A better approach would be to relock the wallet at the end of this test case ( `self.nodes[1].walletlock()`). And unlock-lock the wallet in the other cases when it is needed.
💬 willcl-ark commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#discussion_r1265436928)
Ok I've used a `GlobalMutex` for this now. I never Recursive behaviour but was getting clang static analysis errors without. Looking at _sync.h_ it seems that `GlobalMutex` should be the correct type for this I think.
💬 MarcoFalke commented on pull request "test: Bump walletpassphrase timeout to avoid intermittent issue":
(https://github.com/bitcoin/bitcoin/pull/28089#discussion_r1265441187)
Sounds good. An alternative might be to not use a passphrase after this test. Do you want to submit either as a pull request? Happy to close mine then. Otherwise, I'll leave the feedback for a follow-up, as the goal here is to not change any logic, only the constant.
💬 willcl-ark commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#discussion_r1265441313)
`DeleteNode()` now gated behind `if (pnode->nRefCount.compare_exchange_strong(expectedRefCount, -1))` so it can only be called once (by a single thread).
💬 MarcoFalke commented on pull request "validation: use noexcept instead of deprecated throw()":
(https://github.com/bitcoin/bitcoin/pull/28090#issuecomment-1638254580)
lgtm ACK 047daad4f59942488163c6be8516a69291646294