💬 alfonsoromanz commented on pull request "Bugfix: bitcoin-cli: Check length of peer.transport_protocol_type":
(https://github.com/bitcoin/bitcoin/pull/29657#issuecomment-2057612784)
ACK c3e632b44153e314ef946f342c68c2758b1cbc4d

⚠️ laanwj opened an issue: "utils: wallet_dump can create a `database` directory, cross-pollinating records"
(https://github.com/bitcoin/bitcoin/issues/29883)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I created a large directory with wallet dat files for testing #26606. Ran into a weird issue where the output mismatches, but after investigating deeper, this was not due to a bug in that PR.

So I created a script that iterates over all wallets, dumps them with the internal bdb and external bdb, diffs the textual output, stopping on the first wallet which mismatches. Mind that the walle
...

💬 sdaftuar commented on pull request "policy: restrict all TRUC (v3) transactions to 25KvB":
(https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2057668820)
> Would this make things {broken, inconvenient} for potential users of TRUCs, e.g. by requiring swapping between v3 and v2?

Just wanted to add -- I think the case I expect more often would be that users would just create multiple transactions if they had a need to construct a transaction greater than 25kvb (similar to what I think everyone must do today to work around the current 100kvb standardness limit).

The additional transaction overhead from constructing four 25kvb transactions inste
...

✅ laanwj closed an issue: "v27.0 Testing"
(https://github.com/bitcoin/bitcoin/issues/29697)

💬 laanwj commented on issue "v27.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/29697#issuecomment-2057674519)
Closing this issue now that `v27.0` final is tagged.

✅ fanquake closed an issue: "27.0 RC Testing Guide Feedback"
(https://github.com/bitcoin/bitcoin/issues/29685)

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1566348693)
Dropped `uint160`, changed to use an array.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1566349302)
Changed to `int64_t` with casting.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1566349587)
Yes, I've added a check for that.

🤔 sr-gi reviewed a pull request: "net_processing: make any misbehavior trigger immediate discouragement"
(https://github.com/bitcoin/bitcoin/pull/29575#pullrequestreview-2001909800)
A couple of doc nits and a question below

💬 sr-gi commented on pull request "net_processing: make any misbehavior trigger immediate discouragement":
(https://github.com/bitcoin/bitcoin/pull/29575#discussion_r1566280997)
in: 78515e185755f5eba951917b8e2314ec69446d7d

nit: isn't this 99? (`for i in range(1, NUM_HEADERS)`)

💬 sr-gi commented on pull request "net_processing: make any misbehavior trigger immediate discouragement":
(https://github.com/bitcoin/bitcoin/pull/29575#discussion_r1566343004)
in: ce238dd79bd3a1190a2229fac4502fa5742fc6e3

I think the comment 6 lines below is wrong now:

```
// It should be impossible for the getheaders request to fail,
// because we should have cleared the last getheaders timestamp
// when processing the headers that triggered this call.
[...]
```

IIUC, `result.request_more` cannot be true if `result.success` is false (based on my understanding of [HeadersSyncState::ProcessNextHeaders](https://github.com/bitcoin/bitcoin/blob/07720b1cdd773
...

💬 sr-gi commented on pull request "net_processing: make any misbehavior trigger immediate discouragement":
(https://github.com/bitcoin/bitcoin/pull/29575#discussion_r1566293700)
in: ce238dd79bd3a1190a2229fac4502fa5742fc6e3

nit: I found this wording a bit hard to follow, especially the "to make marked responded to" bit.

IIUC, this is sending an empty block as a response to the outstanding getheaders request from the previous loop interaction, otherwise sending `blocks[i]` would be treated as the reply instead. Is that it? If so, I'd suggest something along the lines of:

```suggestion
# Send an empty header as a failed response to the received gethea
...

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2057691808)
> parsing the headers of overflow pages (which have btree level 0)

Interesting, did not know that they would use an invalid btree level. Fixed as suggested.

***

I've also commented out the enums that we do not use. I decided to leave them there for completeness, but they won't be used.

***

For help with reviewing, I suggest looking at the BDB source code. I've uploaded a copy of the version that we use at https://github.com/achow101/bdb-sources/tree/main/4.8.30.NC. The files that
...

💬 laanwj commented on issue "utils: wallet_dump can create a `database` directory, cross-pollinating records":
(https://github.com/bitcoin/bitcoin/issues/29883#issuecomment-2057708243)
Oh, i think part of the trigger here is that some of the wallets are backups of the same database at different times, so the logs can get re-used because the unique id matches.

💬 laanwj commented on pull request "netbase: clean up Proxy logging":
(https://github.com/bitcoin/bitcoin/pull/29882#discussion_r1566368437)
It's probably a good idea to keep the `!IsValid` check here, just to be sure. Fine with removing the log.

💬 maflcko commented on pull request "build: Fix false positive `CHECK_ATOMIC` test":
(https://github.com/bitcoin/bitcoin/pull/29859#issuecomment-2057777762)
Ok, I could reproduce by setting this CI to lunar: `FILE_ENV="./ci/test/00_setup_env_i686_multiprocess.sh" ./ci/test_run_all.sh`. However, on jammy, mantic, and noble it passes.

💬 achow101 commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#issuecomment-2057799622)
There was a comment at CoreDev that this should check that the file is compacted, so I've added a commit that does that. To check that this is correct, see the implementation of [`lsn_reset`](https://github.com/achow101/bdb-sources/blob/main/4.8.30.NC/db/db_setlsn.c#L114), which uses [`LSN_NOT_LOGGED`](https://github.com/achow101/bdb-sources/blob/main/4.8.30.NC/dbinc/db_int.in#L803). The definition of [`DB_LSN`](https://github.com/achow101/bdb-sources/blob/main/4.8.30.NC/dbinc/db.in#L444) is als
...

⚠️ hebasto opened an issue: "`test/streams_tests.cpp` fails to compile on SunOS / illumos"
(https://github.com/bitcoin/bitcoin/issues/29884)
While testing bitcoin/bitcoin#29484, which seems not related to this bug, I've notice the following failure:
```
$ ./autogen.sh
$ ./configure
$ gmake -C src test/test_bitcoin
...
CXX test/test_bitcoin-streams_tests.o
In file included from test/streams_tests.cpp:5:
./streams.h: In instantiation of 'SpanReader& SpanReader::operator>>(T&&) [with T = signed char&]':
test/streams_tests.cpp:148:15: required from here
./streams.h:114:22: error: no matching function for call to 'Unser
...

💬 mzumsande commented on pull request "p2p: opportunistically accept 1-parent-1-child packages":
(https://github.com/bitcoin/bitcoin/pull/28970#discussion_r1566233731)
Should update `ProcessInvalidTx` doc above too (it lists all member variables that the function updates).