💬 ariard commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1496683339)
can add a `LogPrint(BCLog::NET, “Non-signaling reconciliation inbound peers flooding %d Outbound peers flooding %d for debug”);` for debug purpose and observation

📝 kevkevinpal opened a pull request: "test: check_mempool_result negative feerate"
(https://github.com/bitcoin/bitcoin/pull/29459)
Adds test coverage in `mempool_accept.py` to check if a negative `maxfeerate` is input into `check_mempool_result`
Asserts "Amount out of range" error message and `-3` error code

Motivated by this [comment](https://github.com/bitcoin/bitcoin/pull/29434/files#r1491112250)

💬 kevkevinpal commented on pull request "rpc: Fixed signed integer overflow for large feerates":
(https://github.com/bitcoin/bitcoin/pull/29434#discussion_r1496832298)
Created a PR to cover this https://github.com/bitcoin/bitcoin/pull/29459

📝 kevkevinpal opened a pull request: "test: assert rpc error for addnode v2transport not enabled"
(https://github.com/bitcoin/bitcoin/pull/29460)
Added coverage for the `addnode` rpc when v2transport is not enabled,
but is set as true when calling `addnode` rpc.

I ran the following to check if this rpc error message
was covered in the functional tests.
`grep -nr "v2transport requested but not enabled" ./test/functional --binary-files=without-match`

Adds test coverage to this line.
https://github.com/bitcoin/bitcoin/blob/master/src/rpc/net.cpp#L339

🤔 ajtowns reviewed a pull request: "mempool: Log added for dumping mempool transactions to disk"
(https://github.com/bitcoin/bitcoin/pull/29402#pullrequestreview-1892071040)
utACK dbdb7320252fd68415e8b76bad5a2169bd7da241

💬 ajtowns commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#discussion_r1496887607)
Remove "from disk" here, if dropping "disk" mentions elsewhere?

💬 Eunovo commented on pull request "test: Fix SegwitV0SignatureMsg nLockTime signedness":
(https://github.com/bitcoin/bitcoin/pull/29400#issuecomment-1955897276)
Tested ACK https://github.com/bitcoin/bitcoin/pull/29400/commits/fab15723b0518acbb1015e64df47dcac0187e92f
Reproduced the error by setting `nLockTime` to `2147483648`

💬 Eunovo commented on pull request "test: Fix SegwitV0SignatureMsg nLockTime signedness":
(https://github.com/bitcoin/bitcoin/pull/29400#discussion_r1496914400)
> can you add a test that fails before this change, but passes after?

Wouldn't that mean that we added a test that tests the test-framework?

💬 Eunovo commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1496917274)
@starius Might be useful to leave comment explaining why the params size is limited this way

💬 Eunovo commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#issuecomment-1955903839)
Tested ACK
Tried different `signetchallenge` inputs and got the expected results.

💬 maflcko commented on pull request "test: Fix SegwitV0SignatureMsg nLockTime signedness":
(https://github.com/bitcoin/bitcoin/pull/29400#discussion_r1497068772)
> > can you add a test that fails before this change, but passes after?
>
> Wouldn't that mean that we added a test that tests the test-framework?

Yes, it should be possible to write an internal python unit test to check the behavior of `SegwitV0SignatureMsg`. An alternative would be to extend a python functional test to cover more values of `nLockTime` and compare the `SegwitV0SignatureMsg` behavior against Bitcoin Core via the RPC interface.

👍 maflcko approved a pull request: "test: assert rpc error for addnode v2transport not enabled"
(https://github.com/bitcoin/bitcoin/pull/29460#pullrequestreview-1892357591)
lgtm ACK 345169a7523f00209985da88e0171e8687589c25

Checked coverage per https://corecheck.dev/bitcoin/bitcoin/pulls/29460

💬 maflcko commented on pull request "test: assert rpc error for addnode v2transport not enabled":
(https://github.com/bitcoin/bitcoin/pull/29460#discussion_r1497075255)
```suggestion
assert_raises_rpc_error,
```

nit (only if you re-touch for other reasons): Please always add the trailing comma, to avoid having to touch this line again in the future when a new item is added to the list. Also, could sort.

💬 maflcko commented on pull request "test: check_mempool_result negative feerate":
(https://github.com/bitcoin/bitcoin/pull/29459#discussion_r1497080191)
The comment is now wrongly placed

💬 maflcko commented on pull request "Update rpcauth.py":
(https://github.com/bitcoin/bitcoin/pull/29433#issuecomment-1956121171)
Are you still working on this? https://github.com/bitcoin/bitcoin/pull/29433#issuecomment-1943705614

💬 willcl-ark commented on pull request "docs: ci multi-arch requires qemu":
(https://github.com/bitcoin/bitcoin/pull/29456#issuecomment-1956191576)
Seems like a useful addition.

I do vaguely recall installing a `binfmt` package of some sort when I last did this, but could be misremembering something else I was doing with docker and qemu (apologies if so)...

👍 willcl-ark approved a pull request: "docs: ci multi-arch requires qemu"
(https://github.com/bitcoin/bitcoin/pull/29456#pullrequestreview-1892465034)
utACK 540282905dc6137a307273188d6d9291809f0ee9

💬 maflcko commented on pull request "docs: ci multi-arch requires qemu":
(https://github.com/bitcoin/bitcoin/pull/29456#issuecomment-1956206891)
lgtm ACK 540282905dc6137a307273188d6d9291809f0ee9

👍 brunoerg approved a pull request: "test: assert rpc error for addnode v2transport not enabled"
(https://github.com/bitcoin/bitcoin/pull/29460#pullrequestreview-1892780591)
utACK 345169a7523f00209985da88e0171e8687589c25

🤔 ajtowns reviewed a pull request: "refactor: Allow CScript construction from any std::input_iterator"
(https://github.com/bitcoin/bitcoin/pull/29369#pullrequestreview-1892789962)
utACK fa9ef95b4c1114e2d8f7c3307fdf01446efee6fd

LGTM. Could change `template<typename InputIterator>prevector(...)` to also expect `std::input_iterator`.

The prevector functions that take two iterators first increase the capacity by `last - first` then run `fill(ptr, first, last)` -- that's buggy if the passed in iterators specify a greater range than a `size_type` (or `difference_type` for `insert()`) can hold. Could perhaps be worth changing:

```diff
- template<typename InputIterat
...