💬 achow101 commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-2057510602)
> This is probably more than you wanted to do, but I'd say it would be better to use `FeeModeFromString` to re-use the existing (case-insensitive) parsing code, than to re-implement the parsing and use hard-coded "unset" strings in all call places.

I agree. I think it would make more sense for `FeeModeFromString` to handle "unset" and to check its return value rather than having these repeated string comparisons. It's a bit odd that the estimate mode is being validated in 3 places - `Interpre
...

📝 pinheadmz opened a pull request: "netbase: clean up Proxy logging"
(https://github.com/bitcoin/bitcoin/pull/29882)
Follow up to #27375 and see https://github.com/bitcoin/bitcoin/pull/29649#issuecomment-2057456834

This removes an extra log message when we can't connect to our own proxy. It also removes a log message if the Proxy is invalid, which is unreachable code because we check that during init by calling `SetProxy()`:

https://github.com/bitcoin/bitcoin/blob/d1af4422d13ba84253a1555c2fe5a42170fa8b35/src/netbase.cpp#L663-L666

## Before #27375 if proxy is unreachable

```
2024-04-15T17:54:51Z co
...

💬 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
...