Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 ryanofsky commented on pull request "refactor: Use util::Result class for wallet loading":
(https://github.com/bitcoin/bitcoin/pull/25722#issuecomment-1958356339)
Rebased 1b2a5f12b42543d13a4bcafb9585e3d1df488914 -> 6700299bcb6f560e9e4caf34254d7d86d4e78833 ([`pr/bresult-load.19`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult-load.19) -> [`pr/bresult-load.20`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult-load.20), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/bresult-load.19-rebase..pr/bresult-load.20)) on top of #26022 using #26022 to simplify some changes in this PR
💬 andrewtoth commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1498436377)
I think what @naumenkogs is referring to here is that if this condition is true then `pindex` will hit this condition on the first `pindex` in the loop and every `pindex` up until this condition is no longer true. This can be more efficient by just bumping `pindex` to the first `pindex` that is past the network limited blocks threshold.

So, we can either change the loop from range based to regular for-loop and advance the index, or move this check out of this loop and into the loop on line 14
...
💬 andrewtoth commented on pull request "p2p: make block download logic aware of limited peers threshold":
(https://github.com/bitcoin/bitcoin/pull/28120#discussion_r1498411855)
nit: `const bool is_limited_peer{IsLimitedPeer};`
💬 andrewtoth commented on pull request "rpc,rest,zmq: faster getblock, NotifyBlock and rest_block by reading raw block":
(https://github.com/bitcoin/bitcoin/pull/26415#issuecomment-1958362567)
@furszy @TheCharlatan updated with both your suggestions, thanks!
📝 achow101 opened a pull request: "[25.2] Final backports and changes for 25.2rc1"
(https://github.com/bitcoin/bitcoin/pull/29464)
Backport:

* #29176
* #27927

#29176 does not cleanly backport, and it also requires 27927 to work. Both are still fairly simple backports.

Also does the rest of the version bump tasks for 25.2rc1.
💬 achow101 commented on pull request "wallet: Fix use-after-free in WalletBatch::EraseRecords":
(https://github.com/bitcoin/bitcoin/pull/29176#issuecomment-1958402460)
Backported to 25.x in #29464
🤔 tdb3 reviewed a pull request: "init: Add option for rpccookie permissions (replace 26088)"
(https://github.com/bitcoin/bitcoin/pull/28167#pullrequestreview-1894738259)
Test ACK ce9df2aba3e98ba7f2a576d587ba8e59bf341083.

Great and appreciated feature addition.

Performed basic manual testing, receiving all successful/expected results.

1. Confirmed default of 400 perms for the `.cookie` file when no CLI args or config file parameters are provided.
1. Confirmed CLI arg of `-rpccookieperms=440` results in 440 permissions.
1. Confirmed bitcoin.conf with `rpccookieperms=440` results in 440 permissions.
1. Confirmed empty, non-number, and > 3 octal number i
...
💬 BrandonOdiwuor commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1498713898)
fixed
💬 BrandonOdiwuor commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1958792354)
@maflcko When wallet is disabled these RPS are reported as uncovered;

![Screenshot from 2024-02-22 09-26-27](https://github.com/bitcoin/bitcoin/assets/15610188/fc992ae2-6f0f-4c63-9103-3423a9e65467)
💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1498754677)
Not really related. Change these constants in any way, and the `0.001` value won't be affected.
💬 maflcko commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1498772309)
This change is wrong, no?

In general, please pay attention to the details yourself, before and after pushing the changes. The goal of review is not to write the changeset for you line-by-line, but to double check a finished, complete changeset for any accidental oversights.
💬 naumenkogs commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-1958853401)
ACK 4a45b28a3f258bf156eca980aace69e0a90554cd
💬 naumenkogs commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1498775688)
I think we can go ahead with this PR without waiting for clarification.
💬 maflcko commented on pull request "[25.2] Final backports and changes for 25.2rc1":
(https://github.com/bitcoin/bitcoin/pull/29464#issuecomment-1958866836)
lgtm
👋 maflcko's pull request is ready for review: "test: Remove struct.pack from almost all places"
(https://github.com/bitcoin/bitcoin/pull/29401)
💬 naumenkogs commented on pull request "p2p: Don't process mutated blocks":
(https://github.com/bitcoin/bitcoin/pull/29412#issuecomment-1958901201)
Concept ACK. Looking forward to addressing already pending comments, then i review.
💬 naumenkogs commented on issue "Gathering Priorities of 28.0":
(https://github.com/bitcoin/bitcoin/issues/29439#issuecomment-1958905829)
Erlay #28646
💬 maflcko commented on pull request "[fuzz] Avoid partial negative result":
(https://github.com/bitcoin/bitcoin/pull/29462#issuecomment-1958940749)
Tested with:

```
./autogen.sh && CC=clang CXX=clang++ ./configure -q --enable-c++20 --enable-fuzz --with-sanitizers=fuzzer,undefined,integer,float-divide-by-zero && make clean && make
```

and https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1495735626 to confirm the fix

ACK 9dae3b970a7a82e8d9f3f755048d427da78c49da
💬 maflcko commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-1958944099)
ACK 4a45b28a3f258bf156eca980aace69e0a90554cd 🛌

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: ACK 4a45b28a3f258bf156eca980aa
...
💬 BrandonOdiwuor commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1498892353)
fixed